[PATCH 0/4] drm/msm/dp, phy: qcom: improve DP Type-C reconnect handling

[PATCH 0/4] drm/msm/dp, phy: qcom: improve DP Type-C reconnect handling

[Saurabh Anand]

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,
skip LTTPR setup when no LTTPRs are present, 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, especially when AUX link status briefly disagrees
with Type-C cable presence during reconnect.

Signed-off-by: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
---
Saurabh Anand (4):
      phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing
      drm/msm/dp: clear force_link_train flag when link training succeeds
      drm/msm/dp: skip LTTPR init when no LTTPRs are present
      drm/msm/dp: continue link training downgrade when cable is still plugged

 drivers/gpu/drm/msm/dp/dp_ctrl.c          | 4 ++--
 drivers/gpu/drm/msm/dp/dp_ctrl.h          | 1 +
 drivers/gpu/drm/msm/dp/dp_display.c       | 7 +++++++
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 3 ++-
 4 files changed, 12 insertions(+), 3 deletions(-)
---
base-commit: e6664f2b33db9b6811eb4cec109f06cb2b4f458d
change-id: 20260824-qcom-dp-typec-reconnect-fixes-030767868837

Best regards,
--  
Saurabh Anand <saurabh.anand@oss.qualcomm.com>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Saurabh Anand]

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 c39ced168d03..f23aff79ca08 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -4842,7 +4842,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.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 1/4] 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 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for DP PHY state corruption during Type-C orientation changes.
- [High] Type-C mux state updates are silently dropped when DP PHY is active, breaking dynamic pin assignment.
- [High] Integer underflow of `usb_init_count` breaks USB PHY state tracking.
--

--- Patch [1]: [PATCH 1/4] phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing ---
commit 35456fff470c94d1e75b9f16c438e1e8cf5df40f
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 Type-C mux switch guard to check dp_init_count in
addition to dp_powered_on. This prevents a race window during DP PHY
initialization where a Type-C orientation change could corrupt the PHY state.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index c39ced168d035..f23aff79ca08f 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -4842,7 +4842,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]
This is a pre-existing issue, but does this early return cause the driver to
silently drop legitimate mux state updates?

When the Type-C framework negotiates a dynamic pin assignment and calls
qmp_combo_typec_mux_set(), returning 0 here without saving the requested
mode to qmp->qmpphy_mode or queuing it means the hardware could remain
stuck in the old lane configuration, causing subsequent DRM link
retraining to fail.

[Severity: High]
This is a pre-existing issue, but is a similar guard needed in
qmp_combo_typec_switch_set()?

The current patch fixes the race in qmp_combo_typec_mux_set(), but a
Type-C orientation change also triggers qmp_combo_typec_switch_set().
If DP is active or initializing, qmp_combo_typec_switch_set() forcefully
resets the COM block:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_typec_switch_set() {
        ...
        qmp_combo_com_exit(qmp, true);
        qmp_combo_com_init(qmp, true);
        ...
}

Does this hardware reset wipe out the DP PHY lane and link rate
configurations applied by qmp_combo_dp_power_on(), leaving the
software state (dp_powered_on) desynchronized from the hardware?

[Severity: High]
This is a pre-existing issue, but is there a risk of an integer
underflow with usb_init_count if the USB subsystem also decrements it?

When the Type-C mux switches to QMPPHY_MODE_DP_ONLY,
qmp_combo_typec_mux_set() manually decrements the counter:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_typec_mux_set() {
        ...
        if (qmp->usb_init_count)
                qmp->usb_init_count--;
        ...
}

If the USB controller later undergoes a suspend or role switch and
calls phy_exit(), qmp_combo_usb_exit() will decrement it again:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_usb_exit() {
        ...
        qmp->usb_init_count--;
        ...
}

Could this underflow usb_init_count to UINT_MAX, causing a subsequent
Type-C orientation flip to fail to power the USB PHY back on?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-qcom-dp-typec-reconnect-fixes-v1-0-2825e5bf8a96@oss.qualcomm.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 2/4] drm/msm/dp: clear force_link_train flag when link training succeeds

[Saurabh Anand]

