drm/msm/dp, phy: qcom: improve DP Type-C reconnect handli

PATCH New naidu nama 2 participants 7 messages Nov 11

Subsystems (4)

DRM drivers Arm Qualcomm mailing list Generic PHY framework DRM driver for Qualcomm display hardware

lore

Share & tools

lore

naidu nama

author

2026-11-11 10:45 UTC #0 lore raw

v2 cover drm/msm/dp, phy: qcom: improve DP Type-C reconnect handli

This series improves the Qualcomm DP Type-C bring-up and reconnect paths by handling PHY initialization races and link-training corner cases more consistently.

The fixes cover races and link-training corner cases in the Qualcomm DP Type-C path. They prevent Type-C mux switching while the DP PHY is still initializing, avoid unnecessary retraining after a successful link train, and keep the downgrade retry path running across transient AUX disconnects when the cable is still plugged.

Together these changes make DP reconnect and orientation-switch handling more robust.

---

[v2]

- Updated the commit message [Konrad, Dmitry]
- Handled the force_link_train varibale usage [Konrad]
- Handled the plugged field from struct msm_dp_ctrl,
by passing the current dp->plugged value as a function argument [Dmitry]
- Dropped the drm/msm/dp: skip LTTPR init when no LTTPRs are present[Konrad, Dmitry]

---

Saurabh Anand (3):
phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is
initializing
drm/msm/dp: Remove redundant force_link_train flag drm/msm/dp: continue link training downgrade when cable is still
plugged

 drivers/gpu/drm/msm/dp/dp_ctrl.c          | 20 ++++++++++----------
 drivers/gpu/drm/msm/dp/dp_ctrl.h          |  7 +++----
 drivers/gpu/drm/msm/dp/dp_display.c       | 11 ++++-------
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c |  3 ++-
 4 files changed, 19 insertions(+), 22 deletions(-)

Signature · 1 line

2.43.0

naidu nama

author

2026-11-11 10:45 UTC #1 lore raw

PATCH v2 1/3 phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing

From: Saurabh Anand <[email protected]>

The Type-C mux switch guard only checked dp_powered_on, which is set in qmp_combo_dp_power_on(). However there is a race window between qmp_combo_dp_init() and qmp_combo_dp_power_on() during which dp_init_count is non-zero but dp_powered_on is still false. A Type-C orientation change arriving in this window would proceed with the mux switch while the DP PHY is mid-initialization, corrupting the PHY state.

Extend the guard to also block the mux switch when dp_init_count is non-zero, covering the full period from dp_init through dp_power_on.

Signed-off-by Saurabh Anand <[email protected]>

---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/phy/qualcomm/phy-qcom-qmp-combo.c +2 -1

index 61d8b7a5ce8f..3f93043f94e5 100644
@@ -5163,7 +5163,8 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
 		return 0;
 	}
 
-	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY && qmp->dp_powered_on) {
+	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY &&
+	    (qmp->dp_powered_on || qmp->dp_init_count)) {
 		dev_dbg(qmp->dev, "typec_mux_set: DP PHY is still in use, delaying switch\n");
 		return 0;
 	}

Signature · 1 line

2.43.0

[email protected]

AI review

2026-11-11 11:03 UTC #2 lore raw

Re: [PATCH v2 1/3] phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [High] phy: qcom: qmp-combo: qmp_combo_typec_mux_set() drops valid mode switches - [Medium] phy: qcom…

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] phy: qcom: qmp-combo: qmp_combo_typec_mux_set() drops valid mode switches
- [Medium] phy: qcom: qmp-combo: commit message confuses orientation and mode callbacks
--

commit de650bdad284e255575480f09852006eb78a904a
Author: Saurabh Anand <[email protected]>

phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing

This commit extends the guard in qmp_combo_typec_mux_set() to return early when qmp->dp_init_count is non-zero.

Quoted 6 lines

The Type-C mux switch guard only checked dp_powered_on, which is set in qmp_combo_dp_power_on(). However there is a race window between qmp_combo_dp_init() and qmp_combo_dp_power_on() during which dp_init_count is non-zero but dp_powered_on is still false. A Type-C orientation change arriving in this window would proceed with the mux switch while the DP PHY is mid-initialization, corrupting the PHY state.

[Severity: Medium]
Does the commit message confuse Type-C orientation changes with mode changes?

