Re: [PATCH v2 2/2] drm/msm/dp: Introduce link training per-segment for LTTPRs
From: Dmitry Baryshkov
Date: Wed Apr 09 2025 - 21:50:30 EST
On 09/04/2025 01:29, Aleksandrs Vinarskis wrote:
On Tue, 1 Apr 2025 at 02:55, Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxxxxxxxx> wrote:
On Wed, Mar 12, 2025 at 12:38:04AM +0100, 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).
Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@xxxxxxxxx>
Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 137 +++++++++++++++++++---------
drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 4 +-
3 files changed, 99 insertions(+), 44 deletions(-)
[...]
@@ -1129,18 +1144,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 = msm_dp_ctrl_read_link_status(ctrl, link_status);
+ ret = drm_dp_dpcd_read_phy_link_status(ctrl->aux, dp_phy, link_status);
Please rebase this code on top of drm-misc-next.
What is the relation of drm-misc-next to linux-next? When rebasing on
top of drm-misc-next, I lose all displays including internal one. Same
if just build drm-misc-next without this series with config imported
from linux-next. I could of course address comments, test on
linux-next and then rebase before submitting, but that sounds wrong.
Usually drm-misc-next is a part of the linux-next. Except the time
between -rc6 (or -rc7) and -rc1, when the drm-misc-next gets new
patches, but they are not propagated to the linux-next.
As we are past -rc1, linux-next should be getting drm-misc-next as
usual. So, please just rebase onto the linux-next. Be sure to account
for linux-next
```
auxiliary aux_bridge.aux_bridge.0: deferred probe pending:
aux_bridge.aux_bridge: failed to acquire drm_bridge
auxiliary aux_bridge.aux_bridge.1: deferred probe pending:
aux_bridge.aux_bridge: failed to acquire drm_bridge
```
if (ret)
return ret;
[...]
@@ -1902,7 +1954,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);
Just DPRX or should this include all LTTPRs? Could you point out how
this is handled inside Intel or AMD drivers?
Just DPRX since this call follows `rc =
msm_dp_ctrl_setup_main_link(ctrl, &training_step);` [1], which in turn
calls `msm_dp_ctrl_link_train` [2].
The latter one with the proposed changes will attempt to Train
LTTPRx->Clear training pattern on LTTPRx->Proceed. Finally, it will
attempt to Train DPRX, without cleaning the training pattern:
```
for (i = *ctrl->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 of LTTPR(s) failed. ret=%d\n", ret);
goto end;
}
ret = msm_dp_ctrl_link_train_1_2(ctrl, training_step, DP_PHY_DPRX);
if (ret) {
DRM_ERROR("link training on sink failed. ret=%d\n", ret);
goto end;
}
```
The reason for not clearing training pattern on DPRX right after
training like with LTTPRs appears to be needed for compliance, as it
should only be cleared right before stream starts [3]:
```
if (ctrl->link->sink_request & DP_TEST_LINK_PHY_TEST_PATTERN)
return rc;
if (rc == 0) { /* link train successfully */
/*
* do not stop train pattern here
* stop link training at on_stream
* to pass compliance test
*/
} else {
/*
* link training failed
* end txing train pattern here
*/
msm_dp_ctrl_clear_training_pattern(ctrl, DP_PHY_DPRX);
msm_dp_ctrl_deinitialize_mainlink(ctrl);
rc = -ECONNRESET;
}
```
Intel does a somewhat similar approach - they have
`intel_dp_link_train_all_phys` function [4] which would Train
LTTPRx->Clear dpcd training pattern on LTTPRx->Proceed, and finally
train DPRX but not disable training pattern. DPRX's training is
disabled separately in the `intel_dp_stop_link_train` [5] at a much
later stage.
Ack, thanks.
The difference to msm's drm driver is that in case of link training
failure, Intel schedules software hpd event [6] and exists, while msm
stops and restarts training with reduced parameters internally (this
very function), hence it appears more than once.
[1] https://github.com/torvalds/linux/blob/v6.14/drivers/gpu/drm/msm/dp/dp_ctrl.c#L1856
[2] https://github.com/torvalds/linux/blob/v6.14/drivers/gpu/drm/msm/dp/dp_ctrl.c#L1273
[3] https://github.com/torvalds/linux/blob/v6.14/drivers/gpu/drm/msm/dp/dp_ctrl.c#L1917-L1932
[4] https://github.com/torvalds/linux/blob/v6.14/drivers/gpu/drm/i915/display/intel_dp_link_training.c#L1338-L1364
[5] https://github.com/torvalds/linux/blob/v6.14/drivers/gpu/drm/i915/display/intel_dp_link_training.c#L1107-L1136
[6] https://github.com/torvalds/linux/blob/v6.14/drivers/gpu/drm/i915/display/intel_dp_link_training.c#L1313-L1336
}
rc = msm_dp_ctrl_reinitialize_mainlink(ctrl);
--
With best wishes
Dmitry