[PATCH v2 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers

[PATCH v2 0/5] dmaengine: sun6i: Fixes and upgrade for audio transfers

[Jean-Francois Moine]

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(-)

-- 
2.7.3

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

[Jean-Francois Moine]

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 <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 2db12e4..2ec320d 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -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);
-- 
2.7.3

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

[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 <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 2ec320d..3579ee7 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -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) |
-- 
2.7.3

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

[Jean-Francois Moine]

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

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 3579ee7..7c98c0d 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -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:
-- 
2.7.3

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

[Maxime Ripard]

On Fri, Mar 11, 2016 at 12:01:29PM +0100, Jean-Francois Moine wrote:
> Some DMA transfers, as for H3 audio, ask for 4 as a burst value.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
>  drivers/dma/sun6i-dma.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 3579ee7..7c98c0d 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -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

-- 
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>

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

[Jean-Francois Moine]

On Fri, 11 Mar 2016 13:06:01 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> On Fri, Mar 11, 2016 at 12:01:29PM +0100, Jean-Francois Moine wrote:
> > Some DMA transfers, as for H3 audio, ask for 4 as a burst value.
	[snip]
> > @@ -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?

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?

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

[Maxime Ripard]

On Fri, Mar 11, 2016 at 05:28:58PM +0100, Jean-Francois Moine wrote:
> On Fri, 11 Mar 2016 13:06:01 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> 
> > On Fri, Mar 11, 2016 at 12:01:29PM +0100, Jean-Francois Moine wrote:
> > > Some DMA transfers, as for H3 audio, ask for 4 as a burst value.
> 	[snip]
> > > @@ -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?
> 
> 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

-- 
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>

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

[Jean-Francois Moine]

On Fri, 18 Mar 2016 16:09:46 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> On Fri, Mar 11, 2016 at 05:28:58PM +0100, Jean-Francois Moine wrote:
	[snip]
> > 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.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

[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 <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 7c98c0d..0563fb3 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -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)
-- 
2.7.3

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

[Jean-Francois Moine]

DMA cyclic transfers are required by audio streaming.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/dma/sun6i-dma.c | 126 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 119 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 0563fb3..8e42c1d 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -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;
-- 
2.7.3