[PATCH v2 2/2] drm/msm/dp: Introduce link training per-segment for LTTPRs
Dmitry Baryshkov
dmitry.baryshkov at oss.qualcomm.com
Thu Apr 10 01:48:39 UTC 2025
On 09/04/2025 01:29, Aleksandrs Vinarskis wrote:
> On Tue, 1 Apr 2025 at 02:55, Dmitry Baryshkov
> <dmitry.baryshkov at oss.qualcomm.com> 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 at gmail.com>
>>> Reviewed-by: Abel Vesa <abel.vesa at linaro.org>
>>> ---
>>> 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
More information about the Freedreno
mailing list