[alsa-devel] channel swapping issue on OMAP3/TWL4030 is back
Hello,
I'm running Linux 3.7 on a beagle-xm and observe very reproducible stereo channel swapping issue on playback of a stereo stream (one channel has a sine, the other is zero); the channel swap occurs on starting the playback
with a scope I have observed that the audio data indeed swaps relative to the FSX; so the problem is on the I2S bus
I see the issue with the TWL4030 audio codec; McBSP2 is slave, TWL4030 is master
on a custom board I see the same issue with a TLV320AIC3104 on McBSP3 (have tried to configured the TLV320AIC3104 as slave and master, same channel swapping issue for both configs)
the channel swapping issue has been discussed here, http://comments.gmane.org/gmane.linux.alsa.devel/82178, and patches have been pointed out for 2.6.32
it appears that the swapping issue is back, probably due to the DMA rework in recent kernel?
can someone confirm the issue? any suggestions?
thanks, regards, p.
On 22.03.2013 09:48, Peter Meerwald wrote:
I'm running Linux 3.7 on a beagle-xm and observe very reproducible stereo channel swapping issue on playback of a stereo stream (one channel has a sine, the other is zero); the channel swap occurs on starting the playback
Always? Or in like 50% of the runs? And is the record stream (IOW: the clocks) already running when you switch on playback?
Maybe it's unreleated, but your report reminded me of this commit, which fixed a similar bug for pxa socs: 273b72c8c ("ASoC: pxa-ssp: atomically set stream active masks").
Daniel
I'm running Linux 3.7 on a beagle-xm and observe very reproducible stereo channel swapping issue on playback of a stereo stream (one channel has a sine, the other is zero); the channel swap occurs on starting the playback
Always? Or in like 50% of the runs? And is the record stream (IOW: the clocks) already running when you switch on playback?
in 10 runs, the swap can maybe observed 3 times; so: not always
I can easily turn on/off the clock for the McBSP3/TLV320AIC3104 setup; with McBSP3 being master I ONLY have the issue when the clock is already running; when I start the stream first, then turn on the clock, there is NO SWAP
I am not sure about the McBSP2/TWL4030 setup
Maybe it's unreleated, but your report reminded me of this commit, which fixed a similar bug for pxa socs: 273b72c8c ("ASoC: pxa-ssp: atomically set stream active masks").
will have a look, thanks!
p.
On 22.03.2013 10:03, Peter Meerwald wrote:
I'm running Linux 3.7 on a beagle-xm and observe very reproducible stereo channel swapping issue on playback of a stereo stream (one channel has a sine, the other is zero); the channel swap occurs on starting the playback
Always? Or in like 50% of the runs? And is the record stream (IOW: the clocks) already running when you switch on playback?
in 10 runs, the swap can maybe observed 3 times; so: not always
I can easily turn on/off the clock for the McBSP3/TLV320AIC3104 setup; with McBSP3 being master I ONLY have the issue when the clock is already running; when I start the stream first, then turn on the clock, there is NO SWAP
Yes, that sounds familiar. I guess the problem is that the serial line driver does not respect a specific slope (like lo->hi) but will sync up to *any* slope once it gets its enable bit set. Which will give you arbitrary channel swaps.
Daniel
I'm running Linux 3.7 on a beagle-xm and observe very reproducible stereo channel swapping issue on playback of a stereo stream (one channel has a sine, the other is zero); the channel swap occurs on starting the playback
it appears that the swapping issue is back, probably due to the DMA rework in recent kernel?
kernel 3.4 does not have the issue
regards, p.
On 22.03.2013 10:56, Peter Meerwald wrote:
I'm running Linux 3.7 on a beagle-xm and observe very reproducible stereo channel swapping issue on playback of a stereo stream (one channel has a sine, the other is zero); the channel swap occurs on starting the playback
it appears that the swapping issue is back, probably due to the DMA rework in recent kernel?
kernel 3.4 does not have the issue
Well, then, I guess, you'll have to bisect it down.
Daniel
I'm running Linux 3.7 on a beagle-xm and observe very reproducible stereo channel swapping issue on playback of a stereo stream (one channel has a sine, the other is zero); the channel swap occurs on starting the playback
it appears that the swapping issue is back, probably due to the DMA rework in recent kernel?
kernel 3.4 does not have the issue
Well, then, I guess, you'll have to bisect it down.
here it is: 946cc36ae550ea52adee0f42ac5034a34b5393be is the first bad commit commit 946cc36ae550ea52adee0f42ac5034a34b5393be Author: Peter Ujfalusi peter.ujfalusi@ti.com Date: Fri Sep 14 15:05:58 2012 +0300
ASoC: omap-pcm: Convert to use dmaengine
Original author: Russell King rmk+kernel@arm.linux.org.uk
Switch the omap-pcm to use dmaengine. Certain features are not supported by after dmaengine conversion: 1. No period wakeup mode DMA engine has no way to communicate this information through standard channels.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Tested-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com
Hi,
On 03/22/2013 01:49 PM, Peter Meerwald wrote:
I'm running Linux 3.7 on a beagle-xm and observe very reproducible stereo channel swapping issue on playback of a stereo stream (one channel has a sine, the other is zero); the channel swap occurs on starting the playback
it appears that the swapping issue is back, probably due to the DMA rework in recent kernel?
kernel 3.4 does not have the issue
Well, then, I guess, you'll have to bisect it down.
here it is: 946cc36ae550ea52adee0f42ac5034a34b5393be is the first bad commit commit 946cc36ae550ea52adee0f42ac5034a34b5393be Author: Peter Ujfalusi peter.ujfalusi@ti.com Date: Fri Sep 14 15:05:58 2012 +0300
ASoC: omap-pcm: Convert to use dmaengine Original author: Russell King <rmk+kernel@arm.linux.org.uk> Switch the omap-pcm to use dmaengine. Certain features are not supported by after dmaengine conversion: 1. No period wakeup mode DMA engine has no way to communicate this information through standard channels. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> Tested-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Yes the issue surfaced again with the dmaengine conversion. But at the end it boils down to the OMAP dmaengine implementation. For audio to work correctly we expect that the DMA _is_ started after the omap_dma_issue_pending() call, but it is not true for OMAP.
omap_dma_issue_pending() will start a tasklet which is in charge to start the DMA.
In audio case this is how the call chain looks like:
beagle-gentoo ~ # aplay media/2ch-left-since.wav -q [ 473.128753] omap-dma-engine omap-dma-engine: allocating channel for 33 [ 473.139587] omap_mcbsp_dai_set_dai_fmt [ 473.148345] omap_mcbsp_dai_hw_params: [ 473.155822] omap_mcbsp_config: [ 473.160369] omap_pcm_trigger: start [ 473.164062] omap_dma_prep_dma_cyclic (dma_ch: 2) [ 473.173828] omap_dma_issue_pending (dma_ch: 2) [ 473.177795] omap_mcbsp_dai_trigger: start [ 473.182006] omap_mcbsp_start [ 473.189025] omap_dma_sched [ 473.191864] omap_dma_start_sg (dma_ch: 2)
You can see that McBSP is started before the DMA because of the tasklet in drivers/dma/dma-omap.c
Russell: can we remove the tasklet use from dma-omap and start the DMA right away in omap_dma_issue_pending()? This is the only way to prevent channel swap when starting audio. Other devices might not care if the DMA is not started at the time we tell it to start, but for audio we need to make sure it is.
On Fri, Mar 22, 2013 at 02:04:42PM +0100, Peter Ujfalusi wrote:
Russell: can we remove the tasklet use from dma-omap and start the DMA right away in omap_dma_issue_pending()? This is the only way to prevent channel swap when starting audio.
What I fear is that we may run up against having too many DMA channels tied up to the peripherals. I structured the driver in this way to allow us to move the physical DMA channel allocation to that tasklet when that becomes a problem.
Not only that but I was hoping to lift some more of this code out of DMA engine drivers, so DMA engine drivers had even less code in them.
I guess we could keep the tasklet, but mark the audio DMA channels as "pre-reserved" and arrange for pre-reserved channels to avoid the tasklet, rather than throwing the tasklet out completely.
On 03/22/2013 05:35 PM, Russell King - ARM Linux wrote:
On Fri, Mar 22, 2013 at 02:04:42PM +0100, Peter Ujfalusi wrote:
Russell: can we remove the tasklet use from dma-omap and start the DMA right away in omap_dma_issue_pending()? This is the only way to prevent channel swap when starting audio.
What I fear is that we may run up against having too many DMA channels tied up to the peripherals. I structured the driver in this way to allow us to move the physical DMA channel allocation to that tasklet when that becomes a problem.
Not only that but I was hoping to lift some more of this code out of DMA engine drivers, so DMA engine drivers had even less code in them.
Not sure what you mean by this. Move dmaengine core's issue_pending to a tasklet or is only OMAP internal restructuring?
I guess we could keep the tasklet, but mark the audio DMA channels as "pre-reserved" and arrange for pre-reserved channels to avoid the tasklet, rather than throwing the tasklet out completely.
Not sure if we really want to pre-reserve the DMA channels for audio: on OMAP3 we have 5 McBSP -> 10 DMA channels on OMAP4 we have 4 McBSP, one McPDM, one DMIC and one McASP + we have the AESS/ABE -> 8 + 2 + 1 + 1 + 8 (AESS/ABE). Do we really want to pre-allocate DMA channel for all of these?
We could look omap_chan.cyclic which gives good indication that the channel is used for audio since only audio is using cyclic mode.
I have a patch which removes the tasklet from omap-dma.c and it is working fine on my systems (OMAP3 Zoom2, BeagleBoard, OMAP4 PandBoard, OMAP4 Blaze).
I can modify it to not remove the taskelt, but when the omap_chan.cyclic is true we would instead of starting the tasklet we just start the DMA right away.
On Mon, Mar 25, 2013 at 01:39:29PM +0100, Peter Ujfalusi wrote:
On 03/22/2013 05:35 PM, Russell King - ARM Linux wrote:
On Fri, Mar 22, 2013 at 02:04:42PM +0100, Peter Ujfalusi wrote:
Russell: can we remove the tasklet use from dma-omap and start the DMA right away in omap_dma_issue_pending()? This is the only way to prevent channel swap when starting audio.
What I fear is that we may run up against having too many DMA channels tied up to the peripherals. I structured the driver in this way to allow us to move the physical DMA channel allocation to that tasklet when that becomes a problem.
Not only that but I was hoping to lift some more of this code out of DMA engine drivers, so DMA engine drivers had even less code in them.
Not sure what you mean by this. Move dmaengine core's issue_pending to a tasklet or is only OMAP internal restructuring?
There isn't any "core" issue_pending code. DMA engine is nothing much beyond a collection of drivers sort-of implementing a common API. Very little code is shared between drivers.
What I'm talking about is having a physical channel scheduler in place across DMA engines which have more virtual channels than physical channels. Some DMA engine implementations sort of do this already (eg, AMBA PL08x stuff) but again, every driver implements this kind of thing by themselves.
I guess we could keep the tasklet, but mark the audio DMA channels as "pre-reserved" and arrange for pre-reserved channels to avoid the tasklet, rather than throwing the tasklet out completely.
Not sure if we really want to pre-reserve the DMA channels for audio: on OMAP3 we have 5 McBSP -> 10 DMA channels on OMAP4 we have 4 McBSP, one McPDM, one DMIC and one McASP + we have the AESS/ABE -> 8 + 2 + 1 + 1 + 8 (AESS/ABE). Do we really want to pre-allocate DMA channel for all of these?
Well, at the moment we are effectively pre-allocating a physical channel for every virtual channel - as each virtual channel gets allocated. So, the physical channels are currently being permanently tied up.
With a scheduling core, we need some way to dynamically reallocate physical channels depending on the workload presented on the virtual channels. However, as we're still having to deal with the OMAP private DMA API, we can only change physical channels from task (or tasklet) context.
So, the only way to have audio channels in a condition where they can be startable immediately in issue_pending is to avoid the tasklet, and the only way to avoid that tasklet (where that tasklet eventually becomes the channel scheduler) is to have pre-allocated the physical channel.
I have a patch which removes the tasklet from omap-dma.c and it is working fine on my systems (OMAP3 Zoom2, BeagleBoard, OMAP4 PandBoard, OMAP4 Blaze).
I'm sure you have, but that breaks the future direction of the driver to dynamically allocate the physical channels.
Had TI not effectively terminated my contract, I might be further forward with this. As things are, lack of contract and such like, all the OMAP work I was doing got shelved around Christmas time. If you stop paying people, they generally stop doing useful work for you...
On 03/25/2013 06:15 PM, Russell King - ARM Linux wrote:
What I'm talking about is having a physical channel scheduler in place across DMA engines which have more virtual channels than physical channels. Some DMA engine implementations sort of do this already (eg, AMBA PL08x stuff) but again, every driver implements this kind of thing by themselves.
I see. So basically you want to grab a DMA channel for the upcoming transfer at issue_pending time. This is fine, but we do not need to have taskelt to do this. If we do it as the amba-pl08x.c does it it is fine. AMBA does not use tasklet and OMAP should not do that either. We might need to lift the code out from the legacy DMA code (and remove the code) from arch/arm/plat-omap/dma.c...
I guess we could keep the tasklet, but mark the audio DMA channels as "pre-reserved" and arrange for pre-reserved channels to avoid the tasklet, rather than throwing the tasklet out completely.
Not sure if we really want to pre-reserve the DMA channels for audio: on OMAP3 we have 5 McBSP -> 10 DMA channels on OMAP4 we have 4 McBSP, one McPDM, one DMIC and one McASP + we have the AESS/ABE -> 8 + 2 + 1 + 1 + 8 (AESS/ABE). Do we really want to pre-allocate DMA channel for all of these?
Well, at the moment we are effectively pre-allocating a physical channel for every virtual channel - as each virtual channel gets allocated. So, the physical channels are currently being permanently tied up.
With a scheduling core, we need some way to dynamically reallocate physical channels depending on the workload presented on the virtual channels. However, as we're still having to deal with the OMAP private DMA API, we can only change physical channels from task (or tasklet) context.
So, the only way to have audio channels in a condition where they can be startable immediately in issue_pending is to avoid the tasklet, and the only way to avoid that tasklet (where that tasklet eventually becomes the channel scheduler) is to have pre-allocated the physical channel.
The issue has been noticed in audio but it can also affect other drivers as well. It might be only latency/throughput issue for others, while audio suffer with the tasklet based DMA start.
I have a patch which removes the tasklet from omap-dma.c and it is working fine on my systems (OMAP3 Zoom2, BeagleBoard, OMAP4 PandBoard, OMAP4 Blaze).
I'm sure you have, but that breaks the future direction of the driver to dynamically allocate the physical channels.
Had TI not effectively terminated my contract, I might be further forward with this. As things are, lack of contract and such like, all the OMAP work I was doing got shelved around Christmas time. If you stop paying people, they generally stop doing useful work for you...
Yeah, that is bad. If it makes you feel a bit better just look up in google: "Texas Instruments France" Hint: I'm in France...
participants (4)
-
Daniel Mack -
Peter Meerwald -
Peter Ujfalusi -
Russell King - ARM Linux