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