Type-C orientation changes are handled by qmp_combo_typec_switch_set(), which does not check qmp->dp_powered_on or qmp->dp_init_count before resetting the common block. qmp_combo_typec_mux_set() only handles Type-C mode and SVID transitions.

Quoted 5 lines

drivers/phy/qualcomm/phy-qcom-qmp-combo.c +0 -0
index 61d8b7a5ce8f9..3f93043f94e5c 100644

drivers/phy/qualcomm/phy-qcom-qmp-combo.c static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s · line 5170

Quoted 9 lines

drivers/phy/qualcomm/phy-qcom-qmp-combo.c +2 -1

@@ -5163,7 +5163,8 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
 		return 0;
 	}
 
-	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY && qmp->dp_powered_on) {
+	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY &&
+	    (qmp->dp_powered_on || qmp->dp_init_count)) {
 		dev_dbg(qmp->dev, "typec_mux_set: DP PHY is still in use, delaying switch\n");
 		return 0;
 	}

[Severity: High]
Does bailing out when qmp->dp_init_count is non-zero cause regression by dropping valid Type-C mode switches?

qmp_combo_typec_mux_set() discards new_mode on this path and nothing replays it when the DP PHY later exits. Meanwhile, qmp_combo_reconfigure_phy() already handles reconfiguring the PHY while qmp->dp_init_count is non-zero and qmp->dp_powered_on is false by powering down DP AUX, resetting COM with new_mode, and re-running qmp->cfg->dp_aux_init(qmp):

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_reconfigure_phy() {
	...
	if (qmp->dp_init_count)
		writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);

	qmp_combo_com_exit(qmp, true);

	qmp->qmpphy_mode = new_mode;

	/* Now everything's powered down, power up the right PHYs */
	qmp_combo_com_init(qmp, true);

	if ((qmp->usb_init_count || qmp->usb4_init_count) &&
	    new_mode != QMPPHY_MODE_DP_ONLY)
		qmp_combo_usb_power_on(qmp->usb_phy);

	if ((new_mode == QMPPHY_MODE_USB3DP || new_mode == QMPPHY_MODE_DP_ONLY) &&
	    qmp->dp_init_count)
		qmp->cfg->dp_aux_init(qmp);
}

During 4-lane DisplayPort Altmode entry, dp_altmode_configured() signals drm_connector_oob_hotplug_event() before calling dp_altmode_notify() -> typec_mux_set(). The hotplug event synchronously runs msm_dp_hpd_plug_handle() -> phy_init(), setting qmp->dp_init_count = 1 while qmp->qmpphy_mode is QMPPHY_MODE_USB3DP. When typec_mux_set() then requests QMPPHY_MODE_DP_ONLY, this guard drops the switch and leaves 4-lane DP broken.

Similarly, on cable unplug in pmic_glink_altmode_worker(), pmic_glink_altmode_safe() calls typec_mux_set() before drm_aux_hpd_bridge_notify() triggers phy_exit(), so the transition back to QMPPHY_MODE_USB3_ONLY is also dropped.

Signature · 6 lines

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

naidu nama

author

2026-11-11 10:45 UTC #3 lore raw

PATCH v2 2/3 drm/msm/dp: Remove redundant force_link_train flag

In reply to naidu nama

From: Saurabh Anand <[email protected]>

force_link_train is always false when passed to msm_dp_ctrl_prepare_stream_on(): the initial link is trained by msm_dp_ctrl_on_link(), and the flag is cleared before stream preparation. For active streams, it is never set.

Remove the dead flag and parameter. Stream preparation still retrains when channel equalization is incomplete.

Signed-off-by Saurabh Anand <[email protected]>

---
 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 5 ++---
 drivers/gpu/drm/msm/dp/dp_ctrl.h    | 3 +--
 drivers/gpu/drm/msm/dp/dp_display.c | 4 +---
 3 files changed, 4 insertions(+), 8 deletions(-)

drivers/gpu/drm/msm/dp/dp_ctrl.c +2 -3

index 47647fdec9de..6b9d9a76ba05 100644
@@ -2582,8 +2582,7 @@ static void msm_dp_ctrl_config_msa(struct msm_dp_ctrl_private *ctrl,
 }
 
 int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
-				  struct msm_dp_panel *panel,
-				  bool force_link_train)
+				  struct msm_dp_panel *panel)
 {
 	int ret = 0;
 	struct msm_dp_ctrl_private *ctrl;

@@ -2608,7 +2607,7 @@ int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
 		}
 	}
 
