diff mbox series

[3/5] dma: optimize two stage transfer function

Message ID 20230807052028.2854-1-kaiwei.liu@unisoc.com (mailing list archive)
State Changes Requested
Headers show
Series [1/5] dma: delect redundant parameter for dma driver function | expand

Commit Message

Kaiwei Liu Aug. 7, 2023, 5:20 a.m. UTC 
The DMA hardware is updated to optimize two stages transmission
function, so modify relative register and logic. Two
stages transmission mode of dma allows one channel finished
transmission then start another channel transfer automatically.

Signed-off-by: Kaiwei Liu <kaiwei.liu@unisoc.com>
---
 drivers/dma/sprd-dma.c | 124 ++++++++++++++++++++++++++++++-----------
 1 file changed, 91 insertions(+), 33 deletions(-)

Comments

Baolin Wang Aug. 7, 2023, 8:27 a.m. UTC | #1 
On 8/7/2023 1:20 PM, Kaiwei Liu wrote:
> The DMA hardware is updated to optimize two stages transmission
> function, so modify relative register and logic. Two
> stages transmission mode of dma allows one channel finished
> transmission then start another channel transfer automatically.
> 
> Signed-off-by: Kaiwei Liu <kaiwei.liu@unisoc.com>
> ---
>   drivers/dma/sprd-dma.c | 124 ++++++++++++++++++++++++++++++-----------
>   1 file changed, 91 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
> index 0e146550dfbb..01053e106e8a 100644
> --- a/drivers/dma/sprd-dma.c
> +++ b/drivers/dma/sprd-dma.c
> @@ -68,6 +68,7 @@
>   #define SPRD_DMA_GLB_TRANS_DONE_TRG	BIT(18)
>   #define SPRD_DMA_GLB_BLOCK_DONE_TRG	BIT(17)
>   #define SPRD_DMA_GLB_FRAG_DONE_TRG	BIT(16)
> +#define SPRD_DMA_GLB_TRG_MASK		GENMASK(19, 16)
>   #define SPRD_DMA_GLB_TRG_OFFSET		16
>   #define SPRD_DMA_GLB_DEST_CHN_MASK	GENMASK(13, 8)
>   #define SPRD_DMA_GLB_DEST_CHN_OFFSET	8
> @@ -155,6 +156,13 @@
>   
>   #define SPRD_DMA_SOFTWARE_UID		0
>   
> +#define SPRD_DMA_SRC_CHN0_INT		9
> +#define SPRD_DMA_SRC_CHN1_INT		10
> +#define SPRD_DMA_DST_CHN0_INT		11
> +#define SPRD_DMA_DST_CHN1_INT		12
> +#define SPRD_DMA_2STAGE_SET		1
> +#define SPRD_DMA_2STAGE_CLEAR		0

This 2 are not hardware definition, and I don't think they are helpful, 
just use a boolean parameter.

> +
>   /* dma data width values */
>   enum sprd_dma_datawidth {
>   	SPRD_DMA_DATAWIDTH_1_BYTE,
> @@ -212,7 +220,7 @@ struct sprd_dma_dev {
>   	struct clk		*ashb_clk;
>   	int			irq;
>   	u32			total_chns;
> -	struct sprd_dma_chn	channels[];
> +	struct sprd_dma_chn	channels[0];

Please remove redundant changes.

>   };
>   
>   static void sprd_dma_free_desc(struct virt_dma_desc *vd);
> @@ -431,50 +439,90 @@ static enum sprd_dma_req_mode sprd_dma_get_req_type(struct sprd_dma_chn *schan)
>   	return (frag_reg >> SPRD_DMA_REQ_MODE_OFFSET) & SPRD_DMA_REQ_MODE_MASK;
>   }
>   
> -static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
> +static int sprd_dma_2stage_config(struct sprd_dma_chn *schan, u32 config_type)

Why change the function name? 'config_type' should be boolean.

