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

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

[Cixi Geng]

From: Cixi Geng <cixi.geng1@unisoc.com>

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 <cixi.geng1@unisoc.com>
---
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

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

[Baolin Wang]

On 10/4/2022 7:49 AM, Cixi Geng wrote:
> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> 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 <cixi.geng1@unisoc.com>
> ---
> 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,
>   };
>   
>   /*

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

[Vinod Koul]

On 04-10-22, 07:49, Cixi Geng wrote:
> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> 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

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

[Cixi Geng]

Vinod Koul <vkoul@kernel.org> 于2022年10月19日周三 22:07写道：
>
> On 04-10-22, 07:49, Cixi Geng wrote:
> > From: Cixi Geng <cixi.geng1@unisoc.com>
> >
> > 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

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

[Baolin Wang]

On 10/20/2022 3:33 PM, Cixi Geng wrote:
> Vinod Koul <vkoul@kernel.org> 于2022年10月19日周三 22:07写道：
>>
>> On 04-10-22, 07:49, Cixi Geng wrote:
>>> From: Cixi Geng <cixi.geng1@unisoc.com>
>>>
>>> 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

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

[Vinod Koul]

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

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

[Baolin Wang]

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.