-	if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))
+	if (!msm_dp_ctrl_channel_eq_ok(ctrl))
 		msm_dp_ctrl_link_retrain(ctrl, panel);
 
 	/* stop txing train pattern to end link training */

drivers/gpu/drm/msm/dp/dp_ctrl.h +1 -2

index 80d9258501b4..fc884038182f 100644
@@ -20,8 +20,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 			struct msm_dp_panel *panel);
 int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel);
 int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
-				  struct msm_dp_panel *panel,
-				  bool force_link_train);
+				  struct msm_dp_panel *panel);
 void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl,
 			  struct msm_dp_panel *panel);
 void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl, enum msm_dp_stream_id stream_id);

drivers/gpu/drm/msm/dp/dp_display.c +1 -3

index c0c150e2c575..169938ba515e 100644
@@ -633,7 +633,6 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
 {
 	struct msm_dp *msm_dp_display = &dp->msm_dp_display;
 	int rc = 0;
-	bool force_link_train = false;
 
 	drm_dbg_dp(dp->drm_dev, "sink_count=%d\n", dp->link->sink_count);

@@ -651,7 +650,6 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
 
 	if (!msm_dp_display->power_on) {
 		msm_dp_display_host_phy_init(dp);
-		force_link_train = true;
 	}
 
 	rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel);

@@ -661,7 +659,7 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
 		return rc;
 	}
 
-	return msm_dp_ctrl_prepare_stream_on(dp->ctrl, dp->panel, force_link_train);
+	return msm_dp_ctrl_prepare_stream_on(dp->ctrl, dp->panel);
 }
 
 static int msm_dp_display_enable(struct msm_dp_display_private *dp,

Signature · 7 lines

2.43.0


-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

[email protected]

AI review

2026-11-11 10:57 UTC #4 lore raw

Re: [PATCH v2 2/3] drm/msm/dp: Remove redundant force_link_train flag

Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] drm/msm/dp: inaccurate commit message for msm_dp_ctrl_prepare_stream_on() --

Thank you for your contribution! Sashiko AI review found 1 potential issue to consider:
- [Medium] drm/msm/dp: inaccurate commit message for msm_dp_ctrl_prepare_stream_on()
--

commit c06a5a571a6cad37f7087d19d2c71c6677e8fbc7
Author: Saurabh Anand <[email protected]>

Quoted 11 lines

drm/msm/dp: Remove redundant force_link_train flag
force_link_train is always false when passed to msm_dp_ctrl_prepare_stream_on(): the initial link is trained by msm_dp_ctrl_on_link(), and the flag is cleared before stream preparation. For active streams, it is never set.
Remove the dead flag and parameter. Stream preparation still retrains when channel equalization is incomplete.
Signed-off-by Saurabh Anand <[email protected]>

[Severity: Medium]
Is the commit message accurate about force_link_train always being false?

In msm_dp_display_prepare_link(), when !msm_dp_display->power_on holds on display enable, force_link_train is set to true and passed directly to msm_dp_ctrl_prepare_stream_on() without being cleared. Rather than removing a dead flag, this change stops msm_dp_ctrl_prepare_stream_on() from unconditionally calling msm_dp_ctrl_link_retrain() a second time right after msm_dp_ctrl_on_link() has already trained the link.

Signature · 6 lines

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

naidu nama

author

2026-11-11 10:45 UTC #5 lore raw

PATCH v2 3/3 drm/msm/dp: continue link training downgrade when cable is still plugged

In reply to naidu nama

From: Saurabh Anand <[email protected]>

During a Type-C reconnect the AUX channel may report link-disconnected transiently while the physical cable is still present. The link training retry loop in msm_dp_ctrl_on_link() was aborting immediately on any msm_dp_aux_is_link_connected() failure, preventing the rate/lane downgrade path from running.

When the display is known to be plugged (msm_dp_ctrl->plugged), an AUX link-disconnected status is likely a transient glitch rather than a true unplug. Allow the downgrade loop to continue in that case by requiring both conditions before breaking out of the retry loop: AUX reports disconnected and the display is not plugged.

Signed-off-by Saurabh Anand <[email protected]>

---
 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 15 ++++++++-------
 drivers/gpu/drm/msm/dp/dp_ctrl.h    |  4 ++--
 drivers/gpu/drm/msm/dp/dp_display.c |  7 +++----
 3 files changed, 13 insertions(+), 13 deletions(-)

