Re: [PATCH] drm/display: let a bridge configure sink audio from hw_params

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/display: skipping bridge prepare() breaks ALSA resume and XRUN recovery
--

commit 9242b33c411ab2327a1ed1ae0af3ab2560e75b3a
Author: Jean-Francois Bobier <jean-francois.bobier@laposte.net>

drm/display: let a bridge configure sink audio from hw_params

This commit introduces a prepare_early flag to configure a sink's audio
path from the ASoC hw_params callback rather than from prepare. This is
done to support configurations where the CPU side of the DAI link cannot
start until the sink's audio clock is already running.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> index 7d78b02c14462..3ed5d73f17eb0 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> @@ -34,6 +34,28 @@ static int drm_connector_hdmi_audio_prepare(struct device *dev, void *data,
>  	const struct drm_connector_hdmi_audio_funcs *funcs =
>  		connector->hdmi_audio.funcs;
>  
> +	if (connector->hdmi_audio.prepare_early)
> +		return 0;

[Severity: High]
Can skipping hardware initialization here break recovery from XRUNs or system
resume?

When ALSA resumes from system suspend or recovers from an XRUN, the core (or
userspace) calls snd_pcm_prepare() without calling .hw_params. Because
prepare_early is set, drm_connector_hdmi_audio_prepare() returns early and
skips funcs->prepare().

This means the hardware audio clocks and infoframe configurations are never
restored, and the subsequent CPU DAI .prepare can time out waiting for the
missing audio clock.

> +
> +	return funcs->prepare(connector, fmt, hparms);
> +}
> +
> +/*
> + * Only used by connectors that asked for it; everyone else keeps being
> + * configured from prepare() and this returns without doing anything. See
> + * &drm_bridge.hdmi_audio_prepare_early for why a connector would want it.
> + */
> +static int drm_connector_hdmi_audio_hw_params(struct device *dev, void *data,
> +					      struct hdmi_codec_daifmt *fmt,
> +					      struct hdmi_codec_params *hparms)
> +{
> +	struct drm_connector *connector = data;
> +	const struct drm_connector_hdmi_audio_funcs *funcs =
> +		connector->hdmi_audio.funcs;
> +
> +	if (!connector->hdmi_audio.prepare_early)
> +		return 0;
> +
> +	return funcs->prepare(connector, fmt, hparms);

[Severity: High]
Will shifting the hardware setup exclusively to .hw_params leave the hardware
disabled during recovery?

Because ALSA does not invoke .hw_params during XRUN or resume recovery,
shifting the configuration entirely here means the stream may become completely
unusable after an XRUN or system resume. This would require the stream to be
fully closed and reopened to recover.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005131545.23219-1-jean-francois.bobier@laposte.net?part=1