dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] drm/msm/dp: don't gate eDP on DP_SINK_COUNT
@ 2026-09-08  9:17 Vivek Kumar
  2026-09-08  9:31 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Vivek Kumar @ 2026-09-08  9:17 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, Vivek Kumar

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: Vivek Kumar <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] 2+ messages in thread

* Re: [PATCH v2] drm/msm/dp: don't gate eDP on DP_SINK_COUNT
  2026-09-08  9:17 [PATCH v2] drm/msm/dp: don't gate eDP on DP_SINK_COUNT Vivek Kumar
@ 2026-09-08  9:31 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-08  9:31 UTC (permalink / raw)
  To: Vivek Kumar; +Cc: dri-devel

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

New issues:
- [High] Incomplete fix leaves eDP branch devices gated on sink_count and skips critical initialization.

Pre-existing issues:
- [High] PM runtime usage counter underflow on pm_runtime_resume_and_get failure.
--

commit 2e22ab220d8aa377241f1c1461ea7061493df07f
Author: Vivek Kumar <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 can this early return cause a PM runtime
usage counter underflow?

If pm_runtime_resume_and_get() fails here in msm_dp_display_prepare_link(), it
returns a negative error and drops the reference. This failure propagates back,
and eventually the DRM core calls msm_dp_display_atomic_post_disable(), which
unconditionally executes msm_dp_display_unprepare() and calls
pm_runtime_put_sync(), decrementing a counter that was never incremented.

> -	if (dp->link->sink_count == 0)
> +	if (msm_dp_link_no_sink(dp))

[Severity: High]
Does this fix miss the eDP sink count exemption in msm_dp_panel_read_link_caps(),
leaving eDP branch devices gated and causing critical initialization to be
skipped?

When an eDP branch device with a 0 sink_count is connected,
msm_dp_panel_read_link_caps() in dp_panel.c still aborts:

		count = drm_dp_read_sink_count(panel->aux);
		if (!count) {
			panel->link->sink_count = 0;
			return -ENOTCONN;
		}

Because this failure propagates back to msm_dp_display_process_hpd_high(), it
skips EDID reading and PSM link configuration:

	rc = msm_dp_panel_read_link_caps(dp->panel, connector);
	if (rc)
		goto end;

Then, earlier in msm_dp_display_prepare_link(), the return value of
msm_dp_hpd_plug_handle() (which calls msm_dp_display_process_hpd_high) is
ignored for eDP:

	if (msm_dp_display->is_edp)
		msm_dp_hpd_plug_handle(dp);

As a result, the driver proceeds to use the display in an uninitialized and
degraded state.

>  		return -ENOTCONN;
>  
>  	if (!msm_dp_display->power_on) {

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

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

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

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-08  9:17 [PATCH v2] drm/msm/dp: don't gate eDP on DP_SINK_COUNT Vivek Kumar
2026-09-08  9:31 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).