All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dong Aisheng-B29396 <B29396@freescale.com>
To: Lars-Peter Clausen <lars@metafoo.de>, Liam Girdwood <lrg@ti.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"broonie@opensource.wolfsonmicro.com"
	<broonie@opensource.wolfsonmicro.com>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"w.sang@pengutronix.de" <w.sang@pengutronix.de>
Subject: Re: [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active
Date: Fri, 26 Aug 2011 09:16:14 +0000	[thread overview]
Message-ID: <65EE16ACC360FA4D99C96DC085B3F772225E52@039-SN1MPN1-001.039d.mgd.msft.net> (raw)
In-Reply-To: <4E552F5F.9070801@metafoo.de>

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

WARNING: multiple messages have this Message-ID
From: B29396@freescale.com (Dong Aisheng-B29396)
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: Fri, 26 Aug 2011 09:16:14 +0000	[thread overview]
Message-ID: <65EE16ACC360FA4D99C96DC085B3F772225E52@039-SN1MPN1-001.039d.mgd.msft.net> (raw)
In-Reply-To: <4E552F5F.9070801@metafoo.de>

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

  parent reply	other threads:[~2011-08-26  9:16 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-24 11:02 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 [this message]
2011-08-26  9:16       ` 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=65EE16ACC360FA4D99C96DC085B3F772225E52@039-SN1MPN1-001.039d.mgd.msft.net \
    --to=b29396@freescale.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=lars@metafoo.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=lrg@ti.com \
    --cc=s.hauer@pengutronix.de \
    --cc=w.sang@pengutronix.de \
    --subject='Re: [PATCH v2 1/1] ASoC: soc-core: check rate for symmetry only when pcm is active' \
    /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

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.