Subject: [PATCH v2] ASoC: soc-pcm: fix BE dai not hw_free and shutdown during mixer update

Subject: [PATCH v2] ASoC: soc-pcm: fix BE dai not hw_free and shutdown during mixer update

[朱灿灿]

FE state is SND_SOC_DPCM_STATE_PREPARE now, BE1 is
used by FE.

Later when new BE2 is added to FE by mixer update,
it will call dpcm_run_update_startup() to update
BE2's state, but unfortunately BE2 .prepare() meets
error, it will disconnect all non started BE.

This make BE1 dai skip .hw_free() and .shutdown(),
and the BE1 users will never decrease to zero.

Signed-off-by: zhucancan <zhucancan@vivo.com>
---
re-format patch content v2
---
 sound/soc/soc-pcm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 1f302de44052..df34422bd0dd 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2730,12 +2730,12 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
 close:
 	dpcm_be_dai_shutdown(fe, stream);
 disconnect:
-	/* disconnect any non started BEs */
+	/* disconnect any closed BEs */
 	spin_lock_irqsave(&fe->card->dpcm_lock, flags);
 	for_each_dpcm_be(fe, stream, dpcm) {
 		struct snd_soc_pcm_runtime *be = dpcm->be;
-		if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
-				dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE;
+		if (be->dpcm[stream].state == SND_SOC_DPCM_STATE_CLOSE)
+			dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE;
 	}
 	spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
 
-- 
2.21.0

Subject: [PATCH v2] ASoC: soc-pcm: fix BE dai not hw_free and shutdown during mixer update

[朱灿灿]

FE state is SND_SOC_DPCM_STATE_PREPARE now, BE1 is
used by FE.

Later when new BE2 is added to FE by mixer update,
it will call dpcm_run_update_startup() to update
BE2's state, but unfortunately BE2 .prepare() meets
error, it will disconnect all non started BE.

This make BE1 dai skip .hw_free() and .shutdown(),
and the BE1 users will never decrease to zero.

Signed-off-by: zhucancan <zhucancan@vivo.com>
---
re-format patch content v2
---
 sound/soc/soc-pcm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 1f302de44052..df34422bd0dd 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2730,12 +2730,12 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
 close:
 	dpcm_be_dai_shutdown(fe, stream);
 disconnect:
-	/* disconnect any non started BEs */
+	/* disconnect any closed BEs */
 	spin_lock_irqsave(&fe->card->dpcm_lock, flags);
 	for_each_dpcm_be(fe, stream, dpcm) {
 		struct snd_soc_pcm_runtime *be = dpcm->be;
-		if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
-				dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE;
+		if (be->dpcm[stream].state == SND_SOC_DPCM_STATE_CLOSE)
+			dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE;
 	}
 	spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
 
-- 
2.21.0

Re: Subject: [PATCH v2] ASoC: soc-pcm: fix BE dai not hw_free and shutdown during mixer update

[Mark Brown]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

On Fri, 29 May 2020 18:12:44 +0800 (GMT+08:00), 朱灿灿 wrote:
> FE state is SND_SOC_DPCM_STATE_PREPARE now, BE1 is
> used by FE.
> 
> Later when new BE2 is added to FE by mixer update,
> it will call dpcm_run_update_startup() to update
> BE2's state, but unfortunately BE2 .prepare() meets
> error, it will disconnect all non started BE.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] Subject: [PATCH v2] ASoC: soc-pcm: fix BE dai not hw_free and shutdown during mixer update
      commit: 68f8043ed13e636c6f04ac1c0831e17681d392ab

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

Re: Subject: [PATCH v2] ASoC: soc-pcm: fix BE dai not hw_free and shutdown during mixer update

[Pierre-Louis Bossart]

On 5/29/20 5:12 AM, 朱灿灿 wrote:
> FE state is SND_SOC_DPCM_STATE_PREPARE now, BE1 is
> used by FE.
> 
> Later when new BE2 is added to FE by mixer update,
> it will call dpcm_run_update_startup() to update
> BE2's state, but unfortunately BE2 .prepare() meets
> error, it will disconnect all non started BE.
> 
> This make BE1 dai skip .hw_free() and .shutdown(),
> and the BE1 users will never decrease to zero.
> 
> Signed-off-by: zhucancan <zhucancan@vivo.com>
> ---
> re-format patch content v2
> ---
>   sound/soc/soc-pcm.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 1f302de44052..df34422bd0dd 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -2730,12 +2730,12 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
>   close:
>   	dpcm_be_dai_shutdown(fe, stream);
>   disconnect:
> -	/* disconnect any non started BEs */
> +	/* disconnect any closed BEs */
>   	spin_lock_irqsave(&fe->card->dpcm_lock, flags);
>   	for_each_dpcm_be(fe, stream, dpcm) {
>   		struct snd_soc_pcm_runtime *be = dpcm->be;
> -		if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
> -				dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE;
> +		if (be->dpcm[stream].state == SND_SOC_DPCM_STATE_CLOSE)
> +			dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE;
>   	}
>   	spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);

This change is quite hard to review, this error handling can be called 
from multiple places.

I *think* it's correct because in all cases where the 
disconnect/close/hw_free labels are reached, the non-shared BEs either 
remain or are put in the DPCM_STATE_CLOSE state before doing this test.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

It really wouldn't hurt though if others double-checked this change...