All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
@ 2011-08-24 11:02 ` Dong Aisheng
  0 siblings, 0 replies; 14+ messages in thread
From: Dong Aisheng @ 2011-08-24 11:02 UTC (permalink / raw)
  To: alsa-devel; +Cc: s.hauer, broonie, lrg, linux-arm-kernel, w.sang

For the playback and record using different dai links,
checking !rtd->rate for symmetry may not be accurate because that
pcm may be acutually not running and the default new open rate is 0,
then the warning message "Not enforcing symmetric_rates" will
happen each time with running arecord | aplay.

Now we only check rate for symmetry when the pcm is really active
which seems more sensible.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Liam Girdwood <lrg@ti.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/sound/soc.h |    1 +
 sound/soc/soc-pcm.c |    5 +++++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 3fe658e..17970fd 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -851,6 +851,7 @@ struct snd_soc_pcm_runtime  {
 
 	/* Symmetry data - only valid if symmetry is being enforced */
 	unsigned int rate;
+	unsigned int active;
 	long pmdown_time;
 
 	/* runtime devices */
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 1aee9fc..72a7b34 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -39,6 +39,9 @@ static int soc_pcm_apply_symmetry(struct snd_pcm_substream *substream)
 	    !rtd->dai_link->symmetric_rates)
 		return 0;
 
+	if (!rtd->active)
+		return 0;
+
 	/* This can happen if multiple streams are starting simultaneously -
 	 * the second can need to get its constraints before the first has
 	 * picked a rate.  Complain and allow the application to carry on.
@@ -209,6 +212,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	cpu_dai->active++;
 	codec_dai->active++;
 	rtd->codec->active++;
+	rtd->active++;
 	mutex_unlock(&rtd->pcm_mutex);
 	return 0;
 
@@ -287,6 +291,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
 	cpu_dai->active--;
 	codec_dai->active--;
 	codec->active--;
+	rtd->active--;
 
 	if (!cpu_dai->active && !codec_dai->active)
 		rtd->rate = 0;
-- 
1.7.0.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
@ 2011-08-24 11:02 ` Dong Aisheng
  0 siblings, 0 replies; 14+ messages in thread
From: Dong Aisheng @ 2011-08-24 11:02 UTC (permalink / raw)
  To: linux-arm-kernel

For the playback and record using different dai links,
checking !rtd->rate for symmetry may not be accurate because that
pcm may be acutually not running and the default new open rate is 0,
then the warning message "Not enforcing symmetric_rates" will
happen each time with running arecord | aplay.

Now we only check rate for symmetry when the pcm is really active
which seems more sensible.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Liam Girdwood <lrg@ti.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/sound/soc.h |    1 +
 sound/soc/soc-pcm.c |    5 +++++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 3fe658e..17970fd 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -851,6 +851,7 @@ struct snd_soc_pcm_runtime  {
 
 	/* Symmetry data - only valid if symmetry is being enforced */
 	unsigned int rate;
+	unsigned int active;
 	long pmdown_time;
 
 	/* runtime devices */
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 1aee9fc..72a7b34 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -39,6 +39,9 @@ static int soc_pcm_apply_symmetry(struct snd_pcm_substream *substream)
 	    !rtd->dai_link->symmetric_rates)
 		return 0;
 