drivers/gpu/drm/msm/dp/dp_ctrl.c +8 -7

index 6b9d9a76ba05..f8a6bd7b0d10 100644
@@ -2274,7 +2274,8 @@ void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl, enum msm_dp_stre
 }
 
 static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl,
-						struct msm_dp_panel *panel)
+						struct msm_dp_panel *panel,
+						bool plugged)
 {
 	int ret;
 	unsigned long pixel_rate;

@@ -2293,7 +2294,7 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
 	msm_dp_ctrl_off_pixel_clk(&ctrl->msm_dp_ctrl, panel->stream_id);
 	msm_dp_ctrl_off_link(&ctrl->msm_dp_ctrl, panel);
 
-	ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl, panel);
+	ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl, panel, plugged);
 	if (ret) {
 		DRM_ERROR("failed to enable DP link controller\n");
 		return ret;

@@ -2310,7 +2311,7 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
 }
 
 void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl,
-				     struct msm_dp_panel *panel)
+				     struct msm_dp_panel *panel, bool plugged)
 {
 	struct msm_dp_ctrl_private *ctrl;
 	u32 sink_request = 0x0;

@@ -2325,7 +2326,7 @@ void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl,
 
 	if (sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
 		drm_dbg_dp(ctrl->drm_dev, "PHY_TEST_PATTERN request\n");
-		if (msm_dp_ctrl_process_phy_test_request(ctrl, panel)) {
+		if (msm_dp_ctrl_process_phy_test_request(ctrl, panel, plugged)) {
 			DRM_ERROR("process phy_test_req failed\n");
 			return;
 		}

@@ -2377,7 +2378,7 @@ static bool msm_dp_ctrl_channel_eq_ok(struct msm_dp_ctrl_private *ctrl)
 }
 
 int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
-			struct msm_dp_panel *panel)
+			struct msm_dp_panel *panel, bool plugged)
 {
 	int rc = 0;
 	struct msm_dp_ctrl_private *ctrl;

@@ -2427,7 +2428,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 			break;
 		} else if (training_step == DP_TRAINING_1) {
 			/* link train_1 failed */
-			if (!msm_dp_aux_is_link_connected(ctrl->aux))
+			if (!msm_dp_aux_is_link_connected(ctrl->aux) && !plugged)
 				break;
 
 			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);

@@ -2452,7 +2453,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 			}
 		} else if (training_step == DP_TRAINING_2) {
 			/* link train_2 failed */
-			if (!msm_dp_aux_is_link_connected(ctrl->aux))
+			if (!msm_dp_aux_is_link_connected(ctrl->aux) && !plugged)
 				break;
 
 			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);

drivers/gpu/drm/msm/dp/dp_ctrl.h +2 -2

index fc884038182f..3edf954cc805 100644
@@ -17,7 +17,7 @@ struct msm_dp_ctrl {
 struct phy;
 
 int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
-			struct msm_dp_panel *panel);
+			struct msm_dp_panel *panel, bool plugged);
 int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel);
 int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
 				  struct msm_dp_panel *panel);

@@ -28,7 +28,7 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl);
 irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl,
 			    struct msm_dp_panel *panel);
 void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl,
-				     struct msm_dp_panel *panel);
+				     struct msm_dp_panel *panel, bool plugged);
 struct msm_dp_ctrl *msm_dp_ctrl_get(struct device *dev,
 				    struct msm_dp_link *link,
 				    struct drm_dp_aux *aux,

drivers/gpu/drm/msm/dp/dp_display.c +3 -4

index 169938ba515e..551d2c63ef8e 100644
@@ -399,7 +399,7 @@ static int msm_dp_display_handle_irq_hpd(struct msm_dp_display_private *dp)
 
 	drm_dbg_dp(dp->drm_dev, "%d\n", sink_request);
 
-	msm_dp_ctrl_handle_sink_request(dp->ctrl, dp->panel);
+	msm_dp_ctrl_handle_sink_request(dp->ctrl, dp->panel, dp->plugged);
 
 	if (sink_request & DP_TEST_LINK_VIDEO_PATTERN)
 		msm_dp_display_handle_video_request(dp);

@@ -648,11 +648,10 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
 	if (dp->link->sink_count == 0)
 		return -ENOTCONN;
 
-	if (!msm_dp_display->power_on) {
+	if (!msm_dp_display->power_on)
 		msm_dp_display_host_phy_init(dp);
-	}
 
-	rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel);
+	rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel, dp->plugged);
 	if (rc) {
 		DRM_ERROR("Failed link training (rc=%d)\n", rc);
 		// TODO: schedule drm_connector_set_link_status_property()

Signature · 7 lines

2.43.0


-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

[email protected]

AI review

2026-11-11 10:58 UTC #6 lore raw

Re: [PATCH v2 3/3] drm/msm/dp: continue link training downgrade when cable is still plugged

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] drm/msm/dp: pass-by-value plugged in msm_dp_ctrl_on_link() breaks unplug abort - [Medium]…

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [Medium] drm/msm/dp: pass-by-value plugged in msm_dp_ctrl_on_link() breaks unplug abort
- [Medium] drm/msm/dp: uninitialized link_status read in msm_dp_ctrl_on_link()
--

