diff mbox

[v2] dmaengine: pxa_dma: fix cyclic transfers

Message ID 1455659642-7746-1-git-send-email-robert.jarzmik@free.fr (mailing list archive)
State Accepted
Headers show

Commit Message

Robert Jarzmik Feb. 16, 2016, 9:54 p.m. UTC
While testing audio with pxa2xx-ac97, underrun were happening while the
user application was correctly feeding the music. Debug proved that the
cyclic transfer is not cyclic, ie. the last descriptor did not loop on
the first.

Another issue is that the descriptor length was always set to 8192,
because of an trivial operator issue.

This was tested on a pxa27x platform.

Fixes: a57e16cf0333 ("dmaengine: pxa: add pxa dmaengine driver")
Reported-by: Vasily Khoruzhick <anarsoul@gmail.com>
Tested-by: Vasily Khoruzhick <anarsoul@gmail.com>
Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
Since v1: add the mask fix suggested by Vasily
---
 drivers/dma/pxa_dma.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Robert Jarzmik Feb. 26, 2016, 10:22 p.m. UTC | #1
Robert Jarzmik <robert.jarzmik@free.fr> writes:

> While testing audio with pxa2xx-ac97, underrun were happening while the
> user application was correctly feeding the music. Debug proved that the
> cyclic transfer is not cyclic, ie. the last descriptor did not loop on
> the first.
>
> Another issue is that the descriptor length was always set to 8192,
> because of an trivial operator issue.
>
> This was tested on a pxa27x platform.
>
> Fixes: a57e16cf0333 ("dmaengine: pxa: add pxa dmaengine driver")
> Reported-by: Vasily Khoruzhick <anarsoul@gmail.com>
> Tested-by: Vasily Khoruzhick <anarsoul@gmail.com>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v1: add the mask fix suggested by Vasily
Hi Vinod,

Could you consider this patch, it's a fix and I'd like to have it reviewed.

Cheers.

--
Robert

[1] The patch on cyclic transfers
> ---
>  drivers/dma/pxa_dma.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index 8ab4a53e5660..77c1c44009d8 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c
> @@ -586,6 +586,8 @@ static void set_updater_desc(struct pxad_desc_sw *sw_desc,
>  		(PXA_DCMD_LENGTH & sizeof(u32));
>  	if (flags & DMA_PREP_INTERRUPT)
>  		updater->dcmd |= PXA_DCMD_ENDIRQEN;
> +	if (sw_desc->cyclic)
> +		sw_desc->hw_desc[sw_desc->nb_desc - 2]->ddadr = sw_desc->first;
>  }
>  
>  static bool is_desc_completed(struct virt_dma_desc *vd)
> @@ -676,6 +678,10 @@ static irqreturn_t pxad_chan_handler(int irq, void *dev_id)
>  		dev_dbg(&chan->vc.chan.dev->device,
>  			"%s(): checking txd %p[%x]: completed=%d\n",
>  			__func__, vd, vd->tx.cookie, is_desc_completed(vd));
> +		if (to_pxad_sw_desc(vd)->cyclic) {
> +			vchan_cyclic_callback(vd);
> +			break;
> +		}
>  		if (is_desc_completed(vd)) {
>  			list_del(&vd->node);
>  			vchan_cookie_complete(vd);
> @@ -1084,7 +1090,7 @@ pxad_prep_dma_cyclic(struct dma_chan *dchan,
>  		return NULL;
>  
>  	pxad_get_config(chan, dir, &dcmd, &dsadr, &dtadr);
> -	dcmd |= PXA_DCMD_ENDIRQEN | (PXA_DCMD_LENGTH | period_len);
> +	dcmd |= PXA_DCMD_ENDIRQEN | (PXA_DCMD_LENGTH & period_len);
>  	dev_dbg(&chan->vc.chan.dev->device,
>  		"%s(): buf_addr=0x%lx len=%zu period=%zu dir=%d flags=%lx\n",
>  		__func__, (unsigned long)buf_addr, len, period_len, dir, flags);
Vinod Koul March 3, 2016, 3:37 p.m. UTC | #2
On Tue, Feb 16, 2016 at 10:54:02PM +0100, Robert Jarzmik wrote:
> While testing audio with pxa2xx-ac97, underrun were happening while the
> user application was correctly feeding the music. Debug proved that the
> cyclic transfer is not cyclic, ie. the last descriptor did not loop on
> the first.
> 
> Another issue is that the descriptor length was always set to 8192,
> because of an trivial operator issue.
> 
> This was tested on a pxa27x platform.

Applied, thanks
Robert Jarzmik March 3, 2016, 5:19 p.m. UTC | #3
Vinod Koul <vinod.koul@intel.com> writes:

> On Tue, Feb 16, 2016 at 10:54:02PM +0100, Robert Jarzmik wrote:
>> While testing audio with pxa2xx-ac97, underrun were happening while the
>> user application was correctly feeding the music. Debug proved that the
>> cyclic transfer is not cyclic, ie. the last descriptor did not loop on
>> the first.
>> 
>> Another issue is that the descriptor length was always set to 8192,
>> because of an trivial operator issue.
>> 
>> This was tested on a pxa27x platform.
>
> Applied, thanks
Thanks Vinod !

Cheers.
diff mbox

Patch

diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index 8ab4a53e5660..77c1c44009d8 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -586,6 +586,8 @@  static void set_updater_desc(struct pxad_desc_sw *sw_desc,
 		(PXA_DCMD_LENGTH & sizeof(u32));
 	if (flags & DMA_PREP_INTERRUPT)
 		updater->dcmd |= PXA_DCMD_ENDIRQEN;
+	if (sw_desc->cyclic)
+		sw_desc->hw_desc[sw_desc->nb_desc - 2]->ddadr = sw_desc->first;
 }
 
 static bool is_desc_completed(struct virt_dma_desc *vd)
@@ -676,6 +678,10 @@  static irqreturn_t pxad_chan_handler(int irq, void *dev_id)
 		dev_dbg(&chan->vc.chan.dev->device,
 			"%s(): checking txd %p[%x]: completed=%d\n",
 			__func__, vd, vd->tx.cookie, is_desc_completed(vd));
+		if (to_pxad_sw_desc(vd)->cyclic) {
+			vchan_cyclic_callback(vd);
+			break;
+		}
 		if (is_desc_completed(vd)) {
 			list_del(&vd->node);
 			vchan_cookie_complete(vd);
@@ -1084,7 +1090,7 @@  pxad_prep_dma_cyclic(struct dma_chan *dchan,
 		return NULL;
 
 	pxad_get_config(chan, dir, &dcmd, &dsadr, &dtadr);
-	dcmd |= PXA_DCMD_ENDIRQEN | (PXA_DCMD_LENGTH | period_len);
+	dcmd |= PXA_DCMD_ENDIRQEN | (PXA_DCMD_LENGTH & period_len);
 	dev_dbg(&chan->vc.chan.dev->device,
 		"%s(): buf_addr=0x%lx len=%zu period=%zu dir=%d flags=%lx\n",
 		__func__, (unsigned long)buf_addr, len, period_len, dir, flags);