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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux