On Mon, Jul 14, 2025 at 11:00:08AM +0200, Thomas Zimmermann wrote:
>
> Am 14.07.25 um 10:57 schrieb Imre Deak:
> > 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?
>
> Sure, a-b me.
Ok, thanks.
I pushed the patch to drm-intel-next, thanks for the report, testing,
review and ack.
I'd like to request that this patch be applied to drm-intel-fixes as
well, so that it can still make it to v6.16. To do that, the change
needs to be rebased on drm-intel-fixes, I attached the corresponding
patch.
Thanks,
Imre
> Best regards
> Thomas
>
> >
> > 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
> > > >
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>
>From d468740cb8688070ebbd534c2849b49729ae2af8 Mon Sep 17 00:00:00 2001
From: Imre Deak <imre.d...@intel.com>
Date: Wed, 9 Jul 2025 00:23:31 +0300
Subject: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to
TRAINING_PATTERN_SET
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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
Reviewed-by: Jonathan Cavitt <jonathan.cav...@intel.com>
Acked-by: Thomas Zimmermann <tzimmerm...@suse.de>
Link: https://lore.kernel.org/r/20250708212331.112898-1-imre.d...@intel.com
(cherry picked from commit bba9aa41654036534d86b198f5647a9ce15ebd7f)
[Imre: Rebased on drm-intel-fixes]
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 dc622c78db9d..ea78c6c8ca7a 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -725,7 +725,7 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
* monitor doesn't power down exactly after the throw away read.
*/
if (!aux->is_remote) {
- 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
