[alsa-devel] [PATCH 00/10] ASoC: soc-pcm cleanup step3
Hi Mark
These are step3 of soc-pcm cleanup. Basically it do nothing for behavior, but cleanup code. But, only [09/10] patch exchanges behavior (= care Multi Codec). It is one challenge.
Kuninori Morimoto (10): 1) ASoC: soc-pcm: move dai_get_widget() 2) ASoC: soc-pcm: use dai_get_widget() at dpcm_get_be() 3) ASoC: soc-pcm: use dai_get_widget() at dpcm_end_walk_at_be() 4) ASoC: soc-pcm: use dpcm_get_be() at dpcm_end_walk_at_be() 5) ASoC: soc-pcm: remove soc_dpcm_be_digital_mute() 6) ASoC: soc-pcm: remove snd_soc_dpcm_be_get/set_state() 7) ASoC: soc-pcm: add snd_soc_dpcm_can_be() and remove duplicate code 8) ASoC: soc-pcm: use goto and remove multi return 9) ASoC: soc-pcm: care Multi Codec at soc_dpcm_fe_runtime_update() 10) ASoC: soc.h: add for_each_pcm_stream()
include/sound/soc-dpcm.h | 9 -- include/sound/soc.h | 5 + sound/soc/fsl/fsl_asrc_dma.c | 4 +- sound/soc/soc-core.c | 31 ++--- sound/soc/soc-pcm.c | 237 +++++++++++++---------------------- 5 files changed, 105 insertions(+), 181 deletions(-)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
This patch moves dai_get_widget() to top side. This is prepare for cleanup soc-pcm.c
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 6630fadd6e09..a8adc8d06c05 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -82,6 +82,15 @@ static int soc_rtd_trigger(struct snd_soc_pcm_runtime *rtd, return 0; }
+static inline +struct snd_soc_dapm_widget *dai_get_widget(struct snd_soc_dai *dai, int stream) +{ + if (stream == SNDRV_PCM_STREAM_PLAYBACK) + return dai->playback_widget; + else + return dai->capture_widget; +} + static void snd_soc_runtime_action(struct snd_soc_pcm_runtime *rtd, int stream, int action) { @@ -1287,15 +1296,6 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card, return NULL; }
-static inline struct snd_soc_dapm_widget * - dai_get_widget(struct snd_soc_dai *dai, int stream) -{ - if (stream == SNDRV_PCM_STREAM_PLAYBACK) - return dai->playback_widget; - else - return dai->capture_widget; -} - static int widget_in_list(struct snd_soc_dapm_widget_list *list, struct snd_soc_dapm_widget *widget) {
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
dpcm_get_be() has very duplicate code.
dpcm_get_be() { ... if (stream == SNDRV_PCM_STREAM_PLAYBACK) { (1) /* code for Playback */ } else { (2) /* code for Capture */ } }
The difference between Playback (1) and Capture (2) code is pointer only (= "playback_widget" or "caputre_widget"). OTOH, now we already has dai_get_widget() for it. This means we can merge (1) and (2). This patch do it and remove duplicated code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 41 ++++++++++++----------------------------- 1 file changed, 12 insertions(+), 29 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index a8adc8d06c05..e60dfbcf52b4 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1246,47 +1246,30 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card, struct snd_soc_dapm_widget *widget, int stream) { struct snd_soc_pcm_runtime *be; + struct snd_soc_dapm_widget *w; struct snd_soc_dai *dai; int i;
dev_dbg(card->dev, "ASoC: find BE for widget %s\n", widget->name);
- if (stream == SNDRV_PCM_STREAM_PLAYBACK) { - for_each_card_rtds(card, be) { + for_each_card_rtds(card, be) {
- if (!be->dai_link->no_pcm) - continue; + if (!be->dai_link->no_pcm) + continue;
- dev_dbg(card->dev, "ASoC: try BE : %s\n", - be->cpu_dai->playback_widget ? - be->cpu_dai->playback_widget->name : "(not set)"); + w = dai_get_widget(be->cpu_dai, stream);
- if (be->cpu_dai->playback_widget == widget) - return be; + dev_dbg(card->dev, "ASoC: try BE : %s\n", + w ? w->name : "(not set)");
- for_each_rtd_codec_dai(be, i, dai) { - if (dai->playback_widget == widget) - return be; - } - } - } else { - - for_each_card_rtds(card, be) { - - if (!be->dai_link->no_pcm) - continue; + if (w == widget) + return be;
- dev_dbg(card->dev, "ASoC: try BE %s\n", - be->cpu_dai->capture_widget ? - be->cpu_dai->capture_widget->name : "(not set)"); + for_each_rtd_codec_dai(be, i, dai) { + w = dai_get_widget(dai, stream);
- if (be->cpu_dai->capture_widget == widget) + if (w == widget) return be; - - for_each_rtd_codec_dai(be, i, dai) { - if (dai->capture_widget == widget) - return be; - } } }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
dpcm_end_walk_at_be() has very duplicate code.
dpcm_end_walk_at_be() { ... if (stream == SNDRV_PCM_STREAM_PLAYBACK) { (1) /* code for Playback */ } else { (2) /* code for Capture */ } }
The difference between Playback (1) and Capture (2) code is pointer only (= "playback_widget" or "caputre_widget"). OTOH, now we already has dai_get_widget() for it. This means we can merge (1) and (2). This patch do it and remove duplicated code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index e60dfbcf52b4..62ca275f02b0 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1297,34 +1297,29 @@ static bool dpcm_end_walk_at_be(struct snd_soc_dapm_widget *widget, { struct snd_soc_card *card = widget->dapm->card; struct snd_soc_pcm_runtime *rtd; + struct snd_soc_dapm_widget *w; struct snd_soc_dai *dai; + int stream; int i;
- if (dir == SND_SOC_DAPM_DIR_OUT) { - for_each_card_rtds(card, rtd) { - if (!rtd->dai_link->no_pcm) - continue; + /* adjust dir to stream */ + if (dir == SND_SOC_DAPM_DIR_OUT) + stream = SNDRV_PCM_STREAM_PLAYBACK; + else + stream = SNDRV_PCM_STREAM_CAPTURE;
- if (rtd->cpu_dai->playback_widget == widget) - return true; + for_each_card_rtds(card, rtd) { + if (!rtd->dai_link->no_pcm) + continue;
- for_each_rtd_codec_dai(rtd, i, dai) { - if (dai->playback_widget == widget) - return true; - } - } - } else { /* SND_SOC_DAPM_DIR_IN */ - for_each_card_rtds(card, rtd) { - if (!rtd->dai_link->no_pcm) - continue; + w = dai_get_widget(rtd->cpu_dai, stream); + if (w == widget) + return true;
- if (rtd->cpu_dai->capture_widget == widget) + for_each_rtd_codec_dai(rtd, i, dai) { + w = dai_get_widget(dai, stream); + if (w == widget) return true; - - for_each_rtd_codec_dai(rtd, i, dai) { - if (dai->capture_widget == widget) - return true; - } } }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
dpcm_end_walk_at_be() and dpcm_get_be() are almost same code. This patch uses dpcm_get_be() from dpcm_end_walk_at_be().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 62ca275f02b0..18fbf27aad01 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1297,10 +1297,7 @@ static bool dpcm_end_walk_at_be(struct snd_soc_dapm_widget *widget, { struct snd_soc_card *card = widget->dapm->card; struct snd_soc_pcm_runtime *rtd; - struct snd_soc_dapm_widget *w; - struct snd_soc_dai *dai; int stream; - int i;
/* adjust dir to stream */ if (dir == SND_SOC_DAPM_DIR_OUT) @@ -1308,20 +1305,9 @@ static bool dpcm_end_walk_at_be(struct snd_soc_dapm_widget *widget, else stream = SNDRV_PCM_STREAM_CAPTURE;
- for_each_card_rtds(card, rtd) { - if (!rtd->dai_link->no_pcm) - continue; - - w = dai_get_widget(rtd->cpu_dai, stream); - if (w == widget) - return true; - - for_each_rtd_codec_dai(rtd, i, dai) { - w = dai_get_widget(dai, stream); - if (w == widget) - return true; - } - } + rtd = dpcm_get_be(card, widget, stream); + if (rtd) + return true;
return false; }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
No one is using soc_dpcm_be_digital_mute(). If it exists only by assumption that "it may be necessary someday", let's remove it now. Otherwise code maintenance will be difficult. We can revive it when we really needed it. Let's remove it, so far.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc-dpcm.h | 1 - sound/soc/soc-pcm.c | 27 --------------------------- 2 files changed, 28 deletions(-)
diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index b654ebfc8766..665516387671 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -141,7 +141,6 @@ void snd_soc_dpcm_be_set_state(struct snd_soc_pcm_runtime *be, int stream, enum snd_soc_dpcm_state state);
/* internal use only */ -int soc_dpcm_be_digital_mute(struct snd_soc_pcm_runtime *fe, int mute); int soc_dpcm_runtime_update(struct snd_soc_card *);
#ifdef CONFIG_DEBUG_FS diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 18fbf27aad01..aafa97bbcd1d 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2719,33 +2719,6 @@ int soc_dpcm_runtime_update(struct snd_soc_card *card) mutex_unlock(&card->mutex); return ret; } -int soc_dpcm_be_digital_mute(struct snd_soc_pcm_runtime *fe, int mute) -{ - struct snd_soc_dpcm *dpcm; - struct snd_soc_dai *dai; - - for_each_dpcm_be(fe, SNDRV_PCM_STREAM_PLAYBACK, dpcm) { - - struct snd_soc_pcm_runtime *be = dpcm->be; - int i; - - if (be->dai_link->ignore_suspend) - continue; - - for_each_rtd_codec_dai(be, i, dai) { - struct snd_soc_dai_driver *drv = dai->driver; - - dev_dbg(be->dev, "ASoC: BE digital mute %s\n", - be->dai_link->name); - - if (drv->ops && drv->ops->digital_mute && - dai->playback_active) - drv->ops->digital_mute(dai, mute); - } - } - - return 0; -}
static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) {
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
No one is using snd_soc_dpcm_be_get/set_state(). If it exists only by assumption that "it may be necessary someday", let's remove it now. Otherwise code maintenance will be difficult. We can revive it when we really needed it. Let's remove it, so far.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc-dpcm.h | 8 -------- sound/soc/soc-pcm.c | 16 ---------------- 2 files changed, 24 deletions(-)
diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index 665516387671..3e7819d2a6aa 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -132,14 +132,6 @@ int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe, struct snd_pcm_substream * snd_soc_dpcm_get_substream(struct snd_soc_pcm_runtime *be, int stream);
-/* get the BE runtime state */ -enum snd_soc_dpcm_state - snd_soc_dpcm_be_get_state(struct snd_soc_pcm_runtime *be, int stream); - -/* set the BE runtime state */ -void snd_soc_dpcm_be_set_state(struct snd_soc_pcm_runtime *be, int stream, - enum snd_soc_dpcm_state state); - /* internal use only */ int soc_dpcm_runtime_update(struct snd_soc_card *);
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index aafa97bbcd1d..acc8a7eabfbe 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2957,22 +2957,6 @@ struct snd_pcm_substream * } EXPORT_SYMBOL_GPL(snd_soc_dpcm_get_substream);
-/* get the BE runtime state */ -enum snd_soc_dpcm_state - snd_soc_dpcm_be_get_state(struct snd_soc_pcm_runtime *be, int stream) -{ - return be->dpcm[stream].state; -} -EXPORT_SYMBOL_GPL(snd_soc_dpcm_be_get_state); - -/* set the BE runtime state */ -void snd_soc_dpcm_be_set_state(struct snd_soc_pcm_runtime *be, - int stream, enum snd_soc_dpcm_state state) -{ - be->dpcm[stream].state = state; -} -EXPORT_SYMBOL_GPL(snd_soc_dpcm_be_set_state); - /* * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE * are not running, paused or suspended for the specified stream direction.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Below functions are doing very similar things, the difference is only used state.
snd_soc_dpcm_can_be_free_stop() snd_soc_dpcm_can_be_params()
This patch adds common snd_soc_dpcm_can_be(), and use it from snd_soc_dpcm_can_be_free_stop() / snd_soc_dpcm_can_be_params(). This can reduce duplicate code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 70 ++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 35 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index acc8a7eabfbe..140f07b91d6a 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2957,17 +2957,17 @@ struct snd_pcm_substream * } EXPORT_SYMBOL_GPL(snd_soc_dpcm_get_substream);
-/* - * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE - * are not running, paused or suspended for the specified stream direction. - */ -int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, - struct snd_soc_pcm_runtime *be, int stream) +static int snd_soc_dpcm_can_be(struct snd_soc_pcm_runtime *fe, + struct snd_soc_pcm_runtime *be, + int stream, + enum snd_soc_dpcm_state *states, + int num_states) { struct snd_soc_dpcm *dpcm; int state; int ret = 1; unsigned long flags; + int i;
spin_lock_irqsave(&fe->card->dpcm_lock, flags); for_each_dpcm_fe(be, stream, dpcm) { @@ -2976,18 +2976,34 @@ int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, continue;
state = dpcm->fe->dpcm[stream].state; - if (state == SND_SOC_DPCM_STATE_START || - state == SND_SOC_DPCM_STATE_PAUSED || - state == SND_SOC_DPCM_STATE_SUSPEND) { - ret = 0; - break; + for (i = 0; i < num_states; i++) { + if (state == states[i]) { + ret = 0; + break; + } } } spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
- /* it's safe to free/stop this BE DAI */ + /* it's safe to do this BE DAI */ return ret; } + +/* + * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE + * are not running, paused or suspended for the specified stream direction. + */ +int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe, + struct snd_soc_pcm_runtime *be, int stream) +{ + enum snd_soc_dpcm_state state[] = { + SND_SOC_DPCM_STATE_START, + SND_SOC_DPCM_STATE_PAUSED, + SND_SOC_DPCM_STATE_SUSPEND, + }; + + return snd_soc_dpcm_can_be(fe, be, stream, state, ARRAY_SIZE(state)); +} EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop);
/* @@ -2997,30 +3013,14 @@ EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop); int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe, struct snd_soc_pcm_runtime *be, int stream) { - struct snd_soc_dpcm *dpcm; - int state; - int ret = 1; - unsigned long flags; - - spin_lock_irqsave(&fe->card->dpcm_lock, flags); - for_each_dpcm_fe(be, stream, dpcm) { - - if (dpcm->fe == fe) - continue; + enum snd_soc_dpcm_state state[] = { + SND_SOC_DPCM_STATE_START, + SND_SOC_DPCM_STATE_PAUSED, + SND_SOC_DPCM_STATE_SUSPEND, + SND_SOC_DPCM_STATE_PREPARE, + };
- state = dpcm->fe->dpcm[stream].state; - if (state == SND_SOC_DPCM_STATE_START || - state == SND_SOC_DPCM_STATE_PAUSED || - state == SND_SOC_DPCM_STATE_SUSPEND || - state == SND_SOC_DPCM_STATE_PREPARE) { - ret = 0; - break; - } - } - spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); - - /* it's safe to change hw_params */ - return ret; + return snd_soc_dpcm_can_be(fe, be, stream, state, ARRAY_SIZE(state)); } EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params);
+int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
struct snd_soc_pcm_runtime *be, int stream)+{
- enum snd_soc_dpcm_state state[] = {
SND_SOC_DPCM_STATE_START,SND_SOC_DPCM_STATE_PAUSED,SND_SOC_DPCM_STATE_SUSPEND,- };
should this be const?
- enum snd_soc_dpcm_state state[] = {
SND_SOC_DPCM_STATE_START,SND_SOC_DPCM_STATE_PAUSED,SND_SOC_DPCM_STATE_SUSPEND,SND_SOC_DPCM_STATE_PREPARE,- };
const as well?
Hi Pierre-Louis
+int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
struct snd_soc_pcm_runtime *be, int stream)+{
- enum snd_soc_dpcm_state state[] = {
SND_SOC_DPCM_STATE_START,SND_SOC_DPCM_STATE_PAUSED,SND_SOC_DPCM_STATE_SUSPEND,- };
should this be const?
- enum snd_soc_dpcm_state state[] = {
SND_SOC_DPCM_STATE_START,SND_SOC_DPCM_STATE_PAUSED,SND_SOC_DPCM_STATE_SUSPEND,SND_SOC_DPCM_STATE_PREPARE,- };
const as well?
Yeah, indeed. Thanks. Will fix in v2
Thank you for your help !! Best regards --- Kuninori Morimoto
On Thu, 2020-02-13 at 13:26 +0900, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Below functions are doing very similar things, the difference is only used state.
snd_soc_dpcm_can_be_free_stop() snd_soc_dpcm_can_be_params()
This patch adds common snd_soc_dpcm_can_be(), and use it from snd_soc_dpcm_can_be_free_stop() / snd_soc_dpcm_can_be_params(). This can reduce duplicate code.
Morimoto-san,
Only minor but does it make it a bit more intuitive to call this new function snd_soc_dpcm_check_state() or something equivalent maybe?
Thanks, Ranjani
Hi Ranjani
Below functions are doing very similar things, the difference is only used state.
snd_soc_dpcm_can_be_free_stop() snd_soc_dpcm_can_be_params()
This patch adds common snd_soc_dpcm_can_be(), and use it from snd_soc_dpcm_can_be_free_stop() / snd_soc_dpcm_can_be_params(). This can reduce duplicate code.
Morimoto-san,
Only minor but does it make it a bit more intuitive to call this new function snd_soc_dpcm_check_state() or something equivalent maybe?
Thank you for pointing it. I like understandable naming :) Will fix it in v2
Thank you for your help !! Best regards --- Kuninori Morimoto
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
When we use some kind of lock, we need to do unlock. In that time, multi unlock/return is not good implementation. This patch add label and use goto at dpcm_fe_dai_open() to reduce such code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 140f07b91d6a..95fe915f26b0 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2733,8 +2733,7 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream)
ret = dpcm_path_get(fe, stream, &list); if (ret < 0) { - mutex_unlock(&fe->card->mutex); - return ret; + goto open_end; } else if (ret == 0) { dev_dbg(fe->dev, "ASoC: %s no valid %s route\n", fe->dai_link->name, stream ? "capture" : "playback"); @@ -2755,6 +2754,7 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream)
dpcm_clear_pending_state(fe, stream); dpcm_path_put(&list); +open_end: mutex_unlock(&fe->card->mutex); return ret; }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
soc_dpcm_fe_runtime_update() doesn't care Multi Codec now. We need to care it. This patch fixup it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 95fe915f26b0..962fe6cb7d3e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2614,8 +2614,10 @@ static int dpcm_run_old_update(struct snd_soc_pcm_runtime *fe, int stream)
static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) { + struct snd_soc_dai *codec_dai; struct snd_soc_dapm_widget_list *list; int count, paths; + int i;
if (!fe->dai_link->dynamic) return 0; @@ -2629,13 +2631,18 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) new ? "new" : "old", fe->dai_link->name);
/* skip if FE doesn't have playback capability */ - if (!snd_soc_dai_stream_valid(fe->cpu_dai, SNDRV_PCM_STREAM_PLAYBACK) || - !snd_soc_dai_stream_valid(fe->codec_dai, SNDRV_PCM_STREAM_PLAYBACK)) + if (!snd_soc_dai_stream_valid(fe->cpu_dai, SNDRV_PCM_STREAM_PLAYBACK)) goto capture; + for_each_rtd_codec_dai(fe, i, codec_dai) + if (!snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK)) + goto capture;
/* skip if FE isn't currently playing */ - if (!fe->cpu_dai->playback_active || !fe->codec_dai->playback_active) + if (!fe->cpu_dai->playback_active) goto capture; + for_each_rtd_codec_dai(fe, i, codec_dai) + if (!codec_dai->playback_active) + goto capture;
paths = dpcm_path_get(fe, SNDRV_PCM_STREAM_PLAYBACK, &list); if (paths < 0) { @@ -2660,13 +2667,18 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new)
capture: /* skip if FE doesn't have capture capability */ - if (!snd_soc_dai_stream_valid(fe->cpu_dai, SNDRV_PCM_STREAM_CAPTURE) || - !snd_soc_dai_stream_valid(fe->codec_dai, SNDRV_PCM_STREAM_CAPTURE)) + if (!snd_soc_dai_stream_valid(fe->cpu_dai, SNDRV_PCM_STREAM_CAPTURE)) return 0; + for_each_rtd_codec_dai(fe, i, codec_dai) + if (!snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE)) + return 0;
/* skip if FE isn't currently capturing */ - if (!fe->cpu_dai->capture_active || !fe->codec_dai->capture_active) + if (!fe->cpu_dai->capture_active) return 0; + for_each_rtd_codec_dai(fe, i, codec_dai) + if (!codec_dai->capture_active) + return 0;
paths = dpcm_path_get(fe, SNDRV_PCM_STREAM_CAPTURE, &list); if (paths < 0) {
On 2/12/20 10:26 PM, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
soc_dpcm_fe_runtime_update() doesn't care Multi Codec now. We need to care it. This patch fixup it.
Humm, maybe a stupid question but for my education is there an actual case where a front-end dailink uses more that one codec_dai? All the examples I see for Intel rely on COMP_DUMMY().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/soc-pcm.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 95fe915f26b0..962fe6cb7d3e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2614,8 +2614,10 @@ static int dpcm_run_old_update(struct snd_soc_pcm_runtime *fe, int stream)
static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) {
struct snd_soc_dai *codec_dai; struct snd_soc_dapm_widget_list *list; int count, paths;
int i;
if (!fe->dai_link->dynamic) return 0;
@@ -2629,13 +2631,18 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new) new ? "new" : "old", fe->dai_link->name);
/* skip if FE doesn't have playback capability */
- if (!snd_soc_dai_stream_valid(fe->cpu_dai, SNDRV_PCM_STREAM_PLAYBACK) ||
!snd_soc_dai_stream_valid(fe->codec_dai, SNDRV_PCM_STREAM_PLAYBACK))
if (!snd_soc_dai_stream_valid(fe->cpu_dai, SNDRV_PCM_STREAM_PLAYBACK)) goto capture;
for_each_rtd_codec_dai(fe, i, codec_dai)
if (!snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK))goto capture;/* skip if FE isn't currently playing */
- if (!fe->cpu_dai->playback_active || !fe->codec_dai->playback_active)
if (!fe->cpu_dai->playback_active) goto capture;
for_each_rtd_codec_dai(fe, i, codec_dai)
if (!codec_dai->playback_active)goto capture;paths = dpcm_path_get(fe, SNDRV_PCM_STREAM_PLAYBACK, &list); if (paths < 0) {
@@ -2660,13 +2667,18 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new)
capture: /* skip if FE doesn't have capture capability */
- if (!snd_soc_dai_stream_valid(fe->cpu_dai, SNDRV_PCM_STREAM_CAPTURE) ||
!snd_soc_dai_stream_valid(fe->codec_dai, SNDRV_PCM_STREAM_CAPTURE))
if (!snd_soc_dai_stream_valid(fe->cpu_dai, SNDRV_PCM_STREAM_CAPTURE)) return 0;
for_each_rtd_codec_dai(fe, i, codec_dai)
if (!snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE))return 0;/* skip if FE isn't currently capturing */
- if (!fe->cpu_dai->capture_active || !fe->codec_dai->capture_active)
if (!fe->cpu_dai->capture_active) return 0;
for_each_rtd_codec_dai(fe, i, codec_dai)
if (!codec_dai->capture_active)return 0;paths = dpcm_path_get(fe, SNDRV_PCM_STREAM_CAPTURE, &list); if (paths < 0) {
Hi Pierre-Louis
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
soc_dpcm_fe_runtime_update() doesn't care Multi Codec now. We need to care it. This patch fixup it.
Humm, maybe a stupid question but for my education is there an actual case where a front-end dailink uses more that one codec_dai? All the examples I see for Intel rely on COMP_DUMMY().
Hmm..? indeed... We might need it in the future somehow, but not so much so far. OK, let's skip it this time. Thank you for pointing it.
Thank you for your help !! Best regards --- Kuninori Morimoto
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
ALSA SoC has SNDRV_PCM_STREAM_PLAYBACK/CAPTURE everywhere. Having for_each_xxxx macro is useful. This patch adds for_each_pcm_stream() for it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 5 +++++ sound/soc/fsl/fsl_asrc_dma.c | 4 ++-- sound/soc/soc-core.c | 31 +++++++++++++------------------ 3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index f0e4f36f83bf..58af52efa07d 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -419,6 +419,11 @@ enum snd_soc_card_subclass { SND_SOC_CARD_CLASS_RUNTIME = 1, };
+#define for_each_pcm_stream(stream) \ + for (stream = SNDRV_PCM_STREAM_PLAYBACK; \ + stream <= SNDRV_PCM_STREAM_LAST; \ + stream++) + int snd_soc_register_card(struct snd_soc_card *card); int snd_soc_unregister_card(struct snd_soc_card *card); int devm_snd_soc_register_card(struct device *dev, struct snd_soc_card *card); diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c index ece130f59d15..2f5a62381f94 100644 --- a/sound/soc/fsl/fsl_asrc_dma.c +++ b/sound/soc/fsl/fsl_asrc_dma.c @@ -400,7 +400,7 @@ static int fsl_asrc_dma_pcm_new(struct snd_soc_component *component, return ret; }
- for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_LAST; i++) { + for_each_pcm_stream(i) { substream = pcm->streams[i].substream; if (!substream) continue; @@ -428,7 +428,7 @@ static void fsl_asrc_dma_pcm_free(struct snd_soc_component *component, struct snd_pcm_substream *substream; int i;
- for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_LAST; i++) { + for_each_pcm_stream(i) { substream = pcm->streams[i].substream; if (!substream) continue; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 068d809c349a..dc58ce766f3b 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -431,6 +431,7 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime( struct snd_soc_component *component; struct device *dev; int ret; + int stream;
/* * for rtd->dev @@ -465,10 +466,10 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
rtd->dev = dev; INIT_LIST_HEAD(&rtd->list); - INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_PLAYBACK].be_clients); - INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].be_clients); - INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_PLAYBACK].fe_clients); - INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].fe_clients); + for_each_pcm_stream(stream) { + INIT_LIST_HEAD(&rtd->dpcm[stream].be_clients); + INIT_LIST_HEAD(&rtd->dpcm[stream].fe_clients); + } dev_set_drvdata(dev, rtd); INIT_DELAYED_WORK(&rtd->delayed_work, close_delayed_work);
@@ -558,17 +559,14 @@ int snd_soc_suspend(struct device *dev) snd_soc_flush_all_delayed_work(card);
for_each_card_rtds(card, rtd) { + int stream;
if (rtd->dai_link->ignore_suspend) continue;
- snd_soc_dapm_stream_event(rtd, - SNDRV_PCM_STREAM_PLAYBACK, - SND_SOC_DAPM_STREAM_SUSPEND); - - snd_soc_dapm_stream_event(rtd, - SNDRV_PCM_STREAM_CAPTURE, - SND_SOC_DAPM_STREAM_SUSPEND); + for_each_pcm_stream(stream) + snd_soc_dapm_stream_event(rtd, stream, + SND_SOC_DAPM_STREAM_SUSPEND); }
/* Recheck all endpoints too, their state is affected by suspend */ @@ -664,17 +662,14 @@ static void soc_resume_deferred(struct work_struct *work) }
for_each_card_rtds(card, rtd) { + int stream;
if (rtd->dai_link->ignore_suspend) continue;
- snd_soc_dapm_stream_event(rtd, - SNDRV_PCM_STREAM_PLAYBACK, - SND_SOC_DAPM_STREAM_RESUME); - - snd_soc_dapm_stream_event(rtd, - SNDRV_PCM_STREAM_CAPTURE, - SND_SOC_DAPM_STREAM_RESUME); + for_each_pcm_stream(stream) + snd_soc_dapm_stream_event(rtd, stream, + SND_SOC_DAPM_STREAM_RESUME); }
/* unmute any active DACs */
On Thu, 13 Feb 2020 05:26:53 +0100, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
ALSA SoC has SNDRV_PCM_STREAM_PLAYBACK/CAPTURE everywhere. Having for_each_xxxx macro is useful. This patch adds for_each_pcm_stream() for it.
This macro can be put in sound/pcm.h. The similar pattern is found also generically in many non-ASoC codes, too.
thanks,
Takashi
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
include/sound/soc.h | 5 +++++ sound/soc/fsl/fsl_asrc_dma.c | 4 ++-- sound/soc/soc-core.c | 31 +++++++++++++------------------ 3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index f0e4f36f83bf..58af52efa07d 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -419,6 +419,11 @@ enum snd_soc_card_subclass { SND_SOC_CARD_CLASS_RUNTIME = 1, };
+#define for_each_pcm_stream(stream) \
- for (stream = SNDRV_PCM_STREAM_PLAYBACK; \
stream <= SNDRV_PCM_STREAM_LAST; \stream++)int snd_soc_register_card(struct snd_soc_card *card); int snd_soc_unregister_card(struct snd_soc_card *card); int devm_snd_soc_register_card(struct device *dev, struct snd_soc_card *card); diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c index ece130f59d15..2f5a62381f94 100644 --- a/sound/soc/fsl/fsl_asrc_dma.c +++ b/sound/soc/fsl/fsl_asrc_dma.c @@ -400,7 +400,7 @@ static int fsl_asrc_dma_pcm_new(struct snd_soc_component *component, return ret; }
- for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_LAST; i++) {
- for_each_pcm_stream(i) { substream = pcm->streams[i].substream; if (!substream) continue;
@@ -428,7 +428,7 @@ static void fsl_asrc_dma_pcm_free(struct snd_soc_component *component, struct snd_pcm_substream *substream; int i;
- for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_LAST; i++) {
- for_each_pcm_stream(i) { substream = pcm->streams[i].substream; if (!substream) continue;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 068d809c349a..dc58ce766f3b 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -431,6 +431,7 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime( struct snd_soc_component *component; struct device *dev; int ret;
int stream;
/*
- for rtd->dev
@@ -465,10 +466,10 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
rtd->dev = dev; INIT_LIST_HEAD(&rtd->list);
- INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_PLAYBACK].be_clients);
- INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].be_clients);
- INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_PLAYBACK].fe_clients);
- INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].fe_clients);
- for_each_pcm_stream(stream) {
INIT_LIST_HEAD(&rtd->dpcm[stream].be_clients);INIT_LIST_HEAD(&rtd->dpcm[stream].fe_clients);- } dev_set_drvdata(dev, rtd); INIT_DELAYED_WORK(&rtd->delayed_work, close_delayed_work);
@@ -558,17 +559,14 @@ int snd_soc_suspend(struct device *dev) snd_soc_flush_all_delayed_work(card);
for_each_card_rtds(card, rtd) {
int stream;if (rtd->dai_link->ignore_suspend) continue;
snd_soc_dapm_stream_event(rtd,SNDRV_PCM_STREAM_PLAYBACK,SND_SOC_DAPM_STREAM_SUSPEND);snd_soc_dapm_stream_event(rtd,SNDRV_PCM_STREAM_CAPTURE,SND_SOC_DAPM_STREAM_SUSPEND);
for_each_pcm_stream(stream)snd_soc_dapm_stream_event(rtd, stream,SND_SOC_DAPM_STREAM_SUSPEND);}
/* Recheck all endpoints too, their state is affected by suspend */
@@ -664,17 +662,14 @@ static void soc_resume_deferred(struct work_struct *work) }
for_each_card_rtds(card, rtd) {
int stream;if (rtd->dai_link->ignore_suspend) continue;
snd_soc_dapm_stream_event(rtd,SNDRV_PCM_STREAM_PLAYBACK,SND_SOC_DAPM_STREAM_RESUME);snd_soc_dapm_stream_event(rtd,SNDRV_PCM_STREAM_CAPTURE,SND_SOC_DAPM_STREAM_RESUME);
for_each_pcm_stream(stream)snd_soc_dapm_stream_event(rtd, stream,SND_SOC_DAPM_STREAM_RESUME);}
/* unmute any active DACs */
-- 2.17.1
Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Takashi
Thank you for your feedback
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
ALSA SoC has SNDRV_PCM_STREAM_PLAYBACK/CAPTURE everywhere. Having for_each_xxxx macro is useful. This patch adds for_each_pcm_stream() for it.
This macro can be put in sound/pcm.h. The similar pattern is found also generically in many non-ASoC codes, too.
(snip)
+#define for_each_pcm_stream(stream) \
- for (stream = SNDRV_PCM_STREAM_PLAYBACK; \
stream <= SNDRV_PCM_STREAM_LAST; \stream++)
I see. Will do in v2
Thank you for your help !! Best regards --- Kuninori Morimoto
On 2/12/20 10:26 PM, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
ALSA SoC has SNDRV_PCM_STREAM_PLAYBACK/CAPTURE everywhere. Having for_each_xxxx macro is useful. This patch adds for_each_pcm_stream() for it.
Indeed, we also have ugly code in SOF for this:
for (dir = 0; dir <= SNDRV_PCM_STREAM_CAPTURE; dir++) {
a macro would be much nicer, good suggestion.
Hi Pierre-Louis
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
ALSA SoC has SNDRV_PCM_STREAM_PLAYBACK/CAPTURE everywhere. Having for_each_xxxx macro is useful. This patch adds for_each_pcm_stream() for it.
Indeed, we also have ugly code in SOF for this:
for (dir = 0; dir <= SNDRV_PCM_STREAM_CAPTURE; dir++) {
a macro would be much nicer, good suggestion.
Thank you for pointing it. I will care it in v2 :)
Thank you for your help !! Best regards --- Kuninori Morimoto
participants (4)
-
Kuninori Morimoto -
Pierre-Louis Bossart -
Ranjani Sridharan -
Takashi Iwai