drm/msm/dp: Introduce link training per-segment for LTTPRs

drm/msm/dp: Introduce link training per-segment for LTTPRs

[Aleksandrs Vinarskis]

Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
to non-transparent mode to enable video output on X1E-based devices
that come with LTTPR on the motherboards. However, video would not work
if additional LTTPR(s) are present between sink and source, which is
the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).

First, take into account LTTPR capabilities when computing max link
rate, number of lanes. Take into account previous discussion on the
lists - exit early if reading DPCD caps failed. This also fixes
"*ERROR* panel edid read failed" on some monitors which seems to be
caused by msm_dp_panel_read_sink_caps running before LTTPR(s) are
initialized.

Finally, implement link training per-segment. Pass lttpr_count to all
required helpers.
This seems to also partially improve UI (Wayland) hanging when
changing external display's link parameters (resolution, framerate):
* Prior to this series, via direct USB Type-C to display connection,
  attempt to change resolution or framerate hangs the UI, setting does
  not stick. Some back and forth replugging finally sets desired
  parameters.
* With this series, via direct USB Type-C to display connection,
  changing parameters works most of the time, without UI freezing. Via
  docking station/multiple LTTPRs the setting again does not stick.
* On Xorg changing link paramaters works in all combinations.

These appear to be mainlink initialization related, as in all cases LT
passes successfully.

Test matrix:
* Dell XPS 9345, Ubuntu 24.10, Gnome 47, Wayland (myself)
	* Left USB Type-C, Right USB Type-C
	* Direct monitor connection, Dell WD19TB, Dell WD22TB4, USB
          Type-C to HDMI dongle, USB Type-C to DP dongle
	* Dell AW3423DWF, Samsung LS24A600, dual Samsung LS24A600 (one
	  monitor per USB Type-C connector)
* Dell XPS 9345, Ubuntu 24.10, Gnome 47, Wayland (myself)
	* Left USB Type-C, Right USB Type-C
	* Direct monitor connection
	* Samsung S34BG85 (USB Type-C), Dell U2725QE (universal
          Thunderbolt/USB Type-C, probes with an LTTPR when in USB
          Type-C/DP Alt mode)
* Dell XPS 9345, Debian trixie/sid, Gnome 48, Wayland (Stefan Schmidt)
        * Left USB Type-C, Right USB Type-C
        * Dell WD15 Dock with DisplayPort connected
        * Dell HD22Q dock with HDMI connected
        * USB Type-C to HDMI dongle
        * Dell U3417W

In both cases, "Thunderbot Support"/"USB4 PCIE Tunneling" was disabled
in UEFI to force universal Thunderbolt/USB Type-C devices to work in
DP Alt mode.
In both cases laptops had HBR3 patches applied [1], resulting in
maximum successful link at 3440x1440@100hz and 4k@60hz respectively.
When using Dell WD22TB4/U2725QE, USB Type-C pin assigment D got enabled
and USB3.0 devices were working in parallel to video ouput.

Known issues:
* As mentioned above, it appears that on Gnome+Wayland framerate and
  resolution parameter adjustment is not stable.

Due to lack of access to the official DisplayPort specfication, changes
were primarily inspired by/reverse engineered from Intel's i915 driver.

[1] https://lore.kernel.org/all/20250226231436.16138-2-alex.vinarskis@gmail.com/

Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>

---

Changes in v3:
- Split 1st patch into 3
- Simplified handling of max_lttpr_lanes/max_lttpr_rate
- Moved lttpr_common_caps to msm_dp_link (not msm_dp_panel, as LTTPRs
  are link related, not panel related)
- Picked Stefan's T-b tag (last patch only, as 1st one is getting split)
- Droped Abel's R-b tags from 1st patch that got split due to high diff
- Fixed alignment issues, initialization of variables, debug prints
- Moved lttpr_count to avoid ugly pointer
- Link to v2: https://lore.kernel.org/all/20250311234109.136510-1-alex.vinarskis@gmail.com/

Changes in v2:
- Picked up Abel's R-b tags
- Fixed typo as per Abel, fixed readability as per Johan
- Updated cover and commit message on mailink issue which appears to be 
  specific to Gnome+Wayland. No problems on Xorg.
- Link to v1: https://lore.kernel.org/all/20250310211039.29843-1-alex.vinarskis@gmail.com/

Aleksandrs Vinarskis (4):
  drm/msm/dp: Fix support of LTTPR initialization
  drm/msm/dp: Account for LTTPRs capabilities
  drm/msm/dp: Prepare for link training per-segment for LTTPRs
  drm/msm/dp: Introduce link training per-segment for LTTPRs

 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 126 ++++++++++++++++++++--------
 drivers/gpu/drm/msm/dp/dp_display.c |  27 ++++--
 drivers/gpu/drm/msm/dp/dp_link.h    |   4 +
 drivers/gpu/drm/msm/dp/dp_panel.c   |  12 ++-
 4 files changed, 122 insertions(+), 47 deletions(-)

-- 
2.45.2

[PATCH v3 1/4] drm/msm/dp: Fix support of LTTPR initialization

[Aleksandrs Vinarskis]

Initialize LTTPR before msm_dp_panel_read_sink_caps, as DPTX shall
(re)read DPRX caps after LTTPR detection, as required by DP 2.1,
Section 3.6.7.6.1.

Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index bbc47d86ae9e..fc07cce68382 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -367,12 +367,12 @@ static int msm_dp_display_send_hpd_notification(struct msm_dp_display_private *d
 	return 0;
 }
 
-static void msm_dp_display_lttpr_init(struct msm_dp_display_private *dp)
+static void msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8 *dpcd)
 {
 	u8 lttpr_caps[DP_LTTPR_COMMON_CAP_SIZE];
 	int rc;
 
-	if (drm_dp_read_lttpr_common_caps(dp->aux, dp->panel->dpcd, lttpr_caps))
+	if (drm_dp_read_lttpr_common_caps(dp->aux, dpcd, lttpr_caps))
 		return;
 
 	rc = drm_dp_lttpr_init(dp->aux, drm_dp_lttpr_count(lttpr_caps));
@@ -385,12 +385,17 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
 	struct drm_connector *connector = dp->msm_dp_display.connector;
 	const struct drm_display_info *info = &connector->display_info;
 	int rc = 0;
+	u8 dpcd[DP_RECEIVER_CAP_SIZE];
 
-	rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
+	rc = drm_dp_read_dpcd_caps(dp->aux, dpcd);
 	if (rc)
 		goto end;
 
-	msm_dp_display_lttpr_init(dp);
+	msm_dp_display_lttpr_init(dp, dpcd);
+
+	rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
+	if (rc)
+		goto end;
 
 	msm_dp_link_process_request(dp->link);
 
-- 
2.45.2

[PATCH v3 2/4] drm/msm/dp: Account for LTTPRs capabilities

[Aleksandrs Vinarskis]

Take into account LTTPR capabilities when selecting maximum allowed
link rate, number of data lines.

Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c |  5 ++---
 drivers/gpu/drm/msm/dp/dp_link.h    |  3 +++
 drivers/gpu/drm/msm/dp/dp_panel.c   | 12 +++++++++++-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index fc07cce68382..5c57c1d7ac60 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -369,13 +369,12 @@ static int msm_dp_display_send_hpd_notification(struct msm_dp_display_private *d
 
 static void msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8 *dpcd)
 {
-	u8 lttpr_caps[DP_LTTPR_COMMON_CAP_SIZE];
 	int rc;
 
-	if (drm_dp_read_lttpr_common_caps(dp->aux, dpcd, lttpr_caps))
+	if (drm_dp_read_lttpr_common_caps(dp->aux, dpcd, dp->link->lttpr_common_caps))
 		return;
 
-	rc = drm_dp_lttpr_init(dp->aux, drm_dp_lttpr_count(lttpr_caps));
+	rc = drm_dp_lttpr_init(dp->aux, drm_dp_lttpr_count(dp->link->lttpr_common_caps));
 	if (rc)
 		DRM_ERROR("failed to set LTTPRs transparency mode, rc=%d\n", rc);
 }
diff --git a/drivers/gpu/drm/msm/dp/dp_link.h b/drivers/gpu/drm/msm/dp/dp_link.h
index 8db5d5698a97..c47d75cfc720 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.h
+++ b/drivers/gpu/drm/msm/dp/dp_link.h
@@ -7,6 +7,7 @@
 #define _DP_LINK_H_
 
 #include "dp_aux.h"
+#include <drm/display/drm_dp_helper.h>
 
 #define DS_PORT_STATUS_CHANGED 0x200
 #define DP_TEST_BIT_DEPTH_UNKNOWN 0xFFFFFFFF
@@ -60,6 +61,8 @@ struct msm_dp_link_phy_params {
 };
 
 struct msm_dp_link {
+	u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE];
+
 	u32 sink_request;
 	u32 test_response;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c b/drivers/gpu/drm/msm/dp/dp_panel.c
index 92415bf8aa16..4e8ab75c771b 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -47,7 +47,7 @@ static void msm_dp_panel_read_psr_cap(struct msm_dp_panel_private *panel)
 
 static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel)
 {
-	int rc;
+	int rc, max_lttpr_lanes, max_lttpr_rate;
 	struct msm_dp_panel_private *panel;
 	struct msm_dp_link_info *link_info;
 	u8 *dpcd, major, minor;
@@ -75,6 +75,16 @@ static int msm_dp_panel_read_dpcd(struct msm_dp_panel *msm_dp_panel)
 	if (link_info->rate > msm_dp_panel->max_dp_link_rate)
 		link_info->rate = msm_dp_panel->max_dp_link_rate;
 
+	/* Limit data lanes from LTTPR capabilities, if any */
+	max_lttpr_lanes = drm_dp_lttpr_max_lane_count(panel->link->lttpr_common_caps);
+	if (max_lttpr_lanes && max_lttpr_lanes < link_info->num_lanes)
+		link_info->num_lanes = max_lttpr_lanes;
+
+	/* Limit link rate from LTTPR capabilities, if any */
+	max_lttpr_rate = drm_dp_lttpr_max_link_rate(panel->link->lttpr_common_caps);
+	if (max_lttpr_rate && max_lttpr_rate < link_info->rate)
+		link_info->rate = max_lttpr_rate;
+
 	drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
 	drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
 	drm_dbg_dp(panel->drm_dev, "lane_count=%d\n", link_info->num_lanes);
-- 
2.45.2

[PATCH v3 3/4] drm/msm/dp: Prepare for link training per-segment for LTTPRs

[Aleksandrs Vinarskis]

Per-segment link training requires knowing the number of LTTPRs
(if any) present. Store the count during LTTPRs' initialization.

Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
---
 drivers/gpu/drm/msm/dp/dp_display.c | 17 +++++++++++------
 drivers/gpu/drm/msm/dp/dp_link.h    |  1 +
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 5c57c1d7ac60..ab8c1f19dcb4 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -367,16 +367,21 @@ static int msm_dp_display_send_hpd_notification(struct msm_dp_display_private *d
 	return 0;
 }
 
-static void msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8 *dpcd)
+static int msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8 *dpcd)
 {
-	int rc;
+	int rc, lttpr_count;
 
 	if (drm_dp_read_lttpr_common_caps(dp->aux, dpcd, dp->link->lttpr_common_caps))
-		return;
+		return 0;
 
-	rc = drm_dp_lttpr_init(dp->aux, drm_dp_lttpr_count(dp->link->lttpr_common_caps));
-	if (rc)
+	lttpr_count = drm_dp_lttpr_count(dp->link->lttpr_common_caps);
+	rc = drm_dp_lttpr_init(dp->aux, lttpr_count);
+	if (rc) {
 		DRM_ERROR("failed to set LTTPRs transparency mode, rc=%d\n", rc);
+		return 0;
+	}
+
+	return lttpr_count;
 }
 
 static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
