On Mon, 5 May 2025 at 00:06, Aleksandrs Vinarskis
<alex.vinars...@gmail.com> wrote:
>
> On Sun, 4 May 2025 at 05:02, Abhinav Kumar <quic_abhin...@quicinc.com> wrote:
> >
> > Hi Alex
> >
> > Thanks for the response.
> >
> > My updates below. I also had one question for Abel below.
> >
> > Thanks
> >
> > Abhinav
> >
> > On 5/1/2025 8:56 AM, Aleksandrs Vinarskis wrote:
> > > On Thu, 1 May 2025 at 04:11, Abhinav Kumar <quic_abhin...@quicinc.com>
> > > wrote:
> > >>
> > >>
> > >>
> > >> On 4/29/2025 5:09 PM, 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).
> > >>>
> > >>> Fixes: 72d0af4accd9 ("drm/msm/dp: Add support for LTTPR handling")
> > >>>
> > >>
> > >> Thanks for the patch to improve and add support for link training in
> > >> non-transparent mode.
> > >>
> > >> Some questions below as the DP 2.1a spec documentation is not very clear
> > >> about segmented link training as you noted in the cover letter, so I am
> > >> also only reviewing i915 as reference here.
> > >>
> > >>
> > >>> Tested-by: Johan Hovold <johan+lin...@kernel.org>
> > >>> Tested-by: Rob Clark <robdcl...@gmail.com>
> > >>> Tested-by: Stefan Schmidt <stefan.schm...@linaro.org>
> > >>> Signed-off-by: Aleksandrs Vinarskis <alex.vinars...@gmail.com>
> > >>> Reviewed-by: Abel Vesa <abel.v...@linaro.org>
> > >>> Reviewed-by: Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com>
> > >>> ---
> > >>> 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 d8633a596f8d..35b28c2fcd64 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);
> > >>
> > >> For the max voltage and swing levels, it seems like we need to use the
> > >> source (DPTX) or the DPRX immediately upstream of the RX we are trying
> > >> to train. i915 achieves it with below:
> > >>
> > >> /*
> > >> * Get voltage_max from the DPTX_PHY (source or LTTPR) upstream
> > >> from
> > >> * the DPRX_PHY we train.
> > >> */
> > >> if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
> > >> voltage_max = intel_dp->voltage_max(intel_dp,
> > >> crtc_state);
> > >> else
> > >> voltage_max = intel_dp_lttpr_voltage_max(intel_dp,
> > >> dp_phy + 1);
> > >>
> >
> > Before I update on the below set of questions from Alex, let me clarify
> > one point from Abel.
> >
> > Hi Abel
> >
> > Apologies to ask this late, but as per the earlier discussions we had
> > internally, I thought we wanted to set the LTTPR to transparent mode to
> > avoid the issues. The per-segment link training becomes a requirement if
> > we use non-transparent mode iiuc.
> >
> > In the description of the PHY_REPEATER_MODE DPCD register, it states
> > like below:
> >
> > "A DPTX operating with 8b/10b Link Layer (MAIN_LINK_CHANNEL_CODING_SET
> > register (DPCD Address 00108h) is programmed to 01h) may configure LTTPRs
> > to either Transparent (default) or Non-transparent mode.
> > A DPTX that establishes the DP link with 128b/132b channel coding shall
> > write
> > 02h to the MAIN_LINK_CHANNEL_CODING_SET register and configure LTTPRs
> > to Non-transparent mode."
> >
> > As per the msm dp code, we are using 8b/10b encoding, like below
> >
> > static int msm_dp_ctrl_link_train(struct msm_dp_ctrl_private *ctrl,
> > int *training_step)
> > {
> > int ret = 0;
> > const u8 *dpcd = ctrl->panel->dpcd;
> > u8 encoding[] = { 0, DP_SET_ANSI_8B10B };
> >
> > So can you pls elaborate why we set the PHY_REPEATER_MODE to
> > non-transparent mode because drm_dp_lttpr_init() will set the LTTPR to
> > non-transparent mode.
> >
> > The second part of the section is what was described in the commit text
> > of the 72d0af4accd9 ("drm/msm/dp: Add support for LTTPR handling") but
> >
> > "Before performing link training with LTTPR(s), the DPTX may place the
> > LTTPR(s) in
> > Non-transparent mode by first writing 55h to the PHY_REPEATER_MODE
> > register, and then
> > writing AAh. This operation does not need to be performed on subsequent
> > link training actions
> > unless a downstream device unplug event is detected."
> >
> > So just wanted to understand this better that was there any requirement
> > to put it to non-transparent mode other than the section of the spec
> > highlighted above? Because above lines are only suggesting that if we
> > want to put the LTTPR to non-transparent mode, how to do it but not to
> > always do it. Please let me know your comments.
> >
> > I shall also check internally on this to close this.
> >
> >
> > Hi Alex
> >
> > >>
> > >> But I do not see (unless I missed) how this patch takes care of this
> > >> requirement.
> > >>
> > >> Same holds true for preemph too
> > >
> > > Thanks for you review,
> > >
> > > This is a very good point. You are right, in the present state it does
> > > not. Intel's driver is verifying whether LTTPRs supports
> > > DP_TRAIN_LEVEL_3 or only DP_TRAIN_LEVEL_2, while my current change
> > > follows msm-dp's default which was recently set to DP_TRAIN_LEVEL_3
> > > [1]. I came to conclusion that in particular case it was not required
> > > to verify that LTTPR indeed supports training level 3, but do not
> > > remember the details as its been a few months... should've document it
> > > :)
> > >
> >
> > > As I recall, from one of the DP specs onward (has to be 1.4a then,
> > > since LTTPR was initially introduced in DP 1.3, but register for phy
> > > capabilities only added in 1.4a [2]) it mandates training level 3
> > > support for LTTPRs, so the assumption would've be correct in that
> > > case. Is this something you could verify from the official
> > > documentation? Unfortunately I do not have sources to back this
> > > statement, so it may be incorrect...
> > >
> >
> > I went through DP spec 1.4(a), DP 2.0 and DP 2.1(a). This is what is
> > mentioned below:
> >
> >
> > "LTTPR shall support all required voltage swing and pre-emphasis
> > combinations defined
> > in Table 3-2. The LTTPR shall reflect its support of optional Voltage
> > Swing Level 3
> > and Pre-emphasis Level 3 in the VOLTAGE_SWING_LEVEL_3_SUPPORTED and
> > VOLTAGE_SWING_LEVEL_3_SUPPORTED bits, respectively, in the
> > TRANSMITTER_CAPABILITY_PHY_REPEATERx register(s) (e.g., DPCD
> > Address F0021h for LTTPR1, bits 0 and 1, respectively)."
> >
> > From this paragraph, it means that LTTPR support for levels 0/1/2 can
> > be assumed and level 3 is optional. Whether or not level 3 is supported
> > comes from the TRANSMITTER_CAPABILITY_PHY_REPEATERx register(s).
> >
> > This aligns with i915 implementation.
> >
> >
> > Now, right after this, there is another paragraph in the spec:
> >
> > "If the DPTX sets the voltage swing or pre-emphasis to a level that the
> > LTTPR does not support,
> > the LTTPR shall set its transmitter levels as close as possible to those
> > requested by the DPTX.
> > Although the LTTPR’s level choosing is implementation-specific, the
> > levels chosen shall
> > comply with Section 3.5.4."
>
> Hi Abhinav,
>
> Could you please provide the exact section number and DP spec version
> for this paragraph? For reference in the commit message, see below.
>
> >
> > This tells us that even if we try to do a level3 and the LTTPR does not
> > support it, it will use the one closest to this.
> >
> > So overall, even though i915's implementation is the accurate one, the
> > DP spec does mention that the LTTPR can adjust. I just hope all LTTPRs
> > can adjust to this. Hopefully this clarifies the requirements spec-wise.
>
> Thanks for this clarification, this is extremely useful. A bit sad
> that DP spec is only available to VESA members.
> So my assumption was indeed incorrect. This also explains why eg.
> AMD's driver works, nice.
>
> >
> > Hence I am okay with this change as such as multiple folks including us
> > have given a Tested-by but I would like this to be documented in the
> > commit text so that full context is preserved. The only concern I have
> > is I hope that the level to which the LTTPR adjusts will be correct as
> > that again is "implementation specific".
>
> I started implementing i915's approach meanwhile, to see the
> difference in behaviour. POC fixup for patch 3,4 of this series can be
> found in [1]. Discovered something interesting:
> * Dell WD19TB docking station's LTTPR reports support of training level 3
> * PS8833 retimer in Asus Zenbook A14 reports support of training level 3
> * PS8830 retimer in Dell XPS 9345 claims to _not_ report support
> training level 3. This is the case on two different machines with BIOS
> 1.9.0 (PS8830 payload version 5.3.0.14) and BIOS 2.5.0 (PS8830 payload
> version 9.3.0.01).
>
> This leads to interesting test results:
> * Asus Zenbook A14 (PS8833, supports train level 3) with direct
> monitor connection via Type-C works, both in current version of msm-dp
> (aka AMD's approach) and with additional patches I linked above (aka
> i915's approach)
> * Dell XPS 9345 (PS8830, claims to not support train level 3) with
> Dell WD19TB (supports train level 3) works, both in current version of
> msm-dp and with additional patches I linked above. In this
> combination, PS8830->WD19TB segment training succeeds with vs=2, pe=0
> already.
> * Dell XPS 9345 (PS8830, claims to not support train level 3) with
> direct monitor connection via Type-C works with current version of
> msm-dp, but does _not_ work with additional patches I linked above.
> For PS8830->Monitor segment training, after reaching vs=2,pe=0 and
> being stopped from going higher (due to PS8830 claiming it cannot do
> train level 3), link training fails. With current msm-dp state
> however, the same PS8830->Monitor segment training succeeds with
> vs=2,pe=1. This is contrary to retimer reporting it does not support
> train level 3 - it in fact does, and in case with 1m long Type-C to DP
> cable it only works with train level 3. Bug in P8830's LTTPR
> implementation? :)
>
> Combining both patches linked above as well as debug patch to force
> max train level to 3 like it was before [2], here are detailed logs:
> Asus Zenbook A14, BIOS version "UX3407QA.305":
> ```
> phy #1: params reset #
> training DPRX (phy1/PS8833)
> phy #1: max_v_level=3, max_p_level=3 # DPTX source
> (X1E) supports train level 3
> phy #1: forcing max_v_level=3, max_p_level=3
> phy #1: v_level=0, p_level=0 #
> passes with vs=0,ps=0
> phy #1: max_v_level=3, max_p_level=3
> phy #0: params reset
> # training DPRX (phy0/Monitor)
> phy #0: max_v_level=3, max_p_level=3 # DPTX source
> (phy1/PS8833) supports train level 3
> phy #0: forcing max_v_level=3, max_p_level=3
> phy #0: v_level=0, p_level=0
> phy #0: max_v_level=3, max_p_level=3
> phy #X: v_level=2, p_level=0
> phy #0: v_level=2, p_level=0
> phy #0: max_v_level=3, max_p_level=3
> phy #X: v_level=2, p_level=1
> phy #0: v_level=2, p_level=1 #
> training passes with vs=2,ps=1
> phy #0: max_v_level=3, max_p_level=3
> ```
>
> Dell XPS 9345, BIOS version "2.5.0", PS8830 payload version "9.3.0.01":
> ```
> phy #1: params reset #
> training DPRX (phy1/PS8830)
> phy #1: max_v_level=3, max_p_level=3 # DPTX source
> (X1E) supports train level 3
> phy #1: forcing max_v_level=3, max_p_level=3
> phy #1: v_level=0, p_level=0 #
> passes with vs=0,ps=0
> phy #1: max_v_level=3, max_p_level=3
> phy #0: params reset #
> training DPRX (phy0/Monitor)
> phy #0: max_v_level=2, max_p_level=2 # DPTX source
> (phy1/PS8830) claims to not support train level 3
> phy #0: forcing max_v_level=3, max_p_level=3 # Ignore
> advertised levels, force to max=3, otherwise training fails
> phy #0: v_level=0, p_level=0
> phy #0: max_v_level=3, max_p_level=3
> phy #X: v_level=2, p_level=0
> phy #0: v_level=2, p_level=0
> phy #0: max_v_level=3, max_p_level=3
> phy #X: v_level=2, p_level=1
> phy #0: v_level=2, p_level=1 #
> training passes with vs=2,ps=1 (aka train level 3)
> phy #0: max_v_level=3, max_p_level=3
> ```
>
> While, as you correctly mentioned, i915's implementation would be a
> more accurate one, and I can respin to v5 with [1] applied to patches
> 3,4 of this series respectively, it appears that at least on some X1E
> based devices with PS8830 that would break DP output support at least
> in some cases. The fact that the same device with the same monitor
> works on Windows suggests that Windows driver also uses AMD's approach
> of just assuming LTTPR can do train level 3, without verifying it, and
> letting LTTPR figure the rest. I have asked other community members to
> cross-check these findings on another X1E platform with PS8830
> retimers. With this in mind, I am very glad to hear that you are okay
> with this change as such, as it now appears that a more accurate
> implementation would've caused additional issues.
Cross-confirmed on X1E DevKit with PS8830, and HP Omnibook X14 with
PS8830 - in both cases PS8830 reports max train level as 2 instead of
3. In the case of DevKit, training of phy0 (monitor) was already
passing with vs=2,pe=0 (source phy1/ps8830). In case of HP Omnibook
with some dongles attached, in the worst case training of phy0
(monitor) passed with vs=3,pe=0 (source phy1/ps8830), thus confirming
that PS8830 indeed supports training level 3, but reports otherwise in
its capabilities.
Alex
>
> >
> > I would still like to hear from Abel though about whether setting to
> > non-transparent mode was needed in the first place.
>
> Fwiw, without Abel's initial change DP output didn't work on X1E
> platform at all, neither direct monitor connection nor docking
> station. Not sure if that is because PS883x found in most X1E/X1P
> laptops do not work in transparent mode at all (even though they
> should've), or laptop's firmware would leave it in some weird state,
> and perhaps re-enabling transparent mode would've also fixed it.
>
> Lets wait for Abel's answer and the rest of this conversation to be
> resolved, and as I see it the next step would be for me to respin to
> v5 current change as is, in order to update the commit message of 4th
> patch to reflect the new findings and reference DP spec and section,
> as per the first comment of this reply.
>
> Thanks for your help,
> Alex
>
> [1]
> https://github.com/alexVinarskis/linux-x1e80100-dell-tributo/tree/msm/dp-lttpr-segment-lt
> [2]
> https://github.com/alexVinarskis/linux-x1e80100-dell-tributo/tree/msm/dp-lttpr-segment-lt-debug
>
>
> >
> > Thanks
> >
> > Abhinav
> > > Now reviewing it again, my reasoning may to be wrong, as source
> > > supporting training level 3 and DP 1.4a does not necessarily imply
> > > that external LTTPR does, nor that external LTTPR is DP 1.4a
> > > compliant.
> > >
> > > fwiw, after quickly inspecting AMD's driver it seems it also assumes
> > > DP_TRAIN_LEVEL_3 support for LTTPR and does not explicitly verify it.
> > > Similarly to proposed msm solution, iteration over phys [3] calls
> > > `perform_8b_10b_clock_recovery_sequence` [4] which is generic for both
> > > DPRX and LTTPR(s). This eventually calls `dp_is_max_vs_reached` [5] to
> > > check against hardcoded value of 3 [6]. Generally, it appears no other
> > > driver use `
> > > drm_dp_lttpr_voltage_swing_level_3_supported` or
> > > `drm_dp_lttpr_pre_emphasis_level_3_supported` helpers introduced by
> > > Intel, nor directly use register 0xf0021.
> > >
> > > Alternatively, if we cannot verify that LTTPR is expected to always
> > > support DP_TRAIN_LEVEL_3, I change this patch to match Intel's example
> > > of retrieving max vs and pe per phy. As it appears to be a bit time
> > > sensitive, can have it done and re-tested on all available hardware by
> > > Monday. Please let me know your thoughts.
> > >
> > > Thanks,
> > > Alex
> > >
> > > [1]
> > > https://lore.kernel.org/all/20240203-dp-swing-3-v1-1-6545e1706...@linaro.org/
> > > [2] https://patchwork.freedesktop.org/patch/329863/
> > > [3]
> > > https://github.com/torvalds/linux/blob/v6.15-rc4/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training_8b_10b.c#L396-L430
> > > [4]
> > > https://github.com/torvalds/linux/blob/v6.15-rc4/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training_8b_10b.c#L176-L294
> > > [5]
> > > https://github.com/torvalds/linux/blob/v6.15-rc4/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c#L462-L475
> > > [6]
> > > https://github.com/torvalds/linux/blob/v6.15-rc4/drivers/gpu/drm/amd/display/dc/dc_dp_types.h#L80
> > >
> > >>
> > >> if (intel_dp_phy_is_downstream_of_source(intel_dp, dp_phy))
> > >> preemph_max = intel_dp->preemph_max(intel_dp);
> > >> else
> > >> preemph_max = intel_dp_lttpr_preemph_max(intel_dp,
> > >> dp_phy + 1);
> > >>
> > >> drm_WARN_ON_ONCE(display->drm,
> > >> preemph_max != DP_TRAIN_PRE_EMPH_LEVEL_2 &&
> > >> preemph_max != DP_TRAIN_PRE_EMPH_LEVEL_3);
> > >>
> > >>
> > >>> 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,7 +1104,12 @@ 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;
> > >>> }
> > >>>
> > >>> @@ -1115,12 +1128,16 @@ static int msm_dp_ctrl_read_link_status(struct
> > >>> msm_dp_ctrl_private *ctrl,
> > >>> }
> > >>>
> > >>> 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;
> > >>> @@ -1129,18 +1146,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);
> > >>> if (ret)
> > >>> return ret;
> > >>>
> > >>> @@ -1161,7 +1179,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;
> > >>> }
> > >>> @@ -1213,21 +1231,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;
> > >>> @@ -1247,12 +1275,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 = msm_dp_ctrl_read_link_status(ctrl, link_status);
> > >>> + ret = drm_dp_dpcd_read_phy_link_status(ctrl->aux, dp_phy,
> > >>> link_status);
> > >>> if (ret)
> > >>> return ret;
> > >>>
> > >>> @@ -1262,7 +1290,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;
> > >>>
> > >>> @@ -1271,9 +1299,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 };
> > >>> @@ -1286,8 +1337,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))
> > >>> @@ -1302,24 +1351,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);
> > >>>
> > >>> @@ -1636,7 +1688,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);
> > >>>
> > >>> @@ -1660,7 +1712,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);
> > >>> @@ -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);
> > >>> }
> > >>>
> > >>> rc = msm_dp_ctrl_reinitialize_mainlink(ctrl);
> > >>> @@ -1926,7 +1978,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;
> > >>> @@ -1997,7 +2049,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
> > >>
> >
