[alsa-devel] [PATCH v2 0/2] ASoC: core: Ignoring pmdown_time for playback stream power down
Hello Mark,
Changes since v1 (RFC): - move the flag to per codec (snd_soc_codec) - twl6040 to request to ignore the pmdown_time
Add support in core to ignore the pmdown_time when stopping the playback stream. This will inline the DAPM stream stop event, so it is going to be executed without delay at stream close time (for playback stream).
Backround: with the McPDM protocol we are sendning not only the pure audio stream, but OMAP McPDM also transmits additional information (for example offset cancellation). If McPDM is stopped prior to the DAC this information will be not sent to the codec, which can result noise rendered by the twl6040 codec.
Regards, Peter --- Peter Ujfalusi (2): ASoC: core: Add flag to ignore pmdown_time at pcm_close ASoC: twl6040: Request core to inline the DAPM sequence
include/sound/soc.h | 1 + sound/soc/codecs/twl6040.c | 1 + sound/soc/soc-pcm.c | 15 +++++++++++---- 3 files changed, 13 insertions(+), 4 deletions(-)
With this flag codec drivers can indicate that it is desired to ignore the pmdown_time for DAPM shutdown sequence when playback stream is stopped. The DAPM sequence will be executed without delay in this case.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- include/sound/soc.h | 1 + sound/soc/soc-pcm.c | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 858291d..11cfb59 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -578,6 +578,7 @@ struct snd_soc_codec {
/* dapm */ struct snd_soc_dapm_context dapm; + unsigned int ignore_pmdown_time:1; /* pmdown_time is ignored at stop */
#ifdef CONFIG_DEBUG_FS struct dentry *debugfs_codec_root; diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 8eb0f07..ee15337 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -319,10 +319,17 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) cpu_dai->runtime = NULL;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - /* start delayed pop wq here for playback streams */ - codec_dai->pop_wait = 1; - schedule_delayed_work(&rtd->delayed_work, - msecs_to_jiffies(rtd->pmdown_time)); + if (unlikely(codec->ignore_pmdown_time)) { + /* powered down playback stream now */ + snd_soc_dapm_stream_event(rtd, + codec_dai->driver->playback.stream_name, + SND_SOC_DAPM_STREAM_STOP); + } else { + /* start delayed pop wq here for playback streams */ + codec_dai->pop_wait = 1; + schedule_delayed_work(&rtd->delayed_work, + msecs_to_jiffies(rtd->pmdown_time)); + } } else { /* capture streams can be powered down now */ snd_soc_dapm_stream_event(rtd,
--- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -578,6 +578,7 @@ struct snd_soc_codec {
/* dapm */ struct snd_soc_dapm_context dapm;
- unsigned int ignore_pmdown_time:1; /* pmdown_time is ignored at
stop */
I feel ignore_pmdown_time needs to be part of snd_soc_dai_driver structure. It will give flexibility to control individual dai. Individual dai's can choose to Ignore power down delay.
Would it make sense?
In my codec, I need to ignore the power down delay for voice dais, but retain the delay for HiFi dai's.
On Fri, Oct 21, 2011 at 11:42:27AM +0530, Babu, Ramesh wrote:
I feel ignore_pmdown_time needs to be part of snd_soc_dai_driver structure. It will give flexibility to control individual dai. Individual dai's can choose to Ignore power down delay.
Why do you need to do this?
Would it make sense?
In my codec, I need to ignore the power down delay for voice dais, but retain the delay for HiFi dai's.
I'm not sure associating it with DAIs is a good idea, I'd expect any issues to be with the analouge outputs not DAIs.
I feel ignore_pmdown_time needs to be part of snd_soc_dai_driver
structure.
It will give flexibility to control individual dai. Individual dai's
can choose to
Ignore power down delay.
Why do you need to do this?
Would it make sense?
In my codec, I need to ignore the power down delay for voice dais,
but retain the delay
for HiFi dai's.
I'm not sure associating it with DAIs is a good idea, I'd expect any issues to be with the analouge outputs not DAIs.
In my device, clock to the codec is coming from modem (for voice call use case). After voice call termination, application Calls snd_pcm_close for voice device and then turns off the modem clock. Since clock is turned-off while Widgets are still on (power down deferred by pmdown_time), I am noticing the glitches.
I thought, having ignore_pmdown_time for individual codec device would allow flexibility.
On Fri, Oct 21, 2011 at 02:40:04PM +0530, Babu, Ramesh wrote:
In my device, clock to the codec is coming from modem (for voice call use case). After voice call termination, application Calls snd_pcm_close for voice device and then turns off the modem clock. Since clock is turned-off while Widgets are still on (power down deferred by pmdown_time), I am noticing the glitches.
This clearly isn't something that should be set in the CODEC driver, it's a system integration issue - on a different board there might be no such requirement.
This clearly isn't something that should be set in the CODEC driver, it's a system integration issue - on a different board there might be no such requirement.
Hmm... Ok. Thanks for clarification.
If it is a board specific requirement, then would it make sense to have it in dai_link as per Peter Ujfaulsi's RFC [1].
In general, how this kind of situation is handled?
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2011-October/045088.htm...
On Fri, 2011-10-21 at 16:28 +0530, Babu, Ramesh wrote:
This clearly isn't something that should be set in the CODEC driver, it's a system integration issue - on a different board there might be no such requirement.
Hmm... Ok. Thanks for clarification.
If it is a board specific requirement, then would it make sense to have it in dai_link as per Peter Ujfaulsi's RFC [1].
In general, how this kind of situation is handled?
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2011-October/045088.htm...
Then, I think it makes sense to make it for snd_soc_dai_link
I think the requirement is to delay the power down the link during the music playback. There is a possibility that next song will start playing sooner. In this case, try to avoid power down and power up the DAI immediately. In case of voice call, it is not expected to have voice call immediately after a call. It can be closed. Phone stack can go ahead and close the modem path but codec path will still be open. This will cause glitches.
How will this scenario be glitch free?
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Babu, Ramesh Sent: Friday, October 21, 2011 3:59 AM To: Mark Brown Cc: Peter Ujfalusi; alsa-devel@alsa-project.org; Liam Girdwood; Misael Lopez Cruz Subject: Re: [alsa-devel] [PATCH v2 1/2] ASoC: core: Add flag to ignore pmdown_time at pcm_close
This clearly isn't something that should be set in the CODEC driver, it's a system integration issue - on a different board there might be no such requirement.
Hmm... Ok. Thanks for clarification.
If it is a board specific requirement, then would it make sense to have it in dai_link as per Peter Ujfaulsi's RFC [1].
In general, how this kind of situation is handled?
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2011-October/045088.htm...
_______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Fri, Oct 21, 2011 at 10:31:40PM +0530, Nallasellan, Singaravelan wrote:
Fix your mailer to word wrap within paragraphs and don't top post when posting to public mailing lists.
I think the requirement is to delay the power down the link during the music playback. There is a possibility that next song will start playing sooner. In this case, try to avoid power down and power up the DAI immediately. In case of voice call, it is not expected to have voice call immediately after a call. It can be closed. Phone stack can go ahead and close the modem path but codec path will still be open. This will cause glitches.
You can't decide this on the basis of the DAI, though - the system integrator may have thought of another use for the DAI.
Agree. System integrator may not like to delay closing modem ports for 6 seconds either. Need to close the ports immediately or delay. Do you suggest that we shouldn't delay in the driver and introduce delay in the user pace? I prefer to remove the delay altogether from the driver. Do you see any issue which might cause glitches?
-----Original Message----- From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com] Sent: Friday, October 21, 2011 12:42 PM To: Nallasellan, Singaravelan Cc: Babu, Ramesh; Peter Ujfalusi; alsa-devel@alsa-project.org; Liam Girdwood; Misael Lopez Cruz Subject: Re: [alsa-devel] [PATCH v2 1/2] ASoC: core: Add flag to ignore pmdown_time at pcm_close
On Fri, Oct 21, 2011 at 10:31:40PM +0530, Nallasellan, Singaravelan wrote:
Fix your mailer to word wrap within paragraphs and don't top post when posting to public mailing lists.
I think the requirement is to delay the power down the link during the music playback. There is a possibility that next song will start playing sooner. In this case, try to avoid power down and power up the DAI immediately. In case of voice call, it is not expected to have voice call immediately after a call. It can be closed. Phone stack can go ahead and close the modem path but codec path will still be open. This will cause glitches.
You can't decide this on the basis of the DAI, though - the system integrator may have thought of another use for the DAI.
On Sat, Oct 22, 2011 at 01:54:23AM +0530, Nallasellan, Singaravelan wrote:
When I said you shouldn't top post that's exactly what I meant! This is basic mailing list etiquette for public Linux lists because for example...
Agree. System integrator may not like to delay closing modem ports for 6 seconds either. Need to close the ports immediately or delay.
...not having any context quoted means the reader can't easily tell wht it is that you are agreeing with. You should also fix your mail client to word wrap within paragraphs as I also mentioned.
Do you suggest that we shouldn't delay in the driver and introduce delay in the user pace? I prefer to remove the delay altogether from the driver. Do you see any issue which might cause glitches?
The delay is very important for CPU use cases as brief gaps in playback are extremely common there. The power up and power down are generally noticable and to be avoided.
On Fri, Oct 21, 2011 at 04:28:44PM +0530, Babu, Ramesh wrote:
If it is a board specific requirement, then would it make sense to have it in dai_link as per Peter Ujfaulsi's RFC [1].
Possibly for your case - the case this was originally written for was rather different, the CODEC has issues which mean it always needs to skip the delay so having to specify this for every single board is going to be repetitive.
In general, how this kind of situation is handled?
Usually by keeping the clocks available.
On Sat, 2011-10-22 at 10:45 +0100, Mark Brown wrote:
On Fri, Oct 21, 2011 at 04:28:44PM +0530, Babu, Ramesh wrote:
If it is a board specific requirement, then would it make sense to have it in dai_link as per Peter Ujfaulsi's RFC [1].
Possibly for your case - the case this was originally written for was rather different, the CODEC has issues which mean it always needs to skip the delay so having to specify this for every single board is going to be repetitive.
In general, how this kind of situation is handled?
Usually by keeping the clocks available.
That does not seem to be possible here :(
The codec takes the clock from modem. The codec doesn't have control over it. The voice call termination kills the clock and the DAPM power down produces noticeable artifacts.
Since this is board property, machine driver should tell DAPM to power down immediately for this DAI alone. That of course allows us to reap benefits of DAPM not power cycling codec during close/open.
On Sat, Oct 22, 2011 at 10:47:20PM +0530, Vinod Koul wrote:
On Sat, 2011-10-22 at 10:45 +0100, Mark Brown wrote:
Usually by keeping the clocks available.
That does not seem to be possible here :(
Well, I'm sure you can remonstrate with your baseband vendor. :)
Since this is board property, machine driver should tell DAPM to power down immediately for this DAI alone. That of course allows us to reap benefits of DAPM not power cycling codec during close/open.
For this issue adding control in the dai_link is probably good.
On Sun, 2011-10-23 at 16:00 +0200, Mark Brown wrote:
Since this is board property, machine driver should tell DAPM to
power
down immediately for this DAI alone. That of course allows us to
reap
benefits of DAPM not power cycling codec during close/open.
For this issue adding control in the dai_link is probably good.
Yes meant dia_link :)
We need to have as less time between McPDM shutdown, and power down of the DAC on the twl6040 codec as possible. Request core to ignore the pmdown_time for the playback stream. Backround: with the McPDM protocol we are sendning not only the pure audio stream, but OMAP McPDM also transmits additional information (for example offset cancellation). If McPDM is stopped prior to the DAC this information will be not sent to the codec, which can result noise rendered by the twl6040 codec.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/twl6040.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index 6c573c3..73e11f0 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -1504,6 +1504,7 @@ static int twl6040_probe(struct snd_soc_codec *codec)
priv->codec = codec; codec->control_data = dev_get_drvdata(codec->dev->parent); + codec->ignore_pmdown_time = 1;
if (pdata && pdata->hs_left_step && pdata->hs_right_step) { priv->hs_left_step = pdata->hs_left_step;
On Fri, Oct 14, 2011 at 02:43:32PM +0300, Peter Ujfalusi wrote:
Changes since v1 (RFC):
- move the flag to per codec (snd_soc_codec)
- twl6040 to request to ignore the pmdown_time
I've applied both, thanks. We will probably want to extend this to be handlable dynamically - there's a lot of devices will be happy to do this for speaker but which won't be able to do it unconditionally due to headphone or line outputs for example.
participants (5)
-
Babu, Ramesh -
Mark Brown -
Nallasellan, Singaravelan -
Peter Ujfalusi -
Vinod Koul