[PATCH] ASoC: Fix check for symmetric rate enforcement

[PATCH] ASoC: Fix check for symmetric rate enforcement

[Sascha Hauer]

The ASoC core tries to not enforce symmetric rates when
two streams open simultaneously. It does so by checking
rtd->rate being zero. This works exactly once after booting
because it is not set to zero again when the streams close.
Fix this by clearing rtd->rate when no active stream is left.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 sound/soc/soc-core.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index d75043e..c25649a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -746,6 +746,9 @@ static int soc_codec_close(struct snd_pcm_substream *substream)
 	codec_dai->active--;
 	codec->active--;
 
+	if (!cpu_dai->active && !codec_dai->active)
+		rtd->rate = 0;
+
 	/* Muting the DAC suppresses artifacts caused during digital
 	 * shutdown, for example from stopping clocks.
 	 */
-- 
1.7.5.4

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] ASoC: Fix check for symmetric rate enforcement

[Mark Brown]

On Wed, Aug 17, 2011 at 08:27:53AM +0200, Sascha Hauer wrote:

> The ASoC core tries to not enforce symmetric rates when
> two streams open simultaneously. It does so by checking
> rtd->rate being zero. This works exactly once after booting
> because it is not set to zero again when the streams close.
> Fix this by clearing rtd->rate when no active stream is left.

> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Applied, thanks.  Though I suspect that in practice this may actually be
less robust due to the general raciness of the way we configure and
start the streams - I seem to recall the code works this way semi
deliberately so that we always have a rate selected; most systems only
ever use one rate on symmetric audio interfaces.

Re: [PATCH] ASoC: Fix check for symmetric rate enforcement

[Mark Brown]

On Wed, Aug 17, 2011 at 03:50:14PM +0900, Mark Brown wrote:
> On Wed, Aug 17, 2011 at 08:27:53AM +0200, Sascha Hauer wrote:

> > The ASoC core tries to not enforce symmetric rates when
> > two streams open simultaneously. It does so by checking
> > rtd->rate being zero. This works exactly once after booting
> > because it is not set to zero again when the streams close.
> > Fix this by clearing rtd->rate when no active stream is left.

> Applied, thanks.  Though I suspect that in practice this may actually be

...or not since it doesn't actually apply to either 3.1 or 3.2.  Could
you regenerate against 3.1 please?

[PATCH] ASoC: Fix check for symmetric rate enforcement

[Sascha Hauer]

The ASoC core tries to not enforce symmetric rates when
two streams open simultaneously. It does so by checking
rtd->rate being zero. This works exactly once after booting
because it is not set to zero again when the streams close.
Fix this by setting rtd->rate when no active stream is left.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 sound/soc/soc-pcm.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index b575939..2879c88 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -290,6 +290,9 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
 	codec_dai->active--;
 	codec->active--;
 
+	if (!cpu_dai->active && !codec_dai->active)
+		rtd->rate = 0;
+
 	/* Muting the DAC suppresses artifacts caused during digital
 	 * shutdown, for example from stopping clocks.
 	 */
-- 
1.7.5.4

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] ASoC: Fix check for symmetric rate enforcement

[Mark Brown]

On Wed, Aug 17, 2011 at 09:20:01AM +0200, Sascha Hauer wrote:

> The ASoC core tries to not enforce symmetric rates when
> two streams open simultaneously. It does so by checking
> rtd->rate being zero. This works exactly once after booting
> because it is not set to zero again when the streams close.
> Fix this by setting rtd->rate when no active stream is left.

Applied, thanks.

Re: [PATCH] ASoC: Fix check for symmetric rate enforcement

[Sascha Hauer]

On Wed, Aug 17, 2011 at 03:50:16PM +0900, Mark Brown wrote:
> On Wed, Aug 17, 2011 at 08:27:53AM +0200, Sascha Hauer wrote:
> 
> > The ASoC core tries to not enforce symmetric rates when
> > two streams open simultaneously. It does so by checking
> > rtd->rate being zero. This works exactly once after booting
> > because it is not set to zero again when the streams close.
> > Fix this by clearing rtd->rate when no active stream is left.
> 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Applied, thanks.  Though I suspect that in practice this may actually be
> less robust due to the general raciness of the way we configure and
> start the streams - I seem to recall the code works this way semi
> deliberately so that we always have a rate selected; most systems only
> ever use one rate on symmetric audio interfaces.

I looked into it yesterday and I found no way to fix this properly :(

The original problem I had was subsequent calls of:

arecord -r 16000 | aplay
arecord -r 8000 | aplay

The first call after booting worked, but then strange things happen.
rtd->rate is still 16000 when arecord opens. Then aplay opens and
the ASoC core forces 16000Hz because of rtd->rate still being 16000.
arecord can change the rate to 8000Hz because the minmax constraint
was not present at open time. aplay then switches the rate back
to 16000Hz which results in an inconsistent state. I think this
is only one variant of the game.

With my patch applied we get the "Not enforcing symmetric rates.."
warning each time with arecord | aplay which is not nice either.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] ASoC: Fix check for symmetric rate enforcement

[Mark Brown]

On Wed, Aug 17, 2011 at 09:11:23AM +0200, Sascha Hauer wrote:
> On Wed, Aug 17, 2011 at 03:50:16PM +0900, Mark Brown wrote:

> > start the streams - I seem to recall the code works this way semi
> > deliberately so that we always have a rate selected; most systems only
> > ever use one rate on symmetric audio interfaces.

> I looked into it yesterday and I found no way to fix this properly :(

Yeah, it's just a fundamental API think which applications would need to
code around - they need to restart their config process if they fail to
set the params rather than treating a failure as a failure.

> With my patch applied we get the "Not enforcing symmetric rates.."
> warning each time with arecord | aplay which is not nice either.

Indeed.  We have a window where we know there's two users but don't know
what configurations they want to use.