mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Srinivas Kandagatla <srini@kernel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	sashiko-reviews@lists.linux.dev,
	Kumar Anurag <kumar.singh@oss.qualcomm.com>,
	Rob Clark <robin.clark@oss.qualcomm.com>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	Mark Brown <broonie@kernel.org>
Cc: dri-devel@lists.freedesktop.org,
	Abhinav Kumar <abhinav.kumar@linux.dev>,
	Jessica Zhang <jesszhan0024@gmail.com>,
	Sean Paul <sean@poorly.run>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, Liam Girdwood <lgirdwood@gmail.com>,
	linux-sound@vger.kernel.org
Subject: Re: [PATCH v1] drm/msm/dp: return 0 from audio_prepare when cable is disconnected
Date: Fri, 17 Jul 2026 08:26:18 +0100	[thread overview]
Message-ID: <b5c0de1f-75e3-443e-9e29-a70fbdbf4441@kernel.org> (raw)
In-Reply-To: <5ixuxbgngzobl5c5fsyahdsouohwuykoat7s4hy6sczrye7vwf@rj7nmn7x3c56>

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?
> 


      reply	other threads:[~2026-07-17  7:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-16 15:12 [PATCH] " Kumar Anurag
2026-06-16 15:12 ` [PATCH v1] " Kumar Anurag
     [not found]   ` <20260616152536.D81FB1F000E9@smtp.kernel.org>
2026-07-12 10:16     ` Dmitry Baryshkov
2026-07-17  7:26       ` Srinivas Kandagatla [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b5c0de1f-75e3-443e-9e29-a70fbdbf4441@kernel.org \
    --to=srini@kernel.org \
    --cc=abhinav.kumar@linux.dev \
    --cc=airlied@gmail.com \
    --cc=broonie@kernel.org \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jesszhan0024@gmail.com \
    --cc=kumar.singh@oss.qualcomm.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sean@poorly.run \
    --cc=simona@ffwll.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox