Re: [PATCH] ASoC: rt1320-sdw: don't poll PDE state on power-down

[Aaron Ma]

On Wed, Apr 29, 2026 at 4:20 AM Pierre-Louis Bossart
<pierre-louis.bossart@xxxxxxxxx> wrote:
>
> On 4/28/26 09:37, Aaron Ma wrote:
> > rt1320_pde23_event(PRE_PMD) writes REQ_POWER_STATE=PS3 then polls
> > ACTUAL_POWER_STATE. DAPM fires PRE_PMD while the SoundWire data port
> > is still streaming — the codec cannot reach PS3 until the port stops,
> > so the poll always times out during playback. This blocks
> > snd_ctl_elem_write() for 2-3s making mute unresponsive.
> >
> > Remove the poll from PRE_PMD in rt1320_pde23_event() and
> > rt1320_pde11_event(). The codec transitions to PS3 on its own once the
> > data port becomes inactive. Keep the POST_PMU poll — on power-up the
> > domain must reach PS0 before audio can flow.
> >
> > Fixes: f465d10cd731 ("ASoC: rt1320: Add support for version C")
> > Signed-off-by: Aaron Ma <aaron.ma@xxxxxxxxxxxxx>
> > ---
> >  sound/soc/codecs/rt1320-sdw.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/sound/soc/codecs/rt1320-sdw.c b/sound/soc/codecs/rt1320-sdw.c
> > index 192faa431b5e9..e42e8b6de853e 100644
> > --- a/sound/soc/codecs/rt1320-sdw.c
> > +++ b/sound/soc/codecs/rt1320-sdw.c
> > @@ -2000,7 +2000,6 @@ static int rt1320_pde11_event(struct snd_soc_dapm_widget *w,
> >               regmap_write(rt1320->regmap,
> >                       SDW_SDCA_CTL(FUNC_NUM_MIC, RT1320_SDCA_ENT_PDE11,
> >                               RT1320_SDCA_CTL_REQ_POWER_STATE, 0), ps3);
> > -             rt1320_pde_transition_delay(rt1320, FUNC_NUM_MIC, RT1320_SDCA_ENT_PDE11, ps3);
> >               break;
> >       default:
> >               break;
> > @@ -2028,7 +2027,6 @@ static int rt1320_pde23_event(struct snd_soc_dapm_widget *w,
> >               regmap_write(rt1320->regmap,
> >                       SDW_SDCA_CTL(FUNC_NUM_AMP, RT1320_SDCA_ENT_PDE23,
> >                               RT1320_SDCA_CTL_REQ_POWER_STATE, 0), ps3);
> > -             rt1320_pde_transition_delay(rt1320, FUNC_NUM_AMP, RT1320_SDCA_ENT_PDE23, ps3);
>
> no, sorry, that's clearly wrong.
>
> When we touch the requested power state, we *shall* wait for the power state to be reached.
> removing the transition delay is silly.
>
> You would really need to find out why streaming still occurs when you get a DAPM event. That doesn't seem right in the first place.
>

 The stream is active by design — this is a mute, not a stream close.

To reproduce:

  1. Play audio through rt1320 speaker (aplay or PipeWire)
  2. Press speaker mute key (or amixer cset on both FU21 channels)
  3. Second channel mute takes 2-3s to return

Muting both channels removes the last DAPM path consumer of PDE23.
DAPM fires PRE_PMD while PipeWire still holds the PCM open —
DP1 keeps streaming zeros. The codec cannot reach PS3 while its data
port is active, so the poll always times out. The mute register write
that actually silences the speaker happens after this useless wait.

I confirmed via SoundWire debugfs reads: ACTUAL_POWER_STATE stays PS0
while the port streams and only transitions to PS3 after PipeWire calls
hw_free (~3s later). The poll can never succeed in this path.

Every other Realtek SDCA amp driver does fire-and-forget on PRE_PMD with
no transition delay — rt712_sdca_pde23_event(), rt721_sdca_pde41_event(),
rt1316_pde24_event(), and rt1017_sdca_pde23_event() all just write PS3
and return. rt1320 is the only one that polls on power-down.

The POST_PMU poll is kept — on power-up the domain must reach PS0 before
audio can flow, and the port is not yet active at that point so the
transition succeeds immediately.

Thanks for review.
Aaron

>
> >               break;
> >       default:
> >               break;
>