Skip to contentSkip to conversation

dmaengine: sun6i: Fixes and upgrade for audio transfers

PATCH Superseded Jean-Francois Moine 2 participants 10 messages Mar 11, 2016 – Mar 18, 2016

Subsystems (2)
Arm/Allwinner sunXi SoC support dmaengine

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Jean-Francois Moine

author
#0 lore raw

v2 cover dmaengine: sun6i: Fixes and upgrade for audio transfers

This patch series contains the required fixes and changes to the sun6i DMA driver for audio streaming.
These ones have been tested in a Allwinner H3 (Orange PI 2).

Changes since v1 (comments from Vinod Koul and Sergei Shtylyov - thanks)
- typo fixes
- change some comments
- better handling of burst and bus width
- remove useless code in the cyclic capability patch

Jean-Francois Moine (5):
dmaengine: sun6i: Fix the access of the IRQ register
dmaengine: sun6i: Fix impossible settings of burst and bus width
dmaengine: sun6i: Add 4 as a possible burst value
dmaengine: sun6i: Set system values to memory burst and bus width
dmaengine: sun6i: Add cyclic capability

 drivers/dma/sun6i-dma.c | 160 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 146 insertions(+), 14 deletions(-)
Signature · 1 line
2.7.3
author
#1 lore raw

PATCH v2 1/5 dmaengine: sun6i: Fix the access of the IRQ register

The IRQ register number is computed, but this number was not used and the register was the one indexed by the channel index instead. Then, only the first DMA channel was working.

Signed-off-by Jean-Francois Moine <[email protected]>
---
 drivers/dma/sun6i-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/dma/sun6i-dma.c +2 -2
index 2db12e4..2ec320d 100644
@@ -381,9 +381,9 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
 	irq_reg = pchan->idx / DMA_IRQ_CHAN_NR;
 	irq_offset = pchan->idx % DMA_IRQ_CHAN_NR;
 
-	irq_val = readl(sdev->base + DMA_IRQ_EN(irq_offset));
+	irq_val = readl(sdev->base + DMA_IRQ_EN(irq_reg));
 	irq_val |= DMA_IRQ_QUEUE << (irq_offset * DMA_IRQ_CHAN_WIDTH);
-	writel(irq_val, sdev->base + DMA_IRQ_EN(irq_offset));
+	writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
 
 	writel(pchan->desc->p_lli, pchan->base + DMA_CHAN_LLI_ADDR);
 	writel(DMA_CHAN_ENABLE_START, pchan->base + DMA_CHAN_ENABLE);
Signature · 1 line
2.7.3
author
#2 lore raw

PATCH v2 2/5 dmaengine: sun6i: Fix impossible settings of burst and bus width

In reply to Jean-Francois Moine

In the commit 1f9cd915b64bb95f (dmaengine: sun6i: Fix memcpy operation), the signed values returned by convert_burst() and convert_buswidth() were stored in an unsigned value.
Then, these values were considered as errors when non null.

As a result, DMA transfers were rejected when the burst or buswidth had values different from 1, as 8 for the burst or 4 for the bus width.

Signed-off-by Jean-Francois Moine <[email protected]>
---
 drivers/dma/sun6i-dma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/dma/sun6i-dma.c +5 -5
