[alsa-devel] [PATCH] ASoC: soc-compress: Cancel delayed power down if needed
When a new stream is being opened it is necessary to cancel any delayed power down of the audio.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/soc-compress.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index f9fb112..eb8191b 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -194,6 +194,7 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, { struct snd_soc_pcm_runtime *rtd = cstream->private_data; struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_dai *codec_dai = rtd->codec_dai; int ret = 0;
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass); @@ -216,6 +217,12 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, goto out; }
+ /* cancel any delayed stream shutdown that is pending */ + if (codec_dai->pop_wait) { + codec_dai->pop_wait = 0; + cancel_delayed_work(&rtd->delayed_work); + } + snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK, SND_SOC_DAPM_STREAM_START);
When a new stream is being opened it is necessary to cancel any delayed power down of the audio.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com ---
Apologies the last version was incorrectly generated from the wrong tree and used codec_dai instead of rtd for the pop_wait.
sound/soc/soc-compress.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index f9fb112..a3cc6f7 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -216,6 +216,12 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, goto out; }
+ /* cancel any delayed stream shutdown that is pending */ + if (rtd->pop_wait) { + rtd->pop_wait = 0; + cancel_delayed_work(&rtd->delayed_work); + } + snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK, SND_SOC_DAPM_STREAM_START);
On Fri, Jan 25, 2013 at 03:38:19PM +0000, Charles Keepax wrote:
- /* cancel any delayed stream shutdown that is pending */
- if (codec_dai->pop_wait) {
codec_dai->pop_wait = 0;cancel_delayed_work(&rtd->delayed_work);- }
This looks racy - the check for pop_wait can't be doing a huge amount here, the work might already have been scheduled.
When a new stream is being opened it is necessary to cancel any delayed power down of the audio.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com ---
Apologies for the delay on this.
Charles
sound/soc/soc-compress.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index b5b3db7..44bad67 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -198,6 +198,7 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, { struct snd_soc_pcm_runtime *rtd = cstream->private_data; struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_dai *codec_dai = rtd->codec_dai; int ret = 0;
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass); @@ -220,6 +221,10 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, goto out; }
+ /* cancel any delayed stream shutdown that is pending */ + rtd->pop_wait = 0; + cancel_delayed_work(&rtd->delayed_work); + snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK, SND_SOC_DAPM_STREAM_START);
When a new stream is being opened it is necessary to cancel any delayed power down of the audio.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/soc-compress.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index b5b3db7..44bad67 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -198,6 +198,7 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, { struct snd_soc_pcm_runtime *rtd = cstream->private_data; struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_dai *codec_dai = rtd->codec_dai; int ret = 0;
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass); @@ -220,6 +221,10 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, goto out; }
+ /* cancel any delayed stream shutdown that is pending */ + rtd->pop_wait = 0; + cancel_delayed_work_sync(&rtd->delayed_work); + snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK, SND_SOC_DAPM_STREAM_START);
On Mon, Mar 11, 2013 at 09:25:26AM +0000, Charles Keepax wrote:
Sorry about the delay here, I had been waiting for review from Vinod.
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass); @@ -220,6 +221,10 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, goto out; }
- /* cancel any delayed stream shutdown that is pending */
- rtd->pop_wait = 0;
- cancel_delayed_work_sync(&rtd->delayed_work);
The work here uses the pcm mutex but the cancel is done with the mutex held. We need to either cancel outside the lock or just ensure that the work doesn't do anything if we have run (which it looks like the pop_wait flag actually does so we could probably just use a non-sync cancel).
When a new stream is being opened it is necessary to cancel any delayed power down of the audio.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com ---
Sorry I somehow managed to send v3 again as v4, not quite sure how that came about.
Charles
sound/soc/soc-compress.c | 16 +++++++++++++--- 1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index b5b3db7..9eb8b4a 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -198,8 +198,10 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, { struct snd_soc_pcm_runtime *rtd = cstream->private_data; struct snd_soc_platform *platform = rtd->platform; + struct snd_soc_dai *codec_dai = rtd->codec_dai; int ret = 0;
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
/* first we call set_params for the platform driver @@ -211,19 +213,27 @@ static int soc_compr_set_params(struct snd_compr_stream *cstream, if (platform->driver->compr_ops && platform->driver->compr_ops->set_params) { ret = platform->driver->compr_ops->set_params(cstream, params); if (ret < 0) - goto out; + goto err; }
if (rtd->dai_link->compr_ops && rtd->dai_link->compr_ops->set_params) { ret = rtd->dai_link->compr_ops->set_params(cstream); if (ret < 0) - goto out; + goto err; }
snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK, SND_SOC_DAPM_STREAM_START);
-out: + /* cancel any delayed stream shutdown that is pending */ + rtd->pop_wait = 0; + mutex_unlock(&rtd->pcm_mutex); + + cancel_delayed_work_sync(&rtd->delayed_work); + + return ret; + +err: mutex_unlock(&rtd->pcm_mutex); return ret; }
On Fri, Mar 22, 2013 at 08:50:40AM +0000, Charles Keepax wrote:
When a new stream is being opened it is necessary to cancel any delayed power down of the audio.
I was going to apply this but the patch seems to be corrupted in some way so git am gets upset with it. I can't spot any errors visually...
Oh dear, sorry about that. This patch seems to be my nemesis I will generate a fresh version and resend.
Charles
participants (2)
-
Charles Keepax -
Mark Brown