* Re: [PATCH v1] drm/msm/dp: return 0 from audio_prepare when cable is disconnected
2026-07-12 10:16 ` [PATCH v1] drm/msm/dp: return 0 from audio_prepare when cable is disconnected Dmitry Baryshkov
@ 2026-07-17 7:26 ` Srinivas Kandagatla
0 siblings, 0 replies; 2+ messages in thread
From: Srinivas Kandagatla @ 2026-07-17 7:26 UTC (permalink / raw)
To: Dmitry Baryshkov, sashiko-reviews, Kumar Anurag, Rob Clark,
David Airlie, Simona Vetter, Mark Brown
Cc: dri-devel, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, linux-arm-msm, freedreno, linux-kernel,
Liam Girdwood, linux-sound
On 7/12/26 11:16 AM, Dmitry Baryshkov wrote:
> On Tue, Jun 16, 2026 at 03:25:36PM +0000, sashiko-bot@kernel.org 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@oss.qualcomm.com>
>>
>> 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.
AFAIU,There is no special error code for prepare to flag that cable is
not connected, the standard way is to read Jack events. Wireplumber is
is one which deals with Jack event, even before getting to there
pipewire will fail to probe the devices on the card if prepare fails.
Once we are at Wireplumber level we are good, and stream will never sent
to the sinks which are in disconnected state (via Jack).
This has been a known issue for very long time, most of the codecs like
Its also interesting to note that same Qualcomm SoC drivers deal with
this situation by returning zero, so why dp has to be different?
drivers/gpu/drm/msm/hdmi/hdmi_audio.c
Other drivers that do not care about plug state in prepare are.
sound/x86/intel_hdmi_audio.c
drivers/gpu/drm/mediatek/mtk_hdmi.c
sound/hda/codecs/hdmi/hdmi.c
drivers/gpu/drm/xlnx/zynqmp_dp_audio.c
I think there is enough evidence how this is handled in the codec
drivers, we should consider this patch.
--srini
>
> Mark, would you have any particular recommendations?
>
^ permalink raw reply [flat|nested] 2+ messages in thread