On Wed, Jul 09, 2025 at 07:57:15AM +0200, Paul Menzel wrote:
> Dear Imre,
>
> Thank you very much for your patch, and the detailed commit message.
>
> Am 08.07.25 um 23:23 schrieb Imre Deak:
> > Commit a40c5d727b81 ("drm/dp: Change AUX DPCD probe address from
> > DPCD_REV to LANE0_1_STATUS") stopped using the DPCD_REV register for
> > DPCD probing, since this results in link training failures at least when
> > using an Intel Barlow Ridge TBT hub at UHBR link rates (the
> > DP_INTRA_HOP_AUX_REPLY_INDICATION never getting cleared after the failed
> > link training). Since accessing DPCD_REV during link training is
> > prohibited by the DP Standard, LANE0_1_STATUS (0x202) was used instead,
> > as it falls within the Standard's valid register address range
> > (0x102-0x106, 0x202-0x207, 0x200c-0x200f, 0x2216) and it fixed the link
> > training on the above TBT hub.
> >
> > However, reading the LANE0_1_STATUS register also has a side-effect at
> > least on a Novatek eDP panel, as reported on the Closes: link below,
> > resulting in screen flickering on that panel. One clear side-effect when
> > doing the 1-byte probe reads from LANE0_1_STATUS during link training
> > before reading out the full 6 byte link status starting at the same
> > address is that the panel will report the link training as completed
> > with voltage swing 0. This is different from the normal, flicker-free
> > scenario when no DPCD probing is done, the panel reporting the link
> > training complete with voltage swing 2.
>
> For the ignorant like me, adding the debug log lines you deduced this from
> would help.
The following are the annotated events from the failure and success
cases, imo it's better to add this to the bug ticket, keeping the commit
message more succinct; can do that.
The failure case, using LANE0_1_STATUS for DPCD probing, from the bug
ticket's
https://gitlab.freedesktop.org/-/project/4519/uploads/e440bd79e44fe3442716078fb38fc396/20250630--dell-xps-13-9360--linux-6.16.0-rc3-00002-ga3ef3c2da675--messages.txt
:
"""
[ 47.429619] i915 0000:00:02.0: [drm:drm_dp_dpcd_write [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00102 AUX <- (ret= 5) 21 00 00 00 00
The source (driver) uses vs/pre-emp 0/0 ([0x105 0x106] = 0x00 0x00)
[ 47.429942] i915 0000:00:02.0: [drm:drm_dp_dpcd_probe [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00202 AUX -> (ret= 1) 11
[ 47.430289] i915 0000:00:02.0: [drm:drm_dp_dpcd_read [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00202 AUX -> (ret= 6) 11 11 80 00 66 66
The sink inidicates that CR is complete ([0x202 0x203] = 0x11 0x11).
The sink also requests vs/pre-emp 2/1, but the source does not change
to these levels, since the sink already indicated CR as complete.
[ 47.430305] i915 0000:00:02.0: [drm:intel_dp_link_train_phy [i915]]
[CONNECTOR:106:eDP-1][ENCODER:105:DDI A/PHY A][DPRX] Clock recovery OK
...
[ 47.431025] i915 0000:00:02.0: [drm:drm_dp_dpcd_write [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00102 AUX <- (ret= 5) 22 00 00 00 00
The source starts the EQ phase using the 0/0 vs/pre-emp levels ([0x105
0x106] = 0x00 0x00), with which CR got completed as indicated by the
sink above.
[ 47.431720] i915 0000:00:02.0: [drm:drm_dp_dpcd_probe [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00202 AUX -> (ret= 1) 11
[ 47.432072] i915 0000:00:02.0: [drm:drm_dp_dpcd_read [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00202 AUX -> (ret= 6) 77 77 81 00 66 66
The sink indicates that EQ is complete ([0x202-0x204] = 0x77 0x77 0x81).
The sink also requests vs/pre-emp 2/1, but the source does not change to
these levels, since the sink already indicated EQ as complete.
[ 47.432088] i915 0000:00:02.0: [drm:intel_dp_link_train_phy [i915]]
[CONNECTOR:106:eDP-1][ENCODER:105:DDI A/PHY A][DPRX] Channel EQ done. DP
Training successful
"""
The passing case, using TRAINING_PATTERN_SET for DPCD probing, from the
bug ticket's
https://gitlab.freedesktop.org/-/project/4519/uploads/a999486f52bc794d2923557334e297e2/20250630--dell-xps-13-9360--linux-6.16.0-rc4-00001-g072887dff624--messages--several-acpi-s3-cycles.txt
"""
[ 388.585357] i915 0000:00:02.0: [drm:drm_dp_dpcd_write [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00102 AUX <- (ret= 5) 21 00 00 00 00
[ 388.585696] i915 0000:00:02.0: [drm:drm_dp_dpcd_probe [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00102 AUX -> (ret= 1) 21
[ 388.586064] i915 0000:00:02.0: [drm:drm_dp_dpcd_read [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00202 AUX -> (ret= 6) 11 11 80 00 66 66
[ 388.586083] i915 0000:00:02.0: [drm:intel_dp_link_train_phy [i915]]
[CONNECTOR:106:eDP-1][ENCODER:105:DDI A/PHY A][DPRX] Clock recovery OK
Similarly as in the failure case, sink completes CR with vs/pre-emp 0/0,
but requesting already vs/pre-emp 2/1.
[ 388.586725] i915 0000:00:02.0: [drm:drm_dp_dpcd_write [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00102 AUX <- (ret= 5) 22 00 00 00 00
The source starts the EQ phase using the 0/0 vs/pre-emp levels the CR
phase completed with.
[ 388.587421] i915 0000:00:02.0: [drm:drm_dp_dpcd_probe [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00102 AUX -> (ret= 1) 22
[ 388.587774] i915 0000:00:02.0: [drm:drm_dp_dpcd_read [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00202 AUX -> (ret= 6) 11 77 81 00 66 66
The sink indicates that EQ is still not complete for lane 0/1 ([0x202] =
0x11) and requests for vs/pre-emp 2/1 for all lanes.
[ 388.587786] i915 0000:00:02.0: [drm:intel_dp_get_adjust_train [i915]]
[CONNECTOR:106:eDP-1][ENCODER:105:DDI A/PHY A][DPRX] 8b/10b, lanes: 4, vswing
request: 2/2/2/2, pre-emphasis request: 1/1/1/1
[ 388.587919] i915 0000:00:02.0: [drm:intel_dp_set_signal_levels [i915]]
[CONNECTOR:106:eDP-1][ENCODER:105:DDI A/PHY A][DPRX] 8b/10b, lanes: 4, vswing
levels: 2(max)/2(max)/2(max)/2(max), pre-emphasis levels: 1/1/1/1
[ 388.588035] i915 0000:00:02.0: [drm:hsw_set_signal_levels [i915]] Using
signal levels 08000000
[ 388.588319] i915 0000:00:02.0: [drm:drm_dp_dpcd_write [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00103 AUX <- (ret= 4) 0e 0e 0e 0e
The source switches to vs/pre-emp 2/1 for all lanes as the sink
requested.
[ 388.589007] i915 0000:00:02.0: [drm:drm_dp_dpcd_probe [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00102 AUX -> (ret= 1) 22
[ 388.589353] i915 0000:00:02.0: [drm:drm_dp_dpcd_read [drm_display_helper]]
AUX A/DDI A/PHY A: 0x00202 AUX -> (ret= 6) 77 77 01 00 66 66
The sink indicates the EQ as complete.
"""
> > Using the TRAINING_PATTERN_SET register for DPCD probing doesn't have
> > the above side-effect, the panel will link train with voltage swing 2 as
> > expected and it will stay flicker-free. This register is also in the
> > above valid register range and is unlikely to have a side-effect as that
> > of LANE0_1_STATUS: Reading LANE0_1_STATUS is part of the link training
> > CR/EQ sequences and so it may cause a state change in the sink - even if
> > inadvertently as I suspect in the case of the above Novatek panel. As
> > opposed to this, reading TRAINING_PATTERN_SET is not part of the link
> > training sequence (it must be only written once at the beginning of the
> > CR/EQ sequences), so it's unlikely to cause any state change in the
> > sink.
> >
> > As a side-note, this Novatek panel also lacks support for TPS3, while
> > claiming support for HBR2, which violates the DP Standard (the Standard
> > mandating TPS3 for HBR2).
>
> Unrelated, but a warning about this panel firmware/hardware misbehavior
> would probably be warranted.
There is a debug message about it, not sure if it would make sense to
convert that to an error/note message instead:
[ 388.586203] i915 0000:00:02.0: [drm:intel_dp_link_train_phy [i915]]
[CONNECTOR:106:eDP-1][ENCODER:105:DDI A/PHY A][DPRX] >=5.4/6.48 Gbps link rate
without sink TPS3 support
>
> > Besides the Novatek panel (PSR 1), which this change fixes, I also
> > verified the change on a Samsung (PSR 1) and an Analogix (PSR 2) eDP
> > panel as well as on the Intel Barlow Ridge TBT hub.
> >
> > Note that in the drm-tip tree (targeting the v6.17 kernel version) the
> > i915 and xe drivers keep DPCD probing enabled only for the panel known
> > to require this (HP ZR24w), hence those drivers in drm-tip are not
> > affected by the above problem.
> >
> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > Cc: Jani Nikula <jani.nik...@linux.intel.com>
> > Fixes: a40c5d727b81 ("drm/dp: Change AUX DPCD probe address from DPCD_REV
> > to LANE0_1_STATUS")
> > Reported-and-tested-by: Paul Menzel <pmen...@molgen.mpg.de>
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558
> > Signed-off-by: Imre Deak <imre.d...@intel.com>
> > ---
> > drivers/gpu/drm/display/drm_dp_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > b/drivers/gpu/drm/display/drm_dp_helper.c
> > index 1c3920297906b..1ecc3df7e3167 100644
> > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > @@ -742,7 +742,7 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux,
> > unsigned int offset,
> > int ret;
> > if (dpcd_access_needs_probe(aux)) {
> > - ret = drm_dp_dpcd_probe(aux, DP_LANE0_1_STATUS);
> > + ret = drm_dp_dpcd_probe(aux, DP_TRAINING_PATTERN_SET);
> > if (ret < 0)
> > return ret;
> > }
>
> Just for the record, I also tested this on top of commit 733923397fd9 (Merge
> tag 'pwm/for-6.16-rc6-fixes' of
> git://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux), and the
> flickering is gone.
Ok, thanks for testing it!
> Kind regards,
>
> Paul