index 2ec320d..3579ee7 100644
@@ -281,25 +281,25 @@ static inline int sun6i_dma_cfg_lli(struct sun6i_dma_lli *lli,
 				    dma_addr_t dst, u32 len,
 				    struct dma_slave_config *config)
 {
-	u8 src_width, dst_width, src_burst, dst_burst;
+	s8 src_width, dst_width, src_burst, dst_burst;
 
 	if (!config)
 		return -EINVAL;
 
 	src_burst = convert_burst(config->src_maxburst);
-	if (src_burst)
+	if (src_burst < 0)
 		return src_burst;
 
 	dst_burst = convert_burst(config->dst_maxburst);
-	if (dst_burst)
+	if (dst_burst < 0)
 		return dst_burst;
 
 	src_width = convert_buswidth(config->src_addr_width);
-	if (src_width)
+	if (src_width < 0)
 		return src_width;
 
 	dst_width = convert_buswidth(config->dst_addr_width);
-	if (dst_width)
+	if (dst_width < 0)
 		return dst_width;
 
 	lli->cfg = DMA_CHAN_CFG_SRC_BURST(src_burst) |
Signature · 1 line
2.7.3
author
#3 lore raw

PATCH v2 3/5 dmaengine: sun6i: Add 4 as a possible burst value

In reply to Jean-Francois Moine

Some DMA transfers, as for H3 audio, ask for 4 as a burst value.

Signed-off-by Jean-Francois Moine <[email protected]>
---
 drivers/dma/sun6i-dma.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/dma/sun6i-dma.c +2 -0
index 3579ee7..7c98c0d 100644
@@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;
 	case 8:
 		return 2;
 	default:
Signature · 1 line
2.7.3
#4 lore raw

PATCH v2 3/5 dmaengine: sun6i: Add 4 as a possible burst value

Quoted 12 lines · Jean-Francois Moine

Some DMA transfers, as for H3 audio, ask for 4 as a burst value.

Signed-off-by Jean-Francois Moine <[email protected]>
---
 drivers/dma/sun6i-dma.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/dma/sun6i-dma.c +0 -0
index 3579ee7..7c98c0d 100644

drivers/dma/sun6i-dma.c static inline s8 convert_burst(u32 maxburst) · line 242

Quoted 5 lines
drivers/dma/sun6i-dma.c +2 -0
@@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;

This is true only for the H3.

For the other SoCs that we support, the only valid values are 0 and 2, so we need to reject those values.

We should do that based on the compatible.

The easiest solution would be to expose the available burst sizes in the probe, and just our new one if we match that compatible, and any invalid burst size would be rejected by the framework. Vinod, any objection to that?

Thanks!
Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160311/7c32487f/attachment.sig>
author
#5 lore raw

PATCH v2 3/5 dmaengine: sun6i: Add 4 as a possible burst value

Quoted 2 lines · On Fri, 11 Mar 2016 13:06:01 +0100 Maxime Ripard…
Quoted 1 line · Jean-Francois Moine

Some DMA transfers, as for H3 audio, ask for 4 as a burst value.

	[snip]
Quoted 12 lines

drivers/dma/sun6i-dma.c static inline s8 convert_burst(u32 maxburst) · line 242

Quoted 6 lines
drivers/dma/sun6i-dma.c +2 -0
@@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;

This is true only for the H3.

For the other SoCs that we support, the only valid values are 0 and 2, so we need to reject those values.

Quoted 6 lines

We should do that based on the compatible.

The easiest solution would be to expose the available burst sizes in the probe, and just our new one if we match that compatible, and any invalid burst size would be rejected by the framework. Vinod, any objection to that?

Do you think that we should also check if the requested ports are valid, i.e. have a list/bitmap of the possible input/output ports per SoC, instead of just only the ID of the max port?

Signature · 2 lines
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
#6 lore raw

PATCH v2 3/5 dmaengine: sun6i: Add 4 as a possible burst value

Quoted 20 lines · Jean-Francois Moine
Quoted 2 lines · On Fri, 11 Mar 2016 13:06:01 +0100 Maxime Ripard…
Quoted 1 line · Jean-Francois Moine

Some DMA transfers, as for H3 audio, ask for 4 as a burst value.

	[snip]
Quoted 14 lines

drivers/dma/sun6i-dma.c static inline s8 convert_burst(u32 maxburst) · line 242

Quoted 6 lines
drivers/dma/sun6i-dma.c +2 -0
@@ -238,6 +238,8 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;

This is true only for the H3.

For the other SoCs that we support, the only valid values are 0 and 2, so we need to reject those values.

We should do that based on the compatible.

Quoted 8 lines
Quoted 4 lines

The easiest solution would be to expose the available burst sizes in the probe, and just our new one if we match that compatible, and any invalid burst size would be rejected by the framework. Vinod, any objection to that?

Do you think that we should also check if the requested ports are valid, i.e. have a list/bitmap of the possible input/output ports per SoC, instead of just only the ID of the max port?

That would require having a map of the possible endpoints for a given device, which would be quite difficult to accomplish.

I don't think customer drivers have access to that anyway, only the provider driver is parsing the DT to retrieve the ID. And we already trust the DT for so many things it would be a bit pointless to check that.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160318/6b12d817/attachment.sig>
author
#7 lore raw

PATCH v2 3/5 dmaengine: sun6i: Add 4 as a possible burst value

Quoted 1 line · On Fri, 18 Mar 2016 16:09:46 +0100 Maxime Ripard…

On Fri, Mar 11, 2016 at 05:28:58PM +0100, Jean-Francois Moine wrote:

	[snip]
Quoted 11 lines
Quoted 3 lines

Do you think that we should also check if the requested ports are valid, i.e. have a list/bitmap of the possible input/output ports per SoC, instead of just only the ID of the max port?

That would require having a map of the possible endpoints for a given device, which would be quite difficult to accomplish.

I don't think customer drivers have access to that anyway, only the provider driver is parsing the DT to retrieve the ID. And we already trust the DT for so many things it would be a bit pointless to check that.

The ports are already checked against 'nr_max_requests' which is the index of the higher port. I think that this check exists because the ports are given by the DMA clients.

If this check is needed, as 'nr_max_requests' is lower than 32, 2 maps (in and out) on 32 bits would do the job.
If this check is not needed (because we trust the DTs), the actual check (port <= nr_max_requests) could be removed.

Signature · 2 lines
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
author
#8 lore raw

PATCH v2 4/5 dmaengine: sun6i: Set system values to memory burst and bus width

In reply to Jean-Francois Moine

When defining the DMA transfer, the sound PCM DMA engine sets only the burst and bus width values of the device.
This was making the audio transfers to be rejected because of invalid values on the memory side.

Signed-off-by Jean-Francois Moine <[email protected]>
---
 drivers/dma/sun6i-dma.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
drivers/dma/sun6i-dma.c +18 -0
index 7c98c0d..0563fb3 100644
@@ -504,6 +504,22 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 	return ret;
 }
 
