[PATCH 1/1] dma: imx-sdma: buf_tail should be initialize in prepare function

[PATCH 1/1] dma: imx-sdma: buf_tail should be initialize in prepare function

[Richard Zhao]

This fix audio underrun issue. When SNDRV_PCM_TRIGGER_STOP
and SNDRV_PCM_TRIGGER_START, it calls prepare again. buf_tail
should be reset to zero.
So move buf_tail initialization into prepare function.

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 drivers/dma/imx-sdma.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index a472a29..def08f7 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -814,8 +814,6 @@ static int sdma_request_channel(struct sdma_channel *sdmac)
 
 	init_completion(&sdmac->done);
 
-	sdmac->buf_tail = 0;
-
 	return 0;
 out:
 
@@ -924,6 +922,8 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
 
 	sdmac->flags = 0;
 
+	sdmac->buf_tail = 0;
+
 	dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
 			sg_len, channel);
 
@@ -1024,6 +1024,8 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
 
 	sdmac->status = DMA_IN_PROGRESS;
 
+	sdmac->buf_tail = 0;
+
 	sdmac->flags |= IMX_DMA_SG_LOOP;
 	sdmac->direction = direction;
 	ret = sdma_load_context(sdmac);
-- 
1.7.5.4

Re: [PATCH 1/1] dma: imx-sdma: buf_tail should be initialize in prepare function

[Richard Zhao]

Hi Sascha,

Could you please review the patch?

Thanks
Richard 

Re: [PATCH 1/1] dma: imx-sdma: buf_tail should be initialize in prepare function

[Shawn Guo]

On Mon, May 21, 2012 at 08:31:34PM +0800, Richard Zhao wrote:
> This fix audio underrun issue. When SNDRV_PCM_TRIGGER_STOP
> and SNDRV_PCM_TRIGGER_START, it calls prepare again. buf_tail
> should be reset to zero.
> So move buf_tail initialization into prepare function.
> 
I tried the patch on imx51, and it does not fix the issue for me.

Regards,
Shawn

root@linaro-developer:~# aplay audio8k16S.wav
Playing WAVE 'audio8k16S.wav' : Signed 16 bit Little Endian, Rate 8000 Hz, Stere
o
^Z
[1]+  Stopped                 aplay audio8k16S.wav
root@linaro-developer:~# fg
aplay audio8k16S.wav
underrun!!! (at least 2034.382 ms long)

Re: [PATCH 1/1] dma: imx-sdma: buf_tail should be initialize in prepare function

[Richard Zhao]

On Thu, May 24, 2012 at 11:20:29AM +0800, Shawn Guo wrote:
> On Mon, May 21, 2012 at 08:31:34PM +0800, Richard Zhao wrote:
> > This fix audio underrun issue. When SNDRV_PCM_TRIGGER_STOP
> > and SNDRV_PCM_TRIGGER_START, it calls prepare again. buf_tail
> > should be reset to zero.
> > So move buf_tail initialization into prepare function.
> > 
> I tried the patch on imx51, and it does not fix the issue for me.
> 
> Regards,
> Shawn
> 
> root@linaro-developer:~# aplay audio8k16S.wav
> Playing WAVE 'audio8k16S.wav' : Signed 16 bit Little Endian, Rate 8000 Hz, Stere
> o
> ^Z
> [1]+  Stopped                 aplay audio8k16S.wav
> root@linaro-developer:~# fg
> aplay audio8k16S.wav
> underrun!!! (at least 2034.382 ms long)
Does the audio playback recover? Without the patch, sometimes the
playback will have distortion and play too fast.

Thanks
Richard 

Re: [PATCH 1/1] dma: imx-sdma: buf_tail should be initialize in prepare function

[Shawn Guo]

On Thu, May 24, 2012 at 11:27:00AM +0800, Richard Zhao wrote:
> > root@linaro-developer:~# aplay audio8k16S.wav
> > Playing WAVE 'audio8k16S.wav' : Signed 16 bit Little Endian, Rate 8000 Hz, Stere
> > o
> > ^Z
> > [1]+  Stopped                 aplay audio8k16S.wav
> > root@linaro-developer:~# fg
> > aplay audio8k16S.wav
> > underrun!!! (at least 2034.382 ms long)
> Does the audio playback recover? Without the patch, sometimes the
> playback will have distortion and play too fast.
> 
No, it does not recover.

-- 
Regards,
Shawn

Re: [PATCH 1/1] dma: imx-sdma: buf_tail should be initialize in prepare function

[Richard Zhao]

On Thu, May 24, 2012 at 11:33:58AM +0800, Shawn Guo wrote:
> On Thu, May 24, 2012 at 11:27:00AM +0800, Richard Zhao wrote:
> > > root@linaro-developer:~# aplay audio8k16S.wav
> > > Playing WAVE 'audio8k16S.wav' : Signed 16 bit Little Endian, Rate 8000 Hz, Stere
> > > o
> > > ^Z
> > > [1]+  Stopped                 aplay audio8k16S.wav
> > > root@linaro-developer:~# fg
> > > aplay audio8k16S.wav
> > > underrun!!! (at least 2034.382 ms long)
> > Does the audio playback recover? Without the patch, sometimes the
> > playback will have distortion and play too fast.
> > 
> No, it does not recover.
Maybe it's not the same issue. It do recover on imx6q sabrelite board.
And from source code, it do be a bug.

Thanks
Richard
> 
> -- 
> Regards,
> Shawn
> 

Re: [PATCH 1/1] dma: imx-sdma: buf_tail should be initialize in prepare function

[Shawn Guo]

On Fri, May 25, 2012 at 04:09:44PM +0800, Richard Zhao wrote:
> On Thu, May 24, 2012 at 11:33:58AM +0800, Shawn Guo wrote:
> > On Thu, May 24, 2012 at 11:27:00AM +0800, Richard Zhao wrote:
> > > > root@linaro-developer:~# aplay audio8k16S.wav
> > > > Playing WAVE 'audio8k16S.wav' : Signed 16 bit Little Endian, Rate 8000 Hz, Stere
> > > > o
> > > > ^Z
> > > > [1]+  Stopped                 aplay audio8k16S.wav
> > > > root@linaro-developer:~# fg
> > > > aplay audio8k16S.wav
> > > > underrun!!! (at least 2034.382 ms long)
> > > Does the audio playback recover? Without the patch, sometimes the
> > > playback will have distortion and play too fast.
> > > 
> > No, it does not recover.
> Maybe it's not the same issue. It do recover on imx6q sabrelite board.
> And from source code, it do be a bug.
> 
It seems to be the case, so:

Acked-by: Shawn Guo <shawn.guo@linaro.org>

-- 
Regards,
Shawn