msm_dp_display_prepare_link() sets force_link_train = true before calling
msm_dp_ctrl_on_link(). On success the flag was never cleared, so
msm_dp_ctrl_prepare_stream_on() would unconditionally trigger a second
link retrain even though the link was already trained.

Clear force_link_train on the success path so that
msm_dp_ctrl_prepare_stream_on() only retrains when the channel EQ check
fails, as intended.

Signed-off-by: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index bc646d172abe..40e19d506a45 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -649,6 +649,8 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
 		return rc;
 	}
 
+	force_link_train = false;
+
 	return msm_dp_ctrl_prepare_stream_on(dp->ctrl, dp->panel, force_link_train);
 }
 

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 2/4] drm/msm/dp: clear force_link_train flag when link training succeeds

[sashiko-bot]

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

New issues:
- [Low] Dead store of `force_link_train` introduced by the patch.

Pre-existing issues:
- [High] Failure in `pm_runtime_resume_and_get()` leads to a PM runtime usage count underflow.
- [High] Uninitialized stack memory read in `msm_dp_ctrl_channel_eq_ok()`.
--

--- Patch [2]: [PATCH 2/4] drm/msm/dp: clear force_link_train flag when link training succeeds ---
commit 03a67937ce09f7bc428c5c9c18503745e41c62f6
Author: Saurabh Anand <saurabh.anand@oss.qualcomm.com>

drm/msm/dp: clear force_link_train flag when link training succeeds

msm_dp_display_prepare_link() sets force_link_train = true before calling
msm_dp_ctrl_on_link(). On success the flag was never cleared, so
msm_dp_ctrl_prepare_stream_on() would unconditionally trigger a second
link retrain even though the link was already trained. Clear
force_link_train on the success path so that
msm_dp_ctrl_prepare_stream_on() only retrains when the channel EQ check
fails, as intended.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index bc646d172abe0..40e19d506a450 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -649,6 +649,8 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)

[Severity: High]
This isn't a bug introduced by this patch, but does the pm_runtime error
handling cause a usage count underflow?

If pm_runtime_resume_and_get() fails earlier in msm_dp_display_prepare_link(),
it returns an error without acquiring a PM runtime reference:

	rc = pm_runtime_resume_and_get(&msm_dp_display->pdev->dev);
	if (rc) {
		DRM_ERROR("failed to pm_runtime_resume\n");
		return rc;
	}

Since the DRM core atomic_enable callback returns void, the framework
considers the device enabled and will later call the atomic_post_disable
hook. This routes to msm_dp_display_unprepare(), which unconditionally
drops the reference:

	pm_runtime_put_sync(&msm_dp_display->pdev->dev);

Will this drop a reference that was never acquired and lead to possible
device lockups?

>  		return rc;
>  	}
>  
> +	force_link_train = false;
> +

[Severity: Low]
Does this unconditionally overwrite the previous assignment?

Earlier in msm_dp_display_prepare_link(), force_link_train is set to true:

	if (!msm_dp_display->power_on) {
		msm_dp_display_host_phy_init(dp);
		force_link_train = true;
		dp->ctrl->plugged = dp->plugged;
	}

By setting it to false here unconditionally, does the prior assignment become
dead code? Could this parameter just be removed or passed as false directly?

>  	return msm_dp_ctrl_prepare_stream_on(dp->ctrl, dp->panel, force_link_train);

[Severity: High]
This is a pre-existing issue, but by clearing force_link_train on the
success path here, this patch makes msm_dp_ctrl_prepare_stream_on() evaluate
the channel EQ check. Does this lead to an uninitialized stack memory read?

When force_link_train is false, it eventually calls
msm_dp_ctrl_channel_eq_ok() which passes an uninitialized stack buffer to
drm_dp_dpcd_read_link_status():

	u8 link_status[DP_LINK_STATUS_SIZE];
	...
	drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
	return drm_dp_channel_eq_ok(link_status, num_lanes);

If drm_dp_dpcd_read_link_status() fails (e.g., if the AUX channel fails), it
returns an error without initializing the buffer. Its return value is ignored,
and the uninitialized stack buffer is then read by drm_dp_channel_eq_ok(),
which might cause unpredictable behavior depending on stack garbage.

Can we check the return value of drm_dp_dpcd_read_link_status() before using
the buffer?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-qcom-dp-typec-reconnect-fixes-v1-0-2825e5bf8a96@oss.qualcomm.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 2/4] drm/msm/dp: clear force_link_train flag when link training succeeds

[Konrad Dybcio]

On 8/24/26 1:28 PM, Saurabh Anand wrote:
> msm_dp_display_prepare_link() sets force_link_train = true before calling
> msm_dp_ctrl_on_link(). On success the flag was never cleared, so
> msm_dp_ctrl_prepare_stream_on() would unconditionally trigger a second
> link retrain even though the link was already trained.
> 
> Clear force_link_train on the success path so that
> msm_dp_ctrl_prepare_stream_on() only retrains when the channel EQ check
> fails, as intended.
> 
> Signed-off-by: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index bc646d172abe..40e19d506a45 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -649,6 +649,8 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
>  		return rc;
>  	}
>  
> +	force_link_train = false;
> +
>  	return msm_dp_ctrl_prepare_stream_on(dp->ctrl, dp->panel, force_link_train);

This makes the argument of this function useless

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 3/4] drm/msm/dp: skip LTTPR init when no LTTPRs are present

[Saurabh Anand]

drm_dp_lttpr_count() returns 0 when no LTTPRs are detected and a
negative value on error. The previous code passed the result directly
to drm_dp_lttpr_init() without checking, which would call into the
LTTPR transparency-mode setup with a zero or negative repeater count.

Add an early return for lttpr_count <= 0 to skip the init entirely
when there are no repeaters in the link, matching the expected usage
of drm_dp_lttpr_init().

Signed-off-by: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 40e19d506a45..1f60c78a463d 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -253,6 +253,10 @@ static int msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8 *dpcd
 		return 0;
 
 	lttpr_count = drm_dp_lttpr_count(dp->link->lttpr_common_caps);
+
+	if (lttpr_count <= 0)
+		return 0;
+
 	rc = drm_dp_lttpr_init(dp->aux, lttpr_count);
 	if (rc) {
 		DRM_ERROR("failed to set LTTPRs transparency mode, rc=%d\n", rc);

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 3/4] drm/msm/dp: skip LTTPR init when no LTTPRs are present

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Skipping `drm_dp_lttpr_init()` for negative `lttpr_count` bypasses the required LTTPR transparent mode fallback and is based on a flawed premise.
--

--- Patch [3]: [PATCH 3/4] drm/msm/dp: skip LTTPR init when no LTTPRs are present ---
commit 586e314c50820df2341a91e8a30d3963d3346281
Author: Saurabh Anand <saurabh.anand@oss.qualcomm.com>

drm/msm/dp: skip LTTPR init when no LTTPRs are present

This patch adds an early return when drm_dp_lttpr_count() returns a zero or
negative value. The goal is to skip LTTPR initialization when no repeaters
are detected or an error occurs reading the count.

> drm_dp_lttpr_count() returns 0 when no LTTPRs are detected and a
> negative value on error. The previous code passed the result directly
> to drm_dp_lttpr_init() without checking, which would call into the
> LTTPR transparency-mode setup with a zero or negative repeater count.

Is this description of drm_dp_lttpr_init() accurate?

Looking at drm_dp_lttpr_init(), it safely returns 0 on its own if
lttpr_count is zero:

int drm_dp_lttpr_init(struct drm_dp_aux *aux, int lttpr_count)
{
	int ret;

	if (!lttpr_count)
		return 0;
	...
}

It appears it already avoids calling into the transparency-mode setup
when the count is zero.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -253,6 +253,10 @@ static int msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8 *dpcd
>  		return 0;
>  
>  	lttpr_count = drm_dp_lttpr_count(dp->link->lttpr_common_caps);
> +
> +	if (lttpr_count <= 0)
> +		return 0;
> +

[Severity: Medium]
Does skipping drm_dp_lttpr_init() for negative counts bypass the required
transparent mode fallback for non-compliant repeaters?

Looking at drm_dp_lttpr_init() in drivers/gpu/drm/display/drm_dp_helper.c,
it relies on receiving negative counts to trigger its fallback logic:

int drm_dp_lttpr_init(struct drm_dp_aux *aux, int lttpr_count)
{
	...
	ret = drm_dp_lttpr_set_transparent_mode(aux, true);
	if (ret)
		return ret;

	if (lttpr_count < 0)
		return -ENODEV;
	...
}

