[PATCH v3 0/8] ASoC: soc-pcm: cleanup each functions
Hi Mark
These are v2 of soc-pcm cleanup patches. These has no relationship to each other.
My 1 concern is [3/8] patch. I think it is no problem, but I'm not 100% sure why current code was such code. Pierre-Louis / Liam might about something.
v1 -> v2 - soc_cpu/codec_dai_name() is now inline function - rename soc_pcm_care_symmetry() to soc_pcm_update_symmetry()
v2 -> v3 - log fix at [6/8] - Thus, it will be minus users at (2). + Thus, users will be a negative number at (2)
Link: https://lore.kernel.org/r/87tupuqqc8.wl-kuninori.morimoto.gx@renesas.com Link: https://lore.kernel.org/r/87tupqpg9x.wl-kuninori.morimoto.gx@renesas.com
Kuninori Morimoto (8): ASoC: soc-pcm: check DAI activity under soc_pcm_apply_symmetry() ASoC: soc-pcm: add soc_cpu/codec_dai_name() macro ASoC: soc-pcm: direct copy at snd_soc_set_runtime_hwparams() ASoC: soc-pcm: add soc_pcm_update_symmetry() ASoC: soc-pcm: add soc_hw_sanity_check() ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count ASoC: soc-pcm: remove unneeded !rtd->dai_link check ASoC: soc-pcm: share DPCM BE DAI stop operation
include/sound/soc-dpcm.h | 8 +- sound/soc/soc-compress.c | 2 +- sound/soc/soc-pcm.c | 243 ++++++++++++++++----------------------- 3 files changed, 105 insertions(+), 148 deletions(-)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
soc_pcm_apply_symmetry() is used like below in all cases.
if (snd_soc_dai_active(dai)) { err = soc_pcm_apply_symmetry(fe_substream, dai); ... }
Because of this style, the code is deep nested. This patch checks it under soc_pcm_apply_symmetry(), and makes code simple.
static int soc_pcm_apply_symmetry(...) { ... => if (!snd_soc_dai_active(...)) return 0; ... }
=> ret = soc_pcm_apply_symmetry(); if (ret < 0) ...
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index ba8ffbf8a5d3..9b5ab7a05f65 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -349,6 +349,9 @@ static int soc_pcm_apply_symmetry(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); int ret;
+ if (!snd_soc_dai_active(soc_dai)) + return 0; + #define __soc_pcm_apply_symmetry(name, NAME) \ if (soc_dai->name && (soc_dai->driver->symmetric_##name || \ rtd->dai_link->symmetric_##name)) { \ @@ -765,11 +768,9 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
/* Symmetry only applies if we've already got an active stream. */ for_each_rtd_dais(rtd, i, dai) { - if (snd_soc_dai_active(dai)) { - ret = soc_pcm_apply_symmetry(substream, dai); - if (ret != 0) - goto err; - } + ret = soc_pcm_apply_symmetry(substream, dai); + if (ret != 0) + goto err; }
pr_debug("ASoC: %s <-> %s info:\n", @@ -1693,11 +1694,9 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream,
for_each_rtd_cpu_dais (fe, i, fe_cpu_dai) { /* Symmetry only applies if we've got an active stream. */ - if (snd_soc_dai_active(fe_cpu_dai)) { - err = soc_pcm_apply_symmetry(fe_substream, fe_cpu_dai); - if (err < 0) - return err; - } + err = soc_pcm_apply_symmetry(fe_substream, fe_cpu_dai); + if (err < 0) + return err; }
/* apply symmetry for BE */ @@ -1721,11 +1720,9 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream,
/* Symmetry only applies if we've got an active stream. */ for_each_rtd_dais(rtd, i, dai) { - if (snd_soc_dai_active(dai)) { - err = soc_pcm_apply_symmetry(fe_substream, dai); - if (err < 0) - return err; - } + err = soc_pcm_apply_symmetry(fe_substream, dai); + if (err < 0) + return err; } }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
soc-pcm needs DAI name and it will be "multicpu/multicodec" if it has many DAIs. But current code is using very verbose for it. This patch uses macro and makes code simple.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 9b5ab7a05f65..60e688b103d8 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -29,6 +29,15 @@
#define DPCM_MAX_BE_USERS 8
+static inline const char *soc_cpu_dai_name(struct snd_soc_pcm_runtime *rtd) +{ + return (rtd)->num_cpus == 1 ? asoc_rtd_to_cpu(rtd, 0)->name : "multicpu"; +} +static inline const char *soc_codec_dai_name(struct snd_soc_pcm_runtime *rtd) +{ + return (rtd)->num_codecs == 1 ? asoc_rtd_to_codec(rtd, 0)->name : "multicodec"; +} + #ifdef CONFIG_DEBUG_FS static const char *dpcm_state_string(enum snd_soc_dpcm_state state) { @@ -697,8 +706,8 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_component *component; struct snd_soc_dai *dai; - const char *codec_dai_name = "multicodec"; - const char *cpu_dai_name = "multicpu"; + const char *codec_dai_name = soc_codec_dai_name(rtd); + const char *cpu_dai_name = soc_cpu_dai_name(rtd); int i, ret = 0;
for_each_rtd_components(rtd, i, component) @@ -737,12 +746,6 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) /* Check that the codec and cpu DAIs are compatible */ soc_pcm_init_runtime_hw(substream);
- if (rtd->num_codecs == 1) - codec_dai_name = asoc_rtd_to_codec(rtd, 0)->name; - - if (rtd->num_cpus == 1) - cpu_dai_name = asoc_rtd_to_cpu(rtd, 0)->name; - if (soc_pcm_has_symmetry(substream)) runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX;
@@ -2741,8 +2744,7 @@ static int soc_create_pcm(struct snd_pcm **pcm, else snprintf(new_name, sizeof(new_name), "%s %s-%d", rtd->dai_link->stream_name, - (rtd->num_codecs > 1) ? - "multicodec" : asoc_rtd_to_codec(rtd, 0)->name, num); + soc_codec_dai_name(rtd), num);
ret = snd_pcm_new(rtd->card->snd_card, new_name, num, playback, capture, pcm); @@ -2841,8 +2843,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) pcm->no_device_suspend = true; out: dev_dbg(rtd->card->dev, "%s <-> %s mapping ok\n", - (rtd->num_codecs > 1) ? "multicodec" : asoc_rtd_to_codec(rtd, 0)->name, - (rtd->num_cpus > 1) ? "multicpu" : asoc_rtd_to_cpu(rtd, 0)->name); + soc_codec_dai_name(rtd), soc_cpu_dai_name(rtd)); return ret; }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
snd_soc_set_runtime_hwparams() is called from each driver to initialize hw parameters, but coping each parameters one-by-one.
Current code is not copying all parameters, but no big effect if we do it. This patch copies all parameters by simple code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 60e688b103d8..6f2de27cf18f 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -300,15 +300,8 @@ bool snd_soc_runtime_ignore_pmdown_time(struct snd_soc_pcm_runtime *rtd) int snd_soc_set_runtime_hwparams(struct snd_pcm_substream *substream, const struct snd_pcm_hardware *hw) { - struct snd_pcm_runtime *runtime = substream->runtime; - runtime->hw.info = hw->info; - runtime->hw.formats = hw->formats; - runtime->hw.period_bytes_min = hw->period_bytes_min; - runtime->hw.period_bytes_max = hw->period_bytes_max; - runtime->hw.periods_min = hw->periods_min; - runtime->hw.periods_max = hw->periods_max; - runtime->hw.buffer_bytes_max = hw->buffer_bytes_max; - runtime->hw.fifo_size = hw->fifo_size; + substream->runtime->hw = *hw; + return 0; } EXPORT_SYMBOL_GPL(snd_soc_set_runtime_hwparams);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current soc-pcm has soc_pcm_has_symmetry() and using it as
if (soc_pcm_has_symmetry(substream)) substream->runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX;
We want to share same operation as same function. This patch adds soc_pcm_update_symmetry() and pack above code in one function.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 6f2de27cf18f..4ea4e2af9134 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -410,7 +410,7 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream, return 0; }
-static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream) +static void soc_pcm_update_symmetry(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); struct snd_soc_dai_link *link = rtd->dai_link; @@ -427,7 +427,8 @@ static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream) dai->driver->symmetric_channels || dai->driver->symmetric_sample_bits;
- return symmetry; + if (symmetry) + substream->runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX; }
static void soc_pcm_set_msb(struct snd_pcm_substream *substream, int bits) @@ -739,8 +740,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) /* Check that the codec and cpu DAIs are compatible */ soc_pcm_init_runtime_hw(substream);
- if (soc_pcm_has_symmetry(substream)) - runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX; + soc_pcm_update_symmetry(substream);
ret = -EINVAL; if (!runtime->hw.rates) { @@ -1685,8 +1685,7 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, int i;
/* apply symmetry for FE */ - if (soc_pcm_has_symmetry(fe_substream)) - fe_substream->runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX; + soc_pcm_update_symmetry(fe_substream);
for_each_rtd_cpu_dais (fe, i, fe_cpu_dai) { /* Symmetry only applies if we've got an active stream. */ @@ -1711,8 +1710,7 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, if (rtd->dai_link->be_hw_params_fixup) continue;
- if (soc_pcm_has_symmetry(be_substream)) - be_substream->runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX; + soc_pcm_update_symmetry(be_substream);
/* Symmetry only applies if we've got an active stream. */ for_each_rtd_dais(rtd, i, dai) {
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current soc_pcm_open() is checking runtime->hw parameters, but having such function is very helpful for reading code.
This patch adds new soc_hw_sanity_check() and checks runtime->hw parameters there. And print its debug message there, too.
Debug message print out timing is exchanged after this patch, but it is not a big deal, because it is for debug.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 67 +++++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 27 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 4ea4e2af9134..910a6afe9f48 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -689,6 +689,44 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) return soc_pcm_clean(substream, 0); }
+static int soc_hw_sanity_check(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_pcm_hardware *hw = &substream->runtime->hw; + const char *name_cpu = soc_cpu_dai_name(rtd); + const char *name_codec = soc_codec_dai_name(rtd); + const char *err_msg; + struct device *dev = rtd->dev; + + err_msg = "rates"; + if (!hw->rates) + goto config_err; + + err_msg = "formats"; + if (!hw->formats) + goto config_err; + + err_msg = "channels"; + if (!hw->channels_min || !hw->channels_max || + hw->channels_min > hw->channels_max) + goto config_err; + + dev_dbg(dev, "ASoC: %s <-> %s info:\n", name_codec, + name_cpu); + dev_dbg(dev, "ASoC: rate mask 0x%x\n", hw->rates); + dev_dbg(dev, "ASoC: ch min %d max %d\n", hw->channels_min, + hw->channels_max); + dev_dbg(dev, "ASoC: rate min %d max %d\n", hw->rate_min, + hw->rate_max); + + return 0; + +config_err: + dev_err(dev, "ASoC: %s <-> %s No matching %s\n", + name_codec, name_cpu, err_msg); + return -EINVAL; +} + /* * Called by ALSA when a PCM substream is opened, the runtime->hw record is * then initialized and any private data can be allocated. This also calls @@ -697,11 +735,8 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) static int soc_pcm_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); - struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_component *component; struct snd_soc_dai *dai; - const char *codec_dai_name = soc_codec_dai_name(rtd); - const char *cpu_dai_name = soc_cpu_dai_name(rtd); int i, ret = 0;
for_each_rtd_components(rtd, i, component) @@ -742,23 +777,9 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
soc_pcm_update_symmetry(substream);
- ret = -EINVAL; - if (!runtime->hw.rates) { - printk(KERN_ERR "ASoC: %s <-> %s No matching rates\n", - codec_dai_name, cpu_dai_name); - goto err; - } - if (!runtime->hw.formats) { - printk(KERN_ERR "ASoC: %s <-> %s No matching formats\n", - codec_dai_name, cpu_dai_name); - goto err; - } - if (!runtime->hw.channels_min || !runtime->hw.channels_max || - runtime->hw.channels_min > runtime->hw.channels_max) { - printk(KERN_ERR "ASoC: %s <-> %s No matching channels\n", - codec_dai_name, cpu_dai_name); + ret = soc_hw_sanity_check(substream); + if (ret < 0) goto err; - }
soc_pcm_apply_msb(substream);
@@ -768,14 +789,6 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) if (ret != 0) goto err; } - - pr_debug("ASoC: %s <-> %s info:\n", - codec_dai_name, cpu_dai_name); - pr_debug("ASoC: rate mask 0x%x\n", runtime->hw.rates); - pr_debug("ASoC: min ch %d max ch %d\n", runtime->hw.channels_min, - runtime->hw.channels_max); - pr_debug("ASoC: min rate %d max rate %d\n", runtime->hw.rate_min, - runtime->hw.rate_max); dynamic: snd_soc_runtime_activate(rtd, substream->stream); ret = 0;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
At dpcm_be_dai_startup_unwind(), it indicates error message at (1) if this function was called with no users. But, it doesn't use "continue" here. Thus, users will be a negative number at (2)
void dpcm_be_dai_startup_unwind(...) { ... for_each_dpcm_be(...) { ... (1) if (be->dpcm[stream].users == 0) dev_err(...);
(2) if (--be->dpcm[stream].users != 0) continue;
At dpcm_be_dai_startup(), it indicates error message if user reached to MAX USERS at (A). But, it doesn't use "continue" here. Thus, it will be over MAX USERS at (B).
int dpcm_be_dai_startup(...) { ... for_each_dpcm_be(...) { ... (A) if (be->dpcm[stream].users == DPCM_MAX_BE_USERS) dev_err(...);
(B) if (be->dpcm[stream].users++ != 0) continue;
These are just bug. This patch fixup these.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 910a6afe9f48..626d6e0a3a15 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1432,10 +1432,12 @@ static void dpcm_be_dai_startup_unwind(struct snd_soc_pcm_runtime *fe, struct snd_pcm_substream *be_substream = snd_soc_dpcm_get_substream(be, stream);
- if (be->dpcm[stream].users == 0) + if (be->dpcm[stream].users == 0) { dev_err(be->dev, "ASoC: no users %s at close - state %d\n", stream ? "capture" : "playback", be->dpcm[stream].state); + continue; + }
if (--be->dpcm[stream].users != 0) continue; @@ -1472,10 +1474,12 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) continue;
/* first time the dpcm is open ? */ - if (be->dpcm[stream].users == DPCM_MAX_BE_USERS) + if (be->dpcm[stream].users == DPCM_MAX_BE_USERS) { dev_err(be->dev, "ASoC: too many users %s at open %d\n", stream ? "capture" : "playback", be->dpcm[stream].state); + continue; + }
if (be->dpcm[stream].users++ != 0) continue; @@ -1517,10 +1521,12 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) if (!snd_soc_dpcm_be_can_update(fe, be, stream)) continue;
- if (be->dpcm[stream].users == 0) + if (be->dpcm[stream].users == 0) { dev_err(be->dev, "ASoC: no users %s at close %d\n", stream ? "capture" : "playback", be->dpcm[stream].state); + continue; + }
if (--be->dpcm[stream].users != 0) continue;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
rtd->dai_link is setuped at soc_new_pcm_runtime(), thus "rtd->dai_link == NULL" is never happen. This patch removes unneeded !rtd->dai_link check
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-pcm.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 626d6e0a3a15..0ae386f0790e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -165,9 +165,6 @@ static const struct file_operations dpcm_state_fops = {
void soc_dpcm_debugfs_add(struct snd_soc_pcm_runtime *rtd) { - if (!rtd->dai_link) - return; - if (!rtd->dai_link->dynamic) return;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
soc-pcm has very similar but different DPCM BE DAI stop operation at 1) dpcm_be_dai_startup() error case rollback 2) dpcm_be_dai_startup_unwind() 3) dpcm_be_dai_shutdown()
The differences are 1) for rollback 2) Doesn't check by snd_soc_dpcm_be_can_update() (Is this bug ?) 3) Do soc_pcm_hw_free() if it was not !OPENed and !HW_FREEed, and call soc_pcm_close().
We can share same code by 1) hw_free is not needed. Needs last dpcm as rollback. 2) hw_free is not needed. 3) hw_free is needed.
This patch adds new dpcm_be_dai_stop() and share these 3.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc-dpcm.h | 8 +++- sound/soc/soc-compress.c | 2 +- sound/soc/soc-pcm.c | 94 +++++++++------------------------------- 3 files changed, 28 insertions(+), 76 deletions(-)
diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h index 0f6c50b17bba..d76cb1eeeaca 100644 --- a/include/sound/soc-dpcm.h +++ b/include/sound/soc-dpcm.h @@ -149,7 +149,8 @@ void dpcm_path_put(struct snd_soc_dapm_widget_list **list); int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, int stream, struct snd_soc_dapm_widget_list **list, int new); int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream); -int dpcm_be_dai_shutdown(struct snd_soc_pcm_runtime *fe, int stream); +void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, + int do_hw_free, struct snd_soc_dpcm *last); void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream); void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream); int dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream); @@ -159,4 +160,9 @@ int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream); int dpcm_dapm_stream_event(struct snd_soc_pcm_runtime *fe, int dir, int event);
+#define dpcm_be_dai_startup_rollback(fe, stream, last) \ + dpcm_be_dai_stop(fe, stream, 0, last) +#define dpcm_be_dai_startup_unwind(fe, stream) dpcm_be_dai_stop(fe, stream, 0, NULL) +#define dpcm_be_dai_shutdown(fe, stream) dpcm_be_dai_stop(fe, stream, 1, NULL) + #endif diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 246a5e32e22a..89445ba0e86b 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -189,7 +189,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream) if (ret < 0) dev_err(fe->dev, "Compressed ASoC: hw_free failed: %d\n", ret);
- ret = dpcm_be_dai_shutdown(fe, stream); + dpcm_be_dai_shutdown(fe, stream);
/* mark FE's links ready to prune */ for_each_dpcm_be(fe, stream, dpcm) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 0ae386f0790e..a27385ab7b55 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1417,18 +1417,24 @@ void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) spin_unlock_irqrestore(&fe->card->dpcm_lock, flags); }
-static void dpcm_be_dai_startup_unwind(struct snd_soc_pcm_runtime *fe, - int stream) +void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, + int do_hw_free, struct snd_soc_dpcm *last) { struct snd_soc_dpcm *dpcm;
/* disable any enabled and non active backends */ for_each_dpcm_be(fe, stream, dpcm) { - struct snd_soc_pcm_runtime *be = dpcm->be; struct snd_pcm_substream *be_substream = snd_soc_dpcm_get_substream(be, stream);
+ if (dpcm == last) + return; + + /* is this op for this BE ? */ + if (!snd_soc_dpcm_be_can_update(fe, be, stream)) + continue; + if (be->dpcm[stream].users == 0) { dev_err(be->dev, "ASoC: no users %s at close - state %d\n", stream ? "capture" : "playback", @@ -1439,8 +1445,15 @@ static void dpcm_be_dai_startup_unwind(struct snd_soc_pcm_runtime *fe, if (--be->dpcm[stream].users != 0) continue;
- if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_OPEN) - continue; + if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_OPEN) { + if (!do_hw_free) + continue; + + if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_HW_FREE) { + soc_pcm_hw_free(be_substream); + be->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE; + } + }
soc_pcm_close(be_substream); be_substream->runtime = NULL; @@ -1509,32 +1522,7 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) return count;
unwind: - /* disable any enabled and non active backends */ - for_each_dpcm_be_rollback(fe, stream, dpcm) { - struct snd_soc_pcm_runtime *be = dpcm->be; - struct snd_pcm_substream *be_substream = - snd_soc_dpcm_get_substream(be, stream); - - if (!snd_soc_dpcm_be_can_update(fe, be, stream)) - continue; - - if (be->dpcm[stream].users == 0) { - dev_err(be->dev, "ASoC: no users %s at close %d\n", - stream ? "capture" : "playback", - be->dpcm[stream].state); - continue; - } - - if (--be->dpcm[stream].users != 0) - continue; - - if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_OPEN) - continue; - - soc_pcm_close(be_substream); - be_substream->runtime = NULL; - be->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; - } + dpcm_be_dai_startup_rollback(fe, stream, dpcm);
return err; } @@ -1782,46 +1770,6 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) return ret; }
-int dpcm_be_dai_shutdown(struct snd_soc_pcm_runtime *fe, int stream) -{ - struct snd_soc_dpcm *dpcm; - - /* only shutdown BEs that are either sinks or sources to this FE DAI */ - for_each_dpcm_be(fe, stream, dpcm) { - - struct snd_soc_pcm_runtime *be = dpcm->be; - struct snd_pcm_substream *be_substream = - snd_soc_dpcm_get_substream(be, stream); - - /* is this op for this BE ? */ - if (!snd_soc_dpcm_be_can_update(fe, be, stream)) - continue; - - if (be->dpcm[stream].users == 0) - dev_err(be->dev, "ASoC: no users %s at close - state %d\n", - stream ? "capture" : "playback", - be->dpcm[stream].state); - - if (--be->dpcm[stream].users != 0) - continue; - - if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_HW_FREE) && - (be->dpcm[stream].state != SND_SOC_DPCM_STATE_OPEN)) { - soc_pcm_hw_free(be_substream); - be->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE; - } - - dev_dbg(be->dev, "ASoC: close BE %s\n", - be->dai_link->name); - - soc_pcm_close(be_substream); - be_substream->runtime = NULL; - - be->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; - } - return 0; -} - static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream); @@ -2371,9 +2319,7 @@ static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream) if (err < 0) dev_err(fe->dev,"ASoC: hw_free FE failed %d\n", err);
- err = dpcm_be_dai_shutdown(fe, stream); - if (err < 0) - dev_err(fe->dev,"ASoC: shutdown FE failed %d\n", err); + dpcm_be_dai_shutdown(fe, stream);
/* run the stream event for each BE */ dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_NOP);
On 09 Mar 2021 10:07:24 +0900, Kuninori Morimoto wrote:
These are v2 of soc-pcm cleanup patches. These has no relationship to each other.
My 1 concern is [3/8] patch. I think it is no problem, but I'm not 100% sure why current code was such code. Pierre-Louis / Liam might about something.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/8] ASoC: soc-pcm: check DAI activity under soc_pcm_apply_symmetry() commit: f8fc9ec56f341c2a7aa263049340b11c9956962f [2/8] ASoC: soc-pcm: add soc_cpu/codec_dai_name() macro commit: 6fb8944cd2892e018d13955c2d51579a30744904 [3/8] ASoC: soc-pcm: direct copy at snd_soc_set_runtime_hwparams() commit: 56e749ba756fdc2eff332b8eadda8fca231ad782 [4/8] ASoC: soc-pcm: add soc_pcm_update_symmetry() commit: 68cbc557375e22e921c9fd007dfcb35faeff4908 [5/8] ASoC: soc-pcm: add soc_hw_sanity_check() commit: c393281a3c1cb252735c46efbf8501a3782f9afa [6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count commit: 1db19c151819dea7a0dc4d888250d25abaf229ca [7/8] ASoC: soc-pcm: remove unneeded !rtd->dai_link check commit: 20048a9a4070d046a868c3be3b4f7bdc139cc203 [8/8] ASoC: soc-pcm: share DPCM BE DAI stop operation commit: 531590bb40f827fb3c4398148af0797f95bbaee2
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Kuninori Morimoto -
Mark Brown