All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Liam Girdwood <lrg@ti.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"broonie@opensource.wolfsonmicro.com"
	<broonie@opensource.wolfsonmicro.com>,
	"w.sang@pengutronix.de" <w.sang@pengutronix.de>,
	Dong Aisheng <b29396@freescale.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
Date: Wed, 24 Aug 2011 19:05:35 +0200	[thread overview]
Message-ID: <4E552F5F.9070801@metafoo.de> (raw)
In-Reply-To: <4E5520CE.60204@ti.com>

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.

WARNING: multiple messages have this Message-ID (diff)
From: lars@metafoo.de (Lars-Peter Clausen)
To: linux-arm-kernel@lists.infradead.org
Subject: [alsa-devel] [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
Date: Wed, 24 Aug 2011 19:05:35 +0200	[thread overview]
Message-ID: <4E552F5F.9070801@metafoo.de> (raw)
In-Reply-To: <4E5520CE.60204@ti.com>

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.

  reply	other threads:[~2011-08-24 17:06 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E552F5F.9070801@metafoo.de \
    --to=lars@metafoo.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=b29396@freescale.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=lrg@ti.com \
    --cc=s.hauer@pengutronix.de \
    --cc=w.sang@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.