Hi Mark
These are soc-pcm cleanup step5. These are based on Bard Liao's "ASoC: Add Multi CPU DAI support" patch-set.
[7/8] patch might exchange the behavior
Kuninori Morimoto (8): 1) ASoC: soc-pcm: use defined stream 2) ASoC: soc-pcm: remove duplicate be check from dpcm_add_paths() 3) ASoC: soc-pcm: move dpcm_fe_dai_close() 4) ASoC: soc-pcm: add dpcm_fe_dai_clean() 5) ASoC: soc-pcm: use snd_soc_dai_get_pcm_stream() at dpcm_set_fe_runtime() 6) ASoC: soc-pcm: check DAI's activity more simply 7) ASoC: soc-pcm: Do Digital Mute for both CPU/Codec in same timing. 8) ASoC: soc-pcm: tidyup dulicate handing at dpcm_fe_dai_startup()
sound/soc/soc-pcm.c | 107 ++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 59 deletions(-)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Many functions defines "stream = substream->stream", but some of them is using "substream->stream" instead of "stream". It is pointless. This patch uses defined stream.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 90857138c823..8c27eb4d5e4c 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -644,8 +644,7 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) * bailed out on a higher level, since there would be no * CODEC to support the transfer direction in that case. */ - if (!snd_soc_dai_stream_valid(codec_dai, - substream->stream)) + if (!snd_soc_dai_stream_valid(codec_dai, stream)) continue;
codec_stream = snd_soc_dai_get_pcm_stream(codec_dai, stream); @@ -2149,7 +2148,7 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
- ret = dpcm_be_dai_startup(fe, fe_substream->stream); + ret = dpcm_be_dai_startup(fe, stream); if (ret < 0) { dev_err(fe->dev,"ASoC: failed to start some BEs %d\n", ret); goto be_err; @@ -2180,7 +2179,7 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) return 0;
unwind: - dpcm_be_dai_startup_unwind(fe, fe_substream->stream); + dpcm_be_dai_startup_unwind(fe, stream); be_err: dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO); return ret; @@ -2234,7 +2233,7 @@ static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream) dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
/* shutdown the BEs */ - dpcm_be_dai_shutdown(fe, substream->stream); + dpcm_be_dai_shutdown(fe, stream);
dev_dbg(fe->dev, "ASoC: close FE %s\n", fe->dai_link->name);
@@ -2412,9 +2411,9 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream, mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME); dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
- memcpy(&fe->dpcm[substream->stream].hw_params, params, + memcpy(&fe->dpcm[stream].hw_params, params, sizeof(struct snd_pcm_hw_params)); - ret = dpcm_be_dai_hw_params(fe, substream->stream); + ret = dpcm_be_dai_hw_params(fe, stream); if (ret < 0) { dev_err(fe->dev,"ASoC: hw_params BE failed %d\n", ret); goto out; @@ -2736,7 +2735,7 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream) goto out; }
- ret = dpcm_be_dai_prepare(fe, substream->stream); + ret = dpcm_be_dai_prepare(fe, stream); if (ret < 0) goto out;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
dpcm_add_paths() checks returned be from dpcm_get_be()
static int dpcm_add_paths(...) { ... for_each_dapm_widgets(list, i, widget) { ... be = dpcm_get_be(...); ... /* make sure BE is a real BE */ => if (!be->dai_link->no_pcm) continue; ... } ... }
But, dpcm_get_be() itself is checking it already.
dpcm_get_be(...) { ... for_each_card_rtds(card, be) { => if (!be->dai_link->no_pcm) continue; ... if (...) => return be; }
return NULL }
This patch removes duplicate check
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 8c27eb4d5e4c..e3a2c4f7757b 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1690,10 +1690,6 @@ static int dpcm_add_paths(struct snd_soc_pcm_runtime *fe, int stream, continue; }
- /* make sure BE is a real BE */ - if (!be->dai_link->no_pcm) - continue; - /* don't connect if FE is not running */ if (!fe->dpcm[stream].runtime && !fe->fe_compr) continue;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
move dpcm_fe_dai_close() next to dpcm_fe_dai_open(). This is prepare for dpcm_fe_dai_open() cleanup
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index e3a2c4f7757b..3686dda097e2 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2978,6 +2978,26 @@ int soc_dpcm_runtime_update(struct snd_soc_card *card) return ret; }
+static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) +{ + struct snd_soc_pcm_runtime *fe = fe_substream->private_data; + struct snd_soc_dpcm *dpcm; + int stream = fe_substream->stream, ret; + + mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME); + ret = dpcm_fe_dai_shutdown(fe_substream); + + /* mark FE's links ready to prune */ + for_each_dpcm_be(fe, stream, dpcm) + dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; + + dpcm_be_disconnect(fe, stream); + + fe->dpcm[stream].runtime = NULL; + mutex_unlock(&fe->card->mutex); + return ret; +} + static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) { struct snd_soc_pcm_runtime *fe = fe_substream->private_data; @@ -3017,26 +3037,6 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) return ret; }
-static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) -{ - struct snd_soc_pcm_runtime *fe = fe_substream->private_data; - struct snd_soc_dpcm *dpcm; - int stream = fe_substream->stream, ret; - - mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME); - ret = dpcm_fe_dai_shutdown(fe_substream); - - /* mark FE's links ready to prune */ - for_each_dpcm_be(fe, stream, dpcm) - dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; - - dpcm_be_disconnect(fe, stream); - - fe->dpcm[stream].runtime = NULL; - mutex_unlock(&fe->card->mutex); - return ret; -} - /* create a new pcm */ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) {
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
dpcm_fe_dai_close() and error case of dpcm_fe_dai_open() need to do same cleanup operation. To avoid duplicate code, this patch adds dpcm_fe_dai_clean() and use it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 3686dda097e2..d578dbdfa846 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2978,14 +2978,11 @@ int soc_dpcm_runtime_update(struct snd_soc_card *card) return ret; }
-static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) +static void dpcm_fe_dai_clean(struct snd_pcm_substream *fe_substream) { struct snd_soc_pcm_runtime *fe = fe_substream->private_data; struct snd_soc_dpcm *dpcm; - int stream = fe_substream->stream, ret; - - mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME); - ret = dpcm_fe_dai_shutdown(fe_substream); + int stream = fe_substream->stream;
/* mark FE's links ready to prune */ for_each_dpcm_be(fe, stream, dpcm) @@ -2994,6 +2991,18 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) dpcm_be_disconnect(fe, stream);
fe->dpcm[stream].runtime = NULL; +} + +static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) +{ + struct snd_soc_pcm_runtime *fe = fe_substream->private_data; + int ret; + + mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME); + ret = dpcm_fe_dai_shutdown(fe_substream); + + dpcm_fe_dai_clean(fe_substream); + mutex_unlock(&fe->card->mutex); return ret; } @@ -3001,7 +3010,6 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) { struct snd_soc_pcm_runtime *fe = fe_substream->private_data; - struct snd_soc_dpcm *dpcm; struct snd_soc_dapm_widget_list *list; int ret; int stream = fe_substream->stream; @@ -3021,14 +3029,8 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) dpcm_process_paths(fe, stream, &list, 1);
ret = dpcm_fe_dai_startup(fe_substream); - if (ret < 0) { - /* clean up all links */ - for_each_dpcm_be(fe, stream, dpcm) - dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE; - - dpcm_be_disconnect(fe, stream); - fe->dpcm[stream].runtime = NULL; - } + if (ret < 0) + dpcm_fe_dai_clean(fe_substream);
dpcm_clear_pending_state(fe, stream); dpcm_path_put(&list);
On Wed, 2020-02-26 at 15:40 +0900, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
dpcm_fe_dai_close() and error case of dpcm_fe_dai_open() need to do same cleanup operation. To avoid duplicate code, this patch adds dpcm_fe_dai_clean() and use it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/soc-pcm.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 3686dda097e2..d578dbdfa846 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2978,14 +2978,11 @@ int soc_dpcm_runtime_update(struct snd_soc_card *card) return ret; }
-static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) +static void dpcm_fe_dai_clean(struct snd_pcm_substream *fe_substream)
The series looks good to me, Morimoto-san. But a minor suggestion if you're doing a v2 to address other comments. Could you please change the name of the function above to dpcm_fe_dai_cleanup() instead?
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Thanks, Ranjani
Hi Ranjani
Thank you for your review
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 3686dda097e2..d578dbdfa846 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2978,14 +2978,11 @@ int soc_dpcm_runtime_update(struct snd_soc_card *card) return ret; }
-static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) +static void dpcm_fe_dai_clean(struct snd_pcm_substream *fe_substream)
The series looks good to me, Morimoto-san. But a minor suggestion if you're doing a v2 to address other comments. Could you please change the name of the function above to dpcm_fe_dai_cleanup() instead?
Yeah, will do. Thanks
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Thank you for your help !! Best regards --- Kuninori Morimoto
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
We already have snd_soc_dai_get_pcm_stream(), let's use it
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index d578dbdfa846..4a14f10b8c90 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2022,7 +2022,6 @@ static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream) struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_dai *cpu_dai; - struct snd_soc_dai_driver *cpu_dai_drv; int i;
for_each_rtd_cpu_dai(rtd, i, cpu_dai) { @@ -2033,11 +2032,9 @@ static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream) if (!snd_soc_dai_stream_valid(cpu_dai, substream->stream)) continue;
- cpu_dai_drv = cpu_dai->driver; - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - dpcm_init_runtime_hw(runtime, &cpu_dai_drv->playback); - else - dpcm_init_runtime_hw(runtime, &cpu_dai_drv->capture); + dpcm_init_runtime_hw(runtime, + snd_soc_dai_get_pcm_stream(cpu_dai, + substream->stream)); }
dpcm_runtime_merge_format(substream, &runtime->hw.formats);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
DAI is counting its activity, and both playback/capture directional activity. When considering mute, DAI's activity is enough instead of caring both playback/capture. This patch makes mute check simply.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 4a14f10b8c90..7e0464ec802e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1202,7 +1202,6 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_dai *cpu_dai; struct snd_soc_dai *codec_dai; - bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; int i;
mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass); @@ -1226,11 +1225,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
/* apply codec digital mute */ for_each_rtd_codec_dai(rtd, i, codec_dai) { - int playback_active = codec_dai->stream_active[SNDRV_PCM_STREAM_PLAYBACK]; - int capture_active = codec_dai->stream_active[SNDRV_PCM_STREAM_CAPTURE]; - - if ((playback && playback_active == 1) || - (!playback && capture_active == 1)) + if (codec_dai->active == 1) snd_soc_dai_digital_mute(codec_dai, 1, substream->stream); }
On 2/26/20 12:40 AM, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
DAI is counting its activity, and both playback/capture directional activity. When considering mute, DAI's activity is enough instead of caring both playback/capture. This patch makes mute check simply.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/soc-pcm.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 4a14f10b8c90..7e0464ec802e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1202,7 +1202,6 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_dai *cpu_dai; struct snd_soc_dai *codec_dai;
bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; int i;
mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
@@ -1226,11 +1225,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
/* apply codec digital mute */ for_each_rtd_codec_dai(rtd, i, codec_dai) {
int playback_active = codec_dai->stream_active[SNDRV_PCM_STREAM_PLAYBACK];int capture_active = codec_dai->stream_active[SNDRV_PCM_STREAM_CAPTURE];if ((playback && playback_active == 1) ||(!playback && capture_active == 1))
if (codec_dai->active == 1)
nit-pick: we have two tests in soc-pcm.c
if (codec_dai->active) if (codec_dai->active == 1)
The two are functionality equivalent but it'd be good to choose one version - or possibly use 'active' as a boolean.
snd_soc_dai_digital_mute(codec_dai, 1, substream->stream);}
Hi Pierre-Louis
@@ -1226,11 +1225,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) /* apply codec digital mute */ for_each_rtd_codec_dai(rtd, i, codec_dai) {
int playback_active = codec_dai->stream_active[SNDRV_PCM_STREAM_PLAYBACK];int capture_active = codec_dai->stream_active[SNDRV_PCM_STREAM_CAPTURE];if ((playback && playback_active == 1) ||(!playback && capture_active == 1))
if (codec_dai->active == 1)nit-pick: we have two tests in soc-pcm.c
if (codec_dai->active) if (codec_dai->active == 1)
The two are functionality equivalent but it'd be good to choose one version - or possibly use 'active' as a boolean.
In my understanding, dai->active can be 0/1/2. But yes, we want to call mute if codec_dai is working
/* Call Mute if Codec DAI is working */ if (codec_dai->active)
/* Call Mute if Codec DAI is used only for Playback or Capture */ if (codec_dai->active == 1)
Thank you for your help !! Best regards --- Kuninori Morimoto
if ((playback && playback_active == 1) ||(!playback && capture_active == 1))
if (codec_dai->active == 1)nit-pick: we have two tests in soc-pcm.c
if (codec_dai->active) if (codec_dai->active == 1)
The two are functionality equivalent but it'd be good to choose one version - or possibly use 'active' as a boolean.
In my understanding, dai->active can be 0/1/2.
I see, I guess I missed this completely. Thanks Morimoto-san for the precision.
Hi Pierre-Louis
if ((playback && playback_active == 1) ||(!playback && capture_active == 1))
if (codec_dai->active == 1)nit-pick: we have two tests in soc-pcm.c
if (codec_dai->active) if (codec_dai->active == 1)
The two are functionality equivalent but it'd be good to choose one version - or possibly use 'active' as a boolean.
In my understanding, dai->active can be 0/1/2.
I see, I guess I missed this completely. Thanks Morimoto-san for the precision.
But, we want to use "if (codec_dai->active)" anyway. Your review indicated my mistake.
Thank you for your help !! Best regards --- Kuninori Morimoto
if ((playback && playback_active == 1) ||(!playback && capture_active == 1))
if (codec_dai->active == 1)nit-pick: we have two tests in soc-pcm.c
if (codec_dai->active) if (codec_dai->active == 1)
The two are functionality equivalent but it'd be good to choose one version - or possibly use 'active' as a boolean.
In my understanding, dai->active can be 0/1/2.
I see, I guess I missed this completely. Thanks Morimoto-san for the precision.
But, we want to use "if (codec_dai->active)" anyway. Your review indicated my mistake.
Not in this case though, the initial idea was to do the mute when only playback or capture were enabled? If you mute when both are enabled then that's a real change.
Hi Pierre-Louis
Not in this case though, the initial idea was to do the mute when only playback or capture were enabled? If you mute when both are enabled then that's a real change.
Let's check original
if ((playback && playback_active == 1) || (!playback && capture_active == 1)) snd_soc_dai_digital_mute(...)
It calls mute if Playback or Capture is working. and my patch (v2) is
if (codec_dai->activity) snd_soc_dai_digital_mute(...)
It calls mute if Codec DAI is working (= Playback or Capture). I think it doesn't change behavior.
Thank you for your help !! Best regards --- Kuninori Morimoto
Hi Pierre-Louis, again
Not in this case though, the initial idea was to do the mute when only playback or capture were enabled? If you mute when both are enabled then that's a real change.
Let's check original
if ((playback && playback_active == 1) || (!playback && capture_active == 1)) snd_soc_dai_digital_mute(...)
It calls mute if Playback or Capture is working. and my patch (v2) is
if (codec_dai->activity) snd_soc_dai_digital_mute(...)
It calls mute if Codec DAI is working (= Playback or Capture). I think it doesn't change behavior.
Oops ! I was wrong. I will post mail to v2 patch for detail
Thank you for your help !! Best regards --- Kuninori Morimoto
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Digital Mute for CPU is done at soc_pcm_close(), and Digital Mute for Codec is done at soc_pcm_hw_free(). It is just confusable. This patch do Digital Mute for both CPU/Codec in same timing. Then, it cares DAI activity
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 7e0464ec802e..1f7a86c4bc02 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -760,9 +760,6 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
snd_soc_runtime_deactivate(rtd, substream->stream);
- for_each_rtd_cpu_dai(rtd, i, cpu_dai) - snd_soc_dai_digital_mute(cpu_dai, 1, substream->stream); - for_each_rtd_cpu_dai(rtd, i, cpu_dai) snd_soc_dai_shutdown(cpu_dai, substream);
@@ -1229,6 +1226,11 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) snd_soc_dai_digital_mute(codec_dai, 1, substream->stream); } + for_each_rtd_cpu_dai(rtd, i, cpu_dai) { + if (cpu_dai->active == 1) + snd_soc_dai_digital_mute(cpu_dai, 1, + substream->stream); + }
/* free any machine hw params */ soc_rtd_hw_free(rtd, substream);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
error handling at dpcm_fe_dai_startup() has duplicate code. This patch tidyup it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 1f7a86c4bc02..2fdd90437a6f 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2165,11 +2165,9 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) goto unwind; }
- dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO); - return 0; - unwind: - dpcm_be_dai_startup_unwind(fe, stream); + if (ret < 0) + dpcm_be_dai_startup_unwind(fe, stream); be_err: dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO); return ret;
On 2/26/20 12:41 AM, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
error handling at dpcm_fe_dai_startup() has duplicate code. This patch tidyup it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/soc-pcm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 1f7a86c4bc02..2fdd90437a6f 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2165,11 +2165,9 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) goto unwind; }
nit-pick: since the two lines below are removed, the 'goto unwind' above becomes unnecessary. I don't mind if you leave it for symmetry with the rest of the error handling flow.
- dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
- return 0;
- unwind:
- dpcm_be_dai_startup_unwind(fe, stream);
- if (ret < 0)
dpcm_be_dai_startup_unwind(fe, stream);
Hi Pierre-Louis
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 1f7a86c4bc02..2fdd90437a6f 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2165,11 +2165,9 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) goto unwind; }
nit-pick: since the two lines below are removed, the 'goto unwind' above becomes unnecessary. I don't mind if you leave it for symmetry with the rest of the error handling flow.
Oops ? Indeed. Thank you for your review Will fixup it in v2
Thank you for your help !! Best regards --- Kuninori Morimoto
On 2/26/20 12:39 AM, Kuninori Morimoto wrote:
Hi Mark
These are soc-pcm cleanup step5. These are based on Bard Liao's "ASoC: Add Multi CPU DAI support" patch-set.
[7/8] patch might exchange the behavior
Kuninori Morimoto (8):
- ASoC: soc-pcm: use defined stream
- ASoC: soc-pcm: remove duplicate be check from dpcm_add_paths()
- ASoC: soc-pcm: move dpcm_fe_dai_close()
- ASoC: soc-pcm: add dpcm_fe_dai_clean()
- ASoC: soc-pcm: use snd_soc_dai_get_pcm_stream() at dpcm_set_fe_runtime()
- ASoC: soc-pcm: check DAI's activity more simply
- ASoC: soc-pcm: Do Digital Mute for both CPU/Codec in same timing.
- ASoC: soc-pcm: tidyup dulicate handing at dpcm_fe_dai_startup()
Looks good to me (only minor nit-picks)
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
if you give 2-3 days we'll provide a Tested-by tag.
sound/soc/soc-pcm.c | 107 ++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 59 deletions(-)
participants (3)
-
Kuninori Morimoto -
Pierre-Louis Bossart -
Ranjani Sridharan