+	if (!rtd->active)
+		return 0;
+
 	/* This can happen if multiple streams are starting simultaneously -
 	 * the second can need to get its constraints before the first has
 	 * picked a rate.  Complain and allow the application to carry on.
@@ -209,6 +212,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	cpu_dai->active++;
 	codec_dai->active++;
 	rtd->codec->active++;
+	rtd->active++;
 	mutex_unlock(&rtd->pcm_mutex);
 	return 0;
 
@@ -287,6 +291,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
 	cpu_dai->active--;
 	codec_dai->active--;
 	codec->active--;
+	rtd->active--;
 
 	if (!cpu_dai->active && !codec_dai->active)
 		rtd->rate = 0;
-- 
1.7.0.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
  2011-08-24 11:02 ` Dong Aisheng
@ 2011-08-24 16:03   ` Liam Girdwood
  -1 siblings, 0 replies; 14+ messages in thread
From: Liam Girdwood @ 2011-08-24 16:03 UTC (permalink / raw)
  To: Dong Aisheng; +Cc: alsa-devel, broonie, s.hauer, linux-arm-kernel, w.sang

On 24/08/11 12:02, Dong Aisheng wrote:
> For the playback and record using different dai links,
> checking !rtd->rate for symmetry may not be accurate because that
> pcm may be acutually not running and the default new open rate is 0,
> then the warning message "Not enforcing symmetric_rates" will
> happen each time with running arecord | aplay.
> 
> Now we only check rate for symmetry when the pcm is really active
> which seems more sensible.
> 
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Liam Girdwood <lrg@ti.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>

Acked-by: Liam Girdwood <lrg@ti.com>

> ---
>  include/sound/soc.h |    1 +
>  sound/soc/soc-pcm.c |    5 +++++
>  2 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 3fe658e..17970fd 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -851,6 +851,7 @@ struct snd_soc_pcm_runtime  {
>  
>  	/* Symmetry data - only valid if symmetry is being enforced */
>  	unsigned int rate;
> +	unsigned int active;
>  	long pmdown_time;
>  
>  	/* runtime devices */
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 1aee9fc..72a7b34 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -39,6 +39,9 @@ static int soc_pcm_apply_symmetry(struct snd_pcm_substream *substream)
>  	    !rtd->dai_link->symmetric_rates)
>  		return 0;
>  
> +	if (!rtd->active)
> +		return 0;
> +
>  	/* This can happen if multiple streams are starting simultaneously -
>  	 * the second can need to get its constraints before the first has
>  	 * picked a rate.  Complain and allow the application to carry on.
> @@ -209,6 +212,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
>  	cpu_dai->active++;
>  	codec_dai->active++;
>  	rtd->codec->active++;
> +	rtd->active++;
>  	mutex_unlock(&rtd->pcm_mutex);
>  	return 0;
>  
> @@ -287,6 +291,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
>  	cpu_dai->active--;
>  	codec_dai->active--;
>  	codec->active--;
> +	rtd->active--;
>  
>  	if (!cpu_dai->active && !codec_dai->active)
>  		rtd->rate = 0;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
@ 2011-08-24 16:03   ` Liam Girdwood
  0 siblings, 0 replies; 14+ messages in thread
From: Liam Girdwood @ 2011-08-24 16:03 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/08/11 12:02, Dong Aisheng wrote:
> For the playback and record using different dai links,
> checking !rtd->rate for symmetry may not be accurate because that
> pcm may be acutually not running and the default new open rate is 0,
> then the warning message "Not enforcing symmetric_rates" will
> happen each time with running arecord | aplay.
> 
> Now we only check rate for symmetry when the pcm is really active
> which seems more sensible.
> 
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Liam Girdwood <lrg@ti.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>

Acked-by: Liam Girdwood <lrg@ti.com>

> ---
>  include/sound/soc.h |    1 +
>  sound/soc/soc-pcm.c |    5 +++++
>  2 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 3fe658e..17970fd 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -851,6 +851,7 @@ struct snd_soc_pcm_runtime  {
>  
>  	/* Symmetry data - only valid if symmetry is being enforced */
>  	unsigned int rate;
> +	unsigned int active;
>  	long pmdown_time;
>  
>  	/* runtime devices */
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 1aee9fc..72a7b34 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -39,6 +39,9 @@ static int soc_pcm_apply_symmetry(struct snd_pcm_substream *substream)
>  	    !rtd->dai_link->symmetric_rates)
>  		return 0;
>  
> +	if (!rtd->active)
> +		return 0;
> +
>  	/* This can happen if multiple streams are starting simultaneously -
>  	 * the second can need to get its constraints before the first has
>  	 * picked a rate.  Complain and allow the application to carry on.
> @@ -209,6 +212,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
>  	cpu_dai->active++;
>  	codec_dai->active++;
>  	rtd->codec->active++;
> +	rtd->active++;
>  	mutex_unlock(&rtd->pcm_mutex);
>  	return 0;
>  
> @@ -287,6 +291,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
>  	cpu_dai->active--;
>  	codec_dai->active--;
>  	codec->active--;
> +	rtd->active--;
>  
>  	if (!cpu_dai->active && !codec_dai->active)
>  		rtd->rate = 0;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
  2011-08-24 16:03   ` Liam Girdwood
@ 2011-08-24 17:05     ` Lars-Peter Clausen
  -1 siblings, 0 replies; 14+ messages in thread
From: Lars-Peter Clausen @ 2011-08-24 17:05 UTC (permalink / raw)
  To: Liam Girdwood
  Cc: alsa-devel, s.hauer, broonie, w.sang, Dong Aisheng, linux-arm-kernel

On 08/24/2011 06:03 PM, Liam Girdwood wrote:
> On 24/08/11 12:02, Dong Aisheng wrote:
>> For the playback and record using different dai links,
>> checking !rtd->rate for symmetry may not be accurate because that
>> pcm may be acutually not running and the default new open rate is 0,
>> then the warning message "Not enforcing symmetric_rates" will
>> happen each time with running arecord | aplay.
>>
>> Now we only check rate for symmetry when the pcm is really active
>> which seems more sensible.
>>
>> Signed-off-by: Dong Aisheng <b29396@freescale.com>
>> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
>> Cc: Liam Girdwood <lrg@ti.com>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Acked-by: Liam Girdwood <lrg@ti.com>
> 

I don't think the patch in it's current form is a good idea. All it does is
silencing the warning instead of addressing the real issue. The issue at hand
here is, that the same DAI is connected to two other DAIs and the thus shared
between two PCMs. The shared DAI requires symmetric rates. This is something
that is not covered by the current code, since it assumes that a DAI is not
shared between different PCMs. So with this patch the warning will be gone, but
it is still possible that the capture and playback stream of the DAI to be
configured to run at different rates, which will obviously unwanted behavior.

A more sensible solution would in my opinion be to save the current rate in the
dai struct itself and not in the pcm struct. Upon opening a stream check for
cpu and codec DAI whether they require symmetry. If they do and are active
constraint the rate to DAIs rate. This also ensures that if boths DAIs are
already active and are running at different rates there will be no valid rate
for the new stream.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
@ 2011-08-24 17:05     ` Lars-Peter Clausen
  0 siblings, 0 replies; 14+ messages in thread
From: Lars-Peter Clausen @ 2011-08-24 17:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/24/2011 06:03 PM, Liam Girdwood wrote:
> On 24/08/11 12:02, Dong Aisheng wrote:
>> For the playback and record using different dai links,
>> checking !rtd->rate for symmetry may not be accurate because that
>> pcm may be acutually not running and the default new open rate is 0,
>> then the warning message "Not enforcing symmetric_rates" will
>> happen each time with running arecord | aplay.
>>
>> Now we only check rate for symmetry when the pcm is really active
>> which seems more sensible.
>>
>> Signed-off-by: Dong Aisheng <b29396@freescale.com>
>> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
>> Cc: Liam Girdwood <lrg@ti.com>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Acked-by: Liam Girdwood <lrg@ti.com>
> 

I don't think the patch in it's current form is a good idea. All it does is
silencing the warning instead of addressing the real issue. The issue at hand
here is, that the same DAI is connected to two other DAIs and the thus shared
between two PCMs. The shared DAI requires symmetric rates. This is something
that is not covered by the current code, since it assumes that a DAI is not
shared between different PCMs. So with this patch the warning will be gone, but
it is still possible that the capture and playback stream of the DAI to be
configured to run at different rates, which will obviously unwanted behavior.

A more sensible solution would in my opinion be to save the current rate in the
dai struct itself and not in the pcm struct. Upon opening a stream check for
cpu and codec DAI whether they require symmetry. If they do and are active
constraint the rate to DAIs rate. This also ensures that if boths DAIs are
already active and are running at different rates there will be no valid rate
for the new stream.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
  2011-08-24 17:05     ` [alsa-devel] " Lars-Peter Clausen
@ 2011-08-24 17:27       ` Mark Brown
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2011-08-24 17:27 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: alsa-devel, s.hauer, w.sang, Dong Aisheng, Liam Girdwood,
	linux-arm-kernel

On Wed, Aug 24, 2011 at 07:05:35PM +0200, Lars-Peter Clausen wrote:

> A more sensible solution would in my opinion be to save the current rate in the
> dai struct itself and not in the pcm struct. Upon opening a stream check for
> cpu and codec DAI whether they require symmetry. If they do and are active
> constraint the rate to DAIs rate. This also ensures that if boths DAIs are
> already active and are running at different rates there will be no valid rate
> for the new stream.

We need to store in all three places I think - the symmetry requirement
can also come from the link itself.  But generally this is what we're
actually looking for, yes.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
@ 2011-08-24 17:27       ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2011-08-24 17:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Aug 24, 2011 at 07:05:35PM +0200, Lars-Peter Clausen wrote:

> A more sensible solution would in my opinion be to save the current rate in the
> dai struct itself and not in the pcm struct. Upon opening a stream check for
> cpu and codec DAI whether they require symmetry. If they do and are active
> constraint the rate to DAIs rate. This also ensures that if boths DAIs are
> already active and are running at different rates there will be no valid rate
> for the new stream.

We need to store in all three places I think - the symmetry requirement
can also come from the link itself.  But generally this is what we're
actually looking for, yes.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
  2011-08-24 17:27       ` Mark Brown
@ 2011-08-24 18:54         ` Lars-Peter Clausen
  -1 siblings, 0 replies; 14+ messages in thread
From: Lars-Peter Clausen @ 2011-08-24 18:54 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, s.hauer, w.sang, Dong Aisheng, Liam Girdwood,
	linux-arm-kernel

On 08/24/2011 07:27 PM, Mark Brown wrote:
> On Wed, Aug 24, 2011 at 07:05:35PM +0200, Lars-Peter Clausen wrote:
> 
>> A more sensible solution would in my opinion be to save the current rate in the
>> dai struct itself and not in the pcm struct. Upon opening a stream check for
>> cpu and codec DAI whether they require symmetry. If they do and are active
>> constraint the rate to DAIs rate. This also ensures that if boths DAIs are
>> already active and are running at different rates there will be no valid rate
>> for the new stream.
> 
> We need to store in all three places I think - the symmetry requirement
> can also come from the link itself.  But generally this is what we're
> actually looking for, yes.

It should be sufficient to just store the rate for the cpu and codec DAI.
Though the rate constraint should be applied if either the DAI or the link
wants symmetric rates.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
@ 2011-08-24 18:54         ` Lars-Peter Clausen
  0 siblings, 0 replies; 14+ messages in thread
From: Lars-Peter Clausen @ 2011-08-24 18:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/24/2011 07:27 PM, Mark Brown wrote:
> On Wed, Aug 24, 2011 at 07:05:35PM +0200, Lars-Peter Clausen wrote:
> 
>> A more sensible solution would in my opinion be to save the current rate in the
>> dai struct itself and not in the pcm struct. Upon opening a stream check for
>> cpu and codec DAI whether they require symmetry. If they do and are active
>> constraint the rate to DAIs rate. This also ensures that if boths DAIs are
>> already active and are running at different rates there will be no valid rate
>> for the new stream.
> 
> We need to store in all three places I think - the symmetry requirement
> can also come from the link itself.  But generally this is what we're
> actually looking for, yes.

It should be sufficient to just store the rate for the cpu and codec DAI.
Though the rate constraint should be applied if either the DAI or the link
wants symmetric rates.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
  2011-08-24 18:54         ` [alsa-devel] " Lars-Peter Clausen
@ 2011-08-24 19:19           ` Mark Brown
  -1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2011-08-24 19:19 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: alsa-devel, s.hauer, w.sang, Dong Aisheng, Liam Girdwood,
	linux-arm-kernel

On Wed, Aug 24, 2011 at 08:54:55PM +0200, Lars-Peter Clausen wrote:

> It should be sufficient to just store the rate for the cpu and codec DAI.
> Though the rate constraint should be applied if either the DAI or the link
> wants symmetric rates.

Yes, for the actual rate storage.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
@ 2011-08-24 19:19           ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2011-08-24 19:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Aug 24, 2011 at 08:54:55PM +0200, Lars-Peter Clausen wrote:

> It should be sufficient to just store the rate for the cpu and codec DAI.
> Though the rate constraint should be applied if either the DAI or the link
> wants symmetric rates.

Yes, for the actual rate storage.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
  2011-08-24 17:05     ` [alsa-devel] " Lars-Peter Clausen
@ 2011-08-26  9:16       ` Dong Aisheng-B29396
  -1 siblings, 0 replies; 14+ messages in thread
From: Dong Aisheng-B29396 @ 2011-08-26  9:16 UTC (permalink / raw)
  To: Lars-Peter Clausen, Liam Girdwood
  Cc: alsa-devel, broonie, s.hauer, linux-arm-kernel, w.sang

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Thursday, August 25, 2011 1:06 AM
> To: Liam Girdwood
> Cc: Dong Aisheng-B29396; alsa-devel@alsa-project.org;
> broonie@opensource.wolfsonmicro.com; s.hauer@pengutronix.de; linux-arm-
> kernel@lists.infradead.org; w.sang@pengutronix.de
> Subject: Re: [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for
> symmetry only when pcm is active
> 
> On 08/24/2011 06:03 PM, Liam Girdwood wrote:
> > On 24/08/11 12:02, Dong Aisheng wrote:
> >> For the playback and record using different dai links, checking
> >> !rtd->rate for symmetry may not be accurate because that pcm may be
> >> acutually not running and the default new open rate is 0, then the
> >> warning message "Not enforcing symmetric_rates" will happen each time
> >> with running arecord | aplay.
> >>
> >> Now we only check rate for symmetry when the pcm is really active
> >> which seems more sensible.
> >>
> >> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> >> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> >> Cc: Liam Girdwood <lrg@ti.com>
> >> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> >
> > Acked-by: Liam Girdwood <lrg@ti.com>
> >
> 
> I don't think the patch in it's current form is a good idea. All it does
> is silencing the warning instead of addressing the real issue. The issue
> at hand here is, that the same DAI is connected to two other DAIs and the
> thus shared between two PCMs. The shared DAI requires symmetric rates.
> This is something that is not covered by the current code, since it
> assumes that a DAI is not shared between different PCMs. So with this
> patch the warning will be gone, but it is still possible that the capture
> and playback stream of the DAI to be configured to run at different rates,
> which will obviously unwanted behavior.
Although I think that's another issue and it's a special using case.
But I think it would be good if we can fix that issue by covering this wrong
warning message issue also.

> A more sensible solution would in my opinion be to save the current rate
> in the dai struct itself and not in the pcm struct. Upon opening a stream
> check for cpu and codec DAI whether they require symmetry. If they do and
> are active constraint the rate to DAIs rate. This also ensures that if
> boths DAIs are already active and are running at different rates there
> will be no valid rate for the new stream.
I have made another patch based on your idea and will send it out as RFC soon.
Thanks a lot for your good suggestion.

Regards
Dong Aisheng

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
@ 2011-08-26  9:16       ` Dong Aisheng-B29396
  0 siblings, 0 replies; 14+ messages in thread
From: Dong Aisheng-B29396 @ 2011-08-26  9:16 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars at metafoo.de]
> Sent: Thursday, August 25, 2011 1:06 AM
> To: Liam Girdwood
> Cc: Dong Aisheng-B29396; alsa-devel at alsa-project.org;
> broonie at opensource.wolfsonmicro.com; s.hauer at pengutronix.de; linux-arm-
> kernel at lists.infradead.org; w.sang at pengutronix.de
> Subject: Re: [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for
> symmetry only when pcm is active
> 
> On 08/24/2011 06:03 PM, Liam Girdwood wrote:
> > On 24/08/11 12:02, Dong Aisheng wrote:
> >> For the playback and record using different dai links, checking
> >> !rtd->rate for symmetry may not be accurate because that pcm may be
> >> acutually not running and the default new open rate is 0, then the
> >> warning message "Not enforcing symmetric_rates" will happen each time
> >> with running arecord | aplay.
> >>
> >> Now we only check rate for symmetry when the pcm is really active
> >> which seems more sensible.
> >>
> >> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> >> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> >> Cc: Liam Girdwood <lrg@ti.com>
> >> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> >
> > Acked-by: Liam Girdwood <lrg@ti.com>
> >
> 
> I don't think the patch in it's current form is a good idea. All it does
> is silencing the warning instead of addressing the real issue. The issue
> at hand here is, that the same DAI is connected to two other DAIs and the
> thus shared between two PCMs. The shared DAI requires symmetric rates.
> This is something that is not covered by the current code, since it
> assumes that a DAI is not shared between different PCMs. So with this
> patch the warning will be gone, but it is still possible that the capture
> and playback stream of the DAI to be configured to run at different rates,
> which will obviously unwanted behavior.
Although I think that's another issue and it's a special using case.
But I think it would be good if we can fix that issue by covering this wrong
warning message issue also.

> A more sensible solution would in my opinion be to save the current rate
> in the dai struct itself and not in the pcm struct. Upon opening a stream
> check for cpu and codec DAI whether they require symmetry. If they do and
> are active constraint the rate to DAIs rate. This also ensures that if
> boths DAIs are already active and are running at different rates there
> will be no valid rate for the new stream.
I have made another patch based on your idea and will send it out as RFC soon.
Thanks a lot for your good suggestion.

Regards
Dong Aisheng

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-08-26  9:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-24 11:02 [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active Dong Aisheng
2011-08-24 11:02 ` Dong Aisheng
2011-08-24 16:03 ` Liam Girdwood
2011-08-24 16:03   ` Liam Girdwood
2011-08-24 17:05   ` Lars-Peter Clausen
2011-08-24 17:05     ` [alsa-devel] " Lars-Peter Clausen
2011-08-24 17:27     ` Mark Brown
2011-08-24 17:27       ` Mark Brown
2011-08-24 18:54       ` Lars-Peter Clausen
2011-08-24 18:54         ` [alsa-devel] " Lars-Peter Clausen
2011-08-24 19:19         ` Mark Brown
2011-08-24 19:19           ` [alsa-devel] " Mark Brown
2011-08-26  9:16     ` Dong Aisheng-B29396
2011-08-26  9:16       ` [alsa-devel] " Dong Aisheng-B29396

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.