When lttpr_count is negative (e.g., -ERANGE or -EINVAL), the early return in
msm_dp_display_lttpr_init() causes the fallback to force transparent mode to
be entirely skipped. Could this cause link training to fail on setups with
non-compliant LTTPRs where the repeater count is invalid?

>  	rc = drm_dp_lttpr_init(dp->aux, lttpr_count);
>  	if (rc) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-qcom-dp-typec-reconnect-fixes-v1-0-2825e5bf8a96@oss.qualcomm.com?part=3

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 3/4] drm/msm/dp: skip LTTPR init when no LTTPRs are present

[Konrad Dybcio]

On 8/24/26 1:28 PM, Saurabh Anand wrote:
> drm_dp_lttpr_count() returns 0 when no LTTPRs are detected and a
> negative value on error. The previous code passed the result directly
> to drm_dp_lttpr_init() without checking, which would call into the
> LTTPR transparency-mode setup with a zero or negative repeater count.
> 
> Add an early return for lttpr_count <= 0 to skip the init entirely
> when there are no repeaters in the link, matching the expected usage
> of drm_dp_lttpr_init().

Reading the body of that function, I'm not sure your statement is true

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Saurabh Anand]

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.

The plugged state is snapshotted from dp_display into msm_dp_ctrl just
before msm_dp_ctrl_on_link() is called, so the retry loop has an accurate
view of cable presence at the time link training started.

Signed-off-by: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 4 ++--
 drivers/gpu/drm/msm/dp/dp_ctrl.h    | 1 +
 drivers/gpu/drm/msm/dp/dp_display.c | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 59070f399e2d..e8fd8f4c75fe 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2379,7 +2379,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) && !msm_dp_ctrl->plugged)
 				break;
 
 			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
@@ -2404,7 +2404,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) && !msm_dp_ctrl->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 5902cf7e746a..95533b0b6297 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -12,6 +12,7 @@
 
 struct msm_dp_ctrl {
 	bool wide_bus_en;
+	bool plugged;
 };
 
 struct phy;
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 1f60c78a463d..93a37a44273f 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -644,6 +644,7 @@ 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;
+		dp->ctrl->plugged = dp->plugged;
 	}
 
 	rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel);

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 4/4] 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 issue(s) to consider:

New issues:
- [Medium] Lockless read of dp->plugged in msm_dp_display_prepare_link() causes a data race.

Pre-existing issues:
- [High] The return value of drm_dp_dpcd_read_link_status() is ignored, causing uninitialized stack memory to be used if the DPCD read fails.
--

--- Patch [4]: [PATCH 4/4] drm/msm/dp: continue link training downgrade when cable is still plugged ---
commit ad6d2e949e22a448345ebdb67f36f42020cff884
Author: Saurabh Anand <saurabh.anand@oss.qualcomm.com>

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

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.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 59070f399e2d3..e8fd8f4c75fec 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -2379,7 +2379,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) && !msm_dp_ctrl->plugged)
>  				break;
>  
>  			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);

[Severity: High]
This is a pre-existing issue, but by allowing the loop to continue when AUX
is disconnected, is it possible we'll perform a DPCD read that fails, leaving
the link_status stack array uninitialized?

If drm_dp_dpcd_read_link_status() fails (which is likely if the link is
disconnected), the error return value is ignored. Uninitialized stack memory is
then evaluated by msm_dp_ctrl_clock_recovery_any_ok() below to make hardware
configuration decisions.

Does this also affect the DP_TRAINING_2 case below, as well as
msm_dp_ctrl_channel_eq_ok()?

[ ... ]

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 1f60c78a463dd..93a37a44273f0 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -644,6 +644,7 @@ 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;
> +		dp->ctrl->plugged = dp->plugged;

[Severity: Medium]
Is it safe to read dp->plugged here without holding the dp->plugged_lock mutex?

This assignment runs in msm_dp_display_prepare_link() during atomic commit,
which could run concurrently with HPD plug/unplug events that modify
dp->plugged.

Could this lead to a data race where a stale value is read?

>  	}
>  
>  	rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-qcom-dp-typec-reconnect-fixes-v1-0-2825e5bf8a96@oss.qualcomm.com?part=4

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy