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

[Aaron Ma]

On Tue, May 5, 2026 at 11:10 AM Shuming [范書銘] <shumingf@xxxxxxxxxxx> 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.
> > >>>>
> > >>>> well that's an interesting issue. IMHO all those drivers are broken then...
> > >>>> A mute should let the ports active and not start any power transitions on
> > the codec side.
> > >>>> That's what should be fixed first.
> > >>>>
> > >>>> I am not sure either why PipeWire would send a hw_free on a mute
> > either, that's a sure what to have audible delays and glitches. Mute != Stop.
> > >>>>
> > >>>
> > >>> hw_free is not involved here. The mute is purely at the codec's FU
> > >>> level (FU_MUTE=1 silences the output) — the PCM stream remains open
> > >>> and
> > >>> DP1 stays active. SoundWire debugfs confirms: after mute during
> > >>> playback, REQ_POWER_STATE=PS3 but ACTUAL_POWER_STATE remains
> > PS0.
> > >>> The codec cannot transition while the port is streaming. As long as
> > >>> the application keeps playing, this state persists indefinitely.
> > >>>
> > >>> The power transition is not triggered by PipeWire either. It is
> > >>> standard DAPM behavior: muting both FU21 channels removes the last
> > >>> active path consumer of PDE23, so DAPM evaluates the widget as
> > >>> unused and fires PRE_PMD. This is how every rt7xx/rt13xx driver's DAPM
> > graph works today.
> > >>>
> > >>> The poll itself is broken in this path: it cannot succeed while the
> > >>> port is active, always times out, and the handler continues anyway.
> > >>> Removing it fixes the user-facing 2-3s mute latency without changing
> > >>> any DAPM topology or power semantics — the REQ_POWER_STATE=PS3
> > write
> > >>> is still issued, and the codec transitions once the port stops.
> > >>
> > >> well we're talking past each other. The only thing we agree on is that going
> > to PS3 while audio is still streaming is really bad.
> > >>
> > >> To repeat myself, there is *no reason* why a mute should trigger a
> > power-down.
> > >> I am not expert enough in DAPM but that isn't a desired behavior.
> > >>
> > >> Don't fix this unwanted behavior with a change in how the power state
> > transition is implemented, fix the unwanted behavior instead.
> > >>
> > >>
> > >
> > > The current rt1320 behavior is consistent with how this driver models
> > > the playback path in DAPM.
> > >
> > > DAPM is documented to make power decisions from stream activity and
> > > mixer settings, with those decisions derived from the routing graph
> > > (`Documentation/sound/soc/dapm.rst`).
> > >
> > > In rt1320, the speaker mute controls are
> > `SOC_DAPM_SINGLE_AUTODISABLE`
> > > switches on the two `FU21` mute bits, and the playback path is `DP1RX
> > > -> FU21 -> OT23 L/R -> SPOL/SPOR`, with `FU21` also depending on `PDE
> > > 23`.
> > >
> > > With that graph, muting both speaker switches removes the active
> > > speaker path, so `PDE 23` receiving `PRE_PMD` is consistent with the
> > > current DAPM model of this driver.
> > >
> > > Given that, I think the immediate bug is the synchronous wait in
> > > `rt1320_pde23_event(PRE_PMD)`, not the fact that DAPM attempts to
> > > power down the now-unused speaker supply path.
> >
> > My point is that the DAPM model seems wrong. Muting should not have any
> > impacts on power management.
> >
> > For some reason the RT1320 driver differs from previous codec drivers:
> >
> > static const struct snd_kcontrol_new rt1320_spk_l_dac =
> >         SOC_DAPM_SINGLE_AUTODISABLE("Switch",
> >                 SDW_SDCA_CTL(FUNC_NUM_AMP,
> > RT1320_SDCA_ENT_FU21, RT1320_SDCA_CTL_FU_MUTE, CH_01),
> >                 0, 1, 1);
> >
> > static const struct snd_kcontrol_new rt1318_sto_dac =
> >         SOC_DAPM_DOUBLE_R("Switch",
> >                 SDW_SDCA_CTL(FUNC_NUM_SMART_AMP,
> > RT1318_SDCA_ENT_FU21, RT1318_SDCA_CTL_FU_MUTE, CH_L),
> >                 SDW_SDCA_CTL(FUNC_NUM_SMART_AMP,
> > RT1318_SDCA_ENT_FU21, RT1318_SDCA_CTL_FU_MUTE, CH_R),
> >                 0, 1, 1);
> >
> > And the main question is "why"?
> > There's been no change in the topology definition, FU21 plays exactly the
> > same role but is handled differently.
> >
> > Maybe there are separate DACs for left and right, but that doesn't mean we
> > want muting to impact PDE management.
> > If you look at the helper that is being introduced, no one has any plans to
> > remove the synchronous wait after changing power states.
> >
> > Shuming, can you please chime in?
> >
>
> Hi Aaron,
>
> Sorry, I don't have a clear understanding of the root cause at the moment.
> Could you let me know which project this issue was encountered in?
> Also, could you please contact Realtek's PM?
> We can involve the owner of this project to help reproduce and investigate the issue on the Realtek side.
>
>

Hi Shuming,

What I can confirm is the following:

1. playback on `DP1` is active
2. both `FU21` speaker mute bits are set
3. `rt1320_pde23_event(PRE_PMD)` writes `REQ_POWER_STATE=PS3`
4. `ACTUAL_POWER_STATE` remains `PS0` while the data port is still active
5. the synchronous wait then times out, and the mute control returns
after about 2-3 seconds

So the immediate issue I am trying to address is the blocking wait in
the current `PRE_PMD` path.

On Pierre-Louis's question, can you confirm whether using
`SOC_DAPM_SINGLE_AUTODISABLE()` on the `FU21` speaker mute controls in
rt1320 was intentional, and if so what the rationale was?

Regards,
Aaron