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

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

> > 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?

I didn't want to make any assumption on whether the state of the FE is 
updated before or after the BE state, depending on the trigger order, so 
only used the trigger command to drive the state machine changes.

> > +                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;