Re: [PATCH v1] drm/msm/dp: return 0 from audio_prepare when cable is disconnected

[Dmitry Baryshkov]

On Tue, Jun 16, 2026 at 03:25:36PM +0000, sashiko-bot@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Returning success (0) from `msm_dp_audio_prepare` when the display is disconnected causes silent audio failures upon hotplug and NoC errors due to unclocked DSP hardware access.
> 
> Pre-existing issues:
> - [High] Race condition between `msm_dp_audio_prepare` and `msm_dp_display_disable` causes unclocked MMIO accesses.
> --
> 
> commit bfb65364e66313dc1385a42e09172f44a1e1fbd1
> Author: Kumar Anurag <kumar.singh@xxxxxxxxxxxxxxxx>
> 
> drm/msm/dp: return 0 from audio_prepare when cable is disconnected
> 
> PipeWire treats a non-zero return from prepare as fatal, marking the
> DP audio device as a dummy sink when the cable is unplugged.
> 
> > The active_stream_cnt guard already prevents any unclocked hardware access,
> > so return success instead of -EINVAL when the link is not active.
> 
> [Severity: High]
> Is this commit message accurate for the upstream tree? There doesn't appear
> to be an active_stream_cnt guard in the mainline driver. Without this guard,
> will the DSP push data to powered-off, unclocked DP hardware and cause NoC
> errors?

$ git describe
v7.2-rc2-4669-gb9810cd75b9f
$ git grep active_stream_cnt
$

Which tree has this commit message been based on?

> 
> > diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c b/drivers/gpu/drm/msm/dp/dp_audio.c
> > index 41018e82efa10..df222943ae57f 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_audio.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_audio.c
> > @@ -284,10 +284,8 @@ int msm_dp_audio_prepare(struct drm_bridge *bridge,
> >  	 * such cases check for connection status and bail out if not
> >  	 * connected.
> >  	 */
> > -	if (!msm_dp_display->power_on) {
> > -		rc = -EINVAL;
> > +	if (!msm_dp_display->power_on)
> >  		goto end;
> 
> [Severity: High]
> Does returning 0 here trick the ALSA/ASoC framework into marking the stream
> as successfully prepared?
> 
> If the stream is marked RUNNING, ALSA won't re-invoke prepare() when the
> display is later connected. Could this regression result in silent playback
> because the DP hardware is never actually configured?
> 
> [Severity: High]
> This is a pre-existing issue, but does this lockless check of power_on
> race with msm_dp_display_disable()?
> 
> If msm_dp_display_disable() runs concurrently (e.g., from a hotplug interrupt)
> immediately after this check but before audio_enabled is set to true, it will
> skip waiting and disable the DP link and clocks.
> 
> When msm_dp_audio_prepare() resumes, it will call msm_dp_audio_setup_sdp() and
> attempt MMIO operations on unclocked DP audio registers. Could this lead to a
> synchronous external abort?

I'd agree with Sashiko here. Do we guard against the unlocked access? Or
how do we ensure that the stream will be prepared later?

I think this needs to be fixed on the other side of the pipe. Do we need
to define a special prepare error code, which means "working, but not
available now" and make PulseAudio / PipeWire handle those errors
specially? How is it solved for other platforms? I don't think that
msm/dp is unique here.

Mark, would you have any particular recommendations?

-- 
With best wishes
Dmitry