[PATCH v2 0/3] drm/msm/dp, phy: qcom: improve DP Type-C reconnect handli

[PATCH v2 0/3] drm/msm/dp, phy: qcom: improve DP Type-C reconnect handli

[naidu nama]

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(-)

-- 
2.43.0

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

[naidu nama]

From: Saurabh Anand <saurabh.anand@oss.qualcomm.com>

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 <saurabh.anand@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 61d8b7a5ce8f..3f93043f94e5 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -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;
 	}
-- 
2.43.0

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

[sashiko-bot]

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 <saurabh.anand@oss.qualcomm.com>

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.

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

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 61d8b7a5ce8f9..3f93043f94e5c 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111104434.2222182-1-naidu.nama@oss.qualcomm.com?part=1

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

[Dmitry Baryshkov]

On Wed, Nov 11, 2026 at 04:13:53PM +0530, naidu nama wrote:
> From: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
> 
> 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.

How is going to be handled then?

> 
> 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 <saurabh.anand@oss.qualcomm.com>

Naidu, you are resending the other people patches, but the SoB is
missing.

> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 61d8b7a5ce8f..3f93043f94e5 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -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)) {

Can dp_powered_on be dropped then?

>  		dev_dbg(qmp->dev, "typec_mux_set: DP PHY is still in use, delaying switch\n");
>  		return 0;
>  	}
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

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

[naidu nama]

From: Saurabh Anand <saurabh.anand@oss.qualcomm.com>

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 <saurabh.anand@oss.qualcomm.com>
---
 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(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 47647fdec9de..6b9d9a76ba05 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -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 */
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 80d9258501b4..fc884038182f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -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);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index c0c150e2c575..169938ba515e 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -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,
-- 
2.43.0

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

[sashiko-bot]

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 <saurabh.anand@oss.qualcomm.com>

> 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 <saurabh.anand@oss.qualcomm.com>

[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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111104434.2222182-1-naidu.nama@oss.qualcomm.com?part=2

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

[Dmitry Baryshkov]

On Wed, Nov 11, 2026 at 04:13:54PM +0530, naidu nama wrote:
> From: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
> 
> 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 <saurabh.anand@oss.qualcomm.com>
> ---
>  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(-)

The same SoB comment.

-- 
With best wishes
Dmitry

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

[naidu nama]

From: Saurabh Anand <saurabh.anand@oss.qualcomm.com>

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 <saurabh.anand@oss.qualcomm.com>
---
 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(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 6b9d9a76ba05..f8a6bd7b0d10 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -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);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index fc884038182f..3edf954cc805 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -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,
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 169938ba515e..551d2c63ef8e 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -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()
-- 
2.43.0

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

[sashiko-bot]

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 <saurabh.anand@oss.qualcomm.com>

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.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 6b9d9a76ba059..f8a6bd7b0d107 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c

[ ... ]

> @@ -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) && !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.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111104434.2222182-1-naidu.nama@oss.qualcomm.com?part=3

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

[Dmitry Baryshkov]

On Wed, Nov 11, 2026 at 04:13:55PM +0530, naidu nama wrote:
> From: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
> 
> 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.

Is there a reason to assume that DP is not plugged at this point? I
guess not. But the other issue is: what if the DP gets unplugged in the
middle of link training.

> 
> Signed-off-by: Saurabh Anand <saurabh.anand@oss.qualcomm.com>

SoB

> ---
>  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(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 6b9d9a76ba05..f8a6bd7b0d10 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -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)

Can it ever be false here with your patch in place?

>  {
>  	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);
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index fc884038182f..3edf954cc805 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -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,
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 169938ba515e..551d2c63ef8e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -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()
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry