Hi, On Thu, Jul 10, 2025 at 02:28:28PM +0300, Imre Deak wrote: > Hi Thomas, Maxime, Maarten, > > the patch this change fixes (commit a40c5d727b81) was merged via > drm-intel and is also part of v6.16-rc4 (there cherry-picked in commit > a3ef3c2da675). > > Are you ok with merging this fix via drm-intel as well, so that it could > be still merged to v6.16 before that's released?
any objections to merging this patch to drm-intel? If not, could someone ack it? Thanks, Imre > > Thanks, > Imre > > On Wed, Jul 09, 2025 at 12:23:31AM +0300, Imre Deak wrote: > > 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. > > > > 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). > > > > 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; > > } > > -- > > 2.44.2 > >