dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dp: don't gate eDP on DP_SINK_COUNT
@ 2026-09-08  9:01 092vk
  2026-09-08  9:04 ` Konrad Dybcio
  2026-09-08  9:16 ` sashiko-bot
  0 siblings, 2 replies; 3+ messages in thread
From: 092vk @ 2026-09-08  9:01 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, linux-arm-msm, dri-devel, freedreno
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	Yongxing Mou, Konrad Dybcio, David Airlie, Simona Vetter,
	linux-kernel, 092vk

msm_dp_display_prepare_link(), msm_dp_display_disable(), and
msm_dp_hpd_unplug_handle() all treat dp->link->sink_count == 0 as
"nothing is connected", and in msm_dp_display_prepare_link() this
causes probe/enable to fail outright with -ENOTCONN.

This is correct for external DisplayPort, where a compliant sink is
required to report a valid DP_SINK_COUNT over the AUX channel. It is
not correct for eDP: some eDP sinks -- particularly fixed-format
eDP-to-HDMI bridge chips with no downstream AUX-capable panel
behind them -- never populate DP_SINK_COUNT with a nonzero value,
even while otherwise participating normally in DPCD capability
exchange and link training.

drm_dp_read_sink_count_cap() already accounts for this by excluding
eDP connectors from the sink-count requirement for other DPCD-derived
checks (msm_dp_bridge_detect() takes care). Apply the same exemption
to the raw sink_count reads in this file via a small shared
helper, so eDP outputs with a non-AUX-capable downstream sink are
not treated as permanently disconnected.

Tested on a Qualcomm QCS6490-based board driving a fixed-format
eDP-to-HDMI bridge IC (chrontel,ch7218a) with no negotiable
DPCD-capable panel behind it; without this fix,
msm_dp_display_prepare_link() unconditionally returns -ENOTCONN
and the display never comes up.

Fixes: ca16db3d18cf ("drm/msm/dp: break up dp_display_enable into two parts")

Signed-off-by: 092vk <vk092kumar@gmail.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index bc646d172abe..9254f080f0ea 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -400,6 +400,12 @@ static int msm_dp_display_handle_irq_hpd(struct msm_dp_display_private *dp)
 	return 0;
 }
 
+static inline bool msm_dp_link_no_sink(struct msm_dp_display_private *dp)
+{
+	/* eDP sinks are not required to report a valid sink count */
+	return !dp->msm_dp_display.is_edp && dp->link->sink_count == 0;
+}
+
 static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp)
 {
 	int ret;
@@ -467,7 +473,7 @@ static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp)
 		drm_edid_connector_update(dp->msm_dp_display.connector, NULL);
 
 	/* triggered by irq_hdp with sink_count = 0 */
-	if (dp->link->sink_count == 0)
+	if (msm_dp_link_no_sink(dp))
 		msm_dp_display_host_phy_exit(dp);
 
 	/*
@@ -634,7 +640,7 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
 		return rc;
 	}
 
-	if (dp->link->sink_count == 0)
+	if (msm_dp_link_no_sink(dp))
 		return -ENOTCONN;
 
 	if (!msm_dp_display->power_on) {
@@ -723,12 +729,12 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp,
 	msm_dp_ctrl_off_pixel_clk(dp->ctrl);
 
 	/* dongle is still connected but sinks are disconnected */
-	if (dp->link->sink_count == 0)
+	if (msm_dp_link_no_sink(dp))
 		msm_dp_link_psm_config(dp->link, &msm_dp_panel->link_info, true);
 
 	msm_dp_ctrl_off_link(dp->ctrl, msm_dp_panel);
 
-	if (dp->link->sink_count == 0)
+	if (msm_dp_link_no_sink(dp))
 		/* re-init the PHY so that we can listen to Dongle disconnect */
 		msm_dp_ctrl_reinit_phy(dp->ctrl);
 	else
-- 
2.55.0


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

* Re: [PATCH] drm/msm/dp: don't gate eDP on DP_SINK_COUNT
  2026-09-08  9:01 [PATCH] drm/msm/dp: don't gate eDP on DP_SINK_COUNT 092vk
