[alsa-devel] [PATCH 2/2] ASoC: omap: Stop DMA re enabling in self linkage mode
From: Hari Nagalla hnagalla@ti.com
While using self linking, there is a chance that the DMA has re-enabled the channel just after disabling it.
This patch stops the OMAP4 DMA re-enabling after stoping the DMA channel.
Signed-off-by: Hari Nagalla hnagalla@ti.com Signed-off-by: Margarita Olaya Cabrera magi.olaya@ti.com --- sound/soc/omap/omap-pcm.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 6a21447..afe91ad 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -234,6 +234,13 @@ static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd) case SNDRV_PCM_TRIGGER_PAUSE_PUSH: prtd->period_index = -1; omap_stop_dma(prtd->dma_ch); + /* + * Since we are using self linking, there is a + * chance that the DMA as re-enabled the channel + * just after disabling it. + */ + while (omap_get_dma_active_status(prtd->dma_ch)) + omap_stop_dma(prtd->dma_ch); break; default: ret = -EINVAL;
On Mon, Dec 06, 2010 at 04:34:47PM -0600, Olaya, Margarita wrote:
From: Hari Nagalla hnagalla@ti.com
While using self linking, there is a chance that the DMA has re-enabled the channel just after disabling it.
This patch stops the OMAP4 DMA re-enabling after stoping the DMA channel.
Acked-by: Mark Brown broonie@opensource.wolfsonmicro.com
On Tuesday 07 December 2010 00:34:47 ext Olaya, Margarita wrote:
From: Hari Nagalla hnagalla@ti.com
While using self linking, there is a chance that the DMA has re-enabled the channel just after disabling it.
This patch stops the OMAP4 DMA re-enabling after stoping the DMA channel.
I have not seen this happening on OMAP2/3, or at least I'm not aware of it. Is this problem OMAP4 only? Do you know if this happens on playback, capture or in both direction? Is it worth to do this workaround only on OMAP4?
Signed-off-by: Hari Nagalla hnagalla@ti.com Signed-off-by: Margarita Olaya Cabrera magi.olaya@ti.com
sound/soc/omap/omap-pcm.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 6a21447..afe91ad 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -234,6 +234,13 @@ static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd) case SNDRV_PCM_TRIGGER_PAUSE_PUSH: prtd->period_index = -1; omap_stop_dma(prtd->dma_ch);
/** Since we are using self linking, there is a* chance that the DMA as re-enabled the channel* just after disabling it.*/while (omap_get_dma_active_status(prtd->dma_ch))omap_stop_dma(prtd->dma_ch);
On Tue, Dec 07, 2010 at 11:17:26AM +0200, Peter Ujfalusi wrote:
I have not seen this happening on OMAP2/3, or at least I'm not aware of it. Is this problem OMAP4 only? Do you know if this happens on playback, capture or in both direction? Is it worth to do this workaround only on OMAP4?
Given that the patch should cause at most a single read from a register when tearing down DMA if it's not required I'd guess it's safer to just unconditionally enable the workaround on the off chance that it's just reallly rare rather than not needed?
On Tuesday 07 December 2010 16:05:12 ext Mark Brown wrote:
On Tue, Dec 07, 2010 at 11:17:26AM +0200, Peter Ujfalusi wrote:
I have not seen this happening on OMAP2/3, or at least I'm not aware of it. Is this problem OMAP4 only? Do you know if this happens on playback, capture or in both direction? Is it worth to do this workaround only on OMAP4?
Given that the patch should cause at most a single read from a register when tearing down DMA if it's not required I'd guess it's safer to just unconditionally enable the workaround on the off chance that it's just reallly rare rather than not needed?
Sure, it is a single read to a register, but generally I'm a bit nervous, when we have a while loop without timeout counter. In theory we could have infinite loop, if the HW has a bad day... This could be safe on all OMAP platforms, but I think it has been only tested on OMAP4. If this could happen, than IMHO it has to be handled by the omap_stop_dma, since other drivers could be hit by the same problem (there might be ERRATA for it already for OMAP4).
On Tue, Dec 07, 2010 at 04:38:35PM +0200, Peter Ujfalusi wrote:
Sure, it is a single read to a register, but generally I'm a bit nervous, when we have a while loop without timeout counter. In theory we could have infinite loop, if the HW has a bad day... This could be safe on all OMAP platforms, but I think it has been only tested on OMAP4. If this could happen, than IMHO it has to be handled by the omap_stop_dma, since other drivers could be hit by the same problem (there might be ERRATA for it already for OMAP4).
A timeout would be better in general, yes.
On Tue, 2010-12-07 at 16:38 +0200, Peter Ujfalusi wrote:
On Tuesday 07 December 2010 16:05:12 ext Mark Brown wrote:
On Tue, Dec 07, 2010 at 11:17:26AM +0200, Peter Ujfalusi wrote:
I have not seen this happening on OMAP2/3, or at least I'm not aware of it. Is this problem OMAP4 only? Do you know if this happens on playback, capture or in both direction? Is it worth to do this workaround only on OMAP4?
Given that the patch should cause at most a single read from a register when tearing down DMA if it's not required I'd guess it's safer to just unconditionally enable the workaround on the off chance that it's just reallly rare rather than not needed?
Sure, it is a single read to a register, but generally I'm a bit nervous, when we have a while loop without timeout counter. In theory we could have infinite loop, if the HW has a bad day... This could be safe on all OMAP platforms, but I think it has been only tested on OMAP4. If this could happen, than IMHO it has to be handled by the omap_stop_dma, since other drivers could be hit by the same problem (there might be ERRATA for it already for OMAP4).
Afaik, this has also been tested on OMAP3. Not sure about OMAP2 and most likely not tested on OMAP1.
Liam
On Tue, 7 Dec 2010 16:38:35 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
If this could happen, than IMHO it has to be handled by the omap_stop_dma, since other drivers could be hit by the same problem (there might be ERRATA for it already for OMAP4).
I agree with Peter that all DMA stopping related workarounds should be in omap_stop_dma. I'd expect that DMA is stopped when returning from there. Commit 9da65a9 was dealing with similar issue so probably this looping should be implemented somewhere around those lines reading & clearing the _CCR_EN bit?
participants (5)
-
Jarkko Nikula -
Liam Girdwood -
Mark Brown -
Olaya, Margarita -
Peter Ujfalusi