[PATCH 1/3] drm/msm/dp: don't gate link-training fallback on the controller's HPD block

[Jean-Francois Bobier]

msm_dp_ctrl_setup_main_link()'s retry loop is what implements DP link
fallback: on a training failure it shifts the link rate down HBR3 -> HBR2
-> HBR -> RBR and then, if clock recovery succeeded on at least half the
lanes, reduces the lane count. Both failure branches guard that with

	if (!msm_dp_aux_is_link_connected(ctrl->aux))
		break;

which reads REG_DP_DP_HPD_INT_STATUS, i.e. the DP controller's own HPD
block. That is only meaningful when the block is wired to a real HPD
signal.

On a USB-C port it is not. HPD arrives out of band: the Type-C port
driver owns it and hands it over through a drm_dp_hpd_bridge, and the
controller's HPD register reads DISCONNECTED for the entire time a sink
is attached. msm_dp_bridge_hpd_notify() already shows this, logging the
register beside the bridge's own verdict -- on sm8250 (OnePlus 8T) every
attach reports

	msm_dp_bridge_hpd_notify: type=10 link hpd_link_status=0x0, status=1

So the guard is always true, the loop always breaks on the first failed
attempt, and the rate/lane fallback is dead on every Type-C DP board.
msm_dp_ctrl_setup_main_link() then returns -ECONNRESET and the mode set
fails outright.

The cost is real. With a 4K sink attached directly (4 DP lanes, no USB3),
a board that can only carry two DP lanes to the connector trains lanes 0
and 1 and never lanes 2 and 3:

	v_level 3: DPCD 0x202 = 0x11 (lanes 0,1 CR_DONE)
	           DPCD 0x203 = 0x00 (lanes 2,3 nothing)
	link training #1 on phy 0 failed. ret=-11
	Failed link training (rc=-104)

A fallback to two lanes would have trained -- clock recovery was already
good on exactly the lanes that remain, so
msm_dp_ctrl_clock_recovery_any_ok() would have passed -- but it is never
reached. The same sink works through a dock, because the Type-C alternate
mode negotiates two DP lanes up front and the first attempt succeeds.

Use the DPCD read the loop already performs as the liveness test instead.
It expresses the intent directly -- a sink that has gone away cannot
answer AUX -- and it carries no assumption about where HPD comes from.
drm_dp_dpcd_read_link_status() returns 0 on success and a negative errno
on failure.

Signed-off-by: Jean-Francois Bobier <jean-francois.bobier@xxxxxxxxxxx>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index e1614bc49..d6219d87d 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2426,12 +2426,22 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
 			/* training completed successfully */
 			break;
 		} else if (training_step == DP_TRAINING_1) {
-			/* link train_1 failed */
-			if (!msm_dp_aux_is_link_connected(ctrl->aux))
+			/*
+			 * link train_1 failed
+			 *
+			 * Use the DPCD read itself to decide whether the sink
+			 * is still there. msm_dp_aux_is_link_connected() reads
+			 * the DP controller's own HPD block, which is only
+			 * meaningful when that block sees a real HPD signal --
+			 * on a Type-C port HPD arrives out of band through a
+			 * drm_dp_hpd_bridge and the register reads
+			 * DISCONNECTED the whole time the sink is attached.
+			 * Gating the retry on it therefore skipped the entire
+			 * rate/lane fallback on those boards.
+			 */
+			if (drm_dp_dpcd_read_link_status(ctrl->aux, link_status) < 0)
 				break;
 
-			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
-
 			rc = msm_dp_ctrl_link_rate_down_shift(ctrl);
 			if (rc < 0) { /* already in RBR = 1.6G */
 				if (msm_dp_ctrl_clock_recovery_any_ok(link_status,
@@ -2451,12 +2461,10 @@ 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))
+			/* link train_2 failed; same reasoning as above */
+			if (drm_dp_dpcd_read_link_status(ctrl->aux, link_status) < 0)
 				break;
 
-			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
-
 			if (!drm_dp_clock_recovery_ok(link_status,
 					ctrl->link->link_params.num_lanes))
 				rc = msm_dp_ctrl_link_rate_down_shift(ctrl);
-- 
2.55.0