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

[Srinivas Kandagatla <srini@kernel.org>]

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