>   {
>   	struct sprd_dma_dev *sdev = to_sprd_dma_dev(&schan->vc.chan);
>   	u32 val, chn = schan->chn_num + 1;
>   
>   	switch (schan->chn_mode) {
>   	case SPRD_DMA_SRC_CHN0:
> -		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)
> -			val |= SPRD_DMA_GLB_SRC_INT;
> -
> -		sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
> +		if (config_type == SPRD_DMA_2STAGE_SET) {
> +			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_SRC_CHN0_INT)

can you explain why change the interrupt validation? ignore other 
interrupt type?

> +				val |= SPRD_DMA_GLB_SRC_INT;
> +
> +			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1,
> +					    SPRD_DMA_GLB_SRC_INT |
> +					    SPRD_DMA_GLB_TRG_MASK |
> +					    SPRD_DMA_GLB_SRC_CHN_MASK, val);
> +		} else {
> +			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1,
> +					    SPRD_DMA_GLB_SRC_INT |
> +					    SPRD_DMA_GLB_TRG_MASK |
> +					    SPRD_DMA_GLB_2STAGE_EN |
> +					    SPRD_DMA_GLB_SRC_CHN_MASK, 0);
> +		}
>   		break;
>   
>   	case SPRD_DMA_SRC_CHN1:
> -		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)
> -			val |= SPRD_DMA_GLB_SRC_INT;
> -
> -		sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
> +		if (config_type == SPRD_DMA_2STAGE_SET) {
> +			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_SRC_CHN1_INT)
> +				val |= SPRD_DMA_GLB_SRC_INT;
> +
> +			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2,
> +					    SPRD_DMA_GLB_SRC_INT |
> +					    SPRD_DMA_GLB_TRG_MASK |
> +					    SPRD_DMA_GLB_SRC_CHN_MASK, val);
> +		} else {
> +			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2,
> +					    SPRD_DMA_GLB_SRC_INT |
> +					    SPRD_DMA_GLB_TRG_MASK |
> +					    SPRD_DMA_GLB_2STAGE_EN |
> +					    SPRD_DMA_GLB_SRC_CHN_MASK, 0);
> +		}
>   		break;
>   
>   	case SPRD_DMA_DST_CHN0:
> -		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)
> -			val |= SPRD_DMA_GLB_DEST_INT;
> -
> -		sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
> +		if (config_type == SPRD_DMA_2STAGE_SET) {
> +			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_DST_CHN0_INT)
> +				val |= SPRD_DMA_GLB_DEST_INT;
> +
> +			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1,
> +					    SPRD_DMA_GLB_DEST_INT |
> +					    SPRD_DMA_GLB_DEST_CHN_MASK, val);
> +		} else {
> +			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1,
> +					    SPRD_DMA_GLB_DEST_INT |
> +					    SPRD_DMA_GLB_2STAGE_EN |
> +					    SPRD_DMA_GLB_DEST_CHN_MASK, 0);
> +		}
>   		break;
>   
>   	case SPRD_DMA_DST_CHN1:
> -		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)
> -			val |= SPRD_DMA_GLB_DEST_INT;
> -
> -		sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
> +		if (config_type == SPRD_DMA_2STAGE_SET) {
> +			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_DST_CHN1_INT)
> +				val |= SPRD_DMA_GLB_DEST_INT;
> +
> +			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2,
> +					    SPRD_DMA_GLB_DEST_INT |
> +					    SPRD_DMA_GLB_DEST_CHN_MASK, val);
> +		} else {
> +			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2,
> +					    SPRD_DMA_GLB_DEST_INT |
> +					    SPRD_DMA_GLB_2STAGE_EN |
> +					    SPRD_DMA_GLB_DEST_CHN_MASK, 0);
> +		}
>   		break;
>   
>   	default:
> @@ -545,7 +593,7 @@ static void sprd_dma_start(struct sprd_dma_chn *schan)
>   	 * Set 2-stage configuration if the channel starts one 2-stage
>   	 * transfer.
>   	 */
> -	if (schan->chn_mode && sprd_dma_set_2stage_config(schan))
> +	if (schan->chn_mode && sprd_dma_2stage_config(schan, SPRD_DMA_2STAGE_SET))
>   		return;
>   
>   	/*
> @@ -569,6 +617,12 @@ static void sprd_dma_stop(struct sprd_dma_chn *schan)
>   	sprd_dma_set_pending(schan, false);
>   	sprd_dma_unset_uid(schan);
>   	sprd_dma_clear_int(schan);
> +	/*
> +	 * If 2-stage transfer is used, the configuration must be clear
> +	 * when release DMA channel.

Please explain why? not only 'what'.

> +	 */
> +	if (schan->chn_mode)
> +		sprd_dma_2stage_config(schan, SPRD_DMA_2STAGE_CLEAR);

How to ensure backward compatibility with previous SPRD DMA IP?

