[alsa-devel] [RFC] [PATCH] ASoC: OMAP: full duplex mode fix
This patch tries to correct the problem of full duplex mode not working over a single McBSP based CPU DAI.
Created against linux-2.6.31-rc5. Tested on Amstrad Delta.
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl --- --- linux-2.6.31-rc5/sound/soc/omap/omap-mcbsp.c.orig 2009-08-01 02:40:45.000000000 +0200 +++ linux-2.6.31-rc5/sound/soc/omap/omap-mcbsp.c 2009-08-03 03:28:07.000000000 +0200 @@ -183,6 +183,7 @@ static int omap_mcbsp_dai_trigger(struct struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai; struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data); + u16 buf; int err = 0;
switch (cmd) { @@ -191,6 +192,14 @@ static int omap_mcbsp_dai_trigger(struct case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: if (!mcbsp_data->active++) omap_mcbsp_start(mcbsp_data->bus_id); + else if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) + /* looks like capture already in progress, + * start playback by taking it out of error condition */ + omap_mcbsp_pollwrite(mcbsp_data->bus_id, 0x0); + else + /* looks like playback already in progress, + * start capture by taking it out of error condition */ + omap_mcbsp_pollread(mcbsp_data->bus_id, &buf); break;
case SNDRV_PCM_TRIGGER_STOP:
Hi
On Mon, 3 Aug 2009 03:32:04 +0200 Janusz Krzysztofik jkrzyszt@tis.icnet.pl wrote:
This patch tries to correct the problem of full duplex mode not working over a single McBSP based CPU DAI.
Created against linux-2.6.31-rc5. Tested on Amstrad Delta.
Do you have some specific test case how to trigger this? I haven't seen this on 2420 or 34xx (e.g. with 'arecord -d 1 -f dat |aplay') but I have no doubt that this can happen on 1510. At least this doesn't cause any harm on Beagle so I'm fine with the fix.
@@ -191,6 +192,14 @@ static int omap_mcbsp_dai_trigger(struct case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: if (!mcbsp_data->active++) omap_mcbsp_start(mcbsp_data->bus_id);
else if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)/* looks like capture already in progress,* start playback by taking it out of error condition */omap_mcbsp_pollwrite(mcbsp_data->bus_id, 0x0);else/* looks like playback already in progress,* start capture by taking it out of error condition */omap_mcbsp_pollread(mcbsp_data->bus_id, &buf);
Minor note: See preferred style for multi-line comments in Documentation/CodingStyle. I'm not 100 % sure about the braces but I think they are also preferred if there are indented comment lines with the single code line.
On Mon, Aug 03, 2009 at 11:29:50AM +0300, Jarkko Nikula wrote:
Janusz Krzysztofik jkrzyszt@tis.icnet.pl wrote:
else/* looks like playback already in progress,* start capture by taking it out of error condition */omap_mcbsp_pollread(mcbsp_data->bus_id, &buf);
Minor note: See preferred style for multi-line comments in Documentation/CodingStyle. I'm not 100 % sure about the braces but I think they are also preferred if there are indented comment lines with the single code line.
Indeed; only omit the braces if the *only* thing in the block is a single statement.
Jarkko Nikula wrote:
On Mon, 3 Aug 2009 03:32:04 +0200 Janusz Krzysztofik jkrzyszt@tis.icnet.pl wrote:
This patch tries to correct the problem of full duplex mode not working over a single McBSP based CPU DAI.
Created against linux-2.6.31-rc5. Tested on Amstrad Delta.
Do you have some specific test case how to trigger this? I haven't seen this on 2420 or 34xx (e.g. with 'arecord -d 1 -f dat |aplay') but I have no doubt that this can happen on 1510. At least this doesn't cause any harm on Beagle so I'm fine with the fix.
Hi, I made more testing on my OMAP1510 and found out that I could get your example usage working without my patch, but only if started like this:
arecord -D hw:0,0 -f S16_LE|aplay -D hw:0,0
If I start the same with "-D hw:0,0" omitted from aplay, it doesn't work any longer, waiting forever. It definitelly doesn't work if I start capture and playback one after another, no matter which one goes first (record while playing or play while recording). So it looks like starting both streams simultaneously can do the job, but a short delay breaks it.
With my patch, it seems to work fine for me in all cases.
Jarkko, have you ever tried it on your OMAP2/3 with parallel playback and capture started one after another, not simultaneously?
Arun, can your snd-soc-osk9512 work on OMAP1610 in full duplex mode without any limitations?
If the problem appears to be OMAP1510 or AMS_DELTA specific, I can add a check for a machine or cpu type to avoid braking unaffected machines.
Thanks, Janusz
On Mon, 03 Aug 2009 16:00:32 +0200 Janusz Krzysztofik jkrzyszt@tis.icnet.pl wrote:
Hi, I made more testing on my OMAP1510 and found out that I could get your example usage working without my patch, but only if started like this:
arecord -D hw:0,0 -f S16_LE|aplay -D hw:0,0
If I start the same with "-D hw:0,0" omitted from aplay, it doesn't work any longer, waiting forever. It definitelly doesn't work if I start capture and playback one after another, no matter which one goes first (record while playing or play while recording). So it looks like starting both streams simultaneously can do the job, but a short delay breaks it.
With my patch, it seems to work fine for me in all cases.
So it looks that McBSP-DMA for another stream cease to work if there is more than certain delay between first stream start and second one but omap_mcbsp_pollwrite or _pollread will clear the error condition. Can you debug is this due clearing the possible XSYNC_ERR, waiting for transmit/receive confirmation or playing with data registers there?
Jarkko, have you ever tried it on your OMAP2/3 with parallel playback and capture started one after another, not simultaneously?
Yes I have. Like recording into file (arecord /dev/shm/foo) and start playing it after some time (aplay /dev/shm/foo) or playing a file and start recording into another.
If the problem appears to be OMAP1510 or AMS_DELTA specific, I can add a check for a machine or cpu type to avoid braking unaffected machines.
I'm thinking can your platform show some existing problem not noted before... but yes, check for 1510 would be bit safer now and is also kind of revisit comment why 1510 needs special handling.
Monday 03 August 2009 17:14:05 Jarkko Nikula wrote:
So it looks that McBSP-DMA for another stream cease to work if there is more than certain delay between first stream start and second one but omap_mcbsp_pollwrite or _pollread will clear the error condition. Can you debug is this due clearing the possible XSYNC_ERR, waiting for transmit/receive confirmation or playing with data registers there?
I have temporarily modified those omap_mcbsp_pollwrite/_pollread() to do nothing but reporting, put them into omap_mcbsp_dai_trigger() as before and additionally into a newly created and registered omap_mcbsp_dai_prepare(), called before omap_start_dma(), and got the following result:
For both playback start while capturing and capture start while playing, XSYNC_ERR/RSYNC_ERR is clear and XRDY/RRDY is ready, respectively, both before and after omap_start_dma(). No DMA transfer is actually started, so the operation fails with i/o error.
My interpretation based on my SPRU592 and SPRU674 understanding:
While starting the first stream, omap_mcbsp_start(), called by omap_mcbsp_dai_trigger(), instructs McBSP to start shifting bits in both directions, no matter which one has just been requested. After that, the direction, for which a corresponding DMA transfer has just been started from omap_pcm_trigger() with omap_start_dma(), starts doing its job, while the opposite direction, after shifting in one word in case of capture, issues a DMA event that is missed and waits for an I/O to occur.
Then, when omap_pcm_trigger() starts DMA for the opposite direction, the DMA controller, configured for synchronized transfer, waits for a corresponding DMA event before it performs its first I/O operation. That event already occurred far before and will not occur again, so the transfer will not start without any intervention. This time, omap_mcbsp_start() is not called again for an already started hardware action (correct), so there is no chance for the transfer to start that way.
With my patch, performing an I/O operation by calling omap_mcbsp_pollwrite/_pollread() forces McBSP to issue next DMA event, that initiates the transfer.
While starting both streams (semi)simultaneously, it may happen that the DMA event for the opposite direction will occur after the DMA has just been started for that direction as well and will not be missed, so both streams will run correctly.
Does it make sense, or am I missing something?
If my analysis is correct, the best solution I can see would be starting McBSP transfer for one direction only, not both, so the opposite direction can be started when needed. That requires deeper and wider OMAP knowledge and a change in omap_mcbsp_start() API though. I am not in a position to deal with this myself, I'm afraid.
Thanks, Janusz
PS: Not CCing arch/arm/plat-omap/mcbsp.c author as his email address is probably out-of-date.
Hello,
On Tuesday 04 August 2009 23:46:09 ext Janusz Krzysztofik wrote:
I have temporarily modified those omap_mcbsp_pollwrite/_pollread() to do nothing but reporting, put them into omap_mcbsp_dai_trigger() as before and additionally into a newly created and registered omap_mcbsp_dai_prepare(), called before omap_start_dma(), and got the following result:
For both playback start while capturing and capture start while playing, XSYNC_ERR/RSYNC_ERR is clear and XRDY/RRDY is ready,
This means that XRDY/RRDY is set (1)?
respectively, both before and after omap_start_dma(). No DMA transfer is actually started, so the operation fails with i/o error.
In case of playback start while capture: What is the state of the XEMPTY bit (SPCR2:2)? Is it 0? It should.
My interpretation based on my SPRU592 and SPRU674 understanding:
While starting the first stream, omap_mcbsp_start(), called by omap_mcbsp_dai_trigger(), instructs McBSP to start shifting bits in both directions, no matter which one has just been requested. After that, the direction, for which a corresponding DMA transfer has just been started from omap_pcm_trigger() with omap_start_dma(), starts doing its job, while the opposite direction, after shifting in one word in case of capture, issues a DMA event that is missed and waits for an I/O to occur.
Then, when omap_pcm_trigger() starts DMA for the opposite direction, the DMA controller, configured for synchronized transfer, waits for a corresponding DMA event before it performs its first I/O operation. That event already occurred far before and will not occur again, so the transfer will not start without any intervention. This time, omap_mcbsp_start() is not called again for an already started hardware action (correct), so there is no chance for the transfer to start that way.
I think the reason is quite simple: on OMAP1 the DMA request is edge sensitive (compared to OMAP2 and OMAP3 where it is level based). This means: When you start the capture, both RX and TX is started. Since the playback is not running, no data will be written to the DXR register, McBSP asserts the DMA request line, but since there is no DMA configured for playback it stays high (and McBSP will shift out 0). Now if you start the playback (using DMA) the DMA engine waits for a pulse on the request line, which will not occur, the request line is asserted, so the DMA will not start pushing any data to the McBSP DXR register. Now if you force write to the DXR register, than it de-asserts the DMA request line, when the DXR -> XSR copy has been done, McBSP asserts the DMA request line, which will kick the DMA engine to push data to DXR register and the playback will start.
With my patch, performing an I/O operation by calling omap_mcbsp_pollwrite/_pollread() forces McBSP to issue next DMA event, that initiates the transfer.
The danger with the pollwrite/pollread is: If the stream is not mono, than you kind of ensure a certain channel shift (channel switch in case of stereo stream).
While starting both streams (semi)simultaneously, it may happen that the DMA event for the opposite direction will occur after the DMA has just been started for that direction as well and will not be missed, so both streams will run correctly.
Yep, this is kind of a race condition (in a good way): Most likely both DMA has been started before the McBSP ports asserted their DMA request lines, so the DMA engine will push or read the data correctly.
Does it make sense, or am I missing something?
If my analysis is correct, the best solution I can see would be starting McBSP transfer for one direction only, not both, so the opposite direction can be started when needed. That requires deeper and wider OMAP knowledge and a change in omap_mcbsp_start() API though. I am not in a position to deal with this myself, I'm afraid.
I agree, this would be the best solution for the problem.
Thanks, Janusz
PS: Not CCing arch/arm/plat-omap/mcbsp.c author as his email address is probably out-of-date.
Hi,
Peter Ujfalusi wrote:
On Tuesday 04 August 2009 23:46:09 ext Janusz Krzysztofik wrote:
For both playback start while capturing and capture start while playing, XSYNC_ERR/RSYNC_ERR is clear and XRDY/RRDY is ready,
This means that XRDY/RRDY is set (1)?
Yes, trasmit/recieve confirmed at first while(!(readw(...) & XRDY/RRDY)) attempt.
In case of playback start while capture: What is the state of the XEMPTY bit (SPCR2:2)? Is it 0? It should.
Have not checked, but will do for completness.
I think the reason is quite simple: on OMAP1 the DMA request is edge sensitive (compared to OMAP2 and OMAP3 where it is level based). This means:
Good recap.
The danger with the pollwrite/pollread is: If the stream is not mono, than you kind of ensure a certain channel shift (channel switch in case of stereo stream).
Good point, I was trying break all not mono.
If my analysis is correct, the best solution I can see would be starting McBSP transfer for one direction only, not both, so the opposite direction can be started when needed. That requires deeper and wider OMAP knowledge and a change in omap_mcbsp_start() API though. I am not in a position to deal with this myself, I'm afraid.
I agree, this would be the best solution for the problem.
I was also considering omap_mcbsp_restart(id, direction) as a more conservative solution, but now, after Jarkko has subbmitted his patch on omap_mcbsp_start(), it's not an option any longer, I think.
Cheers, Janusz
Wednesday 05 August 2009 08:45:21 Peter Ujfalusi wrote:
In case of playback start while capture: What is the state of the XEMPTY bit (SPCR2:2)? Is it 0? It should.
Yes, XEMPTY was 0, both before and after omap_start_dma().
For completness of the record: RFULL was 1 (again as expected) in case of capture start while playing.
Janusz
Thanks for good debuging!
On Tue, 4 Aug 2009 22:46:09 +0200 Janusz Krzysztofik jkrzyszt@tis.icnet.pl wrote:
For both playback start while capturing and capture start while playing, XSYNC_ERR/RSYNC_ERR is clear and XRDY/RRDY is ready, respectively, both before and after omap_start_dma(). No DMA transfer is actually started, so the operation fails with i/o error.
My interpretation based on my SPRU592 and SPRU674 understanding:
FYI: there is also SPRU708 for OMAP5910 McBSP.
While starting the first stream, omap_mcbsp_start(), called by omap_mcbsp_dai_trigger(), instructs McBSP to start shifting bits in both directions, no matter which one has just been requested. After that, the direction, for which a corresponding DMA transfer has just been started from omap_pcm_trigger() with omap_start_dma(), starts doing its job, while the opposite direction, after shifting in one word in case of capture, issues a DMA event that is missed and waits for an I/O to occur.
Then, when omap_pcm_trigger() starts DMA for the opposite direction, the DMA controller, configured for synchronized transfer, waits for a corresponding DMA event before it performs its first I/O operation. That event already occurred far before and will not occur again, so the transfer will not start without any intervention. This time, omap_mcbsp_start() is not called again for an already started hardware action (correct), so there is no chance for the transfer to start that way.
This sounds very logical explanation. I didn't find any information how the DMA request (or event as stated in those documents) from McBSP of the 1510/5910 is de-asserted but it very likely looks by your observations that it's just an event which can be missed if the DMA is not configured when it happens.
In later OMAP's it looks that the DMA request is level sensite (I didn't check) and will clear only after the data register is accessed by the DMA since the problem doesn't occur there.
If my analysis is correct, the best solution I can see would be starting McBSP transfer for one direction only, not both, so the opposite direction can be started when needed. That requires deeper and wider OMAP knowledge and a change in omap_mcbsp_start() API though. I am not in a position to deal with this myself, I'm afraid.
I favor this change. Actually I remember I was thinking shortly to change API of omap_mcbsp_start and _stop more than year back or so but didn't find it necessary back then.
I think change will be trivial. Basically two new arguments indicating are the TX/RX active and let the first/last caller to deal with sample-rate generator and frame sync activation/de-activation.
This API change would also clean-up these two patches:
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-July/019821.html http://mailman.alsa-project.org/pipermail/alsa-devel/2009-July/019826.html
PS: Not CCing arch/arm/plat-omap/mcbsp.c author as his email address is probably out-of-date.
Yes it is (s/nokia.com/intel.com/).
On Wed, 5 Aug 2009 10:21:49 +0300 Jarkko Nikula jhnikula@gmail.com wrote:
If my analysis is correct, the best solution I can see would be starting McBSP transfer for one direction only, not both, so the opposite direction can be started when needed. That requires deeper and wider OMAP knowledge and a change in omap_mcbsp_start() API though. I am not in a position to deal with this myself, I'm afraid.
I favor this change. Actually I remember I was thinking shortly to change API of omap_mcbsp_start and _stop more than year back or so but didn't find it necessary back then.
I think change will be trivial. Basically two new arguments indicating are the TX/RX active and let the first/last caller to deal with sample-rate generator and frame sync activation/de-activation.
I hacked a patch below. Can you test does it help?
Jarkko Nikula wrote:
On Wed, 5 Aug 2009 10:21:49 +0300 Jarkko Nikula jhnikula@gmail.com wrote:
If my analysis is correct, the best solution I can see would be starting McBSP transfer for one direction only, not both, so the opposite direction can be started when needed. That requires deeper and wider OMAP knowledge and a change in omap_mcbsp_start() API though. I am not in a position to deal with this myself, I'm afraid.
I favor this change. Actually I remember I was thinking shortly to change API of omap_mcbsp_start and _stop more than year back or so but didn't find it necessary back then.
I think change will be trivial. Basically two new arguments indicating are the TX/RX active and let the first/last caller to deal with sample-rate generator and frame sync activation/de-activation.
I hacked a patch below. Can you test does it help?
Will do tonight (CET).
Now seeing how easy it was (for you ;), I think I could try to follow that way an modify omap_mcbsp_config() in the same spirit, if you find it of any use.
Cheers, Janusz
Wednesday 05 August 2009 15:26:47 Janusz Krzysztofik wrote:
Jarkko Nikula wrote:
On Wed, 5 Aug 2009 10:21:49 +0300 I hacked a patch below. Can you test does it help?
Will do tonight (CET).
Sorry, but I was not able to perform any tests in a consistent way. Something strange was happening to my hardware, no matter with or without your patch, omap_mcbsp_dump_reg() readings was somewhat random. Will retry tomorrow.
Now seeing how easy it was (for you ;), I think I could try to follow that way an modify omap_mcbsp_config() in the same spirit, if you find it of any use.
Maybe not omap_mcbsp_config() itself, but rather asoc omap functions that prepare data for it, not sure yet. The idea would be to enable independent (asymmetric) configuration of streams.
Janusz
Wednesday 05 August 2009 10:42:54 Jarkko Nikula wrote:
On Wed, 5 Aug 2009 10:21:49 +0300 Jarkko Nikula jhnikula@gmail.com wrote:
If my analysis is correct, the best solution I can see would be starting McBSP transfer for one direction only, not both, so the opposite direction can be started when needed. That requires deeper and wider OMAP knowledge and a change in omap_mcbsp_start() API though. I am not in a position to deal with this myself, I'm afraid.
I favor this change. Actually I remember I was thinking shortly to change API of omap_mcbsp_start and _stop more than year back or so but didn't find it necessary back then.
I think change will be trivial. Basically two new arguments indicating are the TX/RX active and let the first/last caller to deal with sample-rate generator and frame sync activation/de-activation.
I hacked a patch below. Can you test does it help?
Yes, it does. Works as expected in all possible combinations I have tried: start recording while playing, start playing while recording, start recording and playing simultaneously. Thanks.
Tested-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl
On Mon, Aug 3, 2009 at 7:00 AM, Janusz Krzysztofik jkrzyszt@tis.icnet.plwrote:
Jarkko Nikula wrote:
On Mon, 3 Aug 2009 03:32:04 +0200 Janusz Krzysztofik jkrzyszt@tis.icnet.pl wrote:
This patch tries to correct the problem of full duplex mode not working
over a single McBSP based CPU DAI.
Created against linux-2.6.31-rc5. Tested on Amstrad Delta.
Do you have some specific test case how to trigger this? I haven't
seen this on 2420 or 34xx (e.g. with 'arecord -d 1 -f dat |aplay') but I have no doubt that this can happen on 1510. At least this doesn't cause any harm on Beagle so I'm fine with the fix.
Hi, I made more testing on my OMAP1510 and found out that I could get your example usage working without my patch, but only if started like this:
arecord -D hw:0,0 -f S16_LE|aplay -D hw:0,0If I start the same with "-D hw:0,0" omitted from aplay, it doesn't work any longer, waiting forever. It definitelly doesn't work if I start capture and playback one after another, no matter which one goes first (record while playing or play while recording). So it looks like starting both streams simultaneously can do the job, but a short delay breaks it.
With my patch, it seems to work fine for me in all cases.
Jarkko, have you ever tried it on your OMAP2/3 with parallel playback and capture started one after another, not simultaneously?
Arun, can your snd-soc-osk9512 work on OMAP1610 in full duplex mode without any limitations?
Janusz,
Haven't done testing in full duplex mode. I don't have access to osk5912 board now. If someone has got osk and do the testing it ll be good. It ll take at least another 2 more month for me to do the testing on osk.
Regards, Arun
If the problem appears to be OMAP1510 or AMS_DELTA specific, I can add a check for a machine or cpu type to avoid braking unaffected machines.
Thanks, Janusz
-- 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
CCing Jesslyn Abdul Salam jesslyn.abdulsalam@gmail.com
Hope he has one osk.
On Mon, Aug 3, 2009 at 10:53 AM, Arun KS arunks@mistralsolutions.com wrote:
On Mon, Aug 3, 2009 at 7:00 AM, Janusz Krzysztofik jkrzyszt@tis.icnet.pl wrote:
Jarkko Nikula wrote:
On Mon, 3 Aug 2009 03:32:04 +0200 Janusz Krzysztofik jkrzyszt@tis.icnet.pl wrote:
This patch tries to correct the problem of full duplex mode not working over a single McBSP based CPU DAI.
Created against linux-2.6.31-rc5. Tested on Amstrad Delta.
Do you have some specific test case how to trigger this? I haven't seen this on 2420 or 34xx (e.g. with 'arecord -d 1 -f dat |aplay') but I have no doubt that this can happen on 1510. At least this doesn't cause any harm on Beagle so I'm fine with the fix.
Hi, I made more testing on my OMAP1510 and found out that I could get your example usage working without my patch, but only if started like this:
arecord -D hw:0,0 -f S16_LE|aplay -D hw:0,0
If I start the same with "-D hw:0,0" omitted from aplay, it doesn't work any longer, waiting forever. It definitelly doesn't work if I start capture and playback one after another, no matter which one goes first (record while playing or play while recording). So it looks like starting both streams simultaneously can do the job, but a short delay breaks it.
With my patch, it seems to work fine for me in all cases.
Jarkko, have you ever tried it on your OMAP2/3 with parallel playback and capture started one after another, not simultaneously?
Arun, can your snd-soc-osk9512 work on OMAP1610 in full duplex mode without any limitations?
Janusz,
Haven't done testing in full duplex mode. I don't have access to osk5912 board now. If someone has got osk and do the testing it ll be good. It ll take at least another 2 more month for me to do the testing on osk.
Regards, Arun
If the problem appears to be OMAP1510 or AMS_DELTA specific, I can add a check for a machine or cpu type to avoid braking unaffected machines.
Thanks, Janusz
-- 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
participants (5)
-
Arun KS -
Janusz Krzysztofik -
Jarkko Nikula -
Mark Brown -
Peter Ujfalusi