* [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions
@ 2021-03-05 0:59 Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 1/8] ASoC: soc-pcm: check DAI activity under soc_pcm_apply_symmetry() Kuninori Morimoto
` (7 more replies)
0 siblings, 8 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 0:59 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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.
v1 -> v2
- soc_cpu/codec_dai_name() is now inline function
- rename soc_pcm_care_symmetry() to soc_pcm_update_symmetry()
Link: https://lore.kernel.org/r/87tupuqqc8.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(-)
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/8] ASoC: soc-pcm: check DAI activity under soc_pcm_apply_symmetry()
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
@ 2021-03-05 0:59 ` Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 2/8] ASoC: soc-pcm: add soc_cpu/codec_dai_name() macro Kuninori Morimoto
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 0:59 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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;
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/8] ASoC: soc-pcm: add soc_cpu/codec_dai_name() macro
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 1/8] ASoC: soc-pcm: check DAI activity under soc_pcm_apply_symmetry() Kuninori Morimoto
@ 2021-03-05 0:59 ` Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 3/8] ASoC: soc-pcm: direct copy at snd_soc_set_runtime_hwparams() Kuninori Morimoto
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 0:59 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/8] ASoC: soc-pcm: direct copy at snd_soc_set_runtime_hwparams()
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 1/8] ASoC: soc-pcm: check DAI activity under soc_pcm_apply_symmetry() Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 2/8] ASoC: soc-pcm: add soc_cpu/codec_dai_name() macro Kuninori Morimoto
@ 2021-03-05 0:59 ` Kuninori Morimoto
2021-03-05 16:11 ` Pierre-Louis Bossart
2021-03-05 0:59 ` [PATCH 4/8] ASoC: soc-pcm: add soc_pcm_update_symmetry() Kuninori Morimoto
` (4 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 0:59 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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);
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/8] ASoC: soc-pcm: add soc_pcm_update_symmetry()
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
` (2 preceding siblings ...)
2021-03-05 0:59 ` [PATCH 3/8] ASoC: soc-pcm: direct copy at snd_soc_set_runtime_hwparams() Kuninori Morimoto
@ 2021-03-05 0:59 ` Kuninori Morimoto
2021-03-05 1:00 ` [PATCH 5/8] ASoC: soc-pcm: add soc_hw_sanity_check() Kuninori Morimoto
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 0:59 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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) {
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/8] ASoC: soc-pcm: add soc_hw_sanity_check()
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
` (3 preceding siblings ...)
2021-03-05 0:59 ` [PATCH 4/8] ASoC: soc-pcm: add soc_pcm_update_symmetry() Kuninori Morimoto
@ 2021-03-05 1:00 ` Kuninori Morimoto
2021-03-05 1:00 ` [PATCH 6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count Kuninori Morimoto
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 1:00 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
` (4 preceding siblings ...)
2021-03-05 1:00 ` [PATCH 5/8] ASoC: soc-pcm: add soc_hw_sanity_check() Kuninori Morimoto
@ 2021-03-05 1:00 ` Kuninori Morimoto
2021-03-05 16:14 ` Pierre-Louis Bossart
2021-03-05 1:00 ` [PATCH 7/8] ASoC: soc-pcm: remove unneeded !rtd->dai_link check Kuninori Morimoto
2021-03-05 1:00 ` [PATCH 8/8] ASoC: soc-pcm: share DPCM BE DAI stop operation Kuninori Morimoto
7 siblings, 1 reply; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 1:00 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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, it will be minus
users 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;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 7/8] ASoC: soc-pcm: remove unneeded !rtd->dai_link check
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
` (5 preceding siblings ...)
2021-03-05 1:00 ` [PATCH 6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count Kuninori Morimoto
@ 2021-03-05 1:00 ` Kuninori Morimoto
2021-03-05 1:00 ` [PATCH 8/8] ASoC: soc-pcm: share DPCM BE DAI stop operation Kuninori Morimoto
7 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 1:00 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 8/8] ASoC: soc-pcm: share DPCM BE DAI stop operation
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
` (6 preceding siblings ...)
2021-03-05 1:00 ` [PATCH 7/8] ASoC: soc-pcm: remove unneeded !rtd->dai_link check Kuninori Morimoto
@ 2021-03-05 1:00 ` Kuninori Morimoto
7 siblings, 0 replies; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-05 1:00 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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);
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/8] ASoC: soc-pcm: direct copy at snd_soc_set_runtime_hwparams()
2021-03-05 0:59 ` [PATCH 3/8] ASoC: soc-pcm: direct copy at snd_soc_set_runtime_hwparams() Kuninori Morimoto
@ 2021-03-05 16:11 ` Pierre-Louis Bossart
0 siblings, 0 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-03-05 16:11 UTC (permalink / raw)
To: Kuninori Morimoto, Mark Brown; +Cc: Liam Girdwood, Linux-ALSA
On 3/4/21 6:59 PM, Kuninori Morimoto wrote:
> 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;
This dates from commit db2a416556af0 in 2006 :-)
what was specifically excluded is the information on rates and channels.
Liam, any memories if this was intentional?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count
2021-03-05 1:00 ` [PATCH 6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count Kuninori Morimoto
@ 2021-03-05 16:14 ` Pierre-Louis Bossart
2021-03-07 22:37 ` Kuninori Morimoto
0 siblings, 1 reply; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-03-05 16:14 UTC (permalink / raw)
To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA
On 3/4/21 7:00 PM, Kuninori Morimoto wrote:
>
> 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, it will be minus
> users at (2).
suggested edit:
"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;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count
2021-03-05 16:14 ` Pierre-Louis Bossart
@ 2021-03-07 22:37 ` Kuninori Morimoto
2021-03-08 13:32 ` Mark Brown
0 siblings, 1 reply; 13+ messages in thread
From: Kuninori Morimoto @ 2021-03-07 22:37 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown
Hi Pierre-Louis, Mark
Thank you for your review.
> > 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, it will be minus
> > users at (2).
>
> suggested edit:
>
> "Thus, users will be a negative number at (2)"
Ah it is better :)
Mark, can I repost it ?
I'm happy if you tidyup it if you are OK for appling.
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count
2021-03-07 22:37 ` Kuninori Morimoto
@ 2021-03-08 13:32 ` Mark Brown
0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2021-03-08 13:32 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Linux-ALSA, Pierre-Louis Bossart
[-- Attachment #1: Type: text/plain, Size: 122 bytes --]
On Mon, Mar 08, 2021 at 07:37:28AM +0900, Kuninori Morimoto wrote:
> Ah it is better :)
> Mark, can I repost it ?
Sure.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-03-08 13:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 0:59 [PATCH v2 0/8] ASoC: soc-pcm: cleanup each functions Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 1/8] ASoC: soc-pcm: check DAI activity under soc_pcm_apply_symmetry() Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 2/8] ASoC: soc-pcm: add soc_cpu/codec_dai_name() macro Kuninori Morimoto
2021-03-05 0:59 ` [PATCH 3/8] ASoC: soc-pcm: direct copy at snd_soc_set_runtime_hwparams() Kuninori Morimoto
2021-03-05 16:11 ` Pierre-Louis Bossart
2021-03-05 0:59 ` [PATCH 4/8] ASoC: soc-pcm: add soc_pcm_update_symmetry() Kuninori Morimoto
2021-03-05 1:00 ` [PATCH 5/8] ASoC: soc-pcm: add soc_hw_sanity_check() Kuninori Morimoto
2021-03-05 1:00 ` [PATCH 6/8] ASoC: soc-pcm: fixup dpcm_be_dai_startup() user count Kuninori Morimoto
2021-03-05 16:14 ` Pierre-Louis Bossart
2021-03-07 22:37 ` Kuninori Morimoto
2021-03-08 13:32 ` Mark Brown
2021-03-05 1:00 ` [PATCH 7/8] ASoC: soc-pcm: remove unneeded !rtd->dai_link check Kuninori Morimoto
2021-03-05 1:00 ` [PATCH 8/8] ASoC: soc-pcm: share DPCM BE DAI stop operation Kuninori Morimoto
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).