>   	schan->cur_desc = NULL;
>   }
>   
> @@ -757,7 +811,9 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
>   	phys_addr_t llist_ptr;
>   
>   	if (dir == DMA_MEM_TO_DEV) {
> -		src_step = sprd_dma_get_step(slave_cfg->src_addr_width);
> +		src_step = slave_cfg->src_port_window_size ?
> +			   slave_cfg->src_port_window_size :
> +			   sprd_dma_get_step(slave_cfg->src_addr_width);

Please also explain why.

>   		if (src_step < 0) {
>   			dev_err(sdev->dma_dev.dev, "invalid source step\n");
>   			return src_step;
> @@ -773,7 +829,9 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
>   		else
>   			dst_step = SPRD_DMA_NONE_STEP;
>   	} else {
> -		dst_step = sprd_dma_get_step(slave_cfg->dst_addr_width);
> +		dst_step = slave_cfg->dst_port_window_size ?
> +			   slave_cfg->dst_port_window_size :
> +			   sprd_dma_get_step(slave_cfg->dst_addr_width);
>   		if (dst_step < 0) {
>   			dev_err(sdev->dma_dev.dev, "invalid destination step\n");
>   			return dst_step;
diff mbox series

Patch

diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 0e146550dfbb..01053e106e8a 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -68,6 +68,7 @@ 
 #define SPRD_DMA_GLB_TRANS_DONE_TRG	BIT(18)
 #define SPRD_DMA_GLB_BLOCK_DONE_TRG	BIT(17)
 #define SPRD_DMA_GLB_FRAG_DONE_TRG	BIT(16)
+#define SPRD_DMA_GLB_TRG_MASK		GENMASK(19, 16)
 #define SPRD_DMA_GLB_TRG_OFFSET		16
 #define SPRD_DMA_GLB_DEST_CHN_MASK	GENMASK(13, 8)
 #define SPRD_DMA_GLB_DEST_CHN_OFFSET	8
@@ -155,6 +156,13 @@ 
 
 #define SPRD_DMA_SOFTWARE_UID		0
 
+#define SPRD_DMA_SRC_CHN0_INT		9
+#define SPRD_DMA_SRC_CHN1_INT		10
+#define SPRD_DMA_DST_CHN0_INT		11
+#define SPRD_DMA_DST_CHN1_INT		12
+#define SPRD_DMA_2STAGE_SET		1
+#define SPRD_DMA_2STAGE_CLEAR		0
+
 /* dma data width values */
 enum sprd_dma_datawidth {
 	SPRD_DMA_DATAWIDTH_1_BYTE,
@@ -212,7 +220,7 @@  struct sprd_dma_dev {
 	struct clk		*ashb_clk;
 	int			irq;
 	u32			total_chns;
-	struct sprd_dma_chn	channels[];
+	struct sprd_dma_chn	channels[0];
 };
 
 static void sprd_dma_free_desc(struct virt_dma_desc *vd);
@@ -431,50 +439,90 @@  static enum sprd_dma_req_mode sprd_dma_get_req_type(struct sprd_dma_chn *schan)
 	return (frag_reg >> SPRD_DMA_REQ_MODE_OFFSET) & SPRD_DMA_REQ_MODE_MASK;
 }
 
-static int sprd_dma_set_2stage_config(struct sprd_dma_chn *schan)
+static int sprd_dma_2stage_config(struct sprd_dma_chn *schan, u32 config_type)
 {
 	struct sprd_dma_dev *sdev = to_sprd_dma_dev(&schan->vc.chan);
 	u32 val, chn = schan->chn_num + 1;
 
 	switch (schan->chn_mode) {
 	case SPRD_DMA_SRC_CHN0:
-		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)
-			val |= SPRD_DMA_GLB_SRC_INT;
-
-		sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
+		if (config_type == SPRD_DMA_2STAGE_SET) {
+			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_SRC_CHN0_INT)
+				val |= SPRD_DMA_GLB_SRC_INT;
+
+			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1,
+					    SPRD_DMA_GLB_SRC_INT |
+					    SPRD_DMA_GLB_TRG_MASK |
+					    SPRD_DMA_GLB_SRC_CHN_MASK, val);
+		} else {
+			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1,
+					    SPRD_DMA_GLB_SRC_INT |
+					    SPRD_DMA_GLB_TRG_MASK |
+					    SPRD_DMA_GLB_2STAGE_EN |
+					    SPRD_DMA_GLB_SRC_CHN_MASK, 0);
+		}
 		break;
 
 	case SPRD_DMA_SRC_CHN1:
-		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)
-			val |= SPRD_DMA_GLB_SRC_INT;
-
-		sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
+		if (config_type == SPRD_DMA_2STAGE_SET) {
+			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_SRC_CHN1_INT)
+				val |= SPRD_DMA_GLB_SRC_INT;
+
+			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2,
+					    SPRD_DMA_GLB_SRC_INT |
+					    SPRD_DMA_GLB_TRG_MASK |
+					    SPRD_DMA_GLB_SRC_CHN_MASK, val);
+		} else {
+			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2,
+					    SPRD_DMA_GLB_SRC_INT |
+					    SPRD_DMA_GLB_TRG_MASK |
+					    SPRD_DMA_GLB_2STAGE_EN |
+					    SPRD_DMA_GLB_SRC_CHN_MASK, 0);
+		}
 		break;
 
 	case SPRD_DMA_DST_CHN0:
-		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)
-			val |= SPRD_DMA_GLB_DEST_INT;
-
-		sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1, val, val);
+		if (config_type == SPRD_DMA_2STAGE_SET) {
+			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_DST_CHN0_INT)
+				val |= SPRD_DMA_GLB_DEST_INT;
+
+			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1,
+					    SPRD_DMA_GLB_DEST_INT |
+					    SPRD_DMA_GLB_DEST_CHN_MASK, val);
+		} else {
+			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP1,
+					    SPRD_DMA_GLB_DEST_INT |
+					    SPRD_DMA_GLB_2STAGE_EN |
+					    SPRD_DMA_GLB_DEST_CHN_MASK, 0);
+		}
 		break;
 
 	case SPRD_DMA_DST_CHN1:
-		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)
-			val |= SPRD_DMA_GLB_DEST_INT;
-
-		sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2, val, val);
+		if (config_type == SPRD_DMA_2STAGE_SET) {
+			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_DST_CHN1_INT)
+				val |= SPRD_DMA_GLB_DEST_INT;
+
+			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2,
+					    SPRD_DMA_GLB_DEST_INT |
+					    SPRD_DMA_GLB_DEST_CHN_MASK, val);
+		} else {
+			sprd_dma_glb_update(sdev, SPRD_DMA_GLB_2STAGE_GRP2,
+					    SPRD_DMA_GLB_DEST_INT |
+					    SPRD_DMA_GLB_2STAGE_EN |
+					    SPRD_DMA_GLB_DEST_CHN_MASK, 0);
+		}
 		break;
 
 	default:
@@ -545,7 +593,7 @@  static void sprd_dma_start(struct sprd_dma_chn *schan)
 	 * Set 2-stage configuration if the channel starts one 2-stage
 	 * transfer.
 	 */
-	if (schan->chn_mode && sprd_dma_set_2stage_config(schan))
+	if (schan->chn_mode && sprd_dma_2stage_config(schan, SPRD_DMA_2STAGE_SET))
 		return;
 
 	/*
@@ -569,6 +617,12 @@  static void sprd_dma_stop(struct sprd_dma_chn *schan)
 	sprd_dma_set_pending(schan, false);
 	sprd_dma_unset_uid(schan);
 	sprd_dma_clear_int(schan);
+	/*
+	 * If 2-stage transfer is used, the configuration must be clear
+	 * when release DMA channel.
+	 */
+	if (schan->chn_mode)
+		sprd_dma_2stage_config(schan, SPRD_DMA_2STAGE_CLEAR);
 	schan->cur_desc = NULL;
 }
 
@@ -757,7 +811,9 @@  static int sprd_dma_fill_desc(struct dma_chan *chan,
 	phys_addr_t llist_ptr;
 
 	if (dir == DMA_MEM_TO_DEV) {
-		src_step = sprd_dma_get_step(slave_cfg->src_addr_width);
+		src_step = slave_cfg->src_port_window_size ?
+			   slave_cfg->src_port_window_size :
+			   sprd_dma_get_step(slave_cfg->src_addr_width);
 		if (src_step < 0) {
 			dev_err(sdev->dma_dev.dev, "invalid source step\n");
 			return src_step;
@@ -773,7 +829,9 @@  static int sprd_dma_fill_desc(struct dma_chan *chan,
 		else
 			dst_step = SPRD_DMA_NONE_STEP;
 	} else {
-		dst_step = sprd_dma_get_step(slave_cfg->dst_addr_width);
+		dst_step = slave_cfg->dst_port_window_size ?
+			   slave_cfg->dst_port_window_size :
+			   sprd_dma_get_step(slave_cfg->dst_addr_width);
 		if (dst_step < 0) {
 			dev_err(sdev->dma_dev.dev, "invalid destination step\n");
 			return dst_step;