commit 03d6b22aa9378458ab94c6193f79ad69b24cfe32
Author: Saurabh Anand <[email protected]>

drm/msm/dp: continue link training downgrade when cable is still plugged

This commit passes dp->plugged into msm_dp_ctrl_on_link() so the link training retry loop only aborts when both msm_dp_aux_is_link_connected() and plugged are false.

Quoted 4 lines

drivers/gpu/drm/msm/dp/dp_ctrl.c +0 -0
index 6b9d9a76ba059..f8a6bd7b0d107 100644

[ ... ]

Quoted 10 lines

drivers/gpu/drm/msm/dp/dp_ctrl.c +1 -1

@@ -2377,7 +2378,7 @@ static bool msm_dp_ctrl_channel_eq_ok(struct msm_dp_ctrl_private *ctrl)
 }
 
 int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
-			struct msm_dp_panel *panel)
+			struct msm_dp_panel *panel, bool plugged)
 {
 	int rc = 0;
 	struct msm_dp_ctrl_private *ctrl;

drivers/gpu/drm/msm/dp/dp_ctrl.c int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl, · line 2432

Quoted 6 lines

drivers/gpu/drm/msm/dp/dp_ctrl.c +1 -1

@@ -2427,7 +2428,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 			break;
 		} else if (training_step == DP_TRAINING_1) {
 			/* link train_1 failed */
-			if (!msm_dp_aux_is_link_connected(ctrl->aux) && !plugged)
+			if (!msm_dp_aux_is_link_connected(ctrl->aux) && !plugged)
 				break;

[Severity: Medium]
Does passing dp->plugged by value into msm_dp_ctrl_on_link() prevent the retry loop from ever breaking out on an unplug during link training?

At both callers (msm_dp_display_prepare_link() and msm_dp_display_handle_irq_hpd()), dp->plugged is true when link training starts, and msm_dp_display_prepare_link() also reads dp->plugged without holding dp->plugged_lock. If the cable is unplugged while the retry loop is running, msm_dp_hpd_unplug_handle() sets dp->plugged = false, but the local plugged parameter in msm_dp_ctrl_on_link() stays true for every iteration, so !plugged is always false and all remaining downshift retries and PHY power-cycles still run on the disconnected link.

Also, the commit message refers to msm_dp_ctrl->plugged, which does not exist in struct msm_dp_ctrl or struct msm_dp_ctrl_private.

drivers/gpu/drm/msm/dp/dp_ctrl.c int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl, · line 2459

Quoted 11 lines

drivers/gpu/drm/msm/dp/dp_ctrl.c +1 -1

 			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);

@@ -2452,7 +2453,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 			}
 		} else if (training_step == DP_TRAINING_2) {
 			/* link train_2 failed */
-			if (!msm_dp_aux_is_link_connected(ctrl->aux) && !plugged)
+			if (!msm_dp_aux_is_link_connected(ctrl->aux) && !plugged)
 				break;
 
 			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);

[Severity: Medium]
When continuing past !msm_dp_aux_is_link_connected(ctrl->aux) in the DP_TRAINING_1 and DP_TRAINING_2 failure paths, can this read uninitialized stack bytes from link_status[]?

link_status is declared on the stack without initialization in msm_dp_ctrl_on_link(). When the AUX channel is transiently disconnected or a concurrent unplug disables AUX transfers, drm_dp_dpcd_read_link_status() fails without writing to link_status, and its return value is not checked before msm_dp_ctrl_clock_recovery_any_ok() and drm_dp_clock_recovery_ok() inspect link_status to choose between rate downshift, lane downshift, and abort.

Signature · 6 lines

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy