lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMcHhXoS0NADUmM7MuDSCbkjrpnbyv53dNj3NiJr-tvzhVKsGw@mail.gmail.com>
Date: Thu, 8 May 2025 00:01:09 +0200
From: Aleksandrs Vinarskis <alex.vinarskis@...il.com>
To: Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc: Dmitry Baryshkov <lumag@...nel.org>, linux-arm-msm@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, dmitry.baryshkov@....qualcomm.com, 
	Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>, 
	Marijn Suijten <marijn.suijten@...ainline.org>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, laurentiu.tudor1@...l.com, abel.vesa@...aro.org, 
	johan@...nel.org, Johan Hovold <johan+linaro@...nel.org>, 
	Stefan Schmidt <stefan.schmidt@...aro.org>
Subject: Re: [PATCH v4 4/4] drm/msm/dp: Introduce link training per-segment
 for LTTPRs

On Tue, 6 May 2025 at 01:41, Abhinav Kumar <quic_abhinavk@...cinc.com> wrote:
>
> Hi Alex
>
> On 5/4/2025 3:06 PM, Aleksandrs Vinarskis wrote:
> > On Sun, 4 May 2025 at 05:02, Abhinav Kumar <quic_abhinavk@...cinc.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_abhinavk@...cinc.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+linaro@...nel.org>
> >>>>> Tested-by: Rob Clark <robdclark@...il.com>
> >>>>> Tested-by: Stefan Schmidt <stefan.schmidt@...aro.org>
> >>>>> Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@...il.com>
> >>>>> Reviewed-by: Abel Vesa <abel.vesa@...aro.org>
> >>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@....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 is in the section "3.6.7.2 8b/10b DP Link Layer LTTPR Link Training
> Mandates" of DP spec 2.1(a)"

Perfect, thanks.

>
> >>
> >> 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.
> >
>
> Yes. This was good to know.
>
> >>
> >> 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? :)
> >
>
> Wow, thats a very good finding!
>
> > 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.
> >
>
> Yes seems like it but certainly looks like a bug in PS8830.
>
> >>
> >> 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.
> >
>
> Yes correct, nothing else pending from your side.
>
> Thanks for your deep analysis and interest in this topic.
>
> > Thanks for your help,
> > Alex
> >
>
> By waiting for Abel, I am mostly trying to make sure :
>
> Was enabling non-transparent mode more of a requirement of the parade
> retimer chip in Xelite? Because from our earlier discussion, I thought
> we wanted to enable transparent mode. Then these issues would perhaps
> have not happened as per-segment link training is a requirement of
> non-transparent mode. So I am surprised how Xelite worked without this.
>
> It seems like to me we enabled non-transparent mode to match AMD/i915
> behavior and not as a requirement of the retimer chip of Xelite.
>
> The commit text of
> https://patchwork.freedesktop.org/patch/msgid/20250203-drm-dp-msm-add-lttpr-transparent-mode-set-v5-4-c865d0e56d6e@linaro.org
> mentions
>
> "The section 3.6.6.1 from the DisplayPort v2.0 specification mandates
> that before link training with the LTTPR is started, the DPTX may place
> the LTTPR in non-transparent mode by first switching to transparent mode
> and then to non-transparent mode. This operation seems to be needed only
> on first link training and doesn't need to be done again until device is
> unplugged."
>
> This talks about how to enable non-transparent mode and not why.
>
> But this part "It has been observed on a few X Elite-based platforms
> which have such LTTPRs in their board design that the DPTX needs to
> follow the procedure described above in order for the link training to
> be successful" is really my doubt. Because from my earlier
> understanding, I thought enabling transparent mode was enough.

To speed up the process a little as the 6.15-rcX window shrinks (and
it appears Abel may be OOO?), I run a series of tests to attempt to
answer your questions. In short - PS8830 is a very quirky device, and
you were right that the current implementation could've simply set
transparent mode.

To clarify the test matrix: PS8830 was tested with Dell XPS 9345.
PS8833 was tested with Asus Zenbook A14. Unfortunately, my dock (Dell
WD19TB) is a universal Thunderbolt/DP-Alt mode dock, and only works if
it's forced to DP alt mode, since PCIE tunneling is not yet supported
on qcom. Dell allows to disable thunderbolt/PCIE tunneling support in
BIOS, hence forcing the dock to be Type-C/DP-alt mode (and show up
with LTTPR onboard). No such feature exists on Asus, so I could not
test PS8833 with the docking station at all.

Complete test matrix;

1. Do nothing/pre Abel's series (LTTPRs assumed to be in transparent
mode unless firmware pre-configured them):
PS8833:
- Type-C to HDMI: works almost always
- Type-C to DP: never works, EDID read fails

PS8830:
- Type-C to HDMI: never works, CR LT fails (-22)
- Type-C to DP: never works, EDID read fails
- Type-C to dock: never works, EQ LT fails (-110)

2. Explicitly set LTTPRs to transparent mode (early exit LTTPR helper
introduced by Abel, after setting transparent mode, but before setting
non-transparent mode):
PS8833:
- Type-C to HDMI: works. Occasionally fails to get DP sink modes (may
be unrelated)
- Type-C to DP: works**

PS8830:
- Type-C to HDMI: works
- Type-C to DP: works**
- Type-C to dock: Sometimes all works. Sometimes video works, but USB
(2.0 nor 3.0) is not working. Sometimes EQ LT fails (-110) and nothing
works. Overall extremely unstable.

3. Explicitly set LTTPRs to non-transparent mode (aka Abel's series):
PS8833:
- Type-C to HDMI: never works, CR LT fails, max v_level reached (-11)
- Type-C to DP: never works, CR LT fails, max v_level reached (-11)

PS8830:
- Type-C to HDMI: works
- Type-C to DP: works**
- Type-C to dock: never works, CR LT fails (-110)

4. Explicitly set LTTPRs to non-transparent mode, support per-segment
training (aka Abel's initial LTTPR support series + this series):
PS8833:
- Type-C to HDMI: works
- Type-C to DP: works

PS8830:
- Type-C to HDMI: works
- Type-C to DP: works
- Type-C to dock: works


** At first, Type-C to DP was frequently/always depending on the use
case failing to read panel EDID, just like in the 1st test case. As I
am 100% certain it worked in the past, did a few more tests. It
appears that in an earlier version of Abel's patch (<=v4), DPRX caps
were read _after_ LTTPR init, just like DP standard mandates (don't
have exact quote, something along the lines 'source shall re-read sink
caps after LTTPR init'). In v4 there was a suggestion [1] (from you
actually :)) to first try to read DPRX caps, then init LTTPRs in order
to fail early if caps readout fails. Reverting this change fixes EDID
read error. Since I was running Abel's series long before it landed, I
never used the broken v5. With the order of functions reverted, Type-C
to DP started working/failing in the same way Type-C to HDMI dongle
did, just as expected. Wrt to the issue itself, the first patch of
this very series actually both fixes this issue by conforming to DP
spec, and also takes into account your suggestion in Abel's v4 series
to be able to fail early in case of DPRX caps readout failure.

To summarize the findings:
- PS8830 is a very quirky device. It does not work in
default/transparent mode unless explicitly set. It does work in
non-transparent mode without per-segment training, even though it
should've not. As per last email, it is lying about not supporting
training level 3.
- PS8833 seems to be a fixed version of PS8830. It does work in
default/transparent mode oob. It does not work in non-transparent mode
without per-segment training, just as expected. As per last email, it
correctly reports training level 3 capability.

To answer some of your questions (from a 3rd party view, cannot speak
for the authors):
- "So I am surprised how Xelite worked without this." - From tests
above: PS8830 worked when it should've not, seems because it's quirky.
PS8833 did not work, which makes sense.
- Doubts about non-transparent mode requirement for X1E/X1P systems -
>From tests above: seems you are right. I don't know why it was forced
to be non-transparent without per-segment training, instead of simply
transparent. Though, seeing how weird the PS8830 is, I wouldn't be
surprised if it behaves differently in other laptops... just
speculating though.
 - "Was enabling non-transparent mode more of a requirement of the
parade retimer chip in Xelite?" - cannot fully answer. Initializing
LTTPRs as such appears to be a requirement of the Parade PS8830 (not
PS8833, which wasn't around back then afaik), as it just wouldn't work
oob. Choice of non-transparent instead of transparent mode is not very
clear to me.

Even though it appears initial LTTPR support could've been done
slightly differently, combination of those initial patches + this
series seem to provide both the best practical results, as well as
most (well, almost, excluding LTTPR's train level verification)
accurate LTTPR implementation, while also making msm-dp similar/up to
date with Intel/AMD's LTTPR implementation. Also learned something new
today myself, don't buy PS8830 :)

Looking forward to hearing from you,
Alex

[1] https://lore.kernel.org/all/feb4f780-8fe6-426b-9ba4-ab1fb102ac27@quicinc.com/




>
> > [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
> >
> >
> >>
>
> <snip>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