[PATCH] drm/msm/dp: don't fall back to a link that can't carry the eDP mode

[PATCH] drm/msm/dp: don't fall back to a link that can't carry the eDP mode

[Jesse Casco]

When channel equalisation fails after clock recovery succeeded,
msm_dp_ctrl_on_link() halves the lane count and trains again. Nothing
checks that the reduced link can still carry the current mode.

On the ASUS Zenbook A16, channel equalisation at 4 lanes x HBR2 fails
on some DPMS wakes. The 2-lane retry trains but can't carry the mode
(17.03 Gbps needed, 8.64 Gbps available), so the panel stays dark
while userspace sees the output as on.

For eDP, check the reduced link against the mode with the same
bandwidth check as msm_dp_display_mode_valid(), and retry at the
previous parameters when it falls short. External DisplayPort keeps
the existing fallback.

Tested with 100 DPMS off/on cycles before and after: channel
equalisation failed on 6-7% of wakes either way. Without this patch
each failure left the panel dark; with it, each one recovered on the
first retry at 4 lanes.

Signed-off-by: Jesse Casco <jesse.casco@gmail.com>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index abd3dcd..261645c 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1587,6 +1587,16 @@ static int msm_dp_ctrl_link_rate_down_shift(struct msm_dp_ctrl_private *ctrl)
 	return ret;
 }
 
+static bool msm_dp_ctrl_link_fits_mode(struct msm_dp_ctrl_private *ctrl,
+				       struct msm_dp_panel *panel,
+				       unsigned long pixel_rate)
+{
+	const struct msm_dp_link_info *params = &ctrl->link->link_params;
+
+	return (u64)pixel_rate * panel->msm_dp_mode.bpp <=
+	       (u64)params->num_lanes * params->rate * 8;
+}
+
 static int msm_dp_ctrl_link_lane_down_shift(struct msm_dp_ctrl_private *ctrl,
 					    struct msm_dp_panel *panel)
 {
@@ -2383,6 +2393,9 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 	struct msm_dp_ctrl_private *ctrl;
 	u32 rate;
 	int link_train_max_retries = 5;
+	bool is_edp = panel->connector &&
+		      panel->connector->connector_type == DRM_MODE_CONNECTOR_eDP;
+	struct msm_dp_link_info prev_params;
 	u32 const phy_cts_pixel_clk_khz = 148500;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	unsigned int training_step;
@@ -2420,6 +2433,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 		return rc;
 
 	while (--link_train_max_retries) {
+		prev_params = ctrl->link->link_params;
 		training_step = DP_TRAINING_NONE;
 		rc = msm_dp_ctrl_setup_main_link(ctrl, panel, &training_step);
 		if (rc == 0) {
@@ -2472,6 +2486,18 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 			msm_dp_ctrl_clear_training_pattern(ctrl, panel, DP_PHY_DPRX);
 		}
 
+		/*
+		 * An eDP panel has one fixed mode. A lower rate or lane count that
+		 * cannot carry it trains, but leaves the panel dark: retry at the
+		 * previous parameters instead.
+		 */
+		if (is_edp && !msm_dp_ctrl_link_fits_mode(ctrl, panel, pixel_rate)) {
+			ctrl->link->link_params = prev_params;
+			drm_dbg_dp(ctrl->drm_dev,
+				   "eDP mode needs rate=%d lanes=%d, retrying\n",
+				   prev_params.rate, prev_params.num_lanes);
+		}
+
 		rc = msm_dp_ctrl_reinitialize_mainlink(ctrl, panel);
 		if (rc) {
 			DRM_ERROR("Failed to reinitialize mainlink. rc=%d\n", rc);
-- 
2.56.0

Re: [PATCH] drm/msm/dp: don't fall back to a link that can't carry the eDP mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] drm/msm/dp: unpropagated eDP 1.4 link_info fields in
  msm_dp_ctrl_on_link() [1]
--

commit 2e586911d41c02809032afda8e106d6b81011527
Author: Jesse Casco <jesse.casco@gmail.com>

drm/msm/dp: don't fall back to a link that can't carry the eDP mode

When eDP link training downshifts to a link rate or lane count that lacks
enough bandwidth for the panel mode, this commit restores the previous
link parameters and retries training instead of training a link that
leaves the panel dark.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 47647fdec9de5..c77427aadcb46 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c

[ ... ]

> @@ -2420,6 +2433,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
>  		return rc;
>  
>  	while (--link_train_max_retries) {
> +		prev_params = ctrl->link->link_params;
>  		training_step = DP_TRAINING_NONE;
>  		rc = msm_dp_ctrl_setup_main_link(ctrl, panel, &training_step);
>  		if (rc == 0) {

[Severity: Medium]
This problem wasn't introduced by this patch, but does
msm_dp_ctrl_on_link() drop the eDP 1.4 supported_rates, rate_set, and
use_rate_set fields from panel->link_info when initializing
ctrl->link->link_params [1]?

Earlier in msm_dp_ctrl_on_link(), only rate and num_lanes are copied into
ctrl->link->link_params (and msm_dp_ctrl_link_lane_down_shift() likewise
restores only rate from panel->link_info):

drivers/gpu/drm/msm/dp/dp_ctrl.c:msm_dp_ctrl_on_link() {
    ...
	} else {
		ctrl->link->link_params.rate = rate;
		ctrl->link->link_params.num_lanes =
			panel->link_info.num_lanes;
    ...
}

In addition, msm_dp_ctrl_link_train() copies only num_lanes, rate, and
capabilities into the local link_info struct passed to
msm_dp_aux_link_configure().

Because use_rate_set and rate_set remain zero in both places,
msm_dp_aux_link_configure() never writes DP_LINK_RATE_SET on eDP 1.4+
panels, and msm_dp_ctrl_link_rate_down_shift() bypasses supported_rates[]
and fails with -EINVAL on intermediate eDP 1.4 link rates.

[1] https://sashiko.dev/#/bug/linux-bd6dd9b8-1fc3-40aa-82c1-7f5b2c6afbc1

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