[RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring issue

[RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring issue

[yitian]

Designware I2S uses tx empty and rx available signals as the DMA
handshaking signals. during music playing, if XRUN occurs,
i2s_stop() function will be executed and both tx and rx irq are
masked, when music continues to be played, i2s_start() is executed
but both tx and rx irq are not unmasked which cause I2S stop
sending DMA handshaking signal to DMA controller, and it finally
causes music playing will be stopped once XRUN occurs for the first
time.

Signed-off-by: Yitian Bu <yitian.bu@tangramtek.com>
---
changes in V2:
 - add definition for i and irq
---
 sound/soc/dwc/designware_i2s.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index a3e97b4..76b2e19 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -141,13 +141,22 @@ static inline void i2s_clear_irqs(struct dw_i2s_dev
*dev, u32 stream)
 static void i2s_start(struct dw_i2s_dev *dev,
 		      struct snd_pcm_substream *substream)
 {
-
+	u32 i, irq;
 	i2s_write_reg(dev->i2s_base, IER, 1);
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		for (i = 0; i < 4; i++) {
+			irq = i2s_read_reg(dev->i2s_base, IMR(i));
+			i2s_write_reg(dev->i2s_base, IMR(i), irq & ~0x30);
+		}
 		i2s_write_reg(dev->i2s_base, ITER, 1);
-	else
+	} else {
+		for (i = 0; i < 4; i++) {
+			irq = i2s_read_reg(dev->i2s_base, IMR(i));
+			i2s_write_reg(dev->i2s_base, IMR(i), irq & ~0x03);
+		}
 		i2s_write_reg(dev->i2s_base, IRER, 1);
+	}
 
 	i2s_write_reg(dev->i2s_base, CER, 1);
 }
-- 

1.7.12.4

Re: [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring issue

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On Tue, Sep 29, 2015 at 10:43:17PM +0800, yitian wrote:
> Designware I2S uses tx empty and rx available signals as the DMA
> handshaking signals. during music playing, if XRUN occurs,
> i2s_stop() function will be executed and both tx and rx irq are
> masked, when music continues to be played, i2s_start() is executed
> but both tx and rx irq are not unmasked which cause I2S stop
> sending DMA handshaking signal to DMA controller, and it finally
> causes music playing will be stopped once XRUN occurs for the first
> time.

I'm a bit concerned about how this code ever worked given the above
description - is there some race condition which allows things to work
if we're lucky?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

RE: [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring issue

[yitian]

> From: linux-arm-kernel
> [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Mark
> Brown
> Sent: Thursday, October 1, 2015 2:22 AM
> To: yitian <yitian.bu@tangramtek.com>
> Cc: alsa-devel@alsa-project.org; wsa@the-dreams.de;
> linux-kernel@vger.kernel.org; Andrew.Jackson@arm.com; tiwai@suse.com;
> lgirdwood@gmail.com; perex@perex.cz;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring
> issue
> 
> On Tue, Sep 29, 2015 at 10:43:17PM +0800, yitian wrote:
> > Designware I2S uses tx empty and rx available signals as the DMA
> > handshaking signals. during music playing, if XRUN occurs,
> > i2s_stop() function will be executed and both tx and rx irq are
> > masked, when music continues to be played, i2s_start() is executed
> > but both tx and rx irq are not unmasked which cause I2S stop
> > sending DMA handshaking signal to DMA controller, and it finally
> > causes music playing will be stopped once XRUN occurs for the first
> > time.
> 
> I'm a bit concerned about how this code ever worked given the above
> description - is there some race condition which allows things to work
> if we're lucky?

Hi Mark:

Thanks for your comments.
I think maybe two reasons:
1. designware I2S IP in my chipset(new design) is using tx empty and rx
available signal as the DMA handshaking signal, but it may be not true
for all chipsets. If I2S has separate signal as DMA handshaking signal, mask
irq should not impact DMA transfer. But Synopsys's engineer recommend us to
use
tx and rx irq signal as the DMA handshaking signal, meanwhile we cannot find
separate DMA handshaking signal from designware's IP spec, that's why tx/rx
irq
will impact DMA transfer.

2. I am using a FPGA for test, the cpu frequency of it is only 26MHz, that
means
XRUN is very easy to happen on my board. But I guess most of the developers
are using real chipset which can have at least 600MHz frequency so XRUN is
not easy to be reproduced. As my test, No XUN, no this bug...

RE: [alsa-devel] [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring issue

[yitian]

Hi Mark:

> From: alsa-devel-bounces@alsa-project.org
> [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of yitian
> Sent: Thursday, October 1, 2015 10:25 AM
> To: 'Mark Brown' <broonie@kernel.org>
> Cc: alsa-devel@alsa-project.org; wsa@the-dreams.de;
> linux-kernel@vger.kernel.org; Andrew.Jackson@arm.com;
> lgirdwood@gmail.com; tiwai@suse.com;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [alsa-devel] [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop
> transferring issue
> 
> > From: linux-arm-kernel
> > [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of
> Mark
> > Brown
> > Sent: Thursday, October 1, 2015 2:22 AM
> > To: yitian <yitian.bu@tangramtek.com>
> > Cc: alsa-devel@alsa-project.org; wsa@the-dreams.de;
> > linux-kernel@vger.kernel.org; Andrew.Jackson@arm.com;
> tiwai@suse.com;
> > lgirdwood@gmail.com; perex@perex.cz;
> > linux-arm-kernel@lists.infradead.org
> > Subject: Re: [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring
> > issue
> >
> > On Tue, Sep 29, 2015 at 10:43:17PM +0800, yitian wrote:
> > > Designware I2S uses tx empty and rx available signals as the DMA
> > > handshaking signals. during music playing, if XRUN occurs,
> > > i2s_stop() function will be executed and both tx and rx irq are
> > > masked, when music continues to be played, i2s_start() is executed
> > > but both tx and rx irq are not unmasked which cause I2S stop
> > > sending DMA handshaking signal to DMA controller, and it finally
> > > causes music playing will be stopped once XRUN occurs for the first
> > > time.
> >
> > I'm a bit concerned about how this code ever worked given the above
> > description - is there some race condition which allows things to work
> > if we're lucky?
> 
> Hi Mark:
> 
> Thanks for your comments.
> I think maybe two reasons:
> 1. designware I2S IP in my chipset(new design) is using tx empty and rx
> available signal as the DMA handshaking signal, but it may be not true
> for all chipsets. If I2S has separate signal as DMA handshaking signal,
mask
> irq should not impact DMA transfer. But Synopsys's engineer recommend
> us to
> use
> tx and rx irq signal as the DMA handshaking signal, meanwhile we cannot
> find
> separate DMA handshaking signal from designware's IP spec, that's why
> tx/rx
> irq
> will impact DMA transfer.
> 
> 2. I am using a FPGA for test, the cpu frequency of it is only 26MHz, that
> means
> XRUN is very easy to happen on my board. But I guess most of the
> developers
> are using real chipset which can have at least 600MHz frequency so XRUN
> is
> not easy to be reproduced. As my test, No XUN, no this bug...

Do I need to provide anything else for this patch? Thanks.

Re: [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring issue

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

On Thu, Oct 01, 2015 at 10:24:42AM +0800, yitian wrote:

> I think maybe two reasons:
> 1. designware I2S IP in my chipset(new design) is using tx empty and rx
> available signal as the DMA handshaking signal, but it may be not true
> for all chipsets. If I2S has separate signal as DMA handshaking signal, mask
> irq should not impact DMA transfer. But Synopsys's engineer recommend us to
> use
> tx and rx irq signal as the DMA handshaking signal, meanwhile we cannot find
> separate DMA handshaking signal from designware's IP spec, that's why tx/rx
> irq
> will impact DMA transfer.

If that's what the Synopsis engineers recommend I'd guess that other
integrations are doing the same thing.

> 2. I am using a FPGA for test, the cpu frequency of it is only 26MHz, that
> means
> XRUN is very easy to happen on my board. But I guess most of the developers
> are using real chipset which can have at least 600MHz frequency so XRUN is
> not easy to be reproduced. As my test, No XUN, no this bug...

Ah, that might be it - it might just be that other systems are working
due to race conditions.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring issue

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On Tue, Sep 29, 2015 at 10:43:17PM +0800, yitian wrote:

> --- a/sound/soc/dwc/designware_i2s.c
> +++ b/sound/soc/dwc/designware_i2s.c
> @@ -141,13 +141,22 @@ static inline void i2s_clear_irqs(struct dw_i2s_dev
> *dev, u32 stream)

This doesn't apply because it's been corrupted by line wrapping - git am
can't understand it.  Since it's just that one line I fixed it up by
hand but please look at your mail setup to make sure this works (this
might've been what happened with your other patch yesterday).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

RE: [alsa-devel] [RESEND PATCH v2 1/1] ASoC: dwc: fix dma stop transferring issue

[yitian]

Hi Mark:
> This doesn't apply because it's been corrupted by line wrapping - git am
> can't understand it.  Since it's just that one line I fixed it up by
> hand but please look at your mail setup to make sure this works (this
> might've been what happened with your other patch yesterday).

After you said it corrupted git am, I used git send-email to send the
"correct irq clear method" patch and hopefully that was correct.
I will be careful next time, thanks a lot for your patient help.