@@ -390,7 +395,7 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
 	if (rc)
 		goto end;
 
-	msm_dp_display_lttpr_init(dp, dpcd);
+	dp->link->lttpr_count = msm_dp_display_lttpr_init(dp, dpcd);
 
 	rc = msm_dp_panel_read_sink_caps(dp->panel, connector);
 	if (rc)
diff --git a/drivers/gpu/drm/msm/dp/dp_link.h b/drivers/gpu/drm/msm/dp/dp_link.h
index c47d75cfc720..ba47c6d19fbf 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.h
+++ b/drivers/gpu/drm/msm/dp/dp_link.h
@@ -62,6 +62,7 @@ struct msm_dp_link_phy_params {
 
 struct msm_dp_link {
 	u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE];
+	int lttpr_count;
 
 	u32 sink_request;
 	u32 test_response;
-- 
2.45.2

[PATCH v3 4/4] drm/msm/dp: Introduce link training per-segment for LTTPRs

[Aleksandrs Vinarskis]

DisplayPort requires per-segment link training when LTTPR are switched
to non-transparent mode, starting with LTTPR closest to the source.
Only when each segment is trained individually, source can link train
to sink.

Implement per-segment link traning when LTTPR(s) are detected, to
support external docking stations. On higher level, changes are:

* Pass phy being trained down to all required helpers
* Run CR, EQ link training per phy
* Set voltage swing, pre-emphasis levels per phy

This ensures successful link training both when connected directly to
the monitor (single LTTPR onboard most X1E laptops) and via the docking
station (at least two LTTPRs).

Tested-by: Stefan Schmidt <stefan.schmidt@linaro.org>
Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
Reviewed-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 126 ++++++++++++++++++++++---------
 1 file changed, 89 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 69a26bb5fabd..a50bfafbb4ea 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1034,10 +1034,12 @@ static int msm_dp_ctrl_set_vx_px(struct msm_dp_ctrl_private *ctrl,
 	return 0;
 }
 
-static int msm_dp_ctrl_update_vx_px(struct msm_dp_ctrl_private *ctrl)
+static int msm_dp_ctrl_update_phy_vx_px(struct msm_dp_ctrl_private *ctrl,
+					enum drm_dp_phy dp_phy)
 {
 	struct msm_dp_link *link = ctrl->link;
-	int ret = 0, lane, lane_cnt;
+	int lane, lane_cnt, reg;
+	int ret = 0;
 	u8 buf[4];
 	u32 max_level_reached = 0;
 	u32 voltage_swing_level = link->phy_params.v_level;
@@ -1075,8 +1077,13 @@ static int msm_dp_ctrl_update_vx_px(struct msm_dp_ctrl_private *ctrl)
 
 	drm_dbg_dp(ctrl->drm_dev, "sink: p|v=0x%x\n",
 			voltage_swing_level | pre_emphasis_level);
-	ret = drm_dp_dpcd_write(ctrl->aux, DP_TRAINING_LANE0_SET,
-					buf, lane_cnt);
+
+	if (dp_phy == DP_PHY_DPRX)
+		reg = DP_TRAINING_LANE0_SET;
+	else
+		reg = DP_TRAINING_LANE0_SET_PHY_REPEATER(dp_phy);
+
+	ret = drm_dp_dpcd_write(ctrl->aux, reg, buf, lane_cnt);
 	if (ret == lane_cnt)
 		ret = 0;
 
@@ -1084,9 +1091,10 @@ static int msm_dp_ctrl_update_vx_px(struct msm_dp_ctrl_private *ctrl)
 }
 
 static bool msm_dp_ctrl_train_pattern_set(struct msm_dp_ctrl_private *ctrl,
-		u8 pattern)
+		u8 pattern, enum drm_dp_phy dp_phy)
 {
 	u8 buf;
+	int reg;
 	int ret = 0;
 
 	drm_dbg_dp(ctrl->drm_dev, "sink: pattern=%x\n", pattern);
@@ -1096,17 +1104,26 @@ static bool msm_dp_ctrl_train_pattern_set(struct msm_dp_ctrl_private *ctrl,
 	if (pattern && pattern != DP_TRAINING_PATTERN_4)
 		buf |= DP_LINK_SCRAMBLING_DISABLE;
 
-	ret = drm_dp_dpcd_writeb(ctrl->aux, DP_TRAINING_PATTERN_SET, buf);
+	if (dp_phy == DP_PHY_DPRX)
+		reg = DP_TRAINING_PATTERN_SET;
+	else
+		reg = DP_TRAINING_PATTERN_SET_PHY_REPEATER(dp_phy);
+
+	ret = drm_dp_dpcd_writeb(ctrl->aux, reg, buf);
 	return ret == 1;
 }
 
 static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
-			int *training_step)
+			int *training_step, enum drm_dp_phy dp_phy)
 {
+	int delay_us;
 	int tries, old_v_level, ret = 0;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 	int const maximum_retries = 4;
 
+	delay_us = drm_dp_read_clock_recovery_delay(ctrl->aux,
+						    ctrl->panel->dpcd, dp_phy, false);
+
 	msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, 0);
 
 	*training_step = DP_TRAINING_1;
@@ -1115,18 +1132,19 @@ static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
 	if (ret)
 		return ret;
 	msm_dp_ctrl_train_pattern_set(ctrl, DP_TRAINING_PATTERN_1 |
-		DP_LINK_SCRAMBLING_DISABLE);
+		DP_LINK_SCRAMBLING_DISABLE, dp_phy);
 
-	ret = msm_dp_ctrl_update_vx_px(ctrl);
+	msm_dp_link_reset_phy_params_vx_px(ctrl->link);
+	ret = msm_dp_ctrl_update_phy_vx_px(ctrl, dp_phy);
 	if (ret)
 		return ret;
 
 	tries = 0;
 	old_v_level = ctrl->link->phy_params.v_level;
 	for (tries = 0; tries < maximum_retries; tries++) {
-		drm_dp_link_train_clock_recovery_delay(ctrl->aux, ctrl->panel->dpcd);
+		fsleep(delay_us);
 
-		ret = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
+		ret = drm_dp_dpcd_read_phy_link_status(ctrl->aux, dp_phy, link_status);
 		if (ret)
 			return ret;
 
@@ -1147,7 +1165,7 @@ static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
 		}
 
 		msm_dp_link_adjust_levels(ctrl->link, link_status);
-		ret = msm_dp_ctrl_update_vx_px(ctrl);
+		ret = msm_dp_ctrl_update_phy_vx_px(ctrl, dp_phy);
 		if (ret)
 			return ret;
 	}
@@ -1199,21 +1217,31 @@ static int msm_dp_ctrl_link_lane_down_shift(struct msm_dp_ctrl_private *ctrl)
 	return 0;
 }
 
-static void msm_dp_ctrl_clear_training_pattern(struct msm_dp_ctrl_private *ctrl)
+static void msm_dp_ctrl_clear_training_pattern(struct msm_dp_ctrl_private *ctrl,
+					       enum drm_dp_phy dp_phy)
 {
-	msm_dp_ctrl_train_pattern_set(ctrl, DP_TRAINING_PATTERN_DISABLE);
-	drm_dp_link_train_channel_eq_delay(ctrl->aux, ctrl->panel->dpcd);
+	int delay_us;
+
+	msm_dp_ctrl_train_pattern_set(ctrl, DP_TRAINING_PATTERN_DISABLE, dp_phy);
+
+	delay_us = drm_dp_read_channel_eq_delay(ctrl->aux,
+						ctrl->panel->dpcd, dp_phy, false);
+	fsleep(delay_us);
 }
 
 static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
-			int *training_step)
+			int *training_step, enum drm_dp_phy dp_phy)
 {
+	int delay_us;
 	int tries = 0, ret = 0;
 	u8 pattern;
 	u32 state_ctrl_bit;
 	int const maximum_retries = 5;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
+	delay_us = drm_dp_read_channel_eq_delay(ctrl->aux,
+						ctrl->panel->dpcd, dp_phy, false);
+
 	msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, 0);
 
 	*training_step = DP_TRAINING_2;
@@ -1233,12 +1261,12 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
 	if (ret)
 		return ret;
 
-	msm_dp_ctrl_train_pattern_set(ctrl, pattern);
+	msm_dp_ctrl_train_pattern_set(ctrl, pattern, dp_phy);
 
 	for (tries = 0; tries <= maximum_retries; tries++) {
-		drm_dp_link_train_channel_eq_delay(ctrl->aux, ctrl->panel->dpcd);
+		fsleep(delay_us);
 
-		ret = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
+		ret = drm_dp_dpcd_read_phy_link_status(ctrl->aux, dp_phy, link_status);
 		if (ret)
 			return ret;
 
@@ -1248,7 +1276,7 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
 		}
 
 		msm_dp_link_adjust_levels(ctrl->link, link_status);
-		ret = msm_dp_ctrl_update_vx_px(ctrl);
+		ret = msm_dp_ctrl_update_phy_vx_px(ctrl, dp_phy);
 		if (ret)
 			return ret;
 
@@ -1257,9 +1285,32 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
 	return -ETIMEDOUT;
 }
 
+static int msm_dp_ctrl_link_train_1_2(struct msm_dp_ctrl_private *ctrl,
+				      int *training_step, enum drm_dp_phy dp_phy)
+{
+	int ret;
+
+	ret = msm_dp_ctrl_link_train_1(ctrl, training_step, dp_phy);
+	if (ret) {
+		DRM_ERROR("link training #1 on phy %d failed. ret=%d\n", dp_phy, ret);
+		return ret;
+	}
+	drm_dbg_dp(ctrl->drm_dev, "link training #1 on phy %d successful\n", dp_phy);
+
+	ret = msm_dp_ctrl_link_train_2(ctrl, training_step, dp_phy);
+	if (ret) {
+		DRM_ERROR("link training #2 on phy %d failed. ret=%d\n", dp_phy, ret);
+		return ret;
+	}
+	drm_dbg_dp(ctrl->drm_dev, "link training #2 on phy %d successful\n", dp_phy);
+
+	return 0;
+}
+
 static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
 			int *training_step)
 {
+	int i;
 	int ret = 0;
 	const u8 *dpcd = ctrl->panel->dpcd;
 	u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
@@ -1272,8 +1323,6 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
 	link_info.rate = ctrl->link->link_params.rate;
 	link_info.capabilities = DP_LINK_CAP_ENHANCED_FRAMING;
 
-	msm_dp_link_reset_phy_params_vx_px(ctrl->link);
-
 	msm_dp_aux_link_configure(ctrl->aux, &link_info);
 
 	if (drm_dp_max_downspread(dpcd))
@@ -1288,24 +1337,27 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
 				&assr, 1);
 	}
 
