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@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@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;