* [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET @ 2025-07-08 21:23 Imre Deak 2025-07-08 21:55 ` Cavitt, Jonathan ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Imre Deak @ 2025-07-08 21:23 UTC (permalink / raw) To: intel-gfx, intel-xe, dri-devel Cc: Ville Syrjälä, Jani Nikula, Paul Menzel 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.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 Signed-off-by: Imre Deak <imre.deak@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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-08 21:23 [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET Imre Deak @ 2025-07-08 21:55 ` Cavitt, Jonathan 2025-07-09 11:32 ` Imre Deak 2025-07-09 5:57 ` Paul Menzel 2025-07-10 11:28 ` Imre Deak 2 siblings, 1 reply; 10+ messages in thread From: Cavitt, Jonathan @ 2025-07-08 21:55 UTC (permalink / raw) To: Deak, Imre, intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: Ville Syrjälä, Jani Nikula, Paul Menzel, Cavitt, Jonathan -----Original Message----- From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre Deak Sent: Tuesday, July 8, 2025 2:24 PM To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Jani Nikula <jani.nikula@linux.intel.com>; Paul Menzel <pmenzel@molgen.mpg.de> Subject: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET > > 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.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > Signed-off-by: Imre Deak <imre.deak@intel.com> Some uses of the first person in the commit message could maybe be revised to speak more generally, but I'm not going to make that a requirement. As is, this patch is: Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> -Jonathan Cavitt > --- > 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 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-08 21:55 ` Cavitt, Jonathan @ 2025-07-09 11:32 ` Imre Deak 0 siblings, 0 replies; 10+ messages in thread From: Imre Deak @ 2025-07-09 11:32 UTC (permalink / raw) To: Jonathan Cavitt Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, dri-devel, Ville Syrjälä, Jani Nikula, Paul Menzel On Wed, Jul 09, 2025 at 12:55:16AM +0300, Jonathan Cavitt wrote: > -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre Deak > Sent: Tuesday, July 8, 2025 2:24 PM > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Jani Nikula <jani.nikula@linux.intel.com>; Paul Menzel <pmenzel@molgen.mpg.de> > Subject: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET > > > > 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.syrjala@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > Some uses of the first person in the commit message could maybe > be revised to speak more generally, Since here the panel's firmware is involved, only the debugging details provide some insight into the possible root causes and choices made for the solution, not sure how else that process could have been described. > but I'm not going to make that a requirement. > > As is, this patch is: > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Thanks for the review! > -Jonathan Cavitt > > > --- > > 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 > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-08 21:23 [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET Imre Deak 2025-07-08 21:55 ` Cavitt, Jonathan @ 2025-07-09 5:57 ` Paul Menzel 2025-07-09 12:39 ` Imre Deak 2025-07-10 11:28 ` Imre Deak 2 siblings, 1 reply; 10+ messages in thread From: Paul Menzel @ 2025-07-09 5:57 UTC (permalink / raw) To: Imre Deak Cc: intel-gfx, intel-xe, dri-devel, Ville Syrjälä, Jani Nikula 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. > 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. > 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.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > Signed-off-by: Imre Deak <imre.deak@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. Kind regards, Paul ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-09 5:57 ` Paul Menzel @ 2025-07-09 12:39 ` Imre Deak 0 siblings, 0 replies; 10+ messages in thread From: Imre Deak @ 2025-07-09 12:39 UTC (permalink / raw) To: Paul Menzel Cc: intel-gfx, intel-xe, dri-devel, Ville Syrjälä, Jani Nikula 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.syrjala@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > > Signed-off-by: Imre Deak <imre.deak@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-08 21:23 [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET Imre Deak 2025-07-08 21:55 ` Cavitt, Jonathan 2025-07-09 5:57 ` Paul Menzel @ 2025-07-10 11:28 ` Imre Deak 2025-07-14 8:57 ` Imre Deak 2 siblings, 1 reply; 10+ messages in thread From: Imre Deak @ 2025-07-10 11:28 UTC (permalink / raw) To: Thomas Zimmermann, Maxime Ripard, Maarten Lankhorst Cc: Ville Syrjälä, Jani Nikula, Paul Menzel, Jonathan Cavitt, intel-gfx, intel-xe, dri-devel 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? 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.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > Signed-off-by: Imre Deak <imre.deak@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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-10 11:28 ` Imre Deak @ 2025-07-14 8:57 ` Imre Deak 2025-07-14 9:00 ` Thomas Zimmermann 0 siblings, 1 reply; 10+ messages in thread From: Imre Deak @ 2025-07-14 8:57 UTC (permalink / raw) To: Thomas Zimmermann, Maxime Ripard, Maarten Lankhorst, Ville Syrjälä, Jani Nikula, Paul Menzel, Jonathan Cavitt, intel-gfx, intel-xe, dri-devel 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.syrjala@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > > Signed-off-by: Imre Deak <imre.deak@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 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-14 8:57 ` Imre Deak @ 2025-07-14 9:00 ` Thomas Zimmermann 2025-07-14 20:06 ` Imre Deak 0 siblings, 1 reply; 10+ messages in thread From: Thomas Zimmermann @ 2025-07-14 9:00 UTC (permalink / raw) To: imre.deak, Maxime Ripard, Maarten Lankhorst, Ville Syrjälä, Jani Nikula, Paul Menzel, Jonathan Cavitt, intel-gfx, intel-xe, dri-devel 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. 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.syrjala@linux.intel.com> >>> Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> >>> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 >>> Signed-off-by: Imre Deak <imre.deak@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) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-14 9:00 ` Thomas Zimmermann @ 2025-07-14 20:06 ` Imre Deak 2025-07-15 13:42 ` Rodrigo Vivi 0 siblings, 1 reply; 10+ messages in thread From: Imre Deak @ 2025-07-14 20:06 UTC (permalink / raw) To: Thomas Zimmermann, Rodrigo Vivi, Joonas Lahtinen Cc: Maxime Ripard, Maarten Lankhorst, Ville Syrjälä, Jani Nikula, Paul Menzel, Jonathan Cavitt, intel-gfx, intel-xe, dri-devel [-- Attachment #1: Type: text/plain, Size: 5499 bytes --] 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.syrjala@linux.intel.com> > > > > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > > > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > > > > Signed-off-by: Imre Deak <imre.deak@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) > [-- Attachment #2: 0001-drm-dp-Change-AUX-DPCD-probe-address-from-LANE0_1_ST.patch --] [-- Type: text/x-diff, Size: 4222 bytes --] From d468740cb8688070ebbd534c2849b49729ae2af8 Mon Sep 17 00:00:00 2001 From: Imre Deak <imre.deak@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.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Link: https://lore.kernel.org/r/20250708212331.112898-1-imre.deak@intel.com (cherry picked from commit bba9aa41654036534d86b198f5647a9ce15ebd7f) [Imre: Rebased on drm-intel-fixes] Signed-off-by: Imre Deak <imre.deak@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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET 2025-07-14 20:06 ` Imre Deak @ 2025-07-15 13:42 ` Rodrigo Vivi 0 siblings, 0 replies; 10+ messages in thread From: Rodrigo Vivi @ 2025-07-15 13:42 UTC (permalink / raw) To: Imre Deak Cc: Thomas Zimmermann, Joonas Lahtinen, Maxime Ripard, Maarten Lankhorst, Ville Syrjälä, Jani Nikula, Paul Menzel, Jonathan Cavitt, intel-gfx, intel-xe, dri-devel On Mon, Jul 14, 2025 at 11:06:52PM +0300, Imre Deak wrote: > 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. pushed to drm-intel-fixes. Thanks for the backport > > 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.syrjala@linux.intel.com> > > > > > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > > > > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > > > > > Signed-off-by: Imre Deak <imre.deak@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.deak@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.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@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 <pmenzel@molgen.mpg.de> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14558 > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > Link: https://lore.kernel.org/r/20250708212331.112898-1-imre.deak@intel.com > (cherry picked from commit bba9aa41654036534d86b198f5647a9ce15ebd7f) > [Imre: Rebased on drm-intel-fixes] > Signed-off-by: Imre Deak <imre.deak@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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-07-15 13:42 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-08 21:23 [PATCH] drm/dp: Change AUX DPCD probe address from LANE0_1_STATUS to TRAINING_PATTERN_SET Imre Deak 2025-07-08 21:55 ` Cavitt, Jonathan 2025-07-09 11:32 ` Imre Deak 2025-07-09 5:57 ` Paul Menzel 2025-07-09 12:39 ` Imre Deak 2025-07-10 11:28 ` Imre Deak 2025-07-14 8:57 ` Imre Deak 2025-07-14 9:00 ` Thomas Zimmermann 2025-07-14 20:06 ` Imre Deak 2025-07-15 13:42 ` Rodrigo Vivi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).