-	ret = msm_dp_ctrl_link_train_1(ctrl, training_step);
+	for (i = ctrl->link->lttpr_count - 1; i >= 0; i--) {
+		enum drm_dp_phy dp_phy = DP_PHY_LTTPR(i);
+
+		ret = msm_dp_ctrl_link_train_1_2(ctrl, training_step, dp_phy);
+		msm_dp_ctrl_clear_training_pattern(ctrl, dp_phy);
+
+		if (ret)
+			break;
+	}
+
 	if (ret) {
-		DRM_ERROR("link training #1 failed. ret=%d\n", ret);
+		DRM_ERROR("link training of LTTPR(s) failed. ret=%d\n", ret);
 		goto end;
 	}
 
-	/* print success info as this is a result of user initiated action */
-	drm_dbg_dp(ctrl->drm_dev, "link training #1 successful\n");
-
-	ret = msm_dp_ctrl_link_train_2(ctrl, training_step);
+	ret = msm_dp_ctrl_link_train_1_2(ctrl, training_step, DP_PHY_DPRX);
 	if (ret) {
-		DRM_ERROR("link training #2 failed. ret=%d\n", ret);
+		DRM_ERROR("link training on sink failed. ret=%d\n", ret);
 		goto end;
 	}
 
-	/* print success info as this is a result of user initiated action */
-	drm_dbg_dp(ctrl->drm_dev, "link training #2 successful\n");
-
 end:
 	msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, 0);
 
@@ -1622,7 +1674,7 @@ static int msm_dp_ctrl_link_maintenance(struct msm_dp_ctrl_private *ctrl)
 	if (ret)
 		goto end;
 
-	msm_dp_ctrl_clear_training_pattern(ctrl);
+	msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
 
 	msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, DP_STATE_CTRL_SEND_VIDEO);
 
@@ -1646,7 +1698,7 @@ static bool msm_dp_ctrl_send_phy_test_pattern(struct msm_dp_ctrl_private *ctrl)
 		return false;
 	}
 	msm_dp_catalog_ctrl_send_phy_pattern(ctrl->catalog, pattern_requested);
-	msm_dp_ctrl_update_vx_px(ctrl);
+	msm_dp_ctrl_update_phy_vx_px(ctrl, DP_PHY_DPRX);
 	msm_dp_link_send_test_response(ctrl->link);
 
 	pattern_sent = msm_dp_catalog_ctrl_read_phy_pattern(ctrl->catalog);
@@ -1888,7 +1940,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
 			}
 
 			/* stop link training before start re training  */
-			msm_dp_ctrl_clear_training_pattern(ctrl);
+			msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
 		}
 
 		rc = msm_dp_ctrl_reinitialize_mainlink(ctrl);
@@ -1912,7 +1964,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
 		 * link training failed
 		 * end txing train pattern here
 		 */
-		msm_dp_ctrl_clear_training_pattern(ctrl);
+		msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
 
 		msm_dp_ctrl_deinitialize_mainlink(ctrl);
 		rc = -ECONNRESET;
@@ -1983,7 +2035,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
 		msm_dp_ctrl_link_retrain(ctrl);
 
 	/* stop txing train pattern to end link training */
