Subject: [E3-hacking] [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

From: Janusz Krzysztofik <[email-masked]>
Date: Mon, 3 Aug 2009 03:32:04 +0200

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 <[email-masked]>
---
--- 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:

---

From: Jarkko Nikula <[email-masked]>
Date: Mon, 3 Aug 2009 11:29:50 +0300

Hi

On Mon, 3 Aug 2009 03:32:04 +0200

> Created against linux-2.6.31-rc5.
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.

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

-- 
Jarkko

---

From: Mark Brown <[email-masked]>
Date: Mon, 3 Aug 2009 10:43:47 +0100

> think they are also preferred if there are indented comment lines with

Indeed; only omit the braces if the *only* thing in the block is a
single statement.

---

From: Janusz Krzysztofik <[email-masked]>
Date: Mon, 03 Aug 2009 16:00:32 +0200

> 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

---

From: Jarkko Nikula <[email-masked]>
Date: Mon, 3 Aug 2009 18:14:05 +0300

On Mon, 03 Aug 2009 16:00:32 +0200

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

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

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

-- 
Jarkko

---

From: Arun KS <[email-masked]>
Date: Mon, 3 Aug 2009 10:57:26 -0700

CCing Jesslyn Abdul Salam <[email-masked]>

Hope he has one osk.

On Mon, Aug 3, 2009 at 10:53 AM, Arun KS <[email-masked]> wrote:
>
>
> On Mon, Aug 3, 2009 at 7:00 AM, Janusz Krzysztofik <[email-masked]> wrote:
>>
>> Jarkko Nikula wrote:
>>>
>> 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:
>>
>> 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.
>>
>> 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 [email-masked]
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

---

From: Arun KS <[email-masked]>
Date: Mon, 3 Aug 2009 10:53:31 -0700

On Mon, Aug 3, 2009 at 7:00 AM, Janusz Krzysztofik <[email-masked]>wrote:

> 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.
>
> Jarkko, have you ever tried it on your OMAP2/3 with parallel playback and
> capture started one after another, not simultaneously?
>

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

---

From: Janusz Krzysztofik <[email-masked]>
Date: Thu, 6 Aug 2009 11:16:51 +0200

Wednesday 05 August 2009 10:42:54 Jarkko Nikula wrote:
> On Wed, 5 Aug 2009 10:21:49 +0300
> Jarkko Nikula <[email-masked]> 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 <[email-masked]>