[PATCH 1/2] ASoC: soc-pcm: improve BE transition for PAUSE_RELEASE

[Amadeusz Sławiński]

On 4/6/2022 9:00 PM, Pierre-Louis Bossart wrote:
> Commit 3aa1e96a2b95 ("ASoC: soc-pcm: fix BE handling of PAUSE_RELEASE")
> did not modify the existing logic and kept the same logic for the following
> transition
> 
>      play FE1    -> BE state is START
>      pause FE1   -> BE state is PAUSED
>      play FE2    -> BE state is START
>      stop FE2    -> BE state is STOP <<< !!
>      release FE1 -> BE state is START
>      stop FE1    -> BE state is STOP
> 
> At the time it was identified by reviewers that a better solution
> might consist in
> 
>      play FE1    -> BE state is START
>      pause FE1   -> BE state is PAUSED
>      play FE2    -> BE state is START
>      stop FE2    -> BE state is PAUSE <<< !!
>      release FE1 -> BE state is START
>      stop FE1    -> BE state is STOP
> 
> This patch suggest a transition to PAUSE when all the 'active' streams
> are paused. This would allow for a more consistent resource management
> for platforms where PAUSE and STOP are handled differently.
> 
> To track the special case of an FE going from PAUSE_PUSH to STOP, we
> add a state variable for each FE context. This 'fe_pause' boolean is
> set on PAUSE_PUSH and cleared on either PAUSE_RELEASE and STOP
> triggers.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> Reviewed-by: Rander Wang <rander.wang at intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan at linux.intel.com>
> Reviewed-by: Péter Ujfalusi <peter.ujfalusi at linux.intel.com>
> Reviewed-by: Bard Liao <yung-chuan.liao at linux.intel.com>
> ---
>   include/sound/soc-dpcm.h |  2 ++
>   sound/soc/soc-pcm.c      | 31 ++++++++++++++++++++++++++++---
>   2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h
> index 75b92d883976..5b689c663290 100644
> --- a/include/sound/soc-dpcm.h
> +++ b/include/sound/soc-dpcm.h
> @@ -103,6 +103,8 @@ struct snd_soc_dpcm_runtime {
>   	int trigger_pending; /* trigger cmd + 1 if pending, 0 if not */
>   
>   	int be_start; /* refcount protected by BE stream pcm lock */
> +	int be_pause; /* refcount protected by BE stream pcm lock */
> +	bool fe_pause; /* used to track STOP after PAUSE */
>   };
>   
>   #define for_each_dpcm_fe(be, stream, _dpcm)				\
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 11c9853e9e80..e8700dd1839f 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -2090,6 +2090,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
>   			       int cmd)
>   {
>   	struct snd_soc_pcm_runtime *be;
> +	bool pause_stop_transition;
>   	struct snd_soc_dpcm *dpcm;
>   	unsigned long flags;
>   	int ret = 0;
> @@ -2148,10 +2149,12 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
>   		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>   			if (!be->dpcm[stream].be_start &&
>   			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) &&
> -			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) &&
>   			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
>   				goto next;
>   
> +			fe->dpcm[stream].fe_pause = false;
> +			be->dpcm[stream].be_pause--;
> +
>   			be->dpcm[stream].be_start++;
>   			if (be->dpcm[stream].be_start != 1)
>   				goto next;
> @@ -2175,14 +2178,33 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
>   			if (be->dpcm[stream].be_start != 0)
>   				goto next;
>   
> -			ret = soc_pcm_trigger(be_substream, cmd);
> +			pause_stop_transition = false;
> +			if (fe->dpcm[stream].fe_pause) {

As you access fe here anyway, any chance something like
if (fe->dpcm[stream].state == SND_SOC_DPCM_STATE_PAUSED)
can be used here instead of adding fe_pause to snd_soc_dpcm_runtime?

> +				pause_stop_transition = true;
> +				fe->dpcm[stream].fe_pause = false;
> +				be->dpcm[stream].be_pause--;
> +			}
> +
> +			if (be->dpcm[stream].be_pause != 0)
> +				ret = soc_pcm_trigger(be_substream, SNDRV_PCM_TRIGGER_PAUSE_PUSH);
> +			else
> +				ret = soc_pcm_trigger(be_substream, SNDRV_PCM_TRIGGER_STOP);
> +
>   			if (ret) {
>   				if (be->dpcm[stream].state == SND_SOC_DPCM_STATE_START)
>   					be->dpcm[stream].be_start++;
> +				if (pause_stop_transition) {
> +					fe->dpcm[stream].fe_pause = true;
> +					be->dpcm[stream].be_pause++;
> +				}
>   				goto next;
>   			}
>   
> -			be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
> +			if (be->dpcm[stream].be_pause != 0)
> +				be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED;
> +			else
> +				be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
> +
>   			break;
>   		case SNDRV_PCM_TRIGGER_SUSPEND:
>   			if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
> @@ -2204,6 +2226,9 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
>   			if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
>   				goto next;
>   
> +			fe->dpcm[stream].fe_pause = true;
> +			be->dpcm[stream].be_pause++;
> +
>   			be->dpcm[stream].be_start--;
>   			if (be->dpcm[stream].be_start != 0)
>   				goto next;