-	msm_dp_ctrl_clear_training_pattern(ctrl);
+	msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
 
 	/*
 	 * Set up transfer unit values and set controller state to send
-- 
2.45.2

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Rob Clark]

On Wed, Apr 16, 2025 at 7:13 PM Aleksandrs Vinarskis
<alex.vinarskis@gmail.com> wrote:
>
> Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> to non-transparent mode to enable video output on X1E-based devices
> that come with LTTPR on the motherboards. However, video would not work
> if additional LTTPR(s) are present between sink and source, which is
> the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).
>
> First, take into account LTTPR capabilities when computing max link
> rate, number of lanes. Take into account previous discussion on the
> lists - exit early if reading DPCD caps failed. This also fixes
> "*ERROR* panel edid read failed" on some monitors which seems to be
> caused by msm_dp_panel_read_sink_caps running before LTTPR(s) are
> initialized.
>
> Finally, implement link training per-segment. Pass lttpr_count to all
> required helpers.
> This seems to also partially improve UI (Wayland) hanging when
> changing external display's link parameters (resolution, framerate):
> * Prior to this series, via direct USB Type-C to display connection,
>   attempt to change resolution or framerate hangs the UI, setting does
>   not stick. Some back and forth replugging finally sets desired
>   parameters.
> * With this series, via direct USB Type-C to display connection,
>   changing parameters works most of the time, without UI freezing. Via
>   docking station/multiple LTTPRs the setting again does not stick.
> * On Xorg changing link paramaters works in all combinations.
>
> These appear to be mainlink initialization related, as in all cases LT
> passes successfully.
>
> Test matrix:
> * Dell XPS 9345, Ubuntu 24.10, Gnome 47, Wayland (myself)
>         * Left USB Type-C, Right USB Type-C
>         * Direct monitor connection, Dell WD19TB, Dell WD22TB4, USB
>           Type-C to HDMI dongle, USB Type-C to DP dongle
>         * Dell AW3423DWF, Samsung LS24A600, dual Samsung LS24A600 (one
>           monitor per USB Type-C connector)
> * Dell XPS 9345, Ubuntu 24.10, Gnome 47, Wayland (myself)
>         * Left USB Type-C, Right USB Type-C
>         * Direct monitor connection
>         * Samsung S34BG85 (USB Type-C), Dell U2725QE (universal
>           Thunderbolt/USB Type-C, probes with an LTTPR when in USB
>           Type-C/DP Alt mode)
> * Dell XPS 9345, Debian trixie/sid, Gnome 48, Wayland (Stefan Schmidt)
>         * Left USB Type-C, Right USB Type-C
>         * Dell WD15 Dock with DisplayPort connected
>         * Dell HD22Q dock with HDMI connected
>         * USB Type-C to HDMI dongle
>         * Dell U3417W

For the series,

Tested-by: Rob Clark <robdclark@gmail.com>  # yoga slim 7x

patch 4/4 had a number of conflicting hunks, you might need to rebase
on msm-next

BR,
-R

> In both cases, "Thunderbot Support"/"USB4 PCIE Tunneling" was disabled
> in UEFI to force universal Thunderbolt/USB Type-C devices to work in
> DP Alt mode.
> In both cases laptops had HBR3 patches applied [1], resulting in
> maximum successful link at 3440x1440@100hz and 4k@60hz respectively.
> When using Dell WD22TB4/U2725QE, USB Type-C pin assigment D got enabled
> and USB3.0 devices were working in parallel to video ouput.
>
> Known issues:
> * As mentioned above, it appears that on Gnome+Wayland framerate and
>   resolution parameter adjustment is not stable.
>
> Due to lack of access to the official DisplayPort specfication, changes
> were primarily inspired by/reverse engineered from Intel's i915 driver.
>
> [1] https://lore.kernel.org/all/20250226231436.16138-2-alex.vinarskis@gmail.com/
>
> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
>
> ---
>
> Changes in v3:
> - Split 1st patch into 3
> - Simplified handling of max_lttpr_lanes/max_lttpr_rate
> - Moved lttpr_common_caps to msm_dp_link (not msm_dp_panel, as LTTPRs
>   are link related, not panel related)
> - Picked Stefan's T-b tag (last patch only, as 1st one is getting split)
> - Droped Abel's R-b tags from 1st patch that got split due to high diff
> - Fixed alignment issues, initialization of variables, debug prints
> - Moved lttpr_count to avoid ugly pointer
> - Link to v2: https://lore.kernel.org/all/20250311234109.136510-1-alex.vinarskis@gmail.com/
>
> Changes in v2:
> - Picked up Abel's R-b tags
> - Fixed typo as per Abel, fixed readability as per Johan
> - Updated cover and commit message on mailink issue which appears to be
>   specific to Gnome+Wayland. No problems on Xorg.
> - Link to v1: https://lore.kernel.org/all/20250310211039.29843-1-alex.vinarskis@gmail.com/
>
> Aleksandrs Vinarskis (4):
>   drm/msm/dp: Fix support of LTTPR initialization
>   drm/msm/dp: Account for LTTPRs capabilities
>   drm/msm/dp: Prepare for link training per-segment for LTTPRs
>   drm/msm/dp: Introduce link training per-segment for LTTPRs
>
>  drivers/gpu/drm/msm/dp/dp_ctrl.c    | 126 ++++++++++++++++++++--------
>  drivers/gpu/drm/msm/dp/dp_display.c |  27 ++++--
>  drivers/gpu/drm/msm/dp/dp_link.h    |   4 +
>  drivers/gpu/drm/msm/dp/dp_panel.c   |  12 ++-
>  4 files changed, 122 insertions(+), 47 deletions(-)
>
> --
> 2.45.2
>

Re: [PATCH v3 1/4] drm/msm/dp: Fix support of LTTPR initialization

[Dmitry Baryshkov]

On Thu, Apr 17, 2025 at 04:10:32AM +0200, Aleksandrs Vinarskis wrote:
> Initialize LTTPR before msm_dp_panel_read_sink_caps, as DPTX shall
> (re)read DPRX caps after LTTPR detection, as required by DP 2.1,
> Section 3.6.7.6.1.
> 
> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Dmitry Baryshkov]

On Thu, Apr 24, 2025 at 02:12:15PM -0700, Rob Clark wrote:
> On Wed, Apr 16, 2025 at 7:13 PM Aleksandrs Vinarskis
> <alex.vinarskis@gmail.com> wrote:
> >
> > Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> > to non-transparent mode to enable video output on X1E-based devices
> > that come with LTTPR on the motherboards. However, video would not work
> > if additional LTTPR(s) are present between sink and source, which is
> > the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> > some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).
> >
> > First, take into account LTTPR capabilities when computing max link
> > rate, number of lanes. Take into account previous discussion on the
> > lists - exit early if reading DPCD caps failed. This also fixes
> > "*ERROR* panel edid read failed" on some monitors which seems to be
> > caused by msm_dp_panel_read_sink_caps running before LTTPR(s) are
> > initialized.
> >
> > Finally, implement link training per-segment. Pass lttpr_count to all
> > required helpers.
> > This seems to also partially improve UI (Wayland) hanging when
> > changing external display's link parameters (resolution, framerate):
> > * Prior to this series, via direct USB Type-C to display connection,
> >   attempt to change resolution or framerate hangs the UI, setting does
> >   not stick. Some back and forth replugging finally sets desired
> >   parameters.
> > * With this series, via direct USB Type-C to display connection,
> >   changing parameters works most of the time, without UI freezing. Via
> >   docking station/multiple LTTPRs the setting again does not stick.
> > * On Xorg changing link paramaters works in all combinations.
> >
> > These appear to be mainlink initialization related, as in all cases LT
> > passes successfully.
> >
> > Test matrix:
> > * Dell XPS 9345, Ubuntu 24.10, Gnome 47, Wayland (myself)
> >         * Left USB Type-C, Right USB Type-C
> >         * Direct monitor connection, Dell WD19TB, Dell WD22TB4, USB
> >           Type-C to HDMI dongle, USB Type-C to DP dongle
> >         * Dell AW3423DWF, Samsung LS24A600, dual Samsung LS24A600 (one
> >           monitor per USB Type-C connector)
> > * Dell XPS 9345, Ubuntu 24.10, Gnome 47, Wayland (myself)
> >         * Left USB Type-C, Right USB Type-C
> >         * Direct monitor connection
> >         * Samsung S34BG85 (USB Type-C), Dell U2725QE (universal
> >           Thunderbolt/USB Type-C, probes with an LTTPR when in USB
> >           Type-C/DP Alt mode)
> > * Dell XPS 9345, Debian trixie/sid, Gnome 48, Wayland (Stefan Schmidt)
> >         * Left USB Type-C, Right USB Type-C
> >         * Dell WD15 Dock with DisplayPort connected
> >         * Dell HD22Q dock with HDMI connected
> >         * USB Type-C to HDMI dongle
> >         * Dell U3417W
> 
> For the series,
> 
> Tested-by: Rob Clark <robdclark@gmail.com>  # yoga slim 7x

Could you please send this as a response to one of the patches, so that
it's also recorded by the patchwork?

> 
> patch 4/4 had a number of conflicting hunks, you might need to rebase
> on msm-next
> 
> BR,
> -R
> 
> > In both cases, "Thunderbot Support"/"USB4 PCIE Tunneling" was disabled
> > in UEFI to force universal Thunderbolt/USB Type-C devices to work in
> > DP Alt mode.
> > In both cases laptops had HBR3 patches applied [1], resulting in
> > maximum successful link at 3440x1440@100hz and 4k@60hz respectively.
> > When using Dell WD22TB4/U2725QE, USB Type-C pin assigment D got enabled
> > and USB3.0 devices were working in parallel to video ouput.
> >
> > Known issues:
> > * As mentioned above, it appears that on Gnome+Wayland framerate and
> >   resolution parameter adjustment is not stable.
> >
> > Due to lack of access to the official DisplayPort specfication, changes
> > were primarily inspired by/reverse engineered from Intel's i915 driver.
> >
> > [1] https://lore.kernel.org/all/20250226231436.16138-2-alex.vinarskis@gmail.com/
> >
> > Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
> >
> > ---
> >
> > Changes in v3:
> > - Split 1st patch into 3
> > - Simplified handling of max_lttpr_lanes/max_lttpr_rate
> > - Moved lttpr_common_caps to msm_dp_link (not msm_dp_panel, as LTTPRs
> >   are link related, not panel related)
> > - Picked Stefan's T-b tag (last patch only, as 1st one is getting split)
> > - Droped Abel's R-b tags from 1st patch that got split due to high diff
> > - Fixed alignment issues, initialization of variables, debug prints
> > - Moved lttpr_count to avoid ugly pointer
> > - Link to v2: https://lore.kernel.org/all/20250311234109.136510-1-alex.vinarskis@gmail.com/
> >
> > Changes in v2:
> > - Picked up Abel's R-b tags
> > - Fixed typo as per Abel, fixed readability as per Johan
> > - Updated cover and commit message on mailink issue which appears to be
> >   specific to Gnome+Wayland. No problems on Xorg.
> > - Link to v1: https://lore.kernel.org/all/20250310211039.29843-1-alex.vinarskis@gmail.com/
> >
> > Aleksandrs Vinarskis (4):
> >   drm/msm/dp: Fix support of LTTPR initialization
> >   drm/msm/dp: Account for LTTPRs capabilities
> >   drm/msm/dp: Prepare for link training per-segment for LTTPRs
> >   drm/msm/dp: Introduce link training per-segment for LTTPRs
> >
> >  drivers/gpu/drm/msm/dp/dp_ctrl.c    | 126 ++++++++++++++++++++--------
> >  drivers/gpu/drm/msm/dp/dp_display.c |  27 ++++--
> >  drivers/gpu/drm/msm/dp/dp_link.h    |   4 +
> >  drivers/gpu/drm/msm/dp/dp_panel.c   |  12 ++-
> >  4 files changed, 122 insertions(+), 47 deletions(-)
> >
> > --
> > 2.45.2
> >

-- 
With best wishes
Dmitry

Re: [PATCH v3 3/4] drm/msm/dp: Prepare for link training per-segment for LTTPRs

[Abel Vesa]

On 25-04-17 04:10:34, Aleksandrs Vinarskis wrote:
> Per-segment link training requires knowing the number of LTTPRs
> (if any) present. Store the count during LTTPRs' initialization.
> 
> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH v3 2/4] drm/msm/dp: Account for LTTPRs capabilities

[Abel Vesa]

On 25-04-17 04:10:33, Aleksandrs Vinarskis wrote:
> Take into account LTTPR capabilities when selecting maximum allowed
> link rate, number of data lines.
> 
> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH v3 2/4] drm/msm/dp: Account for LTTPRs capabilities

[Dmitry Baryshkov]

On Thu, Apr 17, 2025 at 04:10:33AM +0200, Aleksandrs Vinarskis wrote:
> Take into account LTTPR capabilities when selecting maximum allowed
> link rate, number of data lines.
> 
> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c |  5 ++---
>  drivers/gpu/drm/msm/dp/dp_link.h    |  3 +++
>  drivers/gpu/drm/msm/dp/dp_panel.c   | 12 +++++++++++-
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH v3 3/4] drm/msm/dp: Prepare for link training per-segment for LTTPRs

[Dmitry Baryshkov]

On Thu, Apr 17, 2025 at 04:10:34AM +0200, Aleksandrs Vinarskis wrote:
> Per-segment link training requires knowing the number of LTTPRs
> (if any) present. Store the count during LTTPRs' initialization.
> 
> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 17 +++++++++++------
>  drivers/gpu/drm/msm/dp/dp_link.h    |  1 +
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH v3 4/4] drm/msm/dp: Introduce link training per-segment for LTTPRs

[Dmitry Baryshkov]

On Thu, Apr 17, 2025 at 04:10:35AM +0200, Aleksandrs Vinarskis wrote:
> DisplayPort requires per-segment link training when LTTPR are switched
> to non-transparent mode, starting with LTTPR closest to the source.
> Only when each segment is trained individually, source can link train
> to sink.
> 
> Implement per-segment link traning when LTTPR(s) are detected, to
> support external docking stations. On higher level, changes are:
> 
> * Pass phy being trained down to all required helpers
> * Run CR, EQ link training per phy
> * Set voltage swing, pre-emphasis levels per phy
> 
> This ensures successful link training both when connected directly to
> the monitor (single LTTPR onboard most X1E laptops) and via the docking
> station (at least two LTTPRs).
> 
> Tested-by: Stefan Schmidt <stefan.schmidt@linaro.org>
> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
> Reviewed-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c | 126 ++++++++++++++++++++++---------
>  1 file changed, 89 insertions(+), 37 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Johan Hovold]

On Thu, Apr 17, 2025 at 04:10:31AM +0200, Aleksandrs Vinarskis wrote:
> Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> to non-transparent mode to enable video output on X1E-based devices
> that come with LTTPR on the motherboards. However, video would not work
> if additional LTTPR(s) are present between sink and source, which is
> the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).

Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
adapters or docks with retimers in transparent mode?

You describe at least one of this patches as a fix but I'm not seeing
any Fixes tags or indication that these need to go into 6.15-rc to fix
a regression.

> Changes in v3:
> - Split 1st patch into 3
> - Simplified handling of max_lttpr_lanes/max_lttpr_rate
> - Moved lttpr_common_caps to msm_dp_link (not msm_dp_panel, as LTTPRs
>   are link related, not panel related)
> - Picked Stefan's T-b tag (last patch only, as 1st one is getting split)
> - Droped Abel's R-b tags from 1st patch that got split due to high diff
> - Fixed alignment issues, initialization of variables, debug prints
> - Moved lttpr_count to avoid ugly pointer
> - Link to v2: https://lore.kernel.org/all/20250311234109.136510-1-alex.vinarskis@gmail.com/

I tested v2 due to the dependencies in msm-next and DP altmode still
works on my X13s and T14s (while not using any docks):

Tested-by: Johan Hovold <johan+linaro@kernel.org>

> Aleksandrs Vinarskis (4):
>   drm/msm/dp: Fix support of LTTPR initialization
>   drm/msm/dp: Account for LTTPRs capabilities
>   drm/msm/dp: Prepare for link training per-segment for LTTPRs
>   drm/msm/dp: Introduce link training per-segment for LTTPRs

Johan

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Aleksandrs Vinarskis]

On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Apr 17, 2025 at 04:10:31AM +0200, Aleksandrs Vinarskis wrote:
> > Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> > to non-transparent mode to enable video output on X1E-based devices
> > that come with LTTPR on the motherboards. However, video would not work
> > if additional LTTPR(s) are present between sink and source, which is
> > the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> > some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).
>
> Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> adapters or docks with retimers in transparent mode?

I am actually not 100% sure.
- If without LTTPR initialization, they default to transparent mode,
then yes, incomplete LTTPR support sets them to non-transparent
without per-segment training and breaks docks with retimers, while it
would've worked if LTTPR(s) would've been left in default transparent
mode. Note that in this case, X1E devices with ps883x are somehow an
exception, because without LTTPR initialization at all the training
always fails.
- If LTTPR has to be initialized either way, and explicitly set to
transparent mode if we do not want non-transparent, then no,
incomplete LTTPR support in 6.15-rcX did not explicitly break docks
with retimers, as those never worked in the first place. As per my
understanding, this is the case, unless something (firmware?) has
already placed LTTPR to transparent mode before the driver takes over
- then 1st case would be applicable.

Docks with retimers do not work in 6.15-rcX, but I am unable to verify
if it did work before, as I do not have a Qualcomm based device
without LTTPR on the baseboard.

> You describe at least one of this patches as a fix but I'm not seeing
> any Fixes tags or indication that these need to go into 6.15-rc to fix
> a regression.

You are right, I will add Fixes tag to the 1st patch to make it clear:
Fixes 72d0af4accd (drm/msm/dp: Add support for LTTPR handling)

Or should I mark the entire series with Fixes, so that the docking
stations with retimers can be fixed in 6.15 already? Landing only the
1st patch will fix inconsistency with DP spec, but will not fix
docking stations with retimers. I guess this comes down to whether
existing LTTPR (but not multiple LTTPRs) support is considered a bug
(and patches 2,3,4 are a fix) or lack of functionality (and patches
2,3,4 are a new feature).

>
> > Changes in v3:
> > - Split 1st patch into 3
> > - Simplified handling of max_lttpr_lanes/max_lttpr_rate
> > - Moved lttpr_common_caps to msm_dp_link (not msm_dp_panel, as LTTPRs
> >   are link related, not panel related)
> > - Picked Stefan's T-b tag (last patch only, as 1st one is getting split)
> > - Droped Abel's R-b tags from 1st patch that got split due to high diff
> > - Fixed alignment issues, initialization of variables, debug prints
> > - Moved lttpr_count to avoid ugly pointer
> > - Link to v2: https://lore.kernel.org/all/20250311234109.136510-1-alex.vinarskis@gmail.com/
>
> I tested v2 due to the dependencies in msm-next and DP altmode still
> works on my X13s and T14s (while not using any docks):
>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>

Thanks,

Alex

>
> > Aleksandrs Vinarskis (4):
> >   drm/msm/dp: Fix support of LTTPR initialization
> >   drm/msm/dp: Account for LTTPRs capabilities
> >   drm/msm/dp: Prepare for link training per-segment for LTTPRs
> >   drm/msm/dp: Introduce link training per-segment for LTTPRs
>
> Johan

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Johan Hovold]

On Mon, Apr 28, 2025 at 11:06:39AM +0200, Aleksandrs Vinarskis wrote:
> On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:
> > On Thu, Apr 17, 2025 at 04:10:31AM +0200, Aleksandrs Vinarskis wrote:
> > > Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> > > to non-transparent mode to enable video output on X1E-based devices
> > > that come with LTTPR on the motherboards. However, video would not work
> > > if additional LTTPR(s) are present between sink and source, which is
> > > the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> > > some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).
> >
> > Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> > adapters or docks with retimers in transparent mode?
> 
> I am actually not 100% sure.
> - If without LTTPR initialization, they default to transparent mode,
> then yes, incomplete LTTPR support sets them to non-transparent
> without per-segment training and breaks docks with retimers, while it
> would've worked if LTTPR(s) would've been left in default transparent
> mode. Note that in this case, X1E devices with ps883x are somehow an
> exception, because without LTTPR initialization at all the training
> always fails.

Right, I'm concerned about breaking working setups for users of machines
like the X13s.

> - If LTTPR has to be initialized either way, and explicitly set to
> transparent mode if we do not want non-transparent, then no,
> incomplete LTTPR support in 6.15-rcX did not explicitly break docks
> with retimers, as those never worked in the first place. As per my
> understanding, this is the case, unless something (firmware?) has
> already placed LTTPR to transparent mode before the driver takes over
> - then 1st case would be applicable.
> 
> Docks with retimers do not work in 6.15-rcX, but I am unable to verify
> if it did work before, as I do not have a Qualcomm based device
> without LTTPR on the baseboard.

Abel (or anyone else), do you have one of these docks that you could
test with the X13s to confirm whether this series fixes a regression or
not?

> > You describe at least one of this patches as a fix but I'm not seeing
> > any Fixes tags or indication that these need to go into 6.15-rc to fix
> > a regression.
> 
> You are right, I will add Fixes tag to the 1st patch to make it clear:
> Fixes 72d0af4accd (drm/msm/dp: Add support for LTTPR handling)
> 
> Or should I mark the entire series with Fixes, so that the docking
> stations with retimers can be fixed in 6.15 already? Landing only the
> 1st patch will fix inconsistency with DP spec, but will not fix
> docking stations with retimers. I guess this comes down to whether
> existing LTTPR (but not multiple LTTPRs) support is considered a bug
> (and patches 2,3,4 are a fix) or lack of functionality (and patches
> 2,3,4 are a new feature).

Indeed. If LTTPR support broke existing setups, then I think all should
be marked with a Fixes tag and merged for 6.15. If we can't get it into
6.15 we may consider just disabling LTTPR support in 6.15 to address the
regression and then enable it again once fixed in 6.16.

But if this series is just enabling support for docks (and USB-C ports)
that did not used to work, then I guess this can all wait for 6.16.

Johan

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Abel Vesa]

On 25-04-28 14:47:04, Johan Hovold wrote:
> On Mon, Apr 28, 2025 at 11:06:39AM +0200, Aleksandrs Vinarskis wrote:
> > On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:
> > > On Thu, Apr 17, 2025 at 04:10:31AM +0200, Aleksandrs Vinarskis wrote:
> > > > Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> > > > to non-transparent mode to enable video output on X1E-based devices
> > > > that come with LTTPR on the motherboards. However, video would not work
> > > > if additional LTTPR(s) are present between sink and source, which is
> > > > the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> > > > some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).
> > >
> > > Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> > > adapters or docks with retimers in transparent mode?
> > 
> > I am actually not 100% sure.
> > - If without LTTPR initialization, they default to transparent mode,
> > then yes, incomplete LTTPR support sets them to non-transparent
> > without per-segment training and breaks docks with retimers, while it
> > would've worked if LTTPR(s) would've been left in default transparent
> > mode. Note that in this case, X1E devices with ps883x are somehow an
> > exception, because without LTTPR initialization at all the training
> > always fails.
> 
> Right, I'm concerned about breaking working setups for users of machines
> like the X13s.
> 
> > - If LTTPR has to be initialized either way, and explicitly set to
> > transparent mode if we do not want non-transparent, then no,
> > incomplete LTTPR support in 6.15-rcX did not explicitly break docks
> > with retimers, as those never worked in the first place. As per my
> > understanding, this is the case, unless something (firmware?) has
> > already placed LTTPR to transparent mode before the driver takes over
> > - then 1st case would be applicable.
> > 
> > Docks with retimers do not work in 6.15-rcX, but I am unable to verify
> > if it did work before, as I do not have a Qualcomm based device
> > without LTTPR on the baseboard.
> 
> Abel (or anyone else), do you have one of these docks that you could
> test with the X13s to confirm whether this series fixes a regression or
> not?

Before the support for LTTPRs has been merged, if you would have one of
those docks (I do not own one) with LTTPRs, link training would've just
failed if the LTTPRs were not by default in transparent mode, which IIRC
is what the standard dictates.

X13s doesn't have LTTPRs on-board so when reading the caps, LTTPRs count
would return 0 and none of the of the transparent/non-transparent setup
would happen. Now, as already mentioned, DP would be considered already
broken (or rather not supported) if you would connect a dock with LTTPRs in it.

With the support in, if one such dock is used, the training should be
successful as all LTTPRs are set in transparent mode. This I was not
able to test myself as I do not own such a dock.

> 
> > > You describe at least one of this patches as a fix but I'm not seeing
> > > any Fixes tags or indication that these need to go into 6.15-rc to fix
> > > a regression.
> > 
> > You are right, I will add Fixes tag to the 1st patch to make it clear:
> > Fixes 72d0af4accd (drm/msm/dp: Add support for LTTPR handling)
> > 
> > Or should I mark the entire series with Fixes, so that the docking
> > stations with retimers can be fixed in 6.15 already? Landing only the
> > 1st patch will fix inconsistency with DP spec, but will not fix
> > docking stations with retimers. I guess this comes down to whether
> > existing LTTPR (but not multiple LTTPRs) support is considered a bug
> > (and patches 2,3,4 are a fix) or lack of functionality (and patches
> > 2,3,4 are a new feature).
> 
> Indeed. If LTTPR support broke existing setups, then I think all should
> be marked with a Fixes tag and merged for 6.15. If we can't get it into
> 6.15 we may consider just disabling LTTPR support in 6.15 to address the
> regression and then enable it again once fixed in 6.16.

The LTTPR support did not break existing (working) setups because on these
setups, LTTPR count would read 0 and would be basically a no-op.

> 
> But if this series is just enabling support for docks (and USB-C ports)
> that did not used to work, then I guess this can all wait for 6.16.

I'm not sure about what this actually fixes. It might be that is
specific to a dock or something. But as far as X Elite boards go, even
without this "fix" display has been working fine.

The change itself makes sense though and I think makes sense to be marked as a fix.

> 
> Johan

Abel

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Aleksandrs Vinarskis]

On Mon, 28 Apr 2025 at 16:17, Abel Vesa <abel.vesa@linaro.org> wrote:
>
> On 25-04-28 14:47:04, Johan Hovold wrote:
> > On Mon, Apr 28, 2025 at 11:06:39AM +0200, Aleksandrs Vinarskis wrote:
> > > On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:
> > > > On Thu, Apr 17, 2025 at 04:10:31AM +0200, Aleksandrs Vinarskis wrote:
> > > > > Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> > > > > to non-transparent mode to enable video output on X1E-based devices
> > > > > that come with LTTPR on the motherboards. However, video would not work
> > > > > if additional LTTPR(s) are present between sink and source, which is
> > > > > the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> > > > > some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).
> > > >
> > > > Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> > > > adapters or docks with retimers in transparent mode?
> > >
> > > I am actually not 100% sure.
> > > - If without LTTPR initialization, they default to transparent mode,
> > > then yes, incomplete LTTPR support sets them to non-transparent
> > > without per-segment training and breaks docks with retimers, while it
> > > would've worked if LTTPR(s) would've been left in default transparent
> > > mode. Note that in this case, X1E devices with ps883x are somehow an
> > > exception, because without LTTPR initialization at all the training
> > > always fails.
> >
> > Right, I'm concerned about breaking working setups for users of machines
> > like the X13s.
> >
> > > - If LTTPR has to be initialized either way, and explicitly set to
> > > transparent mode if we do not want non-transparent, then no,
> > > incomplete LTTPR support in 6.15-rcX did not explicitly break docks
> > > with retimers, as those never worked in the first place. As per my
> > > understanding, this is the case, unless something (firmware?) has
> > > already placed LTTPR to transparent mode before the driver takes over
> > > - then 1st case would be applicable.
> > >
> > > Docks with retimers do not work in 6.15-rcX, but I am unable to verify
> > > if it did work before, as I do not have a Qualcomm based device
> > > without LTTPR on the baseboard.
> >
> > Abel (or anyone else), do you have one of these docks that you could
> > test with the X13s to confirm whether this series fixes a regression or
> > not?
>
> Before the support for LTTPRs has been merged, if you would have one of
> those docks (I do not own one) with LTTPRs, link training would've just
> failed if the LTTPRs were not by default in transparent mode, which IIRC
> is what the standard dictates.
>
> X13s doesn't have LTTPRs on-board so when reading the caps, LTTPRs count
> would return 0 and none of the of the transparent/non-transparent setup
> would happen. Now, as already mentioned, DP would be considered already
> broken (or rather not supported) if you would connect a dock with LTTPRs in it.
>
> With the support in, if one such dock is used, the training should be
> successful as all LTTPRs are set in transparent mode. This I was not
> able to test myself as I do not own such a dock.
>
> >
> > > > You describe at least one of this patches as a fix but I'm not seeing
> > > > any Fixes tags or indication that these need to go into 6.15-rc to fix
> > > > a regression.
> > >
> > > You are right, I will add Fixes tag to the 1st patch to make it clear:
> > > Fixes 72d0af4accd (drm/msm/dp: Add support for LTTPR handling)
> > >
> > > Or should I mark the entire series with Fixes, so that the docking
> > > stations with retimers can be fixed in 6.15 already? Landing only the
> > > 1st patch will fix inconsistency with DP spec, but will not fix
> > > docking stations with retimers. I guess this comes down to whether
> > > existing LTTPR (but not multiple LTTPRs) support is considered a bug
> > > (and patches 2,3,4 are a fix) or lack of functionality (and patches
> > > 2,3,4 are a new feature).
> >
> > Indeed. If LTTPR support broke existing setups, then I think all should
> > be marked with a Fixes tag and merged for 6.15. If we can't get it into
> > 6.15 we may consider just disabling LTTPR support in 6.15 to address the
> > regression and then enable it again once fixed in 6.16.
>
> The LTTPR support did not break existing (working) setups because on these
> setups, LTTPR count would read 0 and would be basically a no-op.
>
> >
> > But if this series is just enabling support for docks (and USB-C ports)
> > that did not used to work, then I guess this can all wait for 6.16.
>
> I'm not sure about what this actually fixes. It might be that is
> specific to a dock or something. But as far as X Elite boards go, even
> without this "fix" display has been working fine.
>
> The change itself makes sense though and I think makes sense to be marked as a fix.

Just to confirm, you mean to mark as fix only the 1st patch, correct?
Since it's obvious now that the currently present partial LTTPR
support did not break anything that used to work.

Thanks,
Alex

>
> >
> > Johan
>
> Abel

Re: [PATCH v3 4/4] drm/msm/dp: Introduce link training per-segment for LTTPRs

[Rob Clark]

On Wed, Apr 16, 2025 at 7:13 PM Aleksandrs Vinarskis
<alex.vinarskis@gmail.com> wrote:
>
> DisplayPort requires per-segment link training when LTTPR are switched
> to non-transparent mode, starting with LTTPR closest to the source.
> Only when each segment is trained individually, source can link train
> to sink.
>
> Implement per-segment link traning when LTTPR(s) are detected, to
> support external docking stations. On higher level, changes are:
>
> * Pass phy being trained down to all required helpers
> * Run CR, EQ link training per phy
> * Set voltage swing, pre-emphasis levels per phy
>
> This ensures successful link training both when connected directly to
> the monitor (single LTTPR onboard most X1E laptops) and via the docking
> station (at least two LTTPRs).
>
> Tested-by: Stefan Schmidt <stefan.schmidt@linaro.org>
> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
> Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Tested-by: Rob Clark <robdclark@gmail.com>  # yoga slim 7x

> ---
>  drivers/gpu/drm/msm/dp/dp_ctrl.c | 126 ++++++++++++++++++++++---------
>  1 file changed, 89 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 69a26bb5fabd..a50bfafbb4ea 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1034,10 +1034,12 @@ static int msm_dp_ctrl_set_vx_px(struct msm_dp_ctrl_private *ctrl,
>         return 0;
>  }
>
> -static int msm_dp_ctrl_update_vx_px(struct msm_dp_ctrl_private *ctrl)
> +static int msm_dp_ctrl_update_phy_vx_px(struct msm_dp_ctrl_private *ctrl,
> +                                       enum drm_dp_phy dp_phy)
>  {
>         struct msm_dp_link *link = ctrl->link;
> -       int ret = 0, lane, lane_cnt;
> +       int lane, lane_cnt, reg;
> +       int ret = 0;
>         u8 buf[4];
>         u32 max_level_reached = 0;
>         u32 voltage_swing_level = link->phy_params.v_level;
> @@ -1075,8 +1077,13 @@ static int msm_dp_ctrl_update_vx_px(struct msm_dp_ctrl_private *ctrl)
>
>         drm_dbg_dp(ctrl->drm_dev, "sink: p|v=0x%x\n",
>                         voltage_swing_level | pre_emphasis_level);
> -       ret = drm_dp_dpcd_write(ctrl->aux, DP_TRAINING_LANE0_SET,
> -                                       buf, lane_cnt);
> +
> +       if (dp_phy == DP_PHY_DPRX)
> +               reg = DP_TRAINING_LANE0_SET;
> +       else
> +               reg = DP_TRAINING_LANE0_SET_PHY_REPEATER(dp_phy);
> +
> +       ret = drm_dp_dpcd_write(ctrl->aux, reg, buf, lane_cnt);
>         if (ret == lane_cnt)
>                 ret = 0;
>
> @@ -1084,9 +1091,10 @@ static int msm_dp_ctrl_update_vx_px(struct msm_dp_ctrl_private *ctrl)
>  }
>
>  static bool msm_dp_ctrl_train_pattern_set(struct msm_dp_ctrl_private *ctrl,
> -               u8 pattern)
> +               u8 pattern, enum drm_dp_phy dp_phy)
>  {
>         u8 buf;
> +       int reg;
>         int ret = 0;
>
>         drm_dbg_dp(ctrl->drm_dev, "sink: pattern=%x\n", pattern);
> @@ -1096,17 +1104,26 @@ static bool msm_dp_ctrl_train_pattern_set(struct msm_dp_ctrl_private *ctrl,
>         if (pattern && pattern != DP_TRAINING_PATTERN_4)
>                 buf |= DP_LINK_SCRAMBLING_DISABLE;
>
> -       ret = drm_dp_dpcd_writeb(ctrl->aux, DP_TRAINING_PATTERN_SET, buf);
> +       if (dp_phy == DP_PHY_DPRX)
> +               reg = DP_TRAINING_PATTERN_SET;
> +       else
> +               reg = DP_TRAINING_PATTERN_SET_PHY_REPEATER(dp_phy);
> +
> +       ret = drm_dp_dpcd_writeb(ctrl->aux, reg, buf);
>         return ret == 1;
>  }
>
>  static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
> -                       int *training_step)
> +                       int *training_step, enum drm_dp_phy dp_phy)
>  {
> +       int delay_us;
>         int tries, old_v_level, ret = 0;
>         u8 link_status[DP_LINK_STATUS_SIZE];
>         int const maximum_retries = 4;
>
> +       delay_us = drm_dp_read_clock_recovery_delay(ctrl->aux,
> +                                                   ctrl->panel->dpcd, dp_phy, false);
> +
>         msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, 0);
>
>         *training_step = DP_TRAINING_1;
> @@ -1115,18 +1132,19 @@ static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
>         if (ret)
>                 return ret;
>         msm_dp_ctrl_train_pattern_set(ctrl, DP_TRAINING_PATTERN_1 |
> -               DP_LINK_SCRAMBLING_DISABLE);
> +               DP_LINK_SCRAMBLING_DISABLE, dp_phy);
>
> -       ret = msm_dp_ctrl_update_vx_px(ctrl);
> +       msm_dp_link_reset_phy_params_vx_px(ctrl->link);
> +       ret = msm_dp_ctrl_update_phy_vx_px(ctrl, dp_phy);
>         if (ret)
>                 return ret;
>
>         tries = 0;
>         old_v_level = ctrl->link->phy_params.v_level;
>         for (tries = 0; tries < maximum_retries; tries++) {
> -               drm_dp_link_train_clock_recovery_delay(ctrl->aux, ctrl->panel->dpcd);
> +               fsleep(delay_us);
>
> -               ret = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
> +               ret = drm_dp_dpcd_read_phy_link_status(ctrl->aux, dp_phy, link_status);
>                 if (ret)
>                         return ret;
>
> @@ -1147,7 +1165,7 @@ static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
>                 }
>
>                 msm_dp_link_adjust_levels(ctrl->link, link_status);
> -               ret = msm_dp_ctrl_update_vx_px(ctrl);
> +               ret = msm_dp_ctrl_update_phy_vx_px(ctrl, dp_phy);
>                 if (ret)
>                         return ret;
>         }
> @@ -1199,21 +1217,31 @@ static int msm_dp_ctrl_link_lane_down_shift(struct msm_dp_ctrl_private *ctrl)
>         return 0;
>  }
>
> -static void msm_dp_ctrl_clear_training_pattern(struct msm_dp_ctrl_private *ctrl)
> +static void msm_dp_ctrl_clear_training_pattern(struct msm_dp_ctrl_private *ctrl,
> +                                              enum drm_dp_phy dp_phy)
>  {
> -       msm_dp_ctrl_train_pattern_set(ctrl, DP_TRAINING_PATTERN_DISABLE);
> -       drm_dp_link_train_channel_eq_delay(ctrl->aux, ctrl->panel->dpcd);
> +       int delay_us;
> +
> +       msm_dp_ctrl_train_pattern_set(ctrl, DP_TRAINING_PATTERN_DISABLE, dp_phy);
> +
> +       delay_us = drm_dp_read_channel_eq_delay(ctrl->aux,
> +                                               ctrl->panel->dpcd, dp_phy, false);
> +       fsleep(delay_us);
>  }
>
>  static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
> -                       int *training_step)
> +                       int *training_step, enum drm_dp_phy dp_phy)
>  {
> +       int delay_us;
>         int tries = 0, ret = 0;
>         u8 pattern;
>         u32 state_ctrl_bit;
>         int const maximum_retries = 5;
>         u8 link_status[DP_LINK_STATUS_SIZE];
>
> +       delay_us = drm_dp_read_channel_eq_delay(ctrl->aux,
> +                                               ctrl->panel->dpcd, dp_phy, false);
> +
>         msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, 0);
>
>         *training_step = DP_TRAINING_2;
> @@ -1233,12 +1261,12 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
>         if (ret)
>                 return ret;
>
> -       msm_dp_ctrl_train_pattern_set(ctrl, pattern);
> +       msm_dp_ctrl_train_pattern_set(ctrl, pattern, dp_phy);
>
>         for (tries = 0; tries <= maximum_retries; tries++) {
> -               drm_dp_link_train_channel_eq_delay(ctrl->aux, ctrl->panel->dpcd);
> +               fsleep(delay_us);
>
> -               ret = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
> +               ret = drm_dp_dpcd_read_phy_link_status(ctrl->aux, dp_phy, link_status);
>                 if (ret)
>                         return ret;
>
> @@ -1248,7 +1276,7 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
>                 }
>
>                 msm_dp_link_adjust_levels(ctrl->link, link_status);
> -               ret = msm_dp_ctrl_update_vx_px(ctrl);
> +               ret = msm_dp_ctrl_update_phy_vx_px(ctrl, dp_phy);
>                 if (ret)
>                         return ret;
>
> @@ -1257,9 +1285,32 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
>         return -ETIMEDOUT;
>  }
>
> +static int msm_dp_ctrl_link_train_1_2(struct msm_dp_ctrl_private *ctrl,
> +                                     int *training_step, enum drm_dp_phy dp_phy)
> +{
> +       int ret;
> +
> +       ret = msm_dp_ctrl_link_train_1(ctrl, training_step, dp_phy);
> +       if (ret) {
> +               DRM_ERROR("link training #1 on phy %d failed. ret=%d\n", dp_phy, ret);
> +               return ret;
> +       }
> +       drm_dbg_dp(ctrl->drm_dev, "link training #1 on phy %d successful\n", dp_phy);
> +
> +       ret = msm_dp_ctrl_link_train_2(ctrl, training_step, dp_phy);
> +       if (ret) {
> +               DRM_ERROR("link training #2 on phy %d failed. ret=%d\n", dp_phy, ret);
> +               return ret;
> +       }
> +       drm_dbg_dp(ctrl->drm_dev, "link training #2 on phy %d successful\n", dp_phy);
> +
> +       return 0;
> +}
> +
>  static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
>                         int *training_step)
>  {
> +       int i;
>         int ret = 0;
>         const u8 *dpcd = ctrl->panel->dpcd;
>         u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
> @@ -1272,8 +1323,6 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
>         link_info.rate = ctrl->link->link_params.rate;
>         link_info.capabilities = DP_LINK_CAP_ENHANCED_FRAMING;
>
> -       msm_dp_link_reset_phy_params_vx_px(ctrl->link);
> -
>         msm_dp_aux_link_configure(ctrl->aux, &link_info);
>
>         if (drm_dp_max_downspread(dpcd))
> @@ -1288,24 +1337,27 @@ static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
>                                 &assr, 1);
>         }
>
> -       ret = msm_dp_ctrl_link_train_1(ctrl, training_step);
> +       for (i = ctrl->link->lttpr_count - 1; i >= 0; i--) {
> +               enum drm_dp_phy dp_phy = DP_PHY_LTTPR(i);
> +
> +               ret = msm_dp_ctrl_link_train_1_2(ctrl, training_step, dp_phy);
> +               msm_dp_ctrl_clear_training_pattern(ctrl, dp_phy);
> +
> +               if (ret)
> +                       break;
> +       }
> +
>         if (ret) {
> -               DRM_ERROR("link training #1 failed. ret=%d\n", ret);
> +               DRM_ERROR("link training of LTTPR(s) failed. ret=%d\n", ret);
>                 goto end;
>         }
>
> -       /* print success info as this is a result of user initiated action */
> -       drm_dbg_dp(ctrl->drm_dev, "link training #1 successful\n");
> -
> -       ret = msm_dp_ctrl_link_train_2(ctrl, training_step);
> +       ret = msm_dp_ctrl_link_train_1_2(ctrl, training_step, DP_PHY_DPRX);
>         if (ret) {
> -               DRM_ERROR("link training #2 failed. ret=%d\n", ret);
> +               DRM_ERROR("link training on sink failed. ret=%d\n", ret);
>                 goto end;
>         }
>
> -       /* print success info as this is a result of user initiated action */
> -       drm_dbg_dp(ctrl->drm_dev, "link training #2 successful\n");
> -
>  end:
>         msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, 0);
>
> @@ -1622,7 +1674,7 @@ static int msm_dp_ctrl_link_maintenance(struct msm_dp_ctrl_private *ctrl)
>         if (ret)
>                 goto end;
>
> -       msm_dp_ctrl_clear_training_pattern(ctrl);
> +       msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
>
>         msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, DP_STATE_CTRL_SEND_VIDEO);
>
> @@ -1646,7 +1698,7 @@ static bool msm_dp_ctrl_send_phy_test_pattern(struct msm_dp_ctrl_private *ctrl)
>                 return false;
>         }
>         msm_dp_catalog_ctrl_send_phy_pattern(ctrl->catalog, pattern_requested);
> -       msm_dp_ctrl_update_vx_px(ctrl);
> +       msm_dp_ctrl_update_phy_vx_px(ctrl, DP_PHY_DPRX);
>         msm_dp_link_send_test_response(ctrl->link);
>
>         pattern_sent = msm_dp_catalog_ctrl_read_phy_pattern(ctrl->catalog);
> @@ -1888,7 +1940,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
>                         }
>
>                         /* stop link training before start re training  */
> -                       msm_dp_ctrl_clear_training_pattern(ctrl);
> +                       msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
>                 }
>
>                 rc = msm_dp_ctrl_reinitialize_mainlink(ctrl);
> @@ -1912,7 +1964,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
>                  * link training failed
>                  * end txing train pattern here
>                  */
> -               msm_dp_ctrl_clear_training_pattern(ctrl);
> +               msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
>
>                 msm_dp_ctrl_deinitialize_mainlink(ctrl);
>                 rc = -ECONNRESET;
> @@ -1983,7 +2035,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
>                 msm_dp_ctrl_link_retrain(ctrl);
>
>         /* stop txing train pattern to end link training */
> -       msm_dp_ctrl_clear_training_pattern(ctrl);
> +       msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
>
>         /*
>          * Set up transfer unit values and set controller state to send
> --
> 2.45.2
>

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Abel Vesa]

On 25-04-28 20:23:45, Aleksandrs Vinarskis wrote:
> On Mon, 28 Apr 2025 at 16:17, Abel Vesa <abel.vesa@linaro.org> wrote:
> >
> > The change itself makes sense though and I think makes sense to be marked as a fix.
> 
> Just to confirm, you mean to mark as fix only the 1st patch, correct?
> Since it's obvious now that the currently present partial LTTPR
> support did not break anything that used to work.

Well, the way I see it, the LTTPR support is broken on some specific
docks, even if it works in most cases. And since this fix improves
the already working cases and fixes broken ones, yes, add the Fixes tag
to the 1st patch only.

I'd even send that first patch separately to ease the merging, but that's
probably just me.

> 
> Thanks,
> Alex
> 

Abel

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Johan Hovold]

On Mon, Apr 28, 2025 at 05:17:21PM +0300, Abel Vesa wrote:
> On 25-04-28 14:47:04, Johan Hovold wrote:
> > On Mon, Apr 28, 2025 at 11:06:39AM +0200, Aleksandrs Vinarskis wrote:
> > > On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:
> > > > On Thu, Apr 17, 2025 at 04:10:31AM +0200, Aleksandrs Vinarskis wrote:
> > > > > Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> > > > > to non-transparent mode to enable video output on X1E-based devices
> > > > > that come with LTTPR on the motherboards. However, video would not work
> > > > > if additional LTTPR(s) are present between sink and source, which is
> > > > > the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> > > > > some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).
> > > >
> > > > Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> > > > adapters or docks with retimers in transparent mode?

> > > Docks with retimers do not work in 6.15-rcX, but I am unable to verify
> > > if it did work before, as I do not have a Qualcomm based device
> > > without LTTPR on the baseboard.
> > 
> > Abel (or anyone else), do you have one of these docks that you could
> > test with the X13s to confirm whether this series fixes a regression or
> > not?
> 
> Before the support for LTTPRs has been merged, if you would have one of
> those docks (I do not own one) with LTTPRs, link training would've just
> failed if the LTTPRs were not by default in transparent mode, which IIRC
> is what the standard dictates.

Ok, but my concern is if they may have worked in a default transparent
mode.

> X13s doesn't have LTTPRs on-board so when reading the caps, LTTPRs count
> would return 0 and none of the of the transparent/non-transparent setup
> would happen.

But this is the crux; does any off-board LTTPRs in transparent mode add
to the count or not? If they don't, how would you ever learn that there
are any LTTPRs? If they do, it seems we may have a problem here.

Johan

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Aleksandrs Vinarskis]

On Tue, 29 Apr 2025 at 08:52, Abel Vesa <abel.vesa@linaro.org> wrote:
>
> On 25-04-28 20:23:45, Aleksandrs Vinarskis wrote:
> > On Mon, 28 Apr 2025 at 16:17, Abel Vesa <abel.vesa@linaro.org> wrote:
> > >
> > > The change itself makes sense though and I think makes sense to be marked as a fix.
> >
> > Just to confirm, you mean to mark as fix only the 1st patch, correct?
> > Since it's obvious now that the currently present partial LTTPR
> > support did not break anything that used to work.
>
> Well, the way I see it, the LTTPR support is broken on some specific
> docks, even if it works in most cases. And since this fix improves
> the already working cases and fixes broken ones, yes, add the Fixes tag
> to the 1st patch only.

That is not entirely correct. Current LTTPR initialization [1] does
properly set LTTPR to transparent, and then non transparent [2]. In
this case LTTPRs are expected to be trained per link, but are not. It
is reasonable that LTTPR(s) may not work as expected, as they are not
link trained as expected, and now they are not in transparent mode
(anymore).

It does work with X1E onboard LTTPR and a simple external display, but
that is about it. It does not work with _any_ dock that has LTTPR. Or
basically anything that has yet another LTTPR in its way. Given that
max length of DP cable at full bandwidth is 2m, and docks typically
have 50-100cm built in cable for PC connection, not counting outgoing
video connection, this basically means that any DP1.4 and onwards
docking station (not an adapter/dongle) does not work.
Additionally, it appears some fancy monitors with Thundebolt/DP alt
mode ports instead of just DP alt mode also have a retimer onboard
(might have to do with particular monitor support DP-Out for MST), and
also do not work in current setup.
Additionally, it was recently found out that without this series, rhs
USB Type-C on Lenovo Yoga Slim7x does not work either, which likely
implies that IO board on the flex cable for that connector features
yet another retimer.

I wouldn't say it's broken on some specific docks, rather the other
way around, current LTTPR support is limited to 1x LTTPR onboard X1E
devices and nothing else.

Not disagrees wrt to Fixes tag, just wanted to clarify current state
and the impact of this change as it is not limited to just 'specific
docks'.

[1] https://github.com/torvalds/linux/blob/v6.15-rc4/drivers/gpu/drm/msm/dp/dp_display.c#L378
[2] https://github.com/torvalds/linux/blob/v6.15-rc4/drivers/gpu/drm/display/drm_dp_helper.c#L2926

>
> I'd even send that first patch separately to ease the merging, but that's
> probably just me.
>
> >
> > Thanks,
> > Alex
> >
>
> Abel

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Abel Vesa]

On 25-04-29 09:23:46, Johan Hovold wrote:
> On Mon, Apr 28, 2025 at 05:17:21PM +0300, Abel Vesa wrote:
> > On 25-04-28 14:47:04, Johan Hovold wrote:
> > > On Mon, Apr 28, 2025 at 11:06:39AM +0200, Aleksandrs Vinarskis wrote:
> > > > On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:
> > > > > On Thu, Apr 17, 2025 at 04:10:31AM +0200, Aleksandrs Vinarskis wrote:
> > > > > > Recently added Initial LTTPR support in msm/dp has configured LTTPR(s)
> > > > > > to non-transparent mode to enable video output on X1E-based devices
> > > > > > that come with LTTPR on the motherboards. However, video would not work
> > > > > > if additional LTTPR(s) are present between sink and source, which is
> > > > > > the case for USB Type-C docks (eg. Dell WD19TB/WD22TB4), and at least
> > > > > > some universal Thunderbolt/USB Type-C monitors (eg. Dell U2725QE).
> > > > >
> > > > > Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> > > > > adapters or docks with retimers in transparent mode?
> 
> > > > Docks with retimers do not work in 6.15-rcX, but I am unable to verify
> > > > if it did work before, as I do not have a Qualcomm based device
> > > > without LTTPR on the baseboard.
> > > 
> > > Abel (or anyone else), do you have one of these docks that you could
> > > test with the X13s to confirm whether this series fixes a regression or
> > > not?
> > 
> > Before the support for LTTPRs has been merged, if you would have one of
> > those docks (I do not own one) with LTTPRs, link training would've just
> > failed if the LTTPRs were not by default in transparent mode, which IIRC
> > is what the standard dictates.
> 
> Ok, but my concern is if they may have worked in a default transparent
> mode.

But if they are by default in transparent mode, doing the setup to
transparent mode will not break it in any way. At least that is my
understanding of the standard. Also, I tested multiple writes to
transparent mode on CRD back when I wrote the LTTPR msm/dp patches and
doing multiple writes doesn't affect the link training that happens
after in any way.

I do not own such dock though to confirm 100%.

> 
> > X13s doesn't have LTTPRs on-board so when reading the caps, LTTPRs count
> > would return 0 and none of the of the transparent/non-transparent setup
> > would happen.
> 
> But this is the crux; does any off-board LTTPRs in transparent mode add
> to the count or not? If they don't, how would you ever learn that there
> are any LTTPRs? If they do, it seems we may have a problem here.

Count gets increased either way. It doesn't matter if they are in
transparent mode or not.

If they are in transparent mode by default, link training will succeed.
No matter how many times the transparent mode is requested.

> 
> Johan

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Johan Hovold]

On Tue, Apr 29, 2025 at 10:50:55AM +0300, Abel Vesa wrote:
> On 25-04-29 09:23:46, Johan Hovold wrote:
> > On Mon, Apr 28, 2025 at 05:17:21PM +0300, Abel Vesa wrote:
> > > On 25-04-28 14:47:04, Johan Hovold wrote:
> > > > On Mon, Apr 28, 2025 at 11:06:39AM +0200, Aleksandrs Vinarskis wrote:
> > > > > On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:

> > > > > > Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> > > > > > adapters or docks with retimers in transparent mode?

> > Ok, but my concern is if they may have worked in a default transparent
> > mode.
> 
> But if they are by default in transparent mode, doing the setup to
> transparent mode will not break it in any way.

It looks like it will, see below.

> > > X13s doesn't have LTTPRs on-board so when reading the caps, LTTPRs count
> > > would return 0 and none of the of the transparent/non-transparent setup
> > > would happen.
> > 
> > But this is the crux; does any off-board LTTPRs in transparent mode add
> > to the count or not? If they don't, how would you ever learn that there
> > are any LTTPRs? If they do, it seems we may have a problem here.
> 
> Count gets increased either way. It doesn't matter if they are in
> transparent mode or not.

Thanks for confirming. So then it seems we do have a problem as since
6.15-rc1 drm_dp_lttpr_init() will switch all LTTPRs to non-transparent
mode.

Johan

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Aleksandrs Vinarskis]

On Tue, 29 Apr 2025 at 10:03, Johan Hovold <johan@kernel.org> wrote:
>
> On Tue, Apr 29, 2025 at 10:50:55AM +0300, Abel Vesa wrote:
> > On 25-04-29 09:23:46, Johan Hovold wrote:
> > > On Mon, Apr 28, 2025 at 05:17:21PM +0300, Abel Vesa wrote:
> > > > On 25-04-28 14:47:04, Johan Hovold wrote:
> > > > > On Mon, Apr 28, 2025 at 11:06:39AM +0200, Aleksandrs Vinarskis wrote:
> > > > > > On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:
>
> > > > > > > Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> > > > > > > adapters or docks with retimers in transparent mode?
>
> > > Ok, but my concern is if they may have worked in a default transparent
> > > mode.
> >
> > But if they are by default in transparent mode, doing the setup to
> > transparent mode will not break it in any way.
>
> It looks like it will, see below.
>
> > > > X13s doesn't have LTTPRs on-board so when reading the caps, LTTPRs count
> > > > would return 0 and none of the of the transparent/non-transparent setup
> > > > would happen.
> > >
> > > But this is the crux; does any off-board LTTPRs in transparent mode add
> > > to the count or not? If they don't, how would you ever learn that there
> > > are any LTTPRs? If they do, it seems we may have a problem here.
> >
> > Count gets increased either way. It doesn't matter if they are in
> > transparent mode or not.
>
> Thanks for confirming. So then it seems we do have a problem as since
> 6.15-rc1 drm_dp_lttpr_init() will switch all LTTPRs to non-transparent
> mode.

In this case, let me add Fixes to the entire series. Do you think we
could land it in 6.15-rcX then? The second option proposed to roll
back current LTTPR support and wait until 6.16 will completely break
DP output on all X1E, so it's very undesirable.

This series was tested quite a bit on at least the X1E/X1P devices,
both with and without docking stations, as it is also (v2 iirc) part
of Ubuntu's concept tree since little over a month ago. You have
confirmed that x13s also works with this change but without a docking
station. If someone could confirm that x13s with this change does work
with a docking station as well, it would be safe to merge the entire
series as fix to 6.15, correct? I could reach out on #aarch64-laptops,
perhaps someone has both x13s (or another qcom-based non X1(E) device)
and a docking station.

Thanks,
Alex

>
> Johan

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Johan Hovold]

On Tue, Apr 29, 2025 at 12:57:16PM +0200, Aleksandrs Vinarskis wrote:
> On Tue, 29 Apr 2025 at 10:03, Johan Hovold <johan@kernel.org> wrote:
> > On Tue, Apr 29, 2025 at 10:50:55AM +0300, Abel Vesa wrote:
> > > On 25-04-29 09:23:46, Johan Hovold wrote:

> > > > But this is the crux; does any off-board LTTPRs in transparent mode add
> > > > to the count or not? If they don't, how would you ever learn that there
> > > > are any LTTPRs? If they do, it seems we may have a problem here.
> > >
> > > Count gets increased either way. It doesn't matter if they are in
> > > transparent mode or not.
> >
> > Thanks for confirming. So then it seems we do have a problem as since
> > 6.15-rc1 drm_dp_lttpr_init() will switch all LTTPRs to non-transparent
> > mode.
> 
> In this case, let me add Fixes to the entire series. Do you think we
> could land it in 6.15-rcX then?

That's for the msm drm maintainers to answer. It should be possible, and
if we agree (or could confirm) that this breaks existing setups then
this needs to be addressed before 6.15 final is out in some way.

Note that you'd need to rebase on rc4 which doesn't have that return
value rework that's in msm-next.

> The second option proposed to roll
> back current LTTPR support and wait until 6.16 will completely break
> DP output on all X1E, so it's very undesirable.

Right, but it wasn't enabled until 6.15-rc1 so that's technically not a
regression (i.e. since 6.14). Essentially, we'd only wait with enabling
DP for another cycle (and I believe DT changes to enable this never made
it into 6.15 either for most laptops anyway).

> This series was tested quite a bit on at least the X1E/X1P devices,
> both with and without docking stations, as it is also (v2 iirc) part
> of Ubuntu's concept tree since little over a month ago. You have
> confirmed that x13s also works with this change but without a docking
> station. If someone could confirm that x13s with this change does work
> with a docking station as well, it would be safe to merge the entire
> series as fix to 6.15, correct? I could reach out on #aarch64-laptops,
> perhaps someone has both x13s (or another qcom-based non X1(E) device)
> and a docking station.

If it fixes a regression then I would merge it for 6.15.

Johan

Re: drm/msm/dp: Introduce link training per-segment for LTTPRs

[Dmitry Baryshkov]

On Tue, Apr 29, 2025 at 12:57:16PM +0200, Aleksandrs Vinarskis wrote:
> On Tue, 29 Apr 2025 at 10:03, Johan Hovold <johan@kernel.org> wrote:
> >
> > On Tue, Apr 29, 2025 at 10:50:55AM +0300, Abel Vesa wrote:
> > > On 25-04-29 09:23:46, Johan Hovold wrote:
> > > > On Mon, Apr 28, 2025 at 05:17:21PM +0300, Abel Vesa wrote:
> > > > > On 25-04-28 14:47:04, Johan Hovold wrote:
> > > > > > On Mon, Apr 28, 2025 at 11:06:39AM +0200, Aleksandrs Vinarskis wrote:
> > > > > > > On Mon, 28 Apr 2025 at 09:45, Johan Hovold <johan@kernel.org> wrote:
> >
> > > > > > > > Does this mean that the incomplete LTTPR support in 6.15-rc1 broke
> > > > > > > > adapters or docks with retimers in transparent mode?
> >
> > > > Ok, but my concern is if they may have worked in a default transparent
> > > > mode.
> > >
> > > But if they are by default in transparent mode, doing the setup to
> > > transparent mode will not break it in any way.
> >
> > It looks like it will, see below.
> >
> > > > > X13s doesn't have LTTPRs on-board so when reading the caps, LTTPRs count
> > > > > would return 0 and none of the of the transparent/non-transparent setup
> > > > > would happen.
> > > >
> > > > But this is the crux; does any off-board LTTPRs in transparent mode add
> > > > to the count or not? If they don't, how would you ever learn that there
> > > > are any LTTPRs? If they do, it seems we may have a problem here.
> > >
> > > Count gets increased either way. It doesn't matter if they are in
> > > transparent mode or not.
> >
> > Thanks for confirming. So then it seems we do have a problem as since
> > 6.15-rc1 drm_dp_lttpr_init() will switch all LTTPRs to non-transparent
> > mode.
> 
> In this case, let me add Fixes to the entire series. Do you think we
> could land it in 6.15-rcX then? The second option proposed to roll
> back current LTTPR support and wait until 6.16 will completely break
> DP output on all X1E, so it's very undesirable.

It should be possible, if you rebase onto 6.15-rc, add Fixes tags and
send it quickly enough.

> This series was tested quite a bit on at least the X1E/X1P devices,
> both with and without docking stations, as it is also (v2 iirc) part
> of Ubuntu's concept tree since little over a month ago. You have
> confirmed that x13s also works with this change but without a docking
> station. If someone could confirm that x13s with this change does work
> with a docking station as well, it would be safe to merge the entire
> series as fix to 6.15, correct? I could reach out on #aarch64-laptops,
> perhaps someone has both x13s (or another qcom-based non X1(E) device)
> and a docking station.
> 
> Thanks,
> Alex
> 
> >
> > Johan

-- 
With best wishes
Dmitry