mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dp: return 0 from audio_prepare when cable is disconnected
@ 2026-06-16 15:12 Kumar Anurag
  2026-06-16 15:12 ` [PATCH v1] " Kumar Anurag
  0 siblings, 1 reply; 4+ messages in thread
From: Kumar Anurag @ 2026-06-16 15:12 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, David Airlie, Simona Vetter
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	linux-arm-msm, dri-devel, freedreno, linux-kernel, Kumar Anurag,
	Kumar Anurag

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.

Signed-off-by: Kumar Anurag <kumar.anurag@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/dp/dp_audio.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c b/drivers/gpu/drm/msm/dp/dp_audio.c
index 035e230201fd..8118130a3220 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->active_stream_cnt) {
-		rc = -EINVAL;
+	if (!msm_dp_display->active_stream_cnt)
 		goto end;
-	}
 
 	audio = msm_dp_audio_get_data(msm_dp_display);
 	if (IS_ERR(audio)) {
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v1] drm/msm/dp: return 0 from audio_prepare when cable is disconnected
  2026-06-16 15:12 [PATCH] drm/msm/dp: return 0 from audio_prepare when cable is disconnected Kumar Anurag
@ 2026-06-16 15:12 ` Kumar Anurag
       [not found]   ` <20260616152536.D81FB1F000E9@smtp.kernel.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Kumar Anurag @ 2026-06-16 15:12 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, David Airlie, Simona Vetter
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	linux-arm-msm, dri-devel, freedreno, linux-kernel, Kumar Anurag,
	Srinivas Kandagatla

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.

Signed-off-by: Kumar Anurag <kumar.singh@oss.qualcomm.com>
Suggested-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/dp/dp_audio.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c b/drivers/gpu/drm/msm/dp/dp_audio.c
index 41018e82efa1..df222943ae57 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;
-	}
 
 	audio = msm_dp_audio_get_data(msm_dp_display);
 	if (IS_ERR(audio)) {
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] drm/msm/dp: return 0 from audio_prepare when cable is disconnected
       [not found]   ` <20260616152536.D81FB1F000E9@smtp.kernel.org>
@ 2026-07-12 10:16     ` Dmitry Baryshkov
  2026-07-17  7:26       ` Srinivas Kandagatla
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Baryshkov @ 2026-07-12 10:16 UTC (permalink / raw)
  To: sashiko-reviews, Kumar Anurag, Rob Clark, David Airlie,
	Simona Vetter, Srinivas Kandagatla, Mark Brown
  Cc: dri-devel, Abhinav Kumar, Jessica Zhang, Sean Paul,
	Marijn Suijten, linux-arm-msm, freedreno, linux-kernel,
	Liam Girdwood, linux-sound

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.

Mark, would you have any particular recommendations?

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] drm/msm/dp: return 0 from audio_prepare when cable is disconnected
  2026-07-12 10:16     ` Dmitry Baryshkov
@ 2026-07-17  7:26       ` Srinivas Kandagatla
  0 siblings, 0 replies; 4+ 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] 4+ messages in thread

end of thread, other threads:[~2026-07-17  7:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-16 15:12 [PATCH] drm/msm/dp: return 0 from audio_prepare when cable is disconnected 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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome