[RFC PATCH] ASoC: qcom: q6apm-lpass-dais: start the graph at prepare

[RFC PATCH] ASoC: qcom: q6apm-lpass-dais: start the graph at prepare

[Jorijn van der Graaf]

The DAPM power-up sequence runs during snd_pcm prepare, but the BE
port graph is only started at trigger time. A codec that powers up
synchronously from a DAPM widget event and needs a running bit clock
at that point - such as aw88261 since commit caea99ac809d ("ASoC:
codecs: aw88261: remove async start") - can therefore never see a
live clock: its power-up check runs before the trigger and fails on
every stream start.

Start the graph at the end of prepare instead, mirroring what
q6afe_dai_prepare() does on the legacy stack, so the interface
clocks already run when DAPM powers up the codec. The FE side
already starts its own graph at prepare in q6apm_dai_prepare();
only the BE waited for trigger. The trigger-time start is kept as
a fallback, guarded by is_port_started.

Tested on the Fairphone (Gen. 6) - 2x aw88261 on Senary MI2S:
without this the amplifiers fail to power up with SYSST reporting
"no clock" on every stream start; with it they start synchronously,
including for the first short stream of the boot.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
Sending as RFC because this changes when the port starts clocking for
every AudioReach platform, and I may be missing the reason the start
was placed in trigger rather than prepare in the first place:

- Is there a downside to starting the graph at prepare on AudioReach
  (power, pop/click, or DSP-side constraints)? The legacy q6afe stack
  has started its ports at the end of prepare all along, and the FE
  side of AudioReach already starts its graph in q6apm_dai_prepare().
- With the BE started at prepare, the BE graph now starts before the
  FE graph for playback (prepare runs BE-first) — does
  APM_CMD_GRAPH_START ordering between the two graphs matter?
- Is the capture direction fine with this, or should it stay
  trigger-started?

If this approach is right, the trigger callback becomes unreachable
and I would drop it entirely in a non-RFC respin, like q6afe (which
has no trigger op); it is kept here to keep the diff minimal.

Note: the test setup also carried my pending aw88261 power-up check
fix, which addresses an unrelated SYSST-check failure on this board:
https://lore.kernel.org/linux-sound/20260704192857.88366-1-jorijnvdgraaf@catcrafts.net/

Context (report and analysis of the aw88261 failure):
https://lore.kernel.org/linux-sound/d2318ba8-43f9-478b-8f9e-2c09d44ae0b1@packett.cool/

 sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index 006b283484d9..36e12a770c72 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -224,6 +224,20 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
 		dev_err(dai->dev, "Failed to prepare Graph %d\n", rc);
 		goto err;
 	}
+
+	/*
+	 * Start the graph here already, like q6afe does: this way the
+	 * interface clocks are running before the DAPM power-up sequence,
+	 * for codecs that need a live bit clock to power up (e.g.
+	 * aw88261). The trigger callback keeps its start as a fallback.
+	 */
+	rc = q6apm_graph_start(dai_data->graph[dai->id]);
+	if (rc < 0) {
+		dev_err(dai->dev, "Failed to start APM port %d\n", dai->id);
+		goto err;
+	}
+	dai_data->is_port_started[dai->id] = true;
+
 	return 0;
 err:
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {

base-commit: be44d21728b6646189779923b841ad3a46d694e5
-- 
2.55.0

Re: [RFC PATCH] ASoC: qcom: q6apm-lpass-dais: start the graph at prepare

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

On Sun, Jul 05, 2026 at 05:38:30AM +0200, Jorijn van der Graaf wrote:

> Sending as RFC because this changes when the port starts clocking for
> every AudioReach platform, and I may be missing the reason the start
> was placed in trigger rather than prepare in the first place:

> - Is there a downside to starting the graph at prepare on AudioReach
>   (power, pop/click, or DSP-side constraints)? The legacy q6afe stack
>   has started its ports at the end of prepare all along, and the FE
>   side of AudioReach already starts its graph in q6apm_dai_prepare().

There was previously a move in the opposite direction in b54a38af7138
(ASoC: qcom: q6apm-lpass-dai: move graph start to trigger) - the commit
there mentions some issues with PipeWire?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH] ASoC: qcom: q6apm-lpass-dais: start the graph at prepare

[Jorijn van der Graaf]

[ +Cc Dmitry Baryshkov and Jianfeng Liu for the drm/msm/dp DP-audio
  context, and Mohammad Rafi Shaik for the MI2S clock series test
  report below ]

