channel swapping issue on OMAP3/TWL4030 is back

channel swapping issue on OMAP3/TWL4030 is back

[Peter Meerwald]

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.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Daniel Mack]

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

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Peter Meerwald]

> > 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.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Daniel Mack]

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

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Peter Meerwald]

> 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.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Daniel Mack]

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

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Peter Meerwald]

> >> 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>

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Peter Ujfalusi]

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.

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Russell King - ARM Linux]

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.

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Peter Ujfalusi]

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.

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Russell King - ARM Linux]

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...

Re: [alsa-devel] channel swapping issue on OMAP3/TWL4030 is back

[Peter Ujfalusi]

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...

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html