On 1/12/2023 7:58 AM, Chancel Liu wrote:
The commit 1da681e52853 ("ASoC: soc-pcm.c: Clear DAIs parameters after
stream_active is updated") tries to make sure DAIs parameters can be
cleared properly through moving the cleanup to the place where
stream_active is updated. However, it will cause the cleanup only
happening in soc_pcm_close().
Suppose a case: aplay -Dhw:0 44100.wav 48000.wav. The case calls
soc_pcm_open()->soc_pcm_hw_params()->soc_pcm_hw_free()->
soc_pcm_hw_params()->soc_pcm_hw_free()->soc_pcm_close() in order. The
parameters would be remained in the system even if the playback of
44100.wav is finished.
The case requires us clearing parameters in phase of soc_pcm_hw_free().
We shouldn't use stream_active to decide if we must do the cleanup
since it is finally updated in soc_pcm_close().
This patch introduces a usage count called hw_params_count in
snd_soc_dai structure. It records the times of setting parameters to
this DAI then decreases each time soc_pcm_hw_free() is called. If the
count decreases to 0, it means this DAI is not used now and we should
clear the parameters.
Can rtd->dpcm[dir].users be somehow used instead or do DAI users need to
be count explicitly?
Fixes: 1da681e52853 ("ASoC: soc-pcm.c: Clear DAIs parameters after stream_active is updated")
Signed-off-by: Chancel Liu <chancel.liu@xxxxxxx>
---
include/sound/soc-dai.h | 3 +++
sound/soc/soc-pcm.c | 16 +++++++++++-----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index ea7509672086..a7e589a0fe72 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -451,6 +451,9 @@ struct snd_soc_dai {
unsigned int channels;
unsigned int sample_bits;
+ /* Count of setting DAI parameters */
+ unsigned int hw_params_count;
+
/* parent platform/codec */
struct snd_soc_component *component;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 579a44d81d9a..2c2a5dcf9e06 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -711,14 +711,10 @@ static int soc_pcm_clean(struct snd_soc_pcm_runtime *rtd,
if (!rollback) {
snd_soc_runtime_deactivate(rtd, substream->stream);
- /* clear the corresponding DAIs parameters when going to be inactive */
- for_each_rtd_dais(rtd, i, dai) {
- if (snd_soc_dai_active(dai) == 0)
- soc_pcm_set_dai_params(dai, NULL);
+ for_each_rtd_dais(rtd, i, dai)
if (snd_soc_dai_stream_active(dai, substream->stream) == 0)
snd_soc_dai_digital_mute(dai, 1, substream->stream);
- }
}
for_each_rtd_dais(rtd, i, dai)
@@ -949,6 +945,14 @@ static int soc_pcm_hw_clean(struct snd_soc_pcm_runtime *rtd,
snd_soc_dpcm_mutex_assert_held(rtd);
+ /* clear the corresponding DAIs parameters when hw_params_count decreases to 0 */
+ for_each_rtd_dais(rtd, i, dai)
+ if (snd_soc_dai_stream_valid(dai, substream->stream)) {
+ dai->hw_params_count--;
+ if (dai->hw_params_count == 0)
+ soc_pcm_set_dai_params(dai, NULL);
+ }
+
/* run the stream event */
snd_soc_dapm_stream_stop(rtd, substream->stream);
@@ -1051,6 +1055,7 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd,
soc_pcm_set_dai_params(codec_dai, &codec_params);
snd_soc_dapm_update_dai(substream, &codec_params, codec_dai);
+ codec_dai->hw_params_count++;
}
for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
@@ -1068,6 +1073,7 @@ static int __soc_pcm_hw_params(struct snd_soc_pcm_runtime *rtd,
/* store the parameters for each DAI */
soc_pcm_set_dai_params(cpu_dai, params);
snd_soc_dapm_update_dai(substream, params, cpu_dai);
+ cpu_dai->hw_params_count++;
}
ret = snd_soc_pcm_component_hw_params(substream, params);