+static void sanitize_config(struct dma_slave_config *sconfig,
+			   enum dma_transfer_direction direction)
+{
+	if (direction == DMA_MEM_TO_DEV) {
+		if (sconfig->src_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+			sconfig->src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		if (!sconfig->src_maxburst)
+			sconfig->src_maxburst = 8;
+	} else {	// DMA_DEV_TO_MEM
+		if (sconfig->dst_addr_width == DMA_SLAVE_BUSWIDTH_UNDEFINED)
+			sconfig->dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		if (!sconfig->dst_maxburst)
+			sconfig->dst_maxburst = 8;
+	}
+}
+
 static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 		struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 		size_t len, unsigned long flags)

@@ -585,6 +601,8 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 	if (!txd)
 		return NULL;
 
+	sanitize_config(sconfig, dir);
+
 	for_each_sg(sgl, sg, sg_len, i) {
 		v_lli = dma_pool_alloc(sdev->pool, GFP_NOWAIT, &p_lli);
 		if (!v_lli)
Signature · 1 line
2.7.3
author
#9 lore raw

PATCH v2 5/5 dmaengine: sun6i: Add cyclic capability

In reply to Jean-Francois Moine

DMA cyclic transfers are required by audio streaming.

Signed-off-by Jean-Francois Moine <[email protected]>
---
 drivers/dma/sun6i-dma.c | 126 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 119 insertions(+), 7 deletions(-)
drivers/dma/sun6i-dma.c +119 -7
index 0563fb3..8e42c1d 100644
@@ -146,6 +146,8 @@ struct sun6i_vchan {
 	struct dma_slave_config	cfg;
 	struct sun6i_pchan	*phy;
 	u8			port;
+	u8			irq_type;
+	bool			cyclic;
 };
 
 struct sun6i_dma_dev {

@@ -256,6 +258,30 @@ static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
 	return addr_width >> 1;
 }
 
+static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
+{
+	struct sun6i_desc *txd = pchan->desc;
+	struct sun6i_dma_lli *lli;
+	size_t bytes;
+	dma_addr_t pos;
+
+	pos = readl(pchan->base + DMA_CHAN_LLI_ADDR);
+	bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
+
+	if (pos == LLI_LAST_ITEM)
+		return bytes;
+
+	for (lli = txd->v_lli; lli; lli = lli->v_lli_next) {
+		if (lli->p_lli_next == pos) {
+			for (lli = lli->v_lli_next; lli; lli = lli->v_lli_next)
+				bytes += lli->len;
+			break;
+		}
+	}
+
+	return bytes;
+}
+
 static void *sun6i_dma_lli_add(struct sun6i_dma_lli *prev,
 			       struct sun6i_dma_lli *next,
 			       dma_addr_t next_phy,

@@ -383,8 +409,12 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
 	irq_reg = pchan->idx / DMA_IRQ_CHAN_NR;
 	irq_offset = pchan->idx % DMA_IRQ_CHAN_NR;
 
+	vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
+
 	irq_val = readl(sdev->base + DMA_IRQ_EN(irq_reg));
-	irq_val |= DMA_IRQ_QUEUE << (irq_offset * DMA_IRQ_CHAN_WIDTH);
+	irq_val &= ~((DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) <<
+			(irq_offset * DMA_IRQ_CHAN_WIDTH));
+	irq_val |= vchan->irq_type << (irq_offset * DMA_IRQ_CHAN_WIDTH);
 	writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
 
 	writel(pchan->desc->p_lli, pchan->base + DMA_CHAN_LLI_ADDR);

@@ -481,11 +511,12 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 		writel(status, sdev->base + DMA_IRQ_STAT(i));
 
 		for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
-			if (status & DMA_IRQ_QUEUE) {
-				pchan = sdev->pchans + j;
-				vchan = pchan->vchan;
-
-				if (vchan) {
+			pchan = sdev->pchans + j;
+			vchan = pchan->vchan;
+			if (vchan && (status & vchan->irq_type)) {
+				if (vchan->cyclic) {
+					vchan_cyclic_callback(&pchan->desc->vd);
+				} else {
 					spin_lock(&vchan->vc.lock);
 					vchan_cookie_complete(&pchan->desc->vd);
 					pchan->done = pchan->desc;

@@ -663,6 +694,75 @@ err_lli_free:
 	return NULL;
 }
 
+static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
+					struct dma_chan *chan,
+					dma_addr_t buf_addr,
+					size_t buf_len,
+					size_t period_len,
+					enum dma_transfer_direction dir,
+					unsigned long flags)
+{
+	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
+	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
+	struct dma_slave_config *sconfig = &vchan->cfg;
+	struct sun6i_dma_lli *v_lli, *prev = NULL;
+	struct sun6i_desc *txd;
+	dma_addr_t p_lli;
+	unsigned int i, periods = buf_len / period_len;
+	int ret;
+
+	txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
+	if (!txd)
+		return NULL;
+
+	sanitize_config(sconfig, dir);
+
+	for (i = 0; i < periods; i++){
+		v_lli = dma_pool_alloc(sdev->pool, GFP_NOWAIT, &p_lli);
+		if (!v_lli) {
+			dev_err(sdev->slave.dev, "Failed to alloc lli memory\n");
+			goto err_lli_free;
+		}
+		if (dir == DMA_MEM_TO_DEV) {
+			ret = sun6i_dma_cfg_lli(v_lli,
+						buf_addr + period_len * i,
+						sconfig->dst_addr,
+						period_len, sconfig);
+			v_lli->cfg |= DMA_CHAN_CFG_DST_IO_MODE |
+					DMA_CHAN_CFG_SRC_LINEAR_MODE |
+					DMA_CHAN_CFG_SRC_DRQ(DRQ_SDRAM) |
+					DMA_CHAN_CFG_DST_DRQ(vchan->port);
+		} else {
+			ret = sun6i_dma_cfg_lli(v_lli,
+						sconfig->src_addr,
+						buf_addr + period_len * i,
+						period_len, sconfig);
+			v_lli->cfg |= DMA_CHAN_CFG_DST_LINEAR_MODE |
+					DMA_CHAN_CFG_SRC_IO_MODE |
+					DMA_CHAN_CFG_DST_DRQ(DRQ_SDRAM) |
+					DMA_CHAN_CFG_SRC_DRQ(vchan->port);
+		}
+		if (ret < 0)
+			goto err_cur_lli_free;
+
+		prev = sun6i_dma_lli_add(prev, v_lli, p_lli, txd);
+	}
+
+	prev->p_lli_next = txd->p_lli;		/* cyclic list */
+
+	vchan->cyclic = true;
+
+	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
+
+err_cur_lli_free:
+	dma_pool_free(sdev->pool, v_lli, p_lli);
+err_lli_free:
+	for (prev = txd->v_lli; prev; prev = prev->v_lli_next)
+		dma_pool_free(sdev->pool, prev, virt_to_phys(prev));
+	kfree(txd);
+	return NULL;
+}
+
 static int sun6i_dma_config(struct dma_chan *chan,
 			    struct dma_slave_config *config)
 {

@@ -732,6 +832,16 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
 
 	spin_lock_irqsave(&vchan->vc.lock, flags);
 
+	if (vchan->cyclic) {
+		vchan->cyclic = false;
+		if (pchan && pchan->desc) {
+			struct virt_dma_desc *vd = &pchan->desc->vd;
+			struct virt_dma_chan *vc = &vchan->vc;
+
+			list_add_tail(&vd->node, &vc->desc_completed);
+		}
+	}
+
 	vchan_get_all_descriptors(&vchan->vc, &head);
 
 	if (pchan) {

@@ -779,7 +889,7 @@ static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan,
 	} else if (!pchan || !pchan->desc) {
 		bytes = 0;
 	} else {
-		bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
+		bytes = sun6i_get_chan_size(pchan);
 	}
 
 	spin_unlock_irqrestore(&vchan->vc.lock, flags);

@@ -983,6 +1093,7 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	dma_cap_set(DMA_PRIVATE, sdc->slave.cap_mask);
 	dma_cap_set(DMA_MEMCPY, sdc->slave.cap_mask);
 	dma_cap_set(DMA_SLAVE, sdc->slave.cap_mask);
+	dma_cap_set(DMA_CYCLIC, sdc->slave.cap_mask);
 
 	INIT_LIST_HEAD(&sdc->slave.channels);
 	sdc->slave.device_free_chan_resources	= sun6i_dma_free_chan_resources;

@@ -990,6 +1101,7 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->slave.device_issue_pending		= sun6i_dma_issue_pending;
 	sdc->slave.device_prep_slave_sg		= sun6i_dma_prep_slave_sg;
 	sdc->slave.device_prep_dma_memcpy	= sun6i_dma_prep_dma_memcpy;
+	sdc->slave.device_prep_dma_cyclic	= sun6i_dma_prep_dma_cyclic;
 	sdc->slave.copy_align			= DMAENGINE_ALIGN_4_BYTES;
 	sdc->slave.device_config		= sun6i_dma_config;
 	sdc->slave.device_pause			= sun6i_dma_pause;
Signature · 1 line
2.7.3