[RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Janusz Krzysztofik]

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:

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Jarkko Nikula]

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);
>  		break;
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

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Mark Brown]

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.

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Janusz Krzysztofik]

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

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Jarkko Nikula]

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.


-- 
Jarkko

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Janusz Krzysztofik]

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.

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Peter Ujfalusi]

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.

-- 
Péter

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Janusz Krzysztofik]

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

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Janusz Krzysztofik]

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

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Jarkko Nikula]

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


-- 
Jarkko

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Jarkko Nikula]

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

--- 
diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h b/arch/arm/plat-omap/include/mach/mcbsp.h
index bb154ea..57249bb 100644
--- a/arch/arm/plat-omap/include/mach/mcbsp.h
+++ b/arch/arm/plat-omap/include/mach/mcbsp.h
@@ -387,8 +387,8 @@ void omap_mcbsp_register_board_cfg(struct omap_mcbsp_platform_data *config,
 void omap_mcbsp_config(unsigned int id, const struct omap_mcbsp_reg_cfg * config);
 int omap_mcbsp_request(unsigned int id);
 void omap_mcbsp_free(unsigned int id);
-void omap_mcbsp_start(unsigned int id);
-void omap_mcbsp_stop(unsigned int id);
+void omap_mcbsp_start(unsigned int id, int tx, int rx);
+void omap_mcbsp_stop(unsigned int id, int tx, int rx);
 void omap_mcbsp_xmit_word(unsigned int id, u32 word);
 u32 omap_mcbsp_recv_word(unsigned int id);
 
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index efa0e01..d7645b3 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -328,14 +328,15 @@ void omap_mcbsp_free(unsigned int id)
 EXPORT_SYMBOL(omap_mcbsp_free);
 
 /*
- * Here we start the McBSP, by enabling the sample
- * generator, both transmitter and receivers,
- * and the frame sync.
+ * Here we start the McBSP, by enabling transmitter, receiver or both.
+ * If no transmitter or receiver is active prior calling, then sample-rate
+ * generator and frame sync are started.
  */
-void omap_mcbsp_start(unsigned int id)
+void omap_mcbsp_start(unsigned int id, int tx, int rx)
 {
 	struct omap_mcbsp *mcbsp;
 	void __iomem *io_base;
+	int idle;
 	u16 w;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
@@ -348,32 +349,40 @@ void omap_mcbsp_start(unsigned int id)
 	mcbsp->rx_word_length = (OMAP_MCBSP_READ(io_base, RCR1) >> 5) & 0x7;
 	mcbsp->tx_word_length = (OMAP_MCBSP_READ(io_base, XCR1) >> 5) & 0x7;
 
-	/* Start the sample generator */
-	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
+	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
+		OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+
+	if (idle) {
+		/* Start the sample generator */
+		w = OMAP_MCBSP_READ(io_base, SPCR2);
+		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
+	}
 
 	/* Enable transmitter and receiver */
 	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w | 1);
+	OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
 
 	w = OMAP_MCBSP_READ(io_base, SPCR1);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, w | 1);
+	OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
 
 	udelay(100);
 
-	/* Start frame sync */
-	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
+	if (idle) {
+		/* Start frame sync */
+		w = OMAP_MCBSP_READ(io_base, SPCR2);
+		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
+	}
 
 	/* Dump McBSP Regs */
 	omap_mcbsp_dump_reg(id);
 }
 EXPORT_SYMBOL(omap_mcbsp_start);
 
-void omap_mcbsp_stop(unsigned int id)
+void omap_mcbsp_stop(unsigned int id, int tx, int rx)
 {
 	struct omap_mcbsp *mcbsp;
 	void __iomem *io_base;
+	int idle;
 	u16 w;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
@@ -386,15 +395,20 @@ void omap_mcbsp_stop(unsigned int id)
 
 	/* Reset transmitter */
 	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1));
+	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
 
 	/* Reset receiver */
 	w = OMAP_MCBSP_READ(io_base, SPCR1);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(1));
+	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
 
-	/* Reset the sample rate generator */
-	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1 << 6));
+	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
+		OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+
+	if (idle) {
+		/* Reset the sample rate generator */
+		w = OMAP_MCBSP_READ(io_base, SPCR2);
+		OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1 << 6));
+	}
 }
 EXPORT_SYMBOL(omap_mcbsp_stop);
 
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index a5d46a7..9368bca 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -183,21 +183,21 @@ static int omap_mcbsp_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 	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);
-	int err = 0;
+	int err = 0, play = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (!mcbsp_data->active++)
-			omap_mcbsp_start(mcbsp_data->bus_id);
+		mcbsp_data->active++;
+		omap_mcbsp_start(mcbsp_data->bus_id, play, !play);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		if (!--mcbsp_data->active)
-			omap_mcbsp_stop(mcbsp_data->bus_id);
+		mcbsp_data->active--;
+		omap_mcbsp_stop(mcbsp_data->bus_id, play, !play);
 		break;
 	default:
 		err = -EINVAL;

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Janusz Krzysztofik]

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

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Janusz Krzysztofik]

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

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Janusz Krzysztofik]

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>

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Arun KS]

[-- Attachment #1.1: Type: text/plain, Size: 2162 bytes --]

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
>

[-- Attachment #1.2: Type: text/html, Size: 3338 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

_______________________________________________
e3-hacking mailing list
e3-hacking@earth.li
http://www.earth.li/cgi-bin/mailman/listinfo/e3-hacking

Re: [RFC] [PATCH] ASoC: OMAP: full duplex mode fix

[Arun KS]

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