* [PATCH] ASoC: codecs: aw88261: start the amplifier on unmute at trigger time
@ 2026-10-14 16:36 Vladyslav Tymoshchuk
2026-10-15 17:50 ` Mark Brown
0 siblings, 1 reply; 3+ messages in thread
From: Vladyslav Tymoshchuk @ 2026-10-14 16:36 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood
Cc: Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla, Luca Weiss,
Jorijn van der Graaf, Val Packett, Federico Amedeo Izzo,
linux-sound, linux-arm-msm, linux-kernel, phone-devel,
~postmarketos/upstreaming, Vladyslav Tymoshchuk
The amplifier is started from the PRE_PMU event of its AIF widget, that
is from DAPM in prepare. aw88261_dev_start() then waits up to 20 ms for
the PLL to lock to the bit clock and powers the amplifier down again if
it does not.
Not every host runs the bit clock by then. The Qualcomm AudioReach
backends only start their graph, and with it the I2S/TDM clocks, in
trigger since commit b54a38af7138 ("ASoC: qcom: q6apm-lpass-dai: move
graph start to trigger"). There every start fails with "start failure
(-1)" and the speaker stays silent for the whole stream, as seen with
six AW88261 on a TDM link of SM8750. The asynchronous start removed by
commit caea99ac809d ("ASoC: codecs: aw88261: remove async start") only
hid this when the trigger came soon enough.
Start the amplifier from .mute_stream instead and set
.mute_unmute_on_trigger, as wsa883x and wsa884x do. The unmute is then
called from snd_soc_pcm_dai_trigger() after the CPU DAI of the link,
so the bit clock runs when the PLL is checked, whatever the host does
in prepare. Hosts that start the clock in prepare (q6afe on the
Fairphone 5) only see the amplifier start at trigger instead. An
amplifier that already runs is just muted and unmuted, and powering it
down is left to POST_PMD as before.
A stream that is prepared but never started, as sound servers do when
they probe a device, now leaves the amplifier off, so POST_PMD finding
it off is expected: make "already power off" a debug message.
The trigger has to be nonatomic for the I2C accesses, which the
Qualcomm DPCM links using this codec are.
Tested on a OnePlus Pad 3 (SM8750, six AW88261 on two I2C buses, TDM
from the AudioReach DSP), with PipeWire and with a plain ALSA test
program:
- no "start failure" at all, where there were six for each prepare
before, including the probes of PipeWire;
- SYSST reads 0x0311 (running) on all six amplifiers during playback,
also after three PipeWire restarts, and 0x0000 when idle and after
a prepare without trigger;
- the write that starts the stream, which includes the start of the
AudioReach graph and of the six amplifiers, takes 52 to 71 ms
(six runs);
- listening: every sound is complete from its start, including the
first one after the device was closed, without clicks or
distortion.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Vladyslav Tymoshchuk <tvlad744@gmail.com>
---
This is the codec-side alternative to the RFC "ASoC: qcom:
q6apm-lpass-dais: start the graph at prepare" by Jorijn van der Graaf
[1]: it leaves q6apm alone, so the DP case that RFC breaks is not
touched. On SM8750 with TDM the PRM IBIT vote of the MI2S clock control
series does not get the bit clock to the pins before the graph starts
either, as Jorijn and Srinivas saw on FP6 and VENTUNO Q.
Luca, Jorijn: could you give it a try on the Fairphone 5 (q6afe, the
clock runs from prepare, so only the start moves to trigger) and the
Fairphone 6 (q6apm, where it should replace the RFC)? A Tested-by from
either would help a lot.
Testing was done with the same change on a v7.3-rc5 based tree that
also carries other aw88261 changes not upstream yet (a battery
monitor); the mute_stream part is the same as here. I have no number
for the graph start alone, so the 52 to 71 ms above are graph start
plus six amplifier starts, on two buses one after the other.
That time is now spent in trigger. A program that starts the stream on
its first 10 ms period (start_threshold of one period) underruns right
away, as the buffer has drained by the time the trigger returns.
PipeWire, which fills the buffer before it starts, saw no xrun in a
60 s tone and in the listening tests. I did not make the start
asynchronous again, as caea99ac809d removed exactly that; if the time
matters, the start of each amplifier could be made shorter in a
follow-up.
Federico Amedeo Izzo's "ASoC: codecs: aw88261: expose hardware mute
control" [2] may conflict textually; aw88261_mute_stream() already
honours mute_st.
The analysis, the change and this text were drafted with an AI
assistant (Claude, see Assisted-by); I reviewed them and ran the tests
above.
[1] https://lore.kernel.org/r/20260705033830.305907-1-jorijnvdgraaf@catcrafts.net
[2] https://lore.kernel.org/r/20261010-aw88261-hardware-mute-v1-1-d0befedd3b4c@izzo.pro
sound/soc/codecs/aw88261.c | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index 8671069f4c18..fefb77d29449 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -563,7 +563,7 @@ static int aw88261_dev_start(struct aw88261 *aw88261)
static int aw88261_dev_stop(struct aw_device *aw_dev)
{
if (aw_dev->status == AW88261_DEV_PW_OFF) {
- dev_info(aw_dev->dev, "already power off");
+ dev_dbg(aw_dev->dev, "already power off");
return 0;
}
@@ -865,10 +865,36 @@ static int aw88261_set_tdm_slot(struct snd_soc_dai *dai,
return 0;
}
+static int aw88261_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
+{
+ struct aw88261 *aw88261 = snd_soc_component_get_drvdata(dai->component);
+ struct aw_device *aw_dev = aw88261->aw_pa;
+
+ guard(mutex)(&aw88261->lock);
+
+ if (mute) {
+ if (aw_dev->status == AW88261_DEV_PW_ON)
+ aw88261_dev_mute(aw_dev, true);
+ } else if (aw_dev->status != AW88261_DEV_PW_ON) {
+ /*
+ * The PLL locks to the bit clock, which some hosts only start
+ * in their trigger: start the amplifier now, after them.
+ */
+ aw88261_start(aw88261);
+ } else if (!aw88261->mute_st) {
+ aw88261_dev_mute(aw_dev, false);
+ }
+
+ return 0;
+}
+
static const struct snd_soc_dai_ops aw88261_dai_ops = {
.set_fmt = aw88261_set_fmt,
.hw_params = aw88261_hw_params,
.set_tdm_slot = aw88261_set_tdm_slot,
+ .mute_stream = aw88261_mute_stream,
+ .mute_unmute_on_trigger = true,
+ .no_capture_mute = 1,
};
static struct snd_soc_dai_driver aw88261_dai[] = {
@@ -1038,9 +1064,6 @@ static int aw88261_playback_event(struct snd_soc_dapm_widget *w,
guard(mutex)(&aw88261->lock);
switch (event) {
- case SND_SOC_DAPM_PRE_PMU:
- aw88261_start(aw88261);
- break;
case SND_SOC_DAPM_POST_PMD:
aw88261_dev_stop(aw88261->aw_pa);
break;
@@ -1055,7 +1078,7 @@ static const struct snd_soc_dapm_widget aw88261_dapm_widgets[] = {
/* playback */
SND_SOC_DAPM_AIF_IN_E("AIF_RX", "Speaker_Playback", 0, 0, 0, 0,
aw88261_playback_event,
- SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+ SND_SOC_DAPM_POST_PMD),
SND_SOC_DAPM_OUTPUT("DAC Output"),
/* capture */
base-commit: 8ef28aef33f2d5da6959ad6b579ec3cfccc009f6
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ASoC: codecs: aw88261: start the amplifier on unmute at trigger time
2026-10-14 16:36 [PATCH] ASoC: codecs: aw88261: start the amplifier on unmute at trigger time Vladyslav Tymoshchuk
@ 2026-10-15 17:50 ` Mark Brown
2026-10-17 10:02 ` Vladyslav Tymoshchuk
0 siblings, 1 reply; 3+ messages in thread
From: Mark Brown @ 2026-10-15 17:50 UTC (permalink / raw)
To: Vladyslav Tymoshchuk
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Srinivas Kandagatla, Luca Weiss, Jorijn van der Graaf,
Val Packett, Federico Amedeo Izzo, linux-sound, linux-arm-msm,
linux-kernel, phone-devel, ~postmarketos/upstreaming
[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]
On Wed, Oct 14, 2026 at 07:36:49PM +0300, Vladyslav Tymoshchuk wrote:
> The amplifier is started from the PRE_PMU event of its AIF widget, that
> is from DAPM in prepare. aw88261_dev_start() then waits up to 20 ms for
> the PLL to lock to the bit clock and powers the amplifier down again if
> it does not.
>
> Not every host runs the bit clock by then. The Qualcomm AudioReach
> backends only start their graph, and with it the I2S/TDM clocks, in
> trigger since commit b54a38af7138 ("ASoC: qcom: q6apm-lpass-dai: move
> graph start to trigger"). There every start fails with "start failure
> (-1)" and the speaker stays silent for the whole stream, as seen with
> six AW88261 on a TDM link of SM8750. The asynchronous start removed by
> commit caea99ac809d ("ASoC: codecs: aw88261: remove async start") only
> hid this when the trigger came soon enough.
OTOH some hosts presumably do have the clock running earlier and will
now get delays added in trigger which isn't ideal. I'm not sure we have
good solutions here given that this is all system dependent.
> The trigger has to be nonatomic for the I2C accesses, which the
> Qualcomm DPCM links using this codec are.
Are we *sure* there are no other users out there?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ASoC: codecs: aw88261: start the amplifier on unmute at trigger time
2026-10-15 17:50 ` Mark Brown
@ 2026-10-17 10:02 ` Vladyslav Tymoshchuk
0 siblings, 0 replies; 3+ messages in thread
From: Vladyslav Tymoshchuk @ 2026-10-17 10:02 UTC (permalink / raw)
To: Mark Brown
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Srinivas Kandagatla, Luca Weiss, Jorijn van der Graaf,
Val Packett, Federico Amedeo Izzo, linux-sound, linux-arm-msm,
linux-kernel, phone-devel, ~postmarketos/upstreaming,
Vladyslav Tymoshchuk
On Thu, Oct 15, 2026 at 06:50:39PM +0100, Mark Brown wrote:
> OTOH some hosts presumably do have the clock running earlier and will
> now get delays added in trigger which isn't ideal. I'm not sure we have
> good solutions here given that this is all system dependent.
Right, v1 moved the whole start into trigger. v2 [1] splits it at the
PLL lock check, the first step that needs the bit clock: PWDN, the
I2S/PLL setup and the boost settings stay in DAPM, and only the lock
check, AMPPD, the SYSST check, the feedback and the unmute run in the
trigger-time unmute, which the mute undoes. That is the split wsa883x
and wsa884x make with GLOBAL_PA_EN (the vendor aw882xx driver also
starts the amplifier from .mute_stream). It does not depend on the
host: one with the clock running from prepare gets the same few steps
in trigger and nothing more.
Measured here with six amplifiers on two I2C buses: the unmute takes
4.3 to 7.3 ms per amplifier (26 to 38 ms for all six), the mute about
4 to 7.5 ms (estimated, see the v2 notes), and the write that starts
the stream went from 52-57 ms with v1 to 28-36 ms. With two
amplifiers, as on the Fairphone 5, it should be a third of that if the
I2C is as fast.
> > The trigger has to be nonatomic for the I2C accesses, which the
> > Qualcomm DPCM links using this codec are.
>
> Are we *sure* there are no other users out there?
In mainline, linux-next and your for-next the only user is the
Fairphone 5 ("fairphone,fp5-sndcard" in sm8250.c), whose DPCM links
qcom_snd_parse_of() makes nonatomic; the driver has no ACPI IDs. The
out-of-tree users I know of (Fairphone 6, OnePlus Pad 3,
motorola-dubai) are Qualcomm too. But v2 no longer relies on that: it
reads substream->pcm->nonatomic in .startup, and on an atomic PCM it
does the whole start from DAPM as now and nothing in the trigger.
[1] https://lore.kernel.org/r/20261016120000.aw88261-trigger-v2-1-tvlad744@gmail.com
Thanks,
Vladyslav
(I used an AI assistant, Claude, for the analysis and to draft this
mail; the tests ran on my device.)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-17 10:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-14 16:36 [PATCH] ASoC: codecs: aw88261: start the amplifier on unmute at trigger time Vladyslav Tymoshchuk
2026-10-15 17:50 ` Mark Brown
2026-10-17 10:02 ` Vladyslav Tymoshchuk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®