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 <[email protected]>
---
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/[email protected]
[2]
https://lore.kernel.org/r/[email protected]
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
