2022-10-04 00:21:25

by Cixi Geng

[permalink] [raw]
Subject: [PATCH V2] dmaengine: sprd: Support two-stage dma interrupt

From: Cixi Geng <[email protected]>

Audio need to request Audio CP global dma interrupt, so Audio CP
DMA should support two-stage interrupt to adapte it.
It will occur interrupt when two-stage dma channel transfer done.

Signed-off-by: Cixi Geng <[email protected]>
---
Changes in v2:
fix the condition of 2stage_config config for each channel interrupt.

drivers/dma/sprd-dma.c | 8 ++++----
include/linux/dma/sprd-dma.h | 12 ++++++++++++
2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 474d3ba8ec9f..dbcfa340a40f 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -441,7 +441,7 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = chn & SPRD_DMA_GLB_SRC_CHN_MASK;
val |= BIT(schan->trg_mode - 1) << SPRD_DMA_GLB_TRG_OFFSET;
val |= SPRD_DMA_GLB_2STAGE_EN;
- if (schan->int_type != SPRD_DMA_NO_INT)
+ if (schan->int_type & SPRD_DMA_SRC_CHN0_INT)
val |= SPRD_DMA_GLB_SRC_INT;

sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
@@ -451,7 +451,7 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = chn & SPRD_DMA_GLB_SRC_CHN_MASK;
val |= BIT(schan->trg_mode - 1) << SPRD_DMA_GLB_TRG_OFFSET;
val |= SPRD_DMA_GLB_2STAGE_EN;
- if (schan->int_type != SPRD_DMA_NO_INT)
+ if (schan->int_type & SPRD_DMA_SRC_CHN1_INT)
val |= SPRD_DMA_GLB_SRC_INT;

sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
@@ -461,7 +461,7 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = (chn << SPRD_DMA_GLB_DEST_CHN_OFFSET) &
SPRD_DMA_GLB_DEST_CHN_MASK;
val |= SPRD_DMA_GLB_2STAGE_EN;
- if (schan->int_type != SPRD_DMA_NO_INT)
+ if (schan->int_type & SPRD_DMA_DST_CHN0_INT)
val |= SPRD_DMA_GLB_DEST_INT;

sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
@@ -471,7 +471,7 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
val = (chn << SPRD_DMA_GLB_DEST_CHN_OFFSET) &
SPRD_DMA_GLB_DEST_CHN_MASK;
val |= SPRD_DMA_GLB_2STAGE_EN;
- if (schan->int_type != SPRD_DMA_NO_INT)
+ if (schan->int_type & SPRD_DMA_DST_CHN1_INT)
val |= SPRD_DMA_GLB_DEST_INT;

sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
diff --git a/include/linux/dma/sprd-dma.h b/include/linux/dma/sprd-dma.h
index d09c6f6f6da5..26de41d6d915 100644
--- a/include/linux/dma/sprd-dma.h
+++ b/include/linux/dma/sprd-dma.h
@@ -101,6 +101,14 @@ enum sprd_dma_req_mode {
* is done.
* @SPRD_DMA_CFGERR_INT: configure error interrupt when configuration is
* incorrect.
+ * @SPRD_DMA_SRC_CHN0_INT: interrupt occurred when source channel0
+ * transfer is done.
+ * @SPRD_DMA_SRC_CHN1_INT: interrupt occurred when source channel1
+ * transfer is done.
+ * @SPRD_DMA_DST_CHN0_INT: interrupt occurred when destination channel0
+ * transfer is done.
+ * @SPRD_DMA_DST_CHN1_INT: interrupt occurred when destination channel1
+ * transfer is done.
*/
enum sprd_dma_int_type {
SPRD_DMA_NO_INT,
@@ -112,6 +120,10 @@ enum sprd_dma_int_type {
SPRD_DMA_TRANS_BLK_INT,
SPRD_DMA_LIST_INT,
SPRD_DMA_CFGERR_INT,
+ SPRD_DMA_SRC_CHN0_INT,
+ SPRD_DMA_SRC_CHN1_INT,
+ SPRD_DMA_DST_CHN0_INT,
+ SPRD_DMA_DST_CHN1_INT,
};

/*
--
2.34.1


2022-10-10 06:55:20

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH V2] dmaengine: sprd: Support two-stage dma interrupt



On 10/4/2022 7:49 AM, Cixi Geng wrote:
> From: Cixi Geng <[email protected]>
>
> Audio need to request Audio CP global dma interrupt, so Audio CP
> DMA should support two-stage interrupt to adapte it.

s/adapte/adapt

> It will occur interrupt when two-stage dma channel transfer done.

I don't understand why not just set the interrupt type as
SPRD_DMA_TRANS_INT.

>
> Signed-off-by: Cixi Geng <[email protected]>
> ---
> Changes in v2:
> fix the condition of 2stage_config config for each channel interrupt.
>
> drivers/dma/sprd-dma.c | 8 ++++----
> include/linux/dma/sprd-dma.h | 12 ++++++++++++
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
> index 474d3ba8ec9f..dbcfa340a40f 100644
> --- a/drivers/dma/sprd-dma.c
> +++ b/drivers/dma/sprd-dma.c
> @@ -441,7 +441,7 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
> val = chn & SPRD_DMA_GLB_SRC_CHN_MASK;
> val |= BIT(schan->trg_mode - 1) << SPRD_DMA_GLB_TRG_OFFSET;
> val |= SPRD_DMA_GLB_2STAGE_EN;
> - if (schan->int_type != SPRD_DMA_NO_INT)
> + if (schan->int_type & SPRD_DMA_SRC_CHN0_INT)
> val |= SPRD_DMA_GLB_SRC_INT;

I think you will break sprd-pcm-compress.c driver, since we will set
SPRD_DMA_TRANS_INT interrupt type for the 2 stage transaction. Have you
tested the sprd-pcm-compress drvier?

https://elixir.bootlin.com/linux/v6.0-rc5/source/sound/soc/sprd/sprd-pcm-compress.c#L129

>
> sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
> @@ -451,7 +451,7 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
> val = chn & SPRD_DMA_GLB_SRC_CHN_MASK;
> val |= BIT(schan->trg_mode - 1) << SPRD_DMA_GLB_TRG_OFFSET;
> val |= SPRD_DMA_GLB_2STAGE_EN;
> - if (schan->int_type != SPRD_DMA_NO_INT)
> + if (schan->int_type & SPRD_DMA_SRC_CHN1_INT)
> val |= SPRD_DMA_GLB_SRC_INT;
>
> sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
> @@ -461,7 +461,7 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
> val = (chn << SPRD_DMA_GLB_DEST_CHN_OFFSET) &
> SPRD_DMA_GLB_DEST_CHN_MASK;
> val |= SPRD_DMA_GLB_2STAGE_EN;
> - if (schan->int_type != SPRD_DMA_NO_INT)
> + if (schan->int_type & SPRD_DMA_DST_CHN0_INT)
> val |= SPRD_DMA_GLB_DEST_INT;
>
> sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
> @@ -471,7 +471,7 @@ static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
> val = (chn << SPRD_DMA_GLB_DEST_CHN_OFFSET) &
> SPRD_DMA_GLB_DEST_CHN_MASK;
> val |= SPRD_DMA_GLB_2STAGE_EN;
> - if (schan->int_type != SPRD_DMA_NO_INT)
> + if (schan->int_type & SPRD_DMA_DST_CHN1_INT)
> val |= SPRD_DMA_GLB_DEST_INT;
>
> sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
> diff --git a/include/linux/dma/sprd-dma.h b/include/linux/dma/sprd-dma.h
> index d09c6f6f6da5..26de41d6d915 100644
> --- a/include/linux/dma/sprd-dma.h
> +++ b/include/linux/dma/sprd-dma.h
> @@ -101,6 +101,14 @@ enum sprd_dma_req_mode {
> * is done.
> * @SPRD_DMA_CFGERR_INT: configure error interrupt when configuration is
> * incorrect.
> + * @SPRD_DMA_SRC_CHN0_INT: interrupt occurred when source channel0
> + * transfer is done.
> + * @SPRD_DMA_SRC_CHN1_INT: interrupt occurred when source channel1
> + * transfer is done.
> + * @SPRD_DMA_DST_CHN0_INT: interrupt occurred when destination channel0
> + * transfer is done.
> + * @SPRD_DMA_DST_CHN1_INT: interrupt occurred when destination channel1
> + * transfer is done.
> */
> enum sprd_dma_int_type {
> SPRD_DMA_NO_INT,
> @@ -112,6 +120,10 @@ enum sprd_dma_int_type {
> SPRD_DMA_TRANS_BLK_INT,
> SPRD_DMA_LIST_INT,
> SPRD_DMA_CFGERR_INT,
> + SPRD_DMA_SRC_CHN0_INT,
> + SPRD_DMA_SRC_CHN1_INT,
> + SPRD_DMA_DST_CHN0_INT,
> + SPRD_DMA_DST_CHN1_INT,
> };
>
> /*

2022-10-19 14:46:58

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH V2] dmaengine: sprd: Support two-stage dma interrupt

On 04-10-22, 07:49, Cixi Geng wrote:
> From: Cixi Geng <[email protected]>
>
> Audio need to request Audio CP global dma interrupt, so Audio CP
> DMA should support two-stage interrupt to adapte it.
> It will occur interrupt when two-stage dma channel transfer done.

The patch looks fine to me but...

> diff --git a/include/linux/dma/sprd-dma.h b/include/linux/dma/sprd-dma.h
> index d09c6f6f6da5..26de41d6d915 100644
> --- a/include/linux/dma/sprd-dma.h
> +++ b/include/linux/dma/sprd-dma.h

> enum sprd_dma_int_type {
> SPRD_DMA_NO_INT,
> @@ -112,6 +120,10 @@ enum sprd_dma_int_type {
> SPRD_DMA_TRANS_BLK_INT,
> SPRD_DMA_LIST_INT,
> SPRD_DMA_CFGERR_INT,
> + SPRD_DMA_SRC_CHN0_INT,
> + SPRD_DMA_SRC_CHN1_INT,
> + SPRD_DMA_DST_CHN0_INT,
> + SPRD_DMA_DST_CHN1_INT,

why is sprd_dma_int_type part of driver interface. sprd_dma_int_type is
used only by this driver and should be moved into the driver..

Can you change that as well please

--
~Vinod

2022-10-20 07:40:50

by Cixi Geng

[permalink] [raw]
Subject: Re: [PATCH V2] dmaengine: sprd: Support two-stage dma interrupt

Vinod Koul <[email protected]> 于2022年10月19日周三 22:07写道:
>
> On 04-10-22, 07:49, Cixi Geng wrote:
> > From: Cixi Geng <[email protected]>
> >
> > Audio need to request Audio CP global dma interrupt, so Audio CP
> > DMA should support two-stage interrupt to adapte it.
> > It will occur interrupt when two-stage dma channel transfer done.
>
> The patch looks fine to me but...
>
> > diff --git a/include/linux/dma/sprd-dma.h b/include/linux/dma/sprd-dma.h
> > index d09c6f6f6da5..26de41d6d915 100644
> > --- a/include/linux/dma/sprd-dma.h
> > +++ b/include/linux/dma/sprd-dma.h
>
> > enum sprd_dma_int_type {
> > SPRD_DMA_NO_INT,
> > @@ -112,6 +120,10 @@ enum sprd_dma_int_type {
> > SPRD_DMA_TRANS_BLK_INT,
> > SPRD_DMA_LIST_INT,
> > SPRD_DMA_CFGERR_INT,
> > + SPRD_DMA_SRC_CHN0_INT,
> > + SPRD_DMA_SRC_CHN1_INT,
> > + SPRD_DMA_DST_CHN0_INT,
> > + SPRD_DMA_DST_CHN1_INT,
>
> why is sprd_dma_int_type part of driver interface. sprd_dma_int_type is
> used only by this driver and should be moved into the driver..
>
> Can you change that as well please
the two-stage interrupts added need more discuss and test,
anyway, I can create a new patch for the change to move init_type into driver,
>
> --
> ~Vinod

2022-10-20 08:52:56

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH V2] dmaengine: sprd: Support two-stage dma interrupt



On 10/20/2022 3:33 PM, Cixi Geng wrote:
> Vinod Koul <[email protected]> 于2022年10月19日周三 22:07写道:
>>
>> On 04-10-22, 07:49, Cixi Geng wrote:
>>> From: Cixi Geng <[email protected]>
>>>
>>> Audio need to request Audio CP global dma interrupt, so Audio CP
>>> DMA should support two-stage interrupt to adapte it.
>>> It will occur interrupt when two-stage dma channel transfer done.
>>
>> The patch looks fine to me but...
>>
>>> diff --git a/include/linux/dma/sprd-dma.h b/include/linux/dma/sprd-dma.h
>>> index d09c6f6f6da5..26de41d6d915 100644
>>> --- a/include/linux/dma/sprd-dma.h
>>> +++ b/include/linux/dma/sprd-dma.h
>>
>>> enum sprd_dma_int_type {
>>> SPRD_DMA_NO_INT,
>>> @@ -112,6 +120,10 @@ enum sprd_dma_int_type {
>>> SPRD_DMA_TRANS_BLK_INT,
>>> SPRD_DMA_LIST_INT,
>>> SPRD_DMA_CFGERR_INT,
>>> + SPRD_DMA_SRC_CHN0_INT,
>>> + SPRD_DMA_SRC_CHN1_INT,
>>> + SPRD_DMA_DST_CHN0_INT,
>>> + SPRD_DMA_DST_CHN1_INT,
>>
>> why is sprd_dma_int_type part of driver interface. sprd_dma_int_type is
>> used only by this driver and should be moved into the driver..

Now we can not move this into dma driver, since we have some drivers in
the mainline will set the DMA interrupt type, such as spi-sprd.c,
sprd_serial.c and sprd-pcm-compress.c.

>>
>> Can you change that as well please
> the two-stage interrupts added need more discuss and test,
> anyway, I can create a new patch for the change to move init_type into driver,
>>
>> --
>> ~Vinod

2022-10-20 11:43:11

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH V2] dmaengine: sprd: Support two-stage dma interrupt

On 20-10-22, 16:15, Baolin Wang wrote:

> > > why is sprd_dma_int_type part of driver interface. sprd_dma_int_type is
> > > used only by this driver and should be moved into the driver..
>
> Now we can not move this into dma driver, since we have some drivers in the
> mainline will set the DMA interrupt type, such as spi-sprd.c, sprd_serial.c
> and sprd-pcm-compress.c.

That may not sound right.. Why should peripheral set the DMA
interrupt..?

--
~Vinod

2022-10-21 01:43:54

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH V2] dmaengine: sprd: Support two-stage dma interrupt



On 10/20/2022 7:22 PM, Vinod Koul wrote:
> On 20-10-22, 16:15, Baolin Wang wrote:
>
>>>> why is sprd_dma_int_type part of driver interface. sprd_dma_int_type is
>>>> used only by this driver and should be moved into the driver..
>>
>> Now we can not move this into dma driver, since we have some drivers in the
>> mainline will set the DMA interrupt type, such as spi-sprd.c, sprd_serial.c
>> and sprd-pcm-compress.c.
>
> That may not sound right.. Why should peripheral set the DMA
> interrupt..?

That's because SPRD DMA controller supplies several different interrupt
types for different scenarios of users, for example:

@SPRD_DMA_FRAG_INT: fragment done interrupt when one fragment request is
done.
@SPRD_DMA_BLK_INT: block done interrupt when one block request is done.
@SPRD_DMA_BLK_FRAG_INT: block and fragment interrupt when one fragment
or one block request is done.
@SPRD_DMA_TRANS_INT: tansaction done interrupt when one transaction
request is done.
......

Some users may want use Linklist interrupt, and others may just want
tansaction interrupt. So exposing these interrupt types for users to
decide seems more suitable.