@ 2026-09-08  9:04 ` Konrad Dybcio
  2026-09-08  9:16 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: Konrad Dybcio @ 2026-09-08  9:04 UTC (permalink / raw)
  To: 092vk, Rob Clark, Dmitry Baryshkov, linux-arm-msm, dri-devel,
	freedreno
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	Yongxing Mou, David Airlie, Simona Vetter, linux-kernel

On 9/8/26 11:01 AM, 092vk wrote:
> msm_dp_display_prepare_link(), msm_dp_display_disable(), and
> msm_dp_hpd_unplug_handle() all treat dp->link->sink_count == 0 as
> "nothing is connected", and in msm_dp_display_prepare_link() this
> causes probe/enable to fail outright with -ENOTCONN.
> 
> This is correct for external DisplayPort, where a compliant sink is
> required to report a valid DP_SINK_COUNT over the AUX channel. It is
> not correct for eDP: some eDP sinks -- particularly fixed-format
> eDP-to-HDMI bridge chips with no downstream AUX-capable panel
> behind them -- never populate DP_SINK_COUNT with a nonzero value,
> even while otherwise participating normally in DPCD capability
> exchange and link training.
> 
> drm_dp_read_sink_count_cap() already accounts for this by excluding
> eDP connectors from the sink-count requirement for other DPCD-derived
> checks (msm_dp_bridge_detect() takes care). Apply the same exemption
> to the raw sink_count reads in this file via a small shared
> helper, so eDP outputs with a non-AUX-capable downstream sink are
> not treated as permanently disconnected.
> 
> Tested on a Qualcomm QCS6490-based board driving a fixed-format
> eDP-to-HDMI bridge IC (chrontel,ch7218a) with no negotiable
> DPCD-capable panel behind it; without this fix,
> msm_dp_display_prepare_link() unconditionally returns -ENOTCONN
> and the display never comes up.
> 
> Fixes: ca16db3d18cf ("drm/msm/dp: break up dp_display_enable into two parts")
> 
> Signed-off-by: 092vk <vk092kumar@gmail.com>
> ---

Hi, please use a known identity

https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

Konrad

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

* Re: [PATCH] drm/msm/dp: don't gate eDP on DP_SINK_COUNT
  2026-09-08  9:01 [PATCH] drm/msm/dp: don't gate eDP on DP_SINK_COUNT 092vk
  2026-09-08  9:04 ` Konrad Dybcio
@ 2026-09-08  9:16 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-08  9:16 UTC (permalink / raw)
  To: 092vk; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] PM runtime usage count underflow when pm_runtime_resume_and_get() fails in msm_dp_display_prepare_link().
- [High] msm_dp_display_disable() calls functions that acquire mutexes while executing in an atomic context.
--

commit 4bcf3ca8adda3a167d25a4acaa736186981bd198
Author: 092vk <vk092kumar@gmail.com>

drm/msm/dp: don't gate eDP on DP_SINK_COUNT

msm_dp_display_prepare_link(), msm_dp_display_disable(), and
msm_dp_hpd_unplug_handle() all treat dp->link->sink_count == 0 as
"nothing is connected", and in msm_dp_display_prepare_link() this
causes probe/enable to fail outright with -ENOTCONN.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index bc646d172abe0..9254f080f0ea6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -634,7 +640,7 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
>  		return rc;
>  	}
>  

[Severity: High]
This is a pre-existing issue, but does returning early here when
pm_runtime_resume_and_get() fails cause a PM runtime usage count underflow
during teardown?

If pm_runtime_resume_and_get() fails in msm_dp_display_prepare_link(), it
returns early without taking a reference. However, during teardown,
msm_dp_display_unprepare() is called unconditionally and performs a
pm_runtime_put_sync(), dropping a reference that was never acquired.

Can this PM usage count underflow break power management for the device,
potentially leaving it permanently on or off?

> -	if (dp->link->sink_count == 0)
> +	if (msm_dp_link_no_sink(dp))
>  		return -ENOTCONN;
>  
>  	if (!msm_dp_display->power_on) {
[ ... ]
> @@ -723,12 +729,12 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp,
>  	msm_dp_ctrl_off_pixel_clk(dp->ctrl);
>  
>  	/* dongle is still connected but sinks are disconnected */
> -	if (dp->link->sink_count == 0)
> +	if (msm_dp_link_no_sink(dp))
>  		msm_dp_link_psm_config(dp->link, &msm_dp_panel->link_info, true);
>  
>  	msm_dp_ctrl_off_link(dp->ctrl, msm_dp_panel);
>  
> -	if (dp->link->sink_count == 0)
> +	if (msm_dp_link_no_sink(dp))
>  		/* re-init the PHY so that we can listen to Dongle disconnect */
>  		msm_dp_ctrl_reinit_phy(dp->ctrl);
>  	else

[Severity: High]
This isn't a bug introduced by this patch, but does msm_dp_display_disable()
acquire sleeping mutexes while executing in an atomic context?

This function is called from the msm_dp_display_atomic_post_disable() encoder
callback, which is an atomic context path where sleeping is forbidden. However,
the branches modified here call functions that sleep:

- msm_dp_link_psm_config() directly acquires a mutex.
- msm_dp_ctrl_reinit_phy() triggers phy_init() and phy_exit(), which also
  acquire mutexes.
- msm_dp_display_host_phy_exit() triggers phy_exit().

Can these calls lead to scheduling-while-atomic warnings, system instability,
or potential deadlocks during normal display disable or modeset operations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908090146.3100787-1-vk092kumar@gmail.com?part=1

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

end of thread, other threads:[~2026-09-09  6:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08  9:01 [PATCH] drm/msm/dp: don't gate eDP on DP_SINK_COUNT 092vk
2026-09-08  9:04 ` Konrad Dybcio
2026-09-08  9:16 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox