On Tue, Jun 16, 2026 at 03:25:36PM +0000, [email protected] 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 <[email protected]>
>
> 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