On Wed, Jul 08, 2026 at 03:05:39PM +0100, Mark Brown wrote:
> On Sun, Jul 05, 2026 at 05:38:30AM +0200, Jorijn van der Graaf wrote:
>
> > Sending as RFC because this changes when the port starts clocking for
> > every AudioReach platform, and I may be missing the reason the start
> > was placed in trigger rather than prepare in the first place:
>
> > - Is there a downside to starting the graph at prepare on AudioReach
> >   (power, pop/click, or DSP-side constraints)? The legacy q6afe stack
> >   has started its ports at the end of prepare all along, and the FE
> >   side of AudioReach already starts its graph in q6apm_dai_prepare().
>
> There was previously a move in the opposite direction in b54a38af7138
> (ASoC: qcom: q6apm-lpass-dai: move graph start to trigger) - the commit
> there mentions some issues with PipeWire?

Thanks, that's exactly the context I was missing - I had checked the
state of the file on the submission base but not the history of the
lines I was moving. Having read up on it now: b54a38af7138 fixes a real
problem, and this patch as-is would reintroduce it for the DisplayPort
DAIs. Here is what I found.

Since 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec
implementation", v6.16) the msm DP audio configuration runs from
hdmi-codec's .prepare (drm_hdmi_audio_helper wires .prepare but not
.hw_params). Within a BE link the CPU DAI prepares before the codec
DAI, so a prepare-time q6apm_graph_start() asks the DSP to start
DISPLAY_PORT_RX before the DP controller's audio engine is configured,
and the DSP rejects it. DP audio on AudioReach platforms was reported
broken this way from v6.16 until b54a38af7138 went in:

https://lore.kernel.org/all/20250925040530.20731-1-liujianfeng1994@gmail.com/

That thread also has Jianfeng Liu's proposed DRM-side fix (wiring
.hw_params to the same helper), which Srinivas was fine with but Dmitry
argued is incorrect since the IEC 958 channel status isn't known at
hw_params time; as far as I can see it never landed, and the thread
ends with Dmitry noting his understanding that the issue should be
fixed by what became b54a38af7138. The legacy q6afe stack, which starts
its port at prepare, hit the same ordering problem on
qcm6490-fairphone-fp5:

https://lore.kernel.org/all/DD8PK8AI24P7.YK0OGVYC0QFM@fairphone.com/

b54a38af7138's commit message itself also cites PipeWire being unhappy
when the DP is not connected during start - presumably because with the
start at trigger, an open-but-idle DP PCM never attempts it.

I cannot exercise the DP path on my hardware - the FP6 has no DP audio
DAI link, so my testing (PipeWire, playback and capture) only covers
MI2S and codec DMA.

I also tested whether the in-flight "ASoC: qcom: qdsp6: Add MI2S clock
control" series can solve the aw88261 case without touching the graph
start:

https://lore.kernel.org/all/20260706132009.1496321-1-mohammad.rafi.shaik@oss.qualcomm.com/

I carried it onto the v7.1.2-based tree my board runs, wired the
FP6's Senary MI2S DAI to the q6prm SEN_MI2S_IBIT clock ("bclk" in the
new dai@ subnode) and enabled the machine-level bclk vote. The
plumbing works as intended - during the stream clk_summary shows the
clock enabled at the right rate (1536000 for 48 kHz 2ch S16) - but
the amplifiers still report "no clocks" (SYSST) through their
power-up check at prepare and fail to start. On this platform, at
least for Senary MI2S, the interface only drives the pads once the
graph has started, so the clock vote alone does not help a codec
that needs the bit clock before APM_CMD_GRAPH_START; conversely,
with the graph started at prepare the same codec powers up with no
clock vote at all. Happy to share the details in that series' thread
if useful (its patch 3/3 also overlaps the Senary set_fmt one-liner
I sent for sc8280xp, which I would coordinate there).

So for v2 I would keep the two behaviours apart per interface type:
start the graph at the end of prepare only for the MI2S DAIs
(q6i2s_ops), where an external codec like aw88261 needs the bit clock
running before the DAPM power-up sequence and where q6afe has always
started the port at prepare. b54a38af7138's other point - that there is
no data transfer yet at prepare - still holds there, but for these
interfaces the early start is about the clocking, not the data. The
trigger-time start stays unchanged for the DP/HDMI DAIs (q6hdmi_ops,
where the prepare-time start is exactly what broke) and the codec DMA
DAIs (q6dma_ops - codecs clocked from the LPASS macros or SoundWire,
not from the port). That also supersedes my note under the fold about
dropping the trigger callback in a respin: it stays, as the only start
path for the DP/HDMI and codec DMA DAIs.

Srinivas, since b54a38af7138 is yours: does that split sound
reasonable, or is there a DSP-side reason the MI2S ports should not
start at prepare either? And since I cannot test the DP side, it would
be great if someone with DP-capable AudioReach hardware (an X Elite
laptop or a QCS6490 board) could give v2 a spin there.

Thanks,
Jorijn

Re: [RFC PATCH] ASoC: qcom: q6apm-lpass-dais: start the graph at prepare

[Srinivas Kandagatla]

On 7/8/26 10:55 PM, Jorijn van der Graaf wrote:
> 
> So for v2 I would keep the two behaviours apart per interface type:
This is not going to scale, please try to enable the required clocks
from machine driver. Pl take a look at this series

https://lore.kernel.org/linux-sound/20260706132009.1496321-1-mohammad.rafi.shaik@oss.qualcomm.com/


--srini
> start the graph at the end of prepare only for the MI2S DAIs
> (q6i2s_ops), where an external codec like aw88261 needs the bit clock
> running before the DAPM power-up sequence and where q6afe has always
> started the port at prepare. b54a38af7138's other point - that there is
> no data transfer yet at prepare - still holds there, but for these
> interfaces the early start is about the clocking, not the data. The
> trigger-time start stays unchanged for the DP/HDMI DAIs (q6hdmi_ops,
> where the prepare-time start is exactly what broke) and the codec DMA
> DAIs (q6dma_ops - codecs clocked from the LPASS macros or SoundWire,
> not from the port). That also supersedes my note under the fold about
> dropping the trigger callback in a respin: it stays, as the only start
> path for the DP/HDMI and codec DMA DAIs.

Re: [RFC PATCH] ASoC: qcom: q6apm-lpass-dais: start the graph at prepare

[Jorijn van der Graaf]

On Wed, Jul 08, 2026 at 11:13:18PM +0100, Srinivas Kandagatla wrote:
> I guess you are referring to the IBIT clk that dsp provides, not the mclk.
>
> Could you not enable this from the machine driver?

Yes - SEN_MI2S_IBIT, provided by the DSP, not the mclk. And enabling
it from the machine driver is what I tested with the series you
point at below:

On Wed, Jul 08, 2026 at 11:18:50PM +0100, Srinivas Kandagatla wrote:
> > So for v2 I would keep the two behaviours apart per interface type:
> This is not going to scale, please try to enable the required clocks
> from machine driver. Pl take a look at this series
>
> https://lore.kernel.org/linux-sound/20260706132009.1496321-1-mohammad.rafi.shaik@oss.qualcomm.com/

I tested that series (v3) before sending my previous mail -
the report was buried in a long mail though, so here is the setup
and result again in a bit more detail. I carried the series onto
the tree my board runs, described SEN_MI2S_IBIT as the "bclk" of a
dai@147 subnode per the new binding, and set mi2s_bclk_enable in
the board data, so the machine driver votes the clock at hw_params.
The vote itself succeeds - the DSP acks it and clk_summary shows
the clock prepared and enabled at 1536000 during the stream - but
the amplifiers' synchronous power-up check at prepare still reports
"no clocks", and with the graph started at prepare the same codec
powers up with no bit-clock vote at all. So on this board the bit
clock only reaches the codec once the graph is started; the PRM
vote alone does not drive it.

The question that decides where the fix belongs: is the DSP expected
to drive IBIT on the pads from the PRM clock request alone, with the
endpoint's graph not (yet) started?

- If yes, milos firmware behaves differently here and I am happy to
  debug - any pointer on what to check would help.
- If no, the machine-driver clock route cannot cover a codec that
  needs the bit clock during the DAPM power-up sequence, on any
  AudioReach platform, and some form of start-before-power-up is
  still needed for such links.

Mohammad, if you have a chance, could you check on one of your
boards whether the bclk vote alone - before the trigger - produces
a running bit clock on the pads? That would tell us whether what I
am seeing is milos-specific.

I do take the scaling point on the per-ops split. If the vote turns
out not to work for this, would an opt-in per link fit better - e.g.
the machine driver (or board match data, in the style of the clock
series' mi2s_bclk_enable) marking the links whose codec needs a
running bit clock at power-up, and only those starting the graph at
prepare? Policy stays with the board, and DP/HDMI and everything
else keep the trigger-time start.

On Wed, Jul 08, 2026 at 11:13:18PM +0100, Srinivas Kandagatla wrote:
> This is going to break pipewire DP audio case, because pipewire will
> call prepare during the startup and if we do not have DP plugged in dsp
> will fail to start the graph and whole pipewire Audio defaults to dummy.

That spells out the severity - every variant proposed above leaves
DP trigger-started for exactly that reason.

Thanks,
Jorijn

Re: [RFC PATCH] ASoC: qcom: q6apm-lpass-dais: start the graph at prepare

[Srinivas Kandagatla]

On 7/5/26 4:38 AM, Jorijn van der Graaf wrote:
> The DAPM power-up sequence runs during snd_pcm prepare, but the BE
> port graph is only started at trigger time. A codec that powers up
> synchronously from a DAPM widget event and needs a running bit clock

I guess you are referring to the IBIT clk that dsp provides, not the mclk.

Could you not enable this from the machine driver?

> at that point - such as aw88261 since commit caea99ac809d ("ASoC:
> codecs: aw88261: remove async start") - can therefore never see a
> live clock: its power-up check runs before the trigger and fails on
> every stream start.>
> Start the graph at the end of prepare instead, mirroring what
> q6afe_dai_prepare() does on the legacy stack, so the interface
> clocks already run when DAPM powers up the codec. The FE side
> already starts its own graph at prepare in q6apm_dai_prepare();
> only the BE waited for trigger. The trigger-time start is kept as
> a fallback, guarded by is_port_started.
> 
> Tested on the Fairphone (Gen. 6) - 2x aw88261 on Senary MI2S:
> without this the amplifiers fail to power up with SYSST reporting
> "no clock" on every stream start; with it they start synchronously,
> including for the first short stream of the boot.
> 
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
> ---
> Sending as RFC because this changes when the port starts clocking for
> every AudioReach platform, and I may be missing the reason the start
> was placed in trigger rather than prepare in the first place:
> 
> - Is there a downside to starting the graph at prepare on AudioReach
>   (power, pop/click, or DSP-side constraints)? The legacy q6afe stack
>   has started its ports at the end of prepare all along, and the FE
>   side of AudioReach already starts its graph in q6apm_dai_prepare().
> - With the BE started at prepare, the BE graph now starts before the
>   FE graph for playback (prepare runs BE-first) — does
>   APM_CMD_GRAPH_START ordering between the two graphs matter?
> - Is the capture direction fine with this, or should it stay
>   trigger-started?
> 
> If this approach is right, the trigger callback becomes unreachable
> and I would drop it entirely in a non-RFC respin, like q6afe (which
> has no trigger op); it is kept here to keep the diff minimal.
> 
> Note: the test setup also carried my pending aw88261 power-up check
> fix, which addresses an unrelated SYSST-check failure on this board:
> https://lore.kernel.org/linux-sound/20260704192857.88366-1-jorijnvdgraaf@catcrafts.net/
> 
> Context (report and analysis of the aw88261 failure):
> https://lore.kernel.org/linux-sound/d2318ba8-43f9-478b-8f9e-2c09d44ae0b1@packett.cool/
> 
>  sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> index 006b283484d9..36e12a770c72 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> @@ -224,6 +224,20 @@ static int q6apm_lpass_dai_prepare(struct snd_pcm_substream *substream, struct s
>  		dev_err(dai->dev, "Failed to prepare Graph %d\n", rc);
>  		goto err;
>  	}
> +
> +	/*
> +	 * Start the graph here already, like q6afe does: this way the
> +	 * interface clocks are running before the DAPM power-up sequence,
> +	 * for codecs that need a live bit clock to power up (e.g.
> +	 * aw88261). The trigger callback keeps its start as a fallback.
> +	 */
> +	rc = q6apm_graph_start(dai_data->graph[dai->id]);

This is going to break pipewire DP audio case, because pipewire will
call prepare during the startup and if we do not have DP plugged in dsp
will fail to start the graph and whole pipewire Audio defaults to dummy.

--srini
> +	if (rc < 0) {
> +		dev_err(dai->dev, "Failed to start APM port %d\n", dai->id);
> +		goto err;
> +	}
> +	dai_data->is_port_started[dai->id] = true;
> +
>  	return 0;
>  err:
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> 
> base-commit: be44d21728b6646189779923b841ad3a46d694e5