[PATCH v2 0/3] drm/i915: Fix DP LTTPR link training mode initialization

[PATCH v2 0/3] drm/i915: Fix DP LTTPR link training mode initialization

[Imre Deak]

This is v2 of
https://patchwork.freedesktop.org/series/88015/

also making sure that LTTPRs are detected and initialized only if the
DPCD and LTTPR revisions are > 1.4 as required by Display Port
specification.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v5.11

Imre Deak (3):
  drm/i915/ilk-glk: Fix link training on links with LTTPRs
  drm/i915: Disable LTTPR support when the DPCD rev < 1.4
  drm/i915: Disable LTTPR support when the LTTPR rev < 1.4

 drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
 drivers/gpu/drm/i915/display/intel_dp_aux.c   |  7 ++
 .../drm/i915/display/intel_dp_link_training.c | 66 ++++++++++++++-----
 .../drm/i915/display/intel_dp_link_training.h |  2 +-
 4 files changed, 58 insertions(+), 21 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Imre Deak]

The spec requires to use at least 3.2ms for the AUX timeout period if
there are LT-tunable PHY Repeaters on the link (2.11.2). An upcoming
spec update makes this more specific, by requiring a 3.2ms minimum
timeout period for the LTTPR detection reading the 0xF0000-0xF0007
range (3.6.5.1).

Accordingly disable LTTPR detection until GLK, where the maximum timeout
we can set is only 1.6ms.

Link training in the non-transparent mode is known to fail at least on
some SKL systems with a WD19 dock on the link, which exposes an LTTPR
(see the References below). While this could have different reasons
besides the too short AUX timeout used, not detecting LTTPRs (and so not
using the non-transparent LT mode) fixes link training on these systems.

While at it add a code comment about the platform specific maximum
timeout values.

v2: Add a comment about the g4x maximum timeout as well. (Ville)

Reported-by: Takashi Iwai <tiwai@suse.de>
Reported-and-tested-by: Santiago Zarate <santiago.zarate@suse.com>
Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
References: https://gitlab.freedesktop.org/drm/intel/-/issues/3166
Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link training")
Cc: <stable@vger.kernel.org> # v5.11
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
 .../gpu/drm/i915/display/intel_dp_link_training.c | 15 ++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
index eaebf123310a..10fe17b7280d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
@@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
 	else
 		precharge = 5;
 
+	/* Max timeout value on G4x-BDW: 1.6ms */
 	if (IS_BROADWELL(dev_priv))
 		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
 	else
@@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
 	u32 ret;
 
+	/*
+	 * Max timeout values:
+	 * SKL-GLK: 1.6ms
+	 * CNL: 3.2ms
+	 * ICL+: 4ms
+	 */
 	ret = DP_AUX_CH_CTL_SEND_BUSY |
 	      DP_AUX_CH_CTL_DONE |
 	      DP_AUX_CH_CTL_INTERRUPT |
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 19ba7c7cbaab..c0e25c75c105 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -82,6 +82,18 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
 
 static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
 {
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+	if (intel_dp_is_edp(intel_dp))
+		return false;
+
+	/*
+	 * Detecting LTTPRs must be avoided on platforms with an AUX timeout
+	 * period < 3.2ms. (see DP Standard v2.0, 2.11.2, 3.6.6.1).
+	 */
+	if (INTEL_GEN(i915) < 10)
+		return false;
+
 	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
 					  intel_dp->lttpr_common_caps) < 0) {
 		memset(intel_dp->lttpr_common_caps, 0,
@@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
 	bool ret;
 	int i;
 
-	if (intel_dp_is_edp(intel_dp))
-		return 0;
-
 	ret = intel_dp_read_lttpr_common_caps(intel_dp);
 	if (!ret)
 		return 0;
-- 
2.25.1

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Ville Syrjälä]

On Wed, Mar 17, 2021 at 08:48:59PM +0200, Imre Deak wrote:
> The spec requires to use at least 3.2ms for the AUX timeout period if
> there are LT-tunable PHY Repeaters on the link (2.11.2). An upcoming
> spec update makes this more specific, by requiring a 3.2ms minimum
> timeout period for the LTTPR detection reading the 0xF0000-0xF0007
> range (3.6.5.1).

I'm pondering if we could reduce the timeout after having determined
wherther LTTPRs are present or not? But maybe that wouldn't really speed
up anything since we can't reduce the timeout until after detecting
*something*. And once there is something there we shouldn't really get
any more timeouts I guess. So probably a totally stupid idea.

Anyways, this seems about the only thing we can do given the limited
hw capabilities.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Accordingly disable LTTPR detection until GLK, where the maximum timeout
> we can set is only 1.6ms.
> 
> Link training in the non-transparent mode is known to fail at least on
> some SKL systems with a WD19 dock on the link, which exposes an LTTPR
> (see the References below). While this could have different reasons
> besides the too short AUX timeout used, not detecting LTTPRs (and so not
> using the non-transparent LT mode) fixes link training on these systems.
> 
> While at it add a code comment about the platform specific maximum
> timeout values.
> 
> v2: Add a comment about the g4x maximum timeout as well. (Ville)
> 
> Reported-by: Takashi Iwai <tiwai@suse.de>
> Reported-and-tested-by: Santiago Zarate <santiago.zarate@suse.com>
> Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link training")
> Cc: <stable@vger.kernel.org> # v5.11
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
>  .../gpu/drm/i915/display/intel_dp_link_training.c | 15 ++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index eaebf123310a..10fe17b7280d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>  	else
>  		precharge = 5;
>  
> +	/* Max timeout value on G4x-BDW: 1.6ms */
>  	if (IS_BROADWELL(dev_priv))
>  		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
>  	else
> @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>  	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
>  	u32 ret;
>  
> +	/*
> +	 * Max timeout values:
> +	 * SKL-GLK: 1.6ms
> +	 * CNL: 3.2ms
> +	 * ICL+: 4ms
> +	 */
>  	ret = DP_AUX_CH_CTL_SEND_BUSY |
>  	      DP_AUX_CH_CTL_DONE |
>  	      DP_AUX_CH_CTL_INTERRUPT |
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index 19ba7c7cbaab..c0e25c75c105 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -82,6 +82,18 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
>  
>  static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
>  {
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +
> +	if (intel_dp_is_edp(intel_dp))
> +		return false;
> +
> +	/*
> +	 * Detecting LTTPRs must be avoided on platforms with an AUX timeout
> +	 * period < 3.2ms. (see DP Standard v2.0, 2.11.2, 3.6.6.1).
> +	 */
> +	if (INTEL_GEN(i915) < 10)
> +		return false;
> +
>  	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
>  					  intel_dp->lttpr_common_caps) < 0) {
>  		memset(intel_dp->lttpr_common_caps, 0,
> @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
>  	bool ret;
>  	int i;
>  
> -	if (intel_dp_is_edp(intel_dp))
> -		return 0;
> -
>  	ret = intel_dp_read_lttpr_common_caps(intel_dp);
>  	if (!ret)
>  		return 0;
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Imre Deak]

On Thu, Mar 18, 2021 at 07:33:20PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 17, 2021 at 08:48:59PM +0200, Imre Deak wrote:
> > The spec requires to use at least 3.2ms for the AUX timeout period if
> > there are LT-tunable PHY Repeaters on the link (2.11.2). An upcoming
> > spec update makes this more specific, by requiring a 3.2ms minimum
> > timeout period for the LTTPR detection reading the 0xF0000-0xF0007
> > range (3.6.5.1).
> 
> I'm pondering if we could reduce the timeout after having determined
> wherther LTTPRs are present or not? But maybe that wouldn't really speed
> up anything since we can't reduce the timeout until after detecting
> *something*. And once there is something there we shouldn't really get
> any more timeouts I guess. So probably a totally stupid idea.

Right, if something is connected it would take anyway as much time as it
takes for the sink to reply whether or not we decreased the timeout.

However if nothing is connected, we have the excessive timeout Khaled
already noticed (160 * 4ms = 6.4 sec on ICL+). I think to improve that
we could scale the total number of retries by making it
total_timeout/platform_specific_timeout (letting total_timeout=2sec for
instance) or just changing the drm retry logic to be time based instead
of the number of retries we use atm. 

> Anyways, this seems about the only thing we can do given the limited
> hw capabilities.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> > Accordingly disable LTTPR detection until GLK, where the maximum timeout
> > we can set is only 1.6ms.
> > 
> > Link training in the non-transparent mode is known to fail at least on
> > some SKL systems with a WD19 dock on the link, which exposes an LTTPR
> > (see the References below). While this could have different reasons
> > besides the too short AUX timeout used, not detecting LTTPRs (and so not
> > using the non-transparent LT mode) fixes link training on these systems.
> > 
> > While at it add a code comment about the platform specific maximum
> > timeout values.
> > 
> > v2: Add a comment about the g4x maximum timeout as well. (Ville)
> > 
> > Reported-by: Takashi Iwai <tiwai@suse.de>
> > Reported-and-tested-by: Santiago Zarate <santiago.zarate@suse.com>
> > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > References: https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link training")
> > Cc: <stable@vger.kernel.org> # v5.11
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15 ++++++++++++---
> >  2 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > index eaebf123310a..10fe17b7280d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> >  	else
> >  		precharge = 5;
> >  
> > +	/* Max timeout value on G4x-BDW: 1.6ms */
> >  	if (IS_BROADWELL(dev_priv))
> >  		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> >  	else
> > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> >  	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> >  	u32 ret;
> >  
> > +	/*
> > +	 * Max timeout values:
> > +	 * SKL-GLK: 1.6ms
> > +	 * CNL: 3.2ms
> > +	 * ICL+: 4ms
> > +	 */
> >  	ret = DP_AUX_CH_CTL_SEND_BUSY |
> >  	      DP_AUX_CH_CTL_DONE |
> >  	      DP_AUX_CH_CTL_INTERRUPT |
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > index 19ba7c7cbaab..c0e25c75c105 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -82,6 +82,18 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> >  
> >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
> >  {
> > +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > +
> > +	if (intel_dp_is_edp(intel_dp))
> > +		return false;
> > +
> > +	/*
> > +	 * Detecting LTTPRs must be avoided on platforms with an AUX timeout
> > +	 * period < 3.2ms. (see DP Standard v2.0, 2.11.2, 3.6.6.1).
> > +	 */
> > +	if (INTEL_GEN(i915) < 10)
> > +		return false;
> > +
> >  	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> >  					  intel_dp->lttpr_common_caps) < 0) {
> >  		memset(intel_dp->lttpr_common_caps, 0,
> > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
> >  	bool ret;
> >  	int i;
> >  
> > -	if (intel_dp_is_edp(intel_dp))
> > -		return 0;
> > -
> >  	ret = intel_dp_read_lttpr_common_caps(intel_dp);
> >  	if (!ret)
> >  		return 0;
> > -- 
> > 2.25.1
> 
> -- 
> Ville Syrjälä
> Intel

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Imre Deak]

On Thu, Mar 18, 2021 at 07:49:13PM +0200, Imre Deak wrote:
> On Thu, Mar 18, 2021 at 07:33:20PM +0200, Ville Syrjälä wrote:
> > On Wed, Mar 17, 2021 at 08:48:59PM +0200, Imre Deak wrote:
> > > The spec requires to use at least 3.2ms for the AUX timeout period if
> > > there are LT-tunable PHY Repeaters on the link (2.11.2). An upcoming
> > > spec update makes this more specific, by requiring a 3.2ms minimum
> > > timeout period for the LTTPR detection reading the 0xF0000-0xF0007
> > > range (3.6.5.1).
> > 
> > I'm pondering if we could reduce the timeout after having determined
> > wherther LTTPRs are present or not? But maybe that wouldn't really speed
> > up anything since we can't reduce the timeout until after detecting
> > *something*. And once there is something there we shouldn't really get
> > any more timeouts I guess. So probably a totally stupid idea.
> 
> Right, if something is connected it would take anyway as much time as it
> takes for the sink to reply whether or not we decreased the timeout.
> 
> However if nothing is connected, we have the excessive timeout Khaled
> already noticed (160 * 4ms = 6.4 sec on ICL+). I think to improve that
> we could scale the total number of retries by making it
> total_timeout/platform_specific_timeout (letting total_timeout=2sec for
> instance) or just changing the drm retry logic to be time based instead
> of the number of retries we use atm. 

Doh, reducing simply the HW timeouts would be enough to fix this.

> > Anyways, this seems about the only thing we can do given the limited
> > hw capabilities.
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > > Accordingly disable LTTPR detection until GLK, where the maximum timeout
> > > we can set is only 1.6ms.
> > > 
> > > Link training in the non-transparent mode is known to fail at least on
> > > some SKL systems with a WD19 dock on the link, which exposes an LTTPR
> > > (see the References below). While this could have different reasons
> > > besides the too short AUX timeout used, not detecting LTTPRs (and so not
> > > using the non-transparent LT mode) fixes link training on these systems.
> > > 
> > > While at it add a code comment about the platform specific maximum
> > > timeout values.
> > > 
> > > v2: Add a comment about the g4x maximum timeout as well. (Ville)
> > > 
> > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > Reported-and-tested-by: Santiago Zarate <santiago.zarate@suse.com>
> > > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > > References: https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link training")
> > > Cc: <stable@vger.kernel.org> # v5.11
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> > >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15 ++++++++++++---
> > >  2 files changed, 19 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > index eaebf123310a..10fe17b7280d 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
> > >  	else
> > >  		precharge = 5;
> > >  
> > > +	/* Max timeout value on G4x-BDW: 1.6ms */
> > >  	if (IS_BROADWELL(dev_priv))
> > >  		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > >  	else
> > > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> > >  	enum phy phy = intel_port_to_phy(i915, dig_port->base.port);
> > >  	u32 ret;
> > >  
> > > +	/*
> > > +	 * Max timeout values:
> > > +	 * SKL-GLK: 1.6ms
> > > +	 * CNL: 3.2ms
> > > +	 * ICL+: 4ms
> > > +	 */
> > >  	ret = DP_AUX_CH_CTL_SEND_BUSY |
> > >  	      DP_AUX_CH_CTL_DONE |
> > >  	      DP_AUX_CH_CTL_INTERRUPT |
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > @@ -82,6 +82,18 @@ static void intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> > >  
> > >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
> > >  {
> > > +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > +
> > > +	if (intel_dp_is_edp(intel_dp))
> > > +		return false;
> > > +
> > > +	/*
> > > +	 * Detecting LTTPRs must be avoided on platforms with an AUX timeout
> > > +	 * period < 3.2ms. (see DP Standard v2.0, 2.11.2, 3.6.6.1).
> > > +	 */
> > > +	if (INTEL_GEN(i915) < 10)
> > > +		return false;
> > > +
> > >  	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > >  					  intel_dp->lttpr_common_caps) < 0) {
> > >  		memset(intel_dp->lttpr_common_caps, 0,
> > > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
> > >  	bool ret;
> > >  	int i;
> > >  
> > > -	if (intel_dp_is_edp(intel_dp))
> > > -		return 0;
> > > -
> > >  	ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > >  	if (!ret)
> > >  		return 0;
> > > -- 
> > > 2.25.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Almahallawy, Khaled]

On Thu, 2021-03-18 at 20:06 +0200, Imre Deak wrote:
> On Thu, Mar 18, 2021 at 07:49:13PM +0200, Imre Deak wrote:
> > On Thu, Mar 18, 2021 at 07:33:20PM +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 17, 2021 at 08:48:59PM +0200, Imre Deak wrote:
> > > > The spec requires to use at least 3.2ms for the AUX timeout
> > > > period if
> > > > there are LT-tunable PHY Repeaters on the link (2.11.2). An
> > > > upcoming
> > > > spec update makes this more specific, by requiring a 3.2ms
> > > > minimum
> > > > timeout period for the LTTPR detection reading the 0xF0000-
> > > > 0xF0007
> > > > range (3.6.5.1).
> > > 
> > > I'm pondering if we could reduce the timeout after having
> > > determined
> > > wherther LTTPRs are present or not? But maybe that wouldn't
> > > really speed
> > > up anything since we can't reduce the timeout until after
> > > detecting
> > > *something*. And once there is something there we shouldn't
> > > really get
> > > any more timeouts I guess. So probably a totally stupid idea.
> > 
> > Right, if something is connected it would take anyway as much time
> > as it
> > takes for the sink to reply whether or not we decreased the
> > timeout.
> > 
> > However if nothing is connected, we have the excessive timeout
> > Khaled
> > already noticed (160 * 4ms = 6.4 sec on ICL+). I think to improve
> > that
> > we could scale the total number of retries by making it
> > total_timeout/platform_specific_timeout (letting total_timeout=2sec
> > for
> > instance) or just changing the drm retry logic to be time based
> > instead
> > of the number of retries we use atm. 
> 
> Doh, reducing simply the HW timeouts would be enough to fix this.

What about Lyude's suggestion ( 
https://patchwork.freedesktop.org/patch/420369/#comment_756572) 
to drop the retries in intel_dp_aux_xfer()
		/* Must try at least 3 times according to DP spec */
		for (try = 0; try < 5; try++) {
 
 
And use only the retries in drm_dpcd_access?

Thanks
Khaled

> 
> > > Anyways, this seems about the only thing we can do given the
> > > limited
> > > hw capabilities.
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > > Accordingly disable LTTPR detection until GLK, where the
> > > > maximum timeout
> > > > we can set is only 1.6ms.
> > > > 
> > > > Link training in the non-transparent mode is known to fail at
> > > > least on
> > > > some SKL systems with a WD19 dock on the link, which exposes an
> > > > LTTPR
> > > > (see the References below). While this could have different
> > > > reasons
> > > > besides the too short AUX timeout used, not detecting LTTPRs
> > > > (and so not
> > > > using the non-transparent LT mode) fixes link training on these
> > > > systems.
> > > > 
> > > > While at it add a code comment about the platform specific
> > > > maximum
> > > > timeout values.
> > > > 
> > > > v2: Add a comment about the g4x maximum timeout as well.
> > > > (Ville)
> > > > 
> > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > Reported-and-tested-by: Santiago Zarate <
> > > > santiago.zarate@suse.com>
> > > > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > > > References: 
> > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent 
> > > > mode link training")
> > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> > > >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15
> > > > ++++++++++++---
> > > >  2 files changed, 19 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > index eaebf123310a..10fe17b7280d 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct
> > > > intel_dp *intel_dp,
> > > >  	else
> > > >  		precharge = 5;
> > > >  
> > > > +	/* Max timeout value on G4x-BDW: 1.6ms */
> > > >  	if (IS_BROADWELL(dev_priv))
> > > >  		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > >  	else
> > > > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct
> > > > intel_dp *intel_dp,
> > > >  	enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > >base.port);
> > > >  	u32 ret;
> > > >  
> > > > +	/*
> > > > +	 * Max timeout values:
> > > > +	 * SKL-GLK: 1.6ms
> > > > +	 * CNL: 3.2ms
> > > > +	 * ICL+: 4ms
> > > > +	 */
> > > >  	ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > >  	      DP_AUX_CH_CTL_DONE |
> > > >  	      DP_AUX_CH_CTL_INTERRUPT |
> > > > diff --git
> > > > a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > @@ -82,6 +82,18 @@ static void
> > > > intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> > > >  
> > > >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > *intel_dp)
> > > >  {
> > > > +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > +
> > > > +	if (intel_dp_is_edp(intel_dp))
> > > > +		return false;
> > > > +
> > > > +	/*
> > > > +	 * Detecting LTTPRs must be avoided on platforms with
> > > > an AUX timeout
> > > > +	 * period < 3.2ms. (see DP Standard v2.0, 2.11.2,
> > > > 3.6.6.1).
> > > > +	 */
> > > > +	if (INTEL_GEN(i915) < 10)
> > > > +		return false;
> > > > +
> > > >  	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > > >  					  intel_dp-
> > > > >lttpr_common_caps) < 0) {
> > > >  		memset(intel_dp->lttpr_common_caps, 0,
> > > > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp
> > > > *intel_dp)
> > > >  	bool ret;
> > > >  	int i;
> > > >  
> > > > -	if (intel_dp_is_edp(intel_dp))
> > > > -		return 0;
> > > > -
> > > >  	ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > >  	if (!ret)
> > > >  		return 0;
> > > > -- 
> > > > 2.25.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Imre Deak]

On Fri, Mar 19, 2021 at 12:04:54AM +0200, Almahallawy, Khaled wrote:
> On Thu, 2021-03-18 at 20:06 +0200, Imre Deak wrote:
> > On Thu, Mar 18, 2021 at 07:49:13PM +0200, Imre Deak wrote:
> > > On Thu, Mar 18, 2021 at 07:33:20PM +0200, Ville Syrjälä wrote:
> > > > On Wed, Mar 17, 2021 at 08:48:59PM +0200, Imre Deak wrote:
> > > > > The spec requires to use at least 3.2ms for the AUX timeout
> > > > > period if
> > > > > there are LT-tunable PHY Repeaters on the link (2.11.2). An
> > > > > upcoming
> > > > > spec update makes this more specific, by requiring a 3.2ms
> > > > > minimum
> > > > > timeout period for the LTTPR detection reading the 0xF0000-
> > > > > 0xF0007
> > > > > range (3.6.5.1).
> > > >
> > > > I'm pondering if we could reduce the timeout after having
> > > > determined
> > > > wherther LTTPRs are present or not? But maybe that wouldn't
> > > > really speed
> > > > up anything since we can't reduce the timeout until after
> > > > detecting
> > > > *something*. And once there is something there we shouldn't
> > > > really get
> > > > any more timeouts I guess. So probably a totally stupid idea.
> > >
> > > Right, if something is connected it would take anyway as much time
> > > as it
> > > takes for the sink to reply whether or not we decreased the
> > > timeout.
> > >
> > > However if nothing is connected, we have the excessive timeout
> > > Khaled
> > > already noticed (160 * 4ms = 6.4 sec on ICL+). I think to improve
> > > that
> > > we could scale the total number of retries by making it
> > > total_timeout/platform_specific_timeout (letting total_timeout=2sec
> > > for
> > > instance) or just changing the drm retry logic to be time based
> > > instead
> > > of the number of retries we use atm.
> >
> > Doh, reducing simply the HW timeouts would be enough to fix this.
> 
> What about Lyude's suggestion (
> https://patchwork.freedesktop.org/patch/420369/#comment_756572)
> to drop the retries in intel_dp_aux_xfer()
> /* Must try at least 3 times according to DP spec */
> for (try = 0; try < 5; try++) {
> 
> And use only the retries in drm_dpcd_access?

I think it would work if we can make the retries configurable and set it
to
	retries = total_timeout / platform_specific_timeout_per_retry

where total_timeout would be something reasonable like 1 sec.

> 
> Thanks
> Khaled
> 
> >
> > > > Anyways, this seems about the only thing we can do given the
> > > > limited
> > > > hw capabilities.
> > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >
> > > > > Accordingly disable LTTPR detection until GLK, where the
> > > > > maximum timeout
> > > > > we can set is only 1.6ms.
> > > > >
> > > > > Link training in the non-transparent mode is known to fail at
> > > > > least on
> > > > > some SKL systems with a WD19 dock on the link, which exposes an
> > > > > LTTPR
> > > > > (see the References below). While this could have different
> > > > > reasons
> > > > > besides the too short AUX timeout used, not detecting LTTPRs
> > > > > (and so not
> > > > > using the non-transparent LT mode) fixes link training on these
> > > > > systems.
> > > > >
> > > > > While at it add a code comment about the platform specific
> > > > > maximum
> > > > > timeout values.
> > > > >
> > > > > v2: Add a comment about the g4x maximum timeout as well.
> > > > > (Ville)
> > > > >
> > > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > > Reported-and-tested-by: Santiago Zarate <
> > > > > santiago.zarate@suse.com>
> > > > > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > > > > References:
> > > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent
> > > > > mode link training")
> > > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> > > > >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15
> > > > > ++++++++++++---
> > > > >  2 files changed, 19 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > index eaebf123310a..10fe17b7280d 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct
> > > > > intel_dp *intel_dp,
> > > > >  else
> > > > >  precharge = 5;
> > > > >
> > > > > +/* Max timeout value on G4x-BDW: 1.6ms */
> > > > >  if (IS_BROADWELL(dev_priv))
> > > > >  timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > > >  else
> > > > > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct
> > > > > intel_dp *intel_dp,
> > > > >  enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > > >base.port);
> > > > >  u32 ret;
> > > > >
> > > > > +/*
> > > > > + * Max timeout values:
> > > > > + * SKL-GLK: 1.6ms
> > > > > + * CNL: 3.2ms
> > > > > + * ICL+: 4ms
> > > > > + */
> > > > >  ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > > >        DP_AUX_CH_CTL_DONE |
> > > > >        DP_AUX_CH_CTL_INTERRUPT |
> > > > > diff --git
> > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > @@ -82,6 +82,18 @@ static void
> > > > > intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> > > > >
> > > > >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > > *intel_dp)
> > > > >  {
> > > > > +struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > > +
> > > > > +if (intel_dp_is_edp(intel_dp))
> > > > > +return false;
> > > > > +
> > > > > +/*
> > > > > + * Detecting LTTPRs must be avoided on platforms with
> > > > > an AUX timeout
> > > > > + * period < 3.2ms. (see DP Standard v2.0, 2.11.2,
> > > > > 3.6.6.1).
> > > > > + */
> > > > > +if (INTEL_GEN(i915) < 10)
> > > > > +return false;
> > > > > +
> > > > >  if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > > > >    intel_dp-
> > > > > >lttpr_common_caps) < 0) {
> > > > >  memset(intel_dp->lttpr_common_caps, 0,
> > > > > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp
> > > > > *intel_dp)
> > > > >  bool ret;
> > > > >  int i;
> > > > >
> > > > > -if (intel_dp_is_edp(intel_dp))
> > > > > -return 0;
> > > > > -
> > > > >  ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > > >  if (!ret)
> > > > >  return 0;
> > > > > --
> > > > > 2.25.1
> > > >
> > > > --
> > > > Ville Syrjälä
> > > > Intel

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Lyude Paul]

On Fri, 2021-03-19 at 01:17 +0200, Imre Deak wrote:
> On Fri, Mar 19, 2021 at 12:04:54AM +0200, Almahallawy, Khaled wrote:
> > On Thu, 2021-03-18 at 20:06 +0200, Imre Deak wrote:
> > > On Thu, Mar 18, 2021 at 07:49:13PM +0200, Imre Deak wrote:
> > > > On Thu, Mar 18, 2021 at 07:33:20PM +0200, Ville Syrjälä wrote:
> > > > > On Wed, Mar 17, 2021 at 08:48:59PM +0200, Imre Deak wrote:
> > > > > > The spec requires to use at least 3.2ms for the AUX timeout
> > > > > > period if
> > > > > > there are LT-tunable PHY Repeaters on the link (2.11.2). An
> > > > > > upcoming
> > > > > > spec update makes this more specific, by requiring a 3.2ms
> > > > > > minimum
> > > > > > timeout period for the LTTPR detection reading the 0xF0000-
> > > > > > 0xF0007
> > > > > > range (3.6.5.1).
> > > > > 
> > > > > I'm pondering if we could reduce the timeout after having
> > > > > determined
> > > > > wherther LTTPRs are present or not? But maybe that wouldn't
> > > > > really speed
> > > > > up anything since we can't reduce the timeout until after
> > > > > detecting
> > > > > *something*. And once there is something there we shouldn't
> > > > > really get
> > > > > any more timeouts I guess. So probably a totally stupid idea.
> > > > 
> > > > Right, if something is connected it would take anyway as much time
> > > > as it
> > > > takes for the sink to reply whether or not we decreased the
> > > > timeout.
> > > > 
> > > > However if nothing is connected, we have the excessive timeout
> > > > Khaled
> > > > already noticed (160 * 4ms = 6.4 sec on ICL+). I think to improve
> > > > that
> > > > we could scale the total number of retries by making it
> > > > total_timeout/platform_specific_timeout (letting total_timeout=2sec
> > > > for
> > > > instance) or just changing the drm retry logic to be time based
> > > > instead
> > > > of the number of retries we use atm.
> > > 
> > > Doh, reducing simply the HW timeouts would be enough to fix this.
> > 
> > What about Lyude's suggestion (
> > https://patchwork.freedesktop.org/patch/420369/#comment_756572)
> > to drop the retries in intel_dp_aux_xfer()
> > /* Must try at least 3 times according to DP spec */
> > for (try = 0; try < 5; try++) {
> > 
> > And use only the retries in drm_dpcd_access?
> 
> I think it would work if we can make the retries configurable and set it
> to
>         retries = total_timeout / platform_specific_timeout_per_retry
> 
> where total_timeout would be something reasonable like 1 sec.

I actually think I'm more open to the idea of configurable retries after
learning that apparently this is a thing that the i2c subsystem does - so
there's more precedence for it in the rest of the kernel than I originally
thought.

I'm still curious if we need these extra retries in here though - there seems to
be one set of retries that is actually platform specific, and then just a random
set of 5 retries that don't seem to have anything to do with platform specific
behavior - so I think it'd still be worth giving a shot at getting rid of that

> 
> > 
> > Thanks
> > Khaled
> > 
> > > 
> > > > > Anyways, this seems about the only thing we can do given the
> > > > > limited
> > > > > hw capabilities.
> > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > > Accordingly disable LTTPR detection until GLK, where the
> > > > > > maximum timeout
> > > > > > we can set is only 1.6ms.
> > > > > > 
> > > > > > Link training in the non-transparent mode is known to fail at
> > > > > > least on
> > > > > > some SKL systems with a WD19 dock on the link, which exposes an
> > > > > > LTTPR
> > > > > > (see the References below). While this could have different
> > > > > > reasons
> > > > > > besides the too short AUX timeout used, not detecting LTTPRs
> > > > > > (and so not
> > > > > > using the non-transparent LT mode) fixes link training on these
> > > > > > systems.
> > > > > > 
> > > > > > While at it add a code comment about the platform specific
> > > > > > maximum
> > > > > > timeout values.
> > > > > > 
> > > > > > v2: Add a comment about the g4x maximum timeout as well.
> > > > > > (Ville)
> > > > > > 
> > > > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > > > Reported-and-tested-by: Santiago Zarate <
> > > > > > santiago.zarate@suse.com>
> > > > > > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > > > > > References:
> > > > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent
> > > > > > mode link training")
> > > > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> > > > > >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15
> > > > > > ++++++++++++---
> > > > > >  2 files changed, 19 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > index eaebf123310a..10fe17b7280d 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct
> > > > > > intel_dp *intel_dp,
> > > > > >  else
> > > > > >  precharge = 5;
> > > > > > 
> > > > > > +/* Max timeout value on G4x-BDW: 1.6ms */
> > > > > >  if (IS_BROADWELL(dev_priv))
> > > > > >  timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > > > >  else
> > > > > > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct
> > > > > > intel_dp *intel_dp,
> > > > > >  enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > > > > base.port);
> > > > > >  u32 ret;
> > > > > > 
> > > > > > +/*
> > > > > > + * Max timeout values:
> > > > > > + * SKL-GLK: 1.6ms
> > > > > > + * CNL: 3.2ms
> > > > > > + * ICL+: 4ms
> > > > > > + */
> > > > > >  ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > > > >        DP_AUX_CH_CTL_DONE |
> > > > > >        DP_AUX_CH_CTL_INTERRUPT |
> > > > > > diff --git
> > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > @@ -82,6 +82,18 @@ static void
> > > > > > intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> > > > > > 
> > > > > >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > > > *intel_dp)
> > > > > >  {
> > > > > > +struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > > > +
> > > > > > +if (intel_dp_is_edp(intel_dp))
> > > > > > +return false;
> > > > > > +
> > > > > > +/*
> > > > > > + * Detecting LTTPRs must be avoided on platforms with
> > > > > > an AUX timeout
> > > > > > + * period < 3.2ms. (see DP Standard v2.0, 2.11.2,
> > > > > > 3.6.6.1).
> > > > > > + */
> > > > > > +if (INTEL_GEN(i915) < 10)
> > > > > > +return false;
> > > > > > +
> > > > > >  if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > > > > >    intel_dp-
> > > > > > > lttpr_common_caps) < 0) {
> > > > > >  memset(intel_dp->lttpr_common_caps, 0,
> > > > > > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp
> > > > > > *intel_dp)
> > > > > >  bool ret;
> > > > > >  int i;
> > > > > > 
> > > > > > -if (intel_dp_is_edp(intel_dp))
> > > > > > -return 0;
> > > > > > -
> > > > > >  ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > > > >  if (!ret)
> > > > > >  return 0;
> > > > > > --
> > > > > > 2.25.1
> > > > > 
> > > > > --
> > > > > Ville Syrjälä
> > > > > Intel
> 

-- 
Sincerely,
   Lyude Paul (she/her)
   Software Engineer at Red Hat
   
Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
asked me a question, are waiting for a review/merge on a patch, etc. and I
haven't responded in a while, please feel free to send me another email to check
on my status. I don't bite!

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Imre Deak]

On Fri, Mar 19, 2021 at 01:25:08PM -0400, Lyude Paul wrote:
> On Fri, 2021-03-19 at 01:17 +0200, Imre Deak wrote:
> > On Fri, Mar 19, 2021 at 12:04:54AM +0200, Almahallawy, Khaled wrote:
> > > On Thu, 2021-03-18 at 20:06 +0200, Imre Deak wrote:
> > > > On Thu, Mar 18, 2021 at 07:49:13PM +0200, Imre Deak wrote:
> > > > > On Thu, Mar 18, 2021 at 07:33:20PM +0200, Ville Syrjälä wrote:
> > > > > > On Wed, Mar 17, 2021 at 08:48:59PM +0200, Imre Deak wrote:
> > > > > > > The spec requires to use at least 3.2ms for the AUX timeout
> > > > > > > period if
> > > > > > > there are LT-tunable PHY Repeaters on the link (2.11.2). An
> > > > > > > upcoming
> > > > > > > spec update makes this more specific, by requiring a 3.2ms
> > > > > > > minimum
> > > > > > > timeout period for the LTTPR detection reading the 0xF0000-
> > > > > > > 0xF0007
> > > > > > > range (3.6.5.1).
> > > > > > 
> > > > > > I'm pondering if we could reduce the timeout after having
> > > > > > determined
> > > > > > wherther LTTPRs are present or not? But maybe that wouldn't
> > > > > > really speed
> > > > > > up anything since we can't reduce the timeout until after
> > > > > > detecting
> > > > > > *something*. And once there is something there we shouldn't
> > > > > > really get
> > > > > > any more timeouts I guess. So probably a totally stupid idea.
> > > > > 
> > > > > Right, if something is connected it would take anyway as much time
> > > > > as it
> > > > > takes for the sink to reply whether or not we decreased the
> > > > > timeout.
> > > > > 
> > > > > However if nothing is connected, we have the excessive timeout
> > > > > Khaled
> > > > > already noticed (160 * 4ms = 6.4 sec on ICL+). I think to improve
> > > > > that
> > > > > we could scale the total number of retries by making it
> > > > > total_timeout/platform_specific_timeout (letting total_timeout=2sec
> > > > > for
> > > > > instance) or just changing the drm retry logic to be time based
> > > > > instead
> > > > > of the number of retries we use atm.
> > > > 
> > > > Doh, reducing simply the HW timeouts would be enough to fix this.
> > > 
> > > What about Lyude's suggestion (
> > > https://patchwork.freedesktop.org/patch/420369/#comment_756572)
> > > to drop the retries in intel_dp_aux_xfer()
> > > /* Must try at least 3 times according to DP spec */
> > > for (try = 0; try < 5; try++) {
> > > 
> > > And use only the retries in drm_dpcd_access?
> > 
> > I think it would work if we can make the retries configurable and set it
> > to
> >         retries = total_timeout / platform_specific_timeout_per_retry
> > 
> > where total_timeout would be something reasonable like 1 sec.
> 
> I actually think I'm more open to the idea of configurable retries after
> learning that apparently this is a thing that the i2c subsystem does - so
> there's more precedence for it in the rest of the kernel than I originally
> thought.
> 
> I'm still curious if we need these extra retries in here though - there seems to
> be one set of retries that is actually platform specific, and then just a random
> set of 5 retries that don't seem to have anything to do with platform specific
> behavior - so I think it'd still be worth giving a shot at getting rid of that

The platform specific part of the timeout is the one desctibed in the
maximum timeout values comments.

> > > Thanks
> > > Khaled
> > > 
> > > > 
> > > > > > Anyways, this seems about the only thing we can do given the
> > > > > > limited
> > > > > > hw capabilities.
> > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > 
> > > > > > > Accordingly disable LTTPR detection until GLK, where the
> > > > > > > maximum timeout
> > > > > > > we can set is only 1.6ms.
> > > > > > > 
> > > > > > > Link training in the non-transparent mode is known to fail at
> > > > > > > least on
> > > > > > > some SKL systems with a WD19 dock on the link, which exposes an
> > > > > > > LTTPR
> > > > > > > (see the References below). While this could have different
> > > > > > > reasons
> > > > > > > besides the too short AUX timeout used, not detecting LTTPRs
> > > > > > > (and so not
> > > > > > > using the non-transparent LT mode) fixes link training on these
> > > > > > > systems.
> > > > > > > 
> > > > > > > While at it add a code comment about the platform specific
> > > > > > > maximum
> > > > > > > timeout values.
> > > > > > > 
> > > > > > > v2: Add a comment about the g4x maximum timeout as well.
> > > > > > > (Ville)
> > > > > > > 
> > > > > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > Reported-and-tested-by: Santiago Zarate <
> > > > > > > santiago.zarate@suse.com>
> > > > > > > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > > > > > > References:
> > > > > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent
> > > > > > > mode link training")
> > > > > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> > > > > > >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15
> > > > > > > ++++++++++++---
> > > > > > >  2 files changed, 19 insertions(+), 3 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > index eaebf123310a..10fe17b7280d 100644
> > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct
> > > > > > > intel_dp *intel_dp,
> > > > > > >  else
> > > > > > >  precharge = 5;
> > > > > > > 
> > > > > > > +/* Max timeout value on G4x-BDW: 1.6ms */
> > > > > > >  if (IS_BROADWELL(dev_priv))
> > > > > > >  timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > > > > >  else
> > > > > > > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct
> > > > > > > intel_dp *intel_dp,
> > > > > > >  enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > > > > > base.port);
> > > > > > >  u32 ret;
> > > > > > > 
> > > > > > > +/*
> > > > > > > + * Max timeout values:
> > > > > > > + * SKL-GLK: 1.6ms
> > > > > > > + * CNL: 3.2ms
> > > > > > > + * ICL+: 4ms
> > > > > > > + */
> > > > > > >  ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > >        DP_AUX_CH_CTL_DONE |
> > > > > > >        DP_AUX_CH_CTL_INTERRUPT |
> > > > > > > diff --git
> > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > @@ -82,6 +82,18 @@ static void
> > > > > > > intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> > > > > > > 
> > > > > > >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > > > > *intel_dp)
> > > > > > >  {
> > > > > > > +struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > > > > +
> > > > > > > +if (intel_dp_is_edp(intel_dp))
> > > > > > > +return false;
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * Detecting LTTPRs must be avoided on platforms with
> > > > > > > an AUX timeout
> > > > > > > + * period < 3.2ms. (see DP Standard v2.0, 2.11.2,
> > > > > > > 3.6.6.1).
> > > > > > > + */
> > > > > > > +if (INTEL_GEN(i915) < 10)
> > > > > > > +return false;
> > > > > > > +
> > > > > > >  if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > > > > > >    intel_dp-
> > > > > > > > lttpr_common_caps) < 0) {
> > > > > > >  memset(intel_dp->lttpr_common_caps, 0,
> > > > > > > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp
> > > > > > > *intel_dp)
> > > > > > >  bool ret;
> > > > > > >  int i;
> > > > > > > 
> > > > > > > -if (intel_dp_is_edp(intel_dp))
> > > > > > > -return 0;
> > > > > > > -
> > > > > > >  ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > > > > >  if (!ret)
> > > > > > >  return 0;
> > > > > > > --
> > > > > > > 2.25.1
> > > > > > 
> > > > > > --
> > > > > > Ville Syrjälä
> > > > > > Intel
> > 
> 
> -- 
> Sincerely,
>    Lyude Paul (she/her)
>    Software Engineer at Red Hat
>    
> Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
> asked me a question, are waiting for a review/merge on a patch, etc. and I
> haven't responded in a while, please feel free to send me another email to check
> on my status. I don't bite!
> 

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Lyude Paul]

On Fri, 2021-03-19 at 19:29 +0200, Imre Deak wrote:
> On Fri, Mar 19, 2021 at 01:25:08PM -0400, Lyude Paul wrote:
> > On Fri, 2021-03-19 at 01:17 +0200, Imre Deak wrote:
> > > On Fri, Mar 19, 2021 at 12:04:54AM +0200, Almahallawy, Khaled wrote:
> > > > On Thu, 2021-03-18 at 20:06 +0200, Imre Deak wrote:
> > > > > On Thu, Mar 18, 2021 at 07:49:13PM +0200, Imre Deak wrote:
> > > > > > On Thu, Mar 18, 2021 at 07:33:20PM +0200, Ville Syrjälä wrote:
> > > > > > > On Wed, Mar 17, 2021 at 08:48:59PM +0200, Imre Deak wrote:
> > > > > > > > The spec requires to use at least 3.2ms for the AUX timeout
> > > > > > > > period if
> > > > > > > > there are LT-tunable PHY Repeaters on the link (2.11.2). An
> > > > > > > > upcoming
> > > > > > > > spec update makes this more specific, by requiring a 3.2ms
> > > > > > > > minimum
> > > > > > > > timeout period for the LTTPR detection reading the 0xF0000-
> > > > > > > > 0xF0007
> > > > > > > > range (3.6.5.1).
> > > > > > > 
> > > > > > > I'm pondering if we could reduce the timeout after having
> > > > > > > determined
> > > > > > > wherther LTTPRs are present or not? But maybe that wouldn't
> > > > > > > really speed
> > > > > > > up anything since we can't reduce the timeout until after
> > > > > > > detecting
> > > > > > > *something*. And once there is something there we shouldn't
> > > > > > > really get
> > > > > > > any more timeouts I guess. So probably a totally stupid idea.
> > > > > > 
> > > > > > Right, if something is connected it would take anyway as much time
> > > > > > as it
> > > > > > takes for the sink to reply whether or not we decreased the
> > > > > > timeout.
> > > > > > 
> > > > > > However if nothing is connected, we have the excessive timeout
> > > > > > Khaled
> > > > > > already noticed (160 * 4ms = 6.4 sec on ICL+). I think to improve
> > > > > > that
> > > > > > we could scale the total number of retries by making it
> > > > > > total_timeout/platform_specific_timeout (letting total_timeout=2sec
> > > > > > for
> > > > > > instance) or just changing the drm retry logic to be time based
> > > > > > instead
> > > > > > of the number of retries we use atm.
> > > > > 
> > > > > Doh, reducing simply the HW timeouts would be enough to fix this.
> > > > 
> > > > What about Lyude's suggestion (
> > > > https://patchwork.freedesktop.org/patch/420369/#comment_756572)
> > > > to drop the retries in intel_dp_aux_xfer()
> > > > /* Must try at least 3 times according to DP spec */
> > > > for (try = 0; try < 5; try++) {
> > > > 
> > > > And use only the retries in drm_dpcd_access?
> > > 
> > > I think it would work if we can make the retries configurable and set it
> > > to
> > >         retries = total_timeout / platform_specific_timeout_per_retry
> > > 
> > > where total_timeout would be something reasonable like 1 sec.
> > 
> > I actually think I'm more open to the idea of configurable retries after
> > learning that apparently this is a thing that the i2c subsystem does - so
> > there's more precedence for it in the rest of the kernel than I originally
> > thought.
> > 
> > I'm still curious if we need these extra retries in here though - there seems
> > to
> > be one set of retries that is actually platform specific, and then just a
> > random
> > set of 5 retries that don't seem to have anything to do with platform specific
> > behavior - so I think it'd still be worth giving a shot at getting rid of that
> 
> The platform specific part of the timeout is the one desctibed in the
> maximum timeout values comments.

You mean the

		/* Must try at least 3 times according to DP spec */
		for (try = 0; try < 5; try++) {

bit? I thought that wasn't related to platform specific retries at all, since
the code in that loop seems to only reference parts of the DP spec, and that the

	while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {

Loop was the portion that was platform specific, since it prompts the driver to
retry the transaction with different aux clock divider rates depending on the
platform in use. Feel free to correct me if I'm wrong though.

Also - with the timeouts we're seeing, does the LTTPR return NAKs at all? That's
still another thing I had suggested alternate workarounds for so that we could
terminate transactions immediately on NAKs, so I wonder if that could save time
here as well. 

> 
> > > > Thanks
> > > > Khaled
> > > > 
> > > > > 
> > > > > > > Anyways, this seems about the only thing we can do given the
> > > > > > > limited
> > > > > > > hw capabilities.
> > > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > 
> > > > > > > > Accordingly disable LTTPR detection until GLK, where the
> > > > > > > > maximum timeout
> > > > > > > > we can set is only 1.6ms.
> > > > > > > > 
> > > > > > > > Link training in the non-transparent mode is known to fail at
> > > > > > > > least on
> > > > > > > > some SKL systems with a WD19 dock on the link, which exposes an
> > > > > > > > LTTPR
> > > > > > > > (see the References below). While this could have different
> > > > > > > > reasons
> > > > > > > > besides the too short AUX timeout used, not detecting LTTPRs
> > > > > > > > (and so not
> > > > > > > > using the non-transparent LT mode) fixes link training on these
> > > > > > > > systems.
> > > > > > > > 
> > > > > > > > While at it add a code comment about the platform specific
> > > > > > > > maximum
> > > > > > > > timeout values.
> > > > > > > > 
> > > > > > > > v2: Add a comment about the g4x maximum timeout as well.
> > > > > > > > (Ville)
> > > > > > > > 
> > > > > > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > Reported-and-tested-by: Santiago Zarate <
> > > > > > > > santiago.zarate@suse.com>
> > > > > > > > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > > > > > > > References:
> > > > > > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > > > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent
> > > > > > > > mode link training")
> > > > > > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > > > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> > > > > > > >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15
> > > > > > > > ++++++++++++---
> > > > > > > >  2 files changed, 19 insertions(+), 3 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > index eaebf123310a..10fe17b7280d 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct
> > > > > > > > intel_dp *intel_dp,
> > > > > > > >  else
> > > > > > > >  precharge = 5;
> > > > > > > > 
> > > > > > > > +/* Max timeout value on G4x-BDW: 1.6ms */
> > > > > > > >  if (IS_BROADWELL(dev_priv))
> > > > > > > >  timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > > > > > >  else
> > > > > > > > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct
> > > > > > > > intel_dp *intel_dp,
> > > > > > > >  enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > > > > > > base.port);
> > > > > > > >  u32 ret;
> > > > > > > > 
> > > > > > > > +/*
> > > > > > > > + * Max timeout values:
> > > > > > > > + * SKL-GLK: 1.6ms
> > > > > > > > + * CNL: 3.2ms
> > > > > > > > + * ICL+: 4ms
> > > > > > > > + */
> > > > > > > >  ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > > >        DP_AUX_CH_CTL_DONE |
> > > > > > > >        DP_AUX_CH_CTL_INTERRUPT |
> > > > > > > > diff --git
> > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > @@ -82,6 +82,18 @@ static void
> > > > > > > > intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> > > > > > > > 
> > > > > > > >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > > > > > *intel_dp)
> > > > > > > >  {
> > > > > > > > +struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > > > > > +
> > > > > > > > +if (intel_dp_is_edp(intel_dp))
> > > > > > > > +return false;
> > > > > > > > +
> > > > > > > > +/*
> > > > > > > > + * Detecting LTTPRs must be avoided on platforms with
> > > > > > > > an AUX timeout
> > > > > > > > + * period < 3.2ms. (see DP Standard v2.0, 2.11.2,
> > > > > > > > 3.6.6.1).
> > > > > > > > + */
> > > > > > > > +if (INTEL_GEN(i915) < 10)
> > > > > > > > +return false;
> > > > > > > > +
> > > > > > > >  if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > > > > > > >    intel_dp-
> > > > > > > > > lttpr_common_caps) < 0) {
> > > > > > > >  memset(intel_dp->lttpr_common_caps, 0,
> > > > > > > > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp
> > > > > > > > *intel_dp)
> > > > > > > >  bool ret;
> > > > > > > >  int i;
> > > > > > > > 
> > > > > > > > -if (intel_dp_is_edp(intel_dp))
> > > > > > > > -return 0;
> > > > > > > > -
> > > > > > > >  ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > > > > > >  if (!ret)
> > > > > > > >  return 0;
> > > > > > > > --
> > > > > > > > 2.25.1
> > > > > > > 
> > > > > > > --
> > > > > > > Ville Syrjälä
> > > > > > > Intel
> > > 
> > 
> > -- 
> > Sincerely,
> >    Lyude Paul (she/her)
> >    Software Engineer at Red Hat
> >    
> > Note: I deal with a lot of emails and have a lot of bugs on my plate. If
> > you've
> > asked me a question, are waiting for a review/merge on a patch, etc. and I
> > haven't responded in a while, please feel free to send me another email to
> > check
> > on my status. I don't bite!
> > 
> 

-- 
Sincerely,
   Lyude Paul (she/her)
   Software Engineer at Red Hat
   
Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
asked me a question, are waiting for a review/merge on a patch, etc. and I
haven't responded in a while, please feel free to send me another email to check
on my status. I don't bite!

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Imre Deak]

On Fri, Mar 19, 2021 at 04:44:26PM -0400, Lyude Paul wrote:
> > > > [...]
> > > > I think it would work if we can make the retries configurable and set it
> > > > to
> > > >         retries = total_timeout / platform_specific_timeout_per_retry
> > > > 
> > > > where total_timeout would be something reasonable like 1 sec.
> > > 
> > > I actually think I'm more open to the idea of configurable retries after
> > > learning that apparently this is a thing that the i2c subsystem does - so
> > > there's more precedence for it in the rest of the kernel than I originally
> > > thought.
> > > 
> > > I'm still curious if we need these extra retries in here though - there seems
> > > to
> > > be one set of retries that is actually platform specific, and then just a
> > > random
> > > set of 5 retries that don't seem to have anything to do with platform specific
> > > behavior - so I think it'd still be worth giving a shot at getting rid of that
> > 
> > The platform specific part of the timeout is the one desctibed in the
> > maximum timeout values comments.
> 
> You mean the
> 
> 		/* Must try at least 3 times according to DP spec */
> 		for (try = 0; try < 5; try++) {
> 
> bit? I thought that wasn't related to platform specific retries at all, since
> the code in that loop seems to only reference parts of the DP spec, and that the
> 
> 	while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> 
> Loop was the portion that was platform specific, since it prompts the driver to
> retry the transaction with different aux clock divider rates depending on the
> platform in use. Feel free to correct me if I'm wrong though.

Nope. I meant every HW transaction will have a platform specific
timeout. For instance it's 1.6ms on SKL, but 4ms on ICL. So now since
the overall retry count is 32 * 5 = 160, on SKL we'll retry for ~2.6
seconds, on ICL we'll retry for ~6.4 seconds (disregarding now the extra
400usec delay inserted by drm_dp_dpcd_access(), which adds a fixed
~1.3ms delay).

This is what I think should be normalized, so that we have the same
amount of overall maximum timeout period on all platforms.

> Also - with the timeouts we're seeing, does the LTTPR return NAKs at all? That's
> still another thing I had suggested alternate workarounds for so that we could
> terminate transactions immediately on NAKs, so I wonder if that could save time
> here as well.

There's not much LTTPR specific in that wrt. what sinks would do
normally (no NAKs for read, only for writes) except LTTPRs may rewrite
NAKs to ACKs to account for buggy monitors returning NAKs when reading
the 0xf0000 -> range. But I'd suggest not dealing with this aspect now,
just sanitize the above retry thing, as you suggested, remove the i915
retry loop and make the drm retry loop configurable.

(In any case I also had the idea to stop transactions early when HPD
 gets deasserted, but not sure if that's completely robust.)

> > > > > Thanks
> > > > > Khaled
> > > > > 
> > > > > > 
> > > > > > > > Anyways, this seems about the only thing we can do given the
> > > > > > > > limited
> > > > > > > > hw capabilities.
> > > > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > 
> > > > > > > > > Accordingly disable LTTPR detection until GLK, where the
> > > > > > > > > maximum timeout
> > > > > > > > > we can set is only 1.6ms.
> > > > > > > > > 
> > > > > > > > > Link training in the non-transparent mode is known to fail at
> > > > > > > > > least on
> > > > > > > > > some SKL systems with a WD19 dock on the link, which exposes an
> > > > > > > > > LTTPR
> > > > > > > > > (see the References below). While this could have different
> > > > > > > > > reasons
> > > > > > > > > besides the too short AUX timeout used, not detecting LTTPRs
> > > > > > > > > (and so not
> > > > > > > > > using the non-transparent LT mode) fixes link training on these
> > > > > > > > > systems.
> > > > > > > > > 
> > > > > > > > > While at it add a code comment about the platform specific
> > > > > > > > > maximum
> > > > > > > > > timeout values.
> > > > > > > > > 
> > > > > > > > > v2: Add a comment about the g4x maximum timeout as well.
> > > > > > > > > (Ville)
> > > > > > > > > 
> > > > > > > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > Reported-and-tested-by: Santiago Zarate <
> > > > > > > > > santiago.zarate@suse.com>
> > > > > > > > > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > > > > > > > > References:
> > > > > > > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > > > > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent
> > > > > > > > > mode link training")
> > > > > > > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > > > > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > > > > > ---
> > > > > > > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> > > > > > > > >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15
> > > > > > > > > ++++++++++++---
> > > > > > > > >  2 files changed, 19 insertions(+), 3 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > index eaebf123310a..10fe17b7280d 100644
> > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct
> > > > > > > > > intel_dp *intel_dp,
> > > > > > > > >  else
> > > > > > > > >  precharge = 5;
> > > > > > > > > 
> > > > > > > > > +/* Max timeout value on G4x-BDW: 1.6ms */
> > > > > > > > >  if (IS_BROADWELL(dev_priv))
> > > > > > > > >  timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > > > > > > >  else
> > > > > > > > > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct
> > > > > > > > > intel_dp *intel_dp,
> > > > > > > > >  enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > > > > > > > base.port);
> > > > > > > > >  u32 ret;
> > > > > > > > > 
> > > > > > > > > +/*
> > > > > > > > > + * Max timeout values:
> > > > > > > > > + * SKL-GLK: 1.6ms
> > > > > > > > > + * CNL: 3.2ms
> > > > > > > > > + * ICL+: 4ms
> > > > > > > > > + */
> > > > > > > > >  ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > > > >        DP_AUX_CH_CTL_DONE |
> > > > > > > > >        DP_AUX_CH_CTL_INTERRUPT |
> > > > > > > > > diff --git
> > > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > > @@ -82,6 +82,18 @@ static void
> > > > > > > > > intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> > > > > > > > > 
> > > > > > > > >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > > > > > > *intel_dp)
> > > > > > > > >  {
> > > > > > > > > +struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > > > > > > +
> > > > > > > > > +if (intel_dp_is_edp(intel_dp))
> > > > > > > > > +return false;
> > > > > > > > > +
> > > > > > > > > +/*
> > > > > > > > > + * Detecting LTTPRs must be avoided on platforms with
> > > > > > > > > an AUX timeout
> > > > > > > > > + * period < 3.2ms. (see DP Standard v2.0, 2.11.2,
> > > > > > > > > 3.6.6.1).
> > > > > > > > > + */
> > > > > > > > > +if (INTEL_GEN(i915) < 10)
> > > > > > > > > +return false;
> > > > > > > > > +
> > > > > > > > >  if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > > > > > > > >    intel_dp-
> > > > > > > > > > lttpr_common_caps) < 0) {
> > > > > > > > >  memset(intel_dp->lttpr_common_caps, 0,
> > > > > > > > > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp
> > > > > > > > > *intel_dp)
> > > > > > > > >  bool ret;
> > > > > > > > >  int i;
> > > > > > > > > 
> > > > > > > > > -if (intel_dp_is_edp(intel_dp))
> > > > > > > > > -return 0;
> > > > > > > > > -
> > > > > > > > >  ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > > > > > > >  if (!ret)
> > > > > > > > >  return 0;
> > > > > > > > > --
> > > > > > > > > 2.25.1
> > > > > > > > 
> > > > > > > > --
> > > > > > > > Ville Syrjälä
> > > > > > > > Intel
> > > > 
> > > 
> > > -- 
> > > Sincerely,
> > >    Lyude Paul (she/her)
> > >    Software Engineer at Red Hat
> > >    
> > > Note: I deal with a lot of emails and have a lot of bugs on my plate. If
> > > you've
> > > asked me a question, are waiting for a review/merge on a patch, etc. and I
> > > haven't responded in a while, please feel free to send me another email to
> > > check
> > > on my status. I don't bite!
> > > 
> > 
> 
> -- 
> Sincerely,
>    Lyude Paul (she/her)
>    Software Engineer at Red Hat
>    
> Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
> asked me a question, are waiting for a review/merge on a patch, etc. and I
> haven't responded in a while, please feel free to send me another email to check
> on my status. I don't bite!
> 

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Imre Deak]

On Fri, Mar 19, 2021 at 11:07:21PM +0200, Imre Deak wrote:
> On Fri, Mar 19, 2021 at 04:44:26PM -0400, Lyude Paul wrote:
> > > > > [...]
> > > > > I think it would work if we can make the retries configurable and set it
> > > > > to
> > > > >         retries = total_timeout / platform_specific_timeout_per_retry
> > > > > 
> > > > > where total_timeout would be something reasonable like 1 sec.
> > > > 
> > > > I actually think I'm more open to the idea of configurable retries after
> > > > learning that apparently this is a thing that the i2c subsystem does - so
> > > > there's more precedence for it in the rest of the kernel than I originally
> > > > thought.
> > > > 
> > > > I'm still curious if we need these extra retries in here though - there seems
> > > > to
> > > > be one set of retries that is actually platform specific, and then just a
> > > > random
> > > > set of 5 retries that don't seem to have anything to do with platform specific
> > > > behavior - so I think it'd still be worth giving a shot at getting rid of that
> > > 
> > > The platform specific part of the timeout is the one desctibed in the
> > > maximum timeout values comments.
> > 
> > You mean the
> > 
> > 		/* Must try at least 3 times according to DP spec */
> > 		for (try = 0; try < 5; try++) {
> > 
> > bit? I thought that wasn't related to platform specific retries at all, since
> > the code in that loop seems to only reference parts of the DP spec, and that the
> > 
> > 	while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> > 
> > Loop was the portion that was platform specific, since it prompts the driver to
> > retry the transaction with different aux clock divider rates depending on the
> > platform in use. Feel free to correct me if I'm wrong though.
> 
> Nope. I meant every HW transaction will have a platform specific
> timeout. For instance it's 1.6ms on SKL, but 4ms on ICL. So now since
> the overall retry count is 32 * 5 = 160, on SKL we'll retry for ~2.6
> seconds, on ICL we'll retry for ~6.4 seconds (disregarding now the extra
> 400usec delay inserted by drm_dp_dpcd_access(), which adds a fixed
> ~1.3ms delay).

Err, looks like I missed some coffee. Max total timeouts atm, which we
would need to make the same on all platforms:

	g4x-glk: 5 * 32 * 1.6ms + 32 * 400us = 268.8ms
	cnl    : 5 * 32 * 3.2ms + 32 * 400us = 524.8ms
	icl+   : 5 * 32 * 4ms   + 32 * 400us = 652.8ms

> This is what I think should be normalized, so that we have the same
> amount of overall maximum timeout period on all platforms.
> 
> > Also - with the timeouts we're seeing, does the LTTPR return NAKs at all? That's
> > still another thing I had suggested alternate workarounds for so that we could
> > terminate transactions immediately on NAKs, so I wonder if that could save time
> > here as well.
> 
> There's not much LTTPR specific in that wrt. what sinks would do
> normally (no NAKs for read, only for writes) except LTTPRs may rewrite
> NAKs to ACKs to account for buggy monitors returning NAKs when reading
> the 0xf0000 -> range. But I'd suggest not dealing with this aspect now,
> just sanitize the above retry thing, as you suggested, remove the i915
> retry loop and make the drm retry loop configurable.
> 
> (In any case I also had the idea to stop transactions early when HPD
>  gets deasserted, but not sure if that's completely robust.)
> 
> > > > > > Thanks
> > > > > > Khaled
> > > > > > 
> > > > > > > 
> > > > > > > > > Anyways, this seems about the only thing we can do given the
> > > > > > > > > limited
> > > > > > > > > hw capabilities.
> > > > > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > > 
> > > > > > > > > > Accordingly disable LTTPR detection until GLK, where the
> > > > > > > > > > maximum timeout
> > > > > > > > > > we can set is only 1.6ms.
> > > > > > > > > > 
> > > > > > > > > > Link training in the non-transparent mode is known to fail at
> > > > > > > > > > least on
> > > > > > > > > > some SKL systems with a WD19 dock on the link, which exposes an
> > > > > > > > > > LTTPR
> > > > > > > > > > (see the References below). While this could have different
> > > > > > > > > > reasons
> > > > > > > > > > besides the too short AUX timeout used, not detecting LTTPRs
> > > > > > > > > > (and so not
> > > > > > > > > > using the non-transparent LT mode) fixes link training on these
> > > > > > > > > > systems.
> > > > > > > > > > 
> > > > > > > > > > While at it add a code comment about the platform specific
> > > > > > > > > > maximum
> > > > > > > > > > timeout values.
> > > > > > > > > > 
> > > > > > > > > > v2: Add a comment about the g4x maximum timeout as well.
> > > > > > > > > > (Ville)
> > > > > > > > > > 
> > > > > > > > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > Reported-and-tested-by: Santiago Zarate <
> > > > > > > > > > santiago.zarate@suse.com>
> > > > > > > > > > Reported-and-tested-by: Bodo Graumann <mail@bodograumann.de>
> > > > > > > > > > References:
> > > > > > > > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > > > > > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent
> > > > > > > > > > mode link training")
> > > > > > > > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > > > > > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > > > > > > ---
> > > > > > > > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c       |  7 +++++++
> > > > > > > > > >  .../gpu/drm/i915/display/intel_dp_link_training.c | 15
> > > > > > > > > > ++++++++++++---
> > > > > > > > > >  2 files changed, 19 insertions(+), 3 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > index eaebf123310a..10fe17b7280d 100644
> > > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > @@ -133,6 +133,7 @@ static u32 g4x_get_aux_send_ctl(struct
> > > > > > > > > > intel_dp *intel_dp,
> > > > > > > > > >  else
> > > > > > > > > >  precharge = 5;
> > > > > > > > > > 
> > > > > > > > > > +/* Max timeout value on G4x-BDW: 1.6ms */
> > > > > > > > > >  if (IS_BROADWELL(dev_priv))
> > > > > > > > > >  timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > > > > > > > >  else
> > > > > > > > > > @@ -159,6 +160,12 @@ static u32 skl_get_aux_send_ctl(struct
> > > > > > > > > > intel_dp *intel_dp,
> > > > > > > > > >  enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > > > > > > > > base.port);
> > > > > > > > > >  u32 ret;
> > > > > > > > > > 
> > > > > > > > > > +/*
> > > > > > > > > > + * Max timeout values:
> > > > > > > > > > + * SKL-GLK: 1.6ms
> > > > > > > > > > + * CNL: 3.2ms
> > > > > > > > > > + * ICL+: 4ms
> > > > > > > > > > + */
> > > > > > > > > >  ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > > > > >        DP_AUX_CH_CTL_DONE |
> > > > > > > > > >        DP_AUX_CH_CTL_INTERRUPT |
> > > > > > > > > > diff --git
> > > > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > > > > > > > > @@ -82,6 +82,18 @@ static void
> > > > > > > > > > intel_dp_read_lttpr_phy_caps(struct intel_dp *intel_dp,
> > > > > > > > > > 
> > > > > > > > > >  static bool intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > > > > > > > *intel_dp)
> > > > > > > > > >  {
> > > > > > > > > > +struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > > > > > > > +
> > > > > > > > > > +if (intel_dp_is_edp(intel_dp))
> > > > > > > > > > +return false;
> > > > > > > > > > +
> > > > > > > > > > +/*
> > > > > > > > > > + * Detecting LTTPRs must be avoided on platforms with
> > > > > > > > > > an AUX timeout
> > > > > > > > > > + * period < 3.2ms. (see DP Standard v2.0, 2.11.2,
> > > > > > > > > > 3.6.6.1).
> > > > > > > > > > + */
> > > > > > > > > > +if (INTEL_GEN(i915) < 10)
> > > > > > > > > > +return false;
> > > > > > > > > > +
> > > > > > > > > >  if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > > > > > > > > >    intel_dp-
> > > > > > > > > > > lttpr_common_caps) < 0) {
> > > > > > > > > >  memset(intel_dp->lttpr_common_caps, 0,
> > > > > > > > > > @@ -127,9 +139,6 @@ int intel_dp_lttpr_init(struct intel_dp
> > > > > > > > > > *intel_dp)
> > > > > > > > > >  bool ret;
> > > > > > > > > >  int i;
> > > > > > > > > > 
> > > > > > > > > > -if (intel_dp_is_edp(intel_dp))
> > > > > > > > > > -return 0;
> > > > > > > > > > -
> > > > > > > > > >  ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > > > > > > > >  if (!ret)
> > > > > > > > > >  return 0;
> > > > > > > > > > --
> > > > > > > > > > 2.25.1
> > > > > > > > > 
> > > > > > > > > --
> > > > > > > > > Ville Syrjälä
> > > > > > > > > Intel
> > > > > 
> > > > 
> > > > -- 
> > > > Sincerely,
> > > >    Lyude Paul (she/her)
> > > >    Software Engineer at Red Hat
> > > >    
> > > > Note: I deal with a lot of emails and have a lot of bugs on my plate. If
> > > > you've
> > > > asked me a question, are waiting for a review/merge on a patch, etc. and I
> > > > haven't responded in a while, please feel free to send me another email to
> > > > check
> > > > on my status. I don't bite!
> > > > 
> > > 
> > 
> > -- 
> > Sincerely,
> >    Lyude Paul (she/her)
> >    Software Engineer at Red Hat
> >    
> > Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
> > asked me a question, are waiting for a review/merge on a patch, etc. and I
> > haven't responded in a while, please feel free to send me another email to check
> > on my status. I don't bite!
> > 

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Almahallawy, Khaled]

On Sat, 2021-03-20 at 09:15 +0200, Imre Deak wrote:
> On Fri, Mar 19, 2021 at 11:07:21PM +0200, Imre Deak wrote:
> > On Fri, Mar 19, 2021 at 04:44:26PM -0400, Lyude Paul wrote:
> > > > > > [...]
> > > > > > I think it would work if we can make the retries
> > > > > > configurable and set it
> > > > > > to
> > > > > >         retries = total_timeout /
> > > > > > platform_specific_timeout_per_retry
> > > > > > 
> > > > > > where total_timeout would be something reasonable like 1
> > > > > > sec.
> > > > > 
> > > > > I actually think I'm more open to the idea of configurable
> > > > > retries after
> > > > > learning that apparently this is a thing that the i2c
> > > > > subsystem does - so
> > > > > there's more precedence for it in the rest of the kernel than
> > > > > I originally
> > > > > thought.
> > > > > 
> > > > > I'm still curious if we need these extra retries in here
> > > > > though - there seems
> > > > > to
> > > > > be one set of retries that is actually platform specific, and
> > > > > then just a
> > > > > random
> > > > > set of 5 retries that don't seem to have anything to do with
> > > > > platform specific
> > > > > behavior - so I think it'd still be worth giving a shot at
> > > > > getting rid of that
> > > > 
> > > > The platform specific part of the timeout is the one desctibed
> > > > in the
> > > > maximum timeout values comments.
> > > 
> > > You mean the
> > > 
> > > 		/* Must try at least 3 times according to DP spec */
> > > 		for (try = 0; try < 5; try++) {
> > > 
> > > bit? I thought that wasn't related to platform specific retries
> > > at all, since
> > > the code in that loop seems to only reference parts of the DP
> > > spec, and that the
> > > 
> > > 	while ((aux_clock_divider = intel_dp-
> > > >get_aux_clock_divider(intel_dp, clock++))) {
> > > 
> > > Loop was the portion that was platform specific, since it prompts
> > > the driver to
> > > retry the transaction with different aux clock divider rates
> > > depending on the
> > > platform in use. Feel free to correct me if I'm wrong though.
> > 
> > Nope. I meant every HW transaction will have a platform specific
> > timeout. For instance it's 1.6ms on SKL, but 4ms on ICL. So now
> > since
> > the overall retry count is 32 * 5 = 160, on SKL we'll retry for
> > ~2.6
> > seconds, on ICL we'll retry for ~6.4 seconds (disregarding now the
> > extra
> > 400usec delay inserted by drm_dp_dpcd_access(), which adds a fixed
> > ~1.3ms delay).
> 
> Err, looks like I missed some coffee. Max total timeouts atm, which
> we
> would need to make the same on all platforms:
> 
> 	g4x-glk: 5 * 32 * 1.6ms + 32 * 400us = 268.8ms
> 	cnl    : 5 * 32 * 3.2ms + 32 * 400us = 524.8ms
> 	icl+   : 5 * 32 * 4ms   + 32 * 400us = 652.8ms
> 


Apology if I'm missing something. but in drm_dpcd_access() I think it
is 500us not 400us?!

		#define AUX_RETRY_INTERVAL 500 /* us */

		if (ret != 0 && ret != -ETIMEDOUT) {
			usleep_range(AUX_RETRY_INTERVAL,
				     AUX_RETRY_INTERVAL + 100);
		}

Thanks
Khaled

> > This is what I think should be normalized, so that we have the same
> > amount of overall maximum timeout period on all platforms.
> > 
> > > Also - with the timeouts we're seeing, does the LTTPR return NAKs
> > > at all? That's
> > > still another thing I had suggested alternate workarounds for so
> > > that we could
> > > terminate transactions immediately on NAKs, so I wonder if that
> > > could save time
> > > here as well.
> > 
> > There's not much LTTPR specific in that wrt. what sinks would do
> > normally (no NAKs for read, only for writes) except LTTPRs may
> > rewrite
> > NAKs to ACKs to account for buggy monitors returning NAKs when
> > reading
> > the 0xf0000 -> range. But I'd suggest not dealing with this aspect
> > now,
> > just sanitize the above retry thing, as you suggested, remove the
> > i915
> > retry loop and make the drm retry loop configurable.
> > 
> > (In any case I also had the idea to stop transactions early when
> > HPD
> >  gets deasserted, but not sure if that's completely robust.)
> > 
> > > > > > > Thanks
> > > > > > > Khaled
> > > > > > > 
> > > > > > > > > > Anyways, this seems about the only thing we can do
> > > > > > > > > > given the
> > > > > > > > > > limited
> > > > > > > > > > hw capabilities.
> > > > > > > > > > Reviewed-by: Ville Syrjälä <
> > > > > > > > > > ville.syrjala@linux.intel.com>
> > > > > > > > > > 
> > > > > > > > > > > Accordingly disable LTTPR detection until GLK,
> > > > > > > > > > > where the
> > > > > > > > > > > maximum timeout
> > > > > > > > > > > we can set is only 1.6ms.
> > > > > > > > > > > 
> > > > > > > > > > > Link training in the non-transparent mode is
> > > > > > > > > > > known to fail at
> > > > > > > > > > > least on
> > > > > > > > > > > some SKL systems with a WD19 dock on the link,
> > > > > > > > > > > which exposes an
> > > > > > > > > > > LTTPR
> > > > > > > > > > > (see the References below). While this could have
> > > > > > > > > > > different
> > > > > > > > > > > reasons
> > > > > > > > > > > besides the too short AUX timeout used, not
> > > > > > > > > > > detecting LTTPRs
> > > > > > > > > > > (and so not
> > > > > > > > > > > using the non-transparent LT mode) fixes link
> > > > > > > > > > > training on these
> > > > > > > > > > > systems.
> > > > > > > > > > > 
> > > > > > > > > > > While at it add a code comment about the platform
> > > > > > > > > > > specific
> > > > > > > > > > > maximum
> > > > > > > > > > > timeout values.
> > > > > > > > > > > 
> > > > > > > > > > > v2: Add a comment about the g4x maximum timeout
> > > > > > > > > > > as well.
> > > > > > > > > > > (Ville)
> > > > > > > > > > > 
> > > > > > > > > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > > Reported-and-tested-by: Santiago Zarate <
> > > > > > > > > > > santiago.zarate@suse.com>
> > > > > > > > > > > Reported-and-tested-by: Bodo Graumann <
> > > > > > > > > > > mail@bodograumann.de>
> > > > > > > > > > > References:
> > > > > > > > > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > > > > > > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR
> > > > > > > > > > > non-transparent
> > > > > > > > > > > mode link training")
> > > > > > > > > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > > > > > > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > > > > > > > ---
> > > > > > > > > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c     
> > > > > > > > > > >   |  7 +++++++
> > > > > > > > > > >  .../gpu/drm/i915/display/intel_dp_link_training.
> > > > > > > > > > > c | 15
> > > > > > > > > > > ++++++++++++---
> > > > > > > > > > >  2 files changed, 19 insertions(+), 3 deletions(-
> > > > > > > > > > > )
> > > > > > > > > > > 
> > > > > > > > > > > diff --git
> > > > > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > index eaebf123310a..10fe17b7280d 100644
> > > > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > @@ -133,6 +133,7 @@ static u32
> > > > > > > > > > > g4x_get_aux_send_ctl(struct
> > > > > > > > > > > intel_dp *intel_dp,
> > > > > > > > > > >  else
> > > > > > > > > > >  precharge = 5;
> > > > > > > > > > > 
> > > > > > > > > > > +/* Max timeout value on G4x-BDW: 1.6ms */
> > > > > > > > > > >  if (IS_BROADWELL(dev_priv))
> > > > > > > > > > >  timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > > > > > > > > >  else
> > > > > > > > > > > @@ -159,6 +160,12 @@ static u32
> > > > > > > > > > > skl_get_aux_send_ctl(struct
> > > > > > > > > > > intel_dp *intel_dp,
> > > > > > > > > > >  enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > > > > > > > > > base.port);
> > > > > > > > > > >  u32 ret;
> > > > > > > > > > > 
> > > > > > > > > > > +/*
> > > > > > > > > > > + * Max timeout values:
> > > > > > > > > > > + * SKL-GLK: 1.6ms
> > > > > > > > > > > + * CNL: 3.2ms
> > > > > > > > > > > + * ICL+: 4ms
> > > > > > > > > > > + */
> > > > > > > > > > >  ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > > > > > >        DP_AUX_CH_CTL_DONE |
> > > > > > > > > > >        DP_AUX_CH_CTL_INTERRUPT |
> > > > > > > > > > > diff --git
> > > > > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_trai
> > > > > > > > > > > ning.c
> > > > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_trai
> > > > > > > > > > > ning.c
> > > > > > > > > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > > > > > > > > ---
> > > > > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_trai
> > > > > > > > > > > ning.c
> > > > > > > > > > > +++
> > > > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_trai
> > > > > > > > > > > ning.c
> > > > > > > > > > > @@ -82,6 +82,18 @@ static void
> > > > > > > > > > > intel_dp_read_lttpr_phy_caps(struct intel_dp
> > > > > > > > > > > *intel_dp,
> > > > > > > > > > > 
> > > > > > > > > > >  static bool
> > > > > > > > > > > intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > > > > > > > > *intel_dp)
> > > > > > > > > > >  {
> > > > > > > > > > > +struct drm_i915_private *i915 =
> > > > > > > > > > > dp_to_i915(intel_dp);
> > > > > > > > > > > +
> > > > > > > > > > > +if (intel_dp_is_edp(intel_dp))
> > > > > > > > > > > +return false;
> > > > > > > > > > > +
> > > > > > > > > > > +/*
> > > > > > > > > > > + * Detecting LTTPRs must be avoided on platforms
> > > > > > > > > > > with
> > > > > > > > > > > an AUX timeout
> > > > > > > > > > > + * period < 3.2ms. (see DP Standard v2.0,
> > > > > > > > > > > 2.11.2,
> > > > > > > > > > > 3.6.6.1).
> > > > > > > > > > > + */
> > > > > > > > > > > +if (INTEL_GEN(i915) < 10)
> > > > > > > > > > > +return false;
> > > > > > > > > > > +
> > > > > > > > > > >  if (drm_dp_read_lttpr_common_caps(&intel_dp-
> > > > > > > > > > > >aux,
> > > > > > > > > > >    intel_dp-
> > > > > > > > > > > > lttpr_common_caps) < 0) {
> > > > > > > > > > >  memset(intel_dp->lttpr_common_caps, 0,
> > > > > > > > > > > @@ -127,9 +139,6 @@ int
> > > > > > > > > > > intel_dp_lttpr_init(struct intel_dp
> > > > > > > > > > > *intel_dp)
> > > > > > > > > > >  bool ret;
> > > > > > > > > > >  int i;
> > > > > > > > > > > 
> > > > > > > > > > > -if (intel_dp_is_edp(intel_dp))
> > > > > > > > > > > -return 0;
> > > > > > > > > > > -
> > > > > > > > > > >  ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > > > > > > > > >  if (!ret)
> > > > > > > > > > >  return 0;
> > > > > > > > > > > --
> > > > > > > > > > > 2.25.1
> > > > > > > > > > 
> > > > > > > > > > --
> > > > > > > > > > Ville Syrjälä
> > > > > > > > > > Intel
> > > > > 
> > > > > -- 
> > > > > Sincerely,
> > > > >    Lyude Paul (she/her)
> > > > >    Software Engineer at Red Hat
> > > > >    
> > > > > Note: I deal with a lot of emails and have a lot of bugs on
> > > > > my plate. If
> > > > > you've
> > > > > asked me a question, are waiting for a review/merge on a
> > > > > patch, etc. and I
> > > > > haven't responded in a while, please feel free to send me
> > > > > another email to
> > > > > check
> > > > > on my status. I don't bite!
> > > > > 
> > > 
> > > -- 
> > > Sincerely,
> > >    Lyude Paul (she/her)
> > >    Software Engineer at Red Hat
> > >    
> > > Note: I deal with a lot of emails and have a lot of bugs on my
> > > plate. If you've
> > > asked me a question, are waiting for a review/merge on a patch,
> > > etc. and I
> > > haven't responded in a while, please feel free to send me another
> > > email to check
> > > on my status. I don't bite!
> > > 

Re: [PATCH v2 1/3] drm/i915/ilk-glk: Fix link training on links with LTTPRs

[Imre Deak]

On Sat, Mar 20, 2021 at 09:40:52AM +0200, Almahallawy, Khaled wrote:
> On Sat, 2021-03-20 at 09:15 +0200, Imre Deak wrote:
> > On Fri, Mar 19, 2021 at 11:07:21PM +0200, Imre Deak wrote:
> > > On Fri, Mar 19, 2021 at 04:44:26PM -0400, Lyude Paul wrote:
> > > > > > > [...]
> > > > > > > I think it would work if we can make the retries
> > > > > > > configurable and set it
> > > > > > > to
> > > > > > >         retries = total_timeout /
> > > > > > > platform_specific_timeout_per_retry
> > > > > > >
> > > > > > > where total_timeout would be something reasonable like 1
> > > > > > > sec.
> > > > > >
> > > > > > I actually think I'm more open to the idea of configurable
> > > > > > retries after
> > > > > > learning that apparently this is a thing that the i2c
> > > > > > subsystem does - so
> > > > > > there's more precedence for it in the rest of the kernel than
> > > > > > I originally
> > > > > > thought.
> > > > > >
> > > > > > I'm still curious if we need these extra retries in here
> > > > > > though - there seems
> > > > > > to
> > > > > > be one set of retries that is actually platform specific, and
> > > > > > then just a
> > > > > > random
> > > > > > set of 5 retries that don't seem to have anything to do with
> > > > > > platform specific
> > > > > > behavior - so I think it'd still be worth giving a shot at
> > > > > > getting rid of that
> > > > >
> > > > > The platform specific part of the timeout is the one desctibed
> > > > > in the
> > > > > maximum timeout values comments.
> > > >
> > > > You mean the
> > > >
> > > > /* Must try at least 3 times according to DP spec */
> > > > for (try = 0; try < 5; try++) {
> > > >
> > > > bit? I thought that wasn't related to platform specific retries
> > > > at all, since
> > > > the code in that loop seems to only reference parts of the DP
> > > > spec, and that the
> > > >
> > > > while ((aux_clock_divider = intel_dp-
> > > > >get_aux_clock_divider(intel_dp, clock++))) {
> > > >
> > > > Loop was the portion that was platform specific, since it prompts
> > > > the driver to
> > > > retry the transaction with different aux clock divider rates
> > > > depending on the
> > > > platform in use. Feel free to correct me if I'm wrong though.
> > >
> > > Nope. I meant every HW transaction will have a platform specific
> > > timeout. For instance it's 1.6ms on SKL, but 4ms on ICL. So now
> > > since
> > > the overall retry count is 32 * 5 = 160, on SKL we'll retry for
> > > ~2.6
> > > seconds, on ICL we'll retry for ~6.4 seconds (disregarding now the
> > > extra
> > > 400usec delay inserted by drm_dp_dpcd_access(), which adds a fixed
> > > ~1.3ms delay).
> >
> > Err, looks like I missed some coffee. Max total timeouts atm, which
> > we
> > would need to make the same on all platforms:
> >
> > g4x-glk: 5 * 32 * 1.6ms + 32 * 400us = 268.8ms
> > cnl    : 5 * 32 * 3.2ms + 32 * 400us = 524.8ms
> > icl+   : 5 * 32 * 4ms   + 32 * 400us = 652.8ms
> >
> 
> 
> Apology if I'm missing something. but in drm_dpcd_access() I think it
> is 500us not 400us?!

Ah, yes, or more like 600us so need to add 6.4ms to all of the above
figures.

> #define AUX_RETRY_INTERVAL 500 /* us */
> 
> if (ret != 0 && ret != -ETIMEDOUT) {
> usleep_range(AUX_RETRY_INTERVAL,
>      AUX_RETRY_INTERVAL + 100);
> }
> 
> Thanks
> Khaled
> 
> > > This is what I think should be normalized, so that we have the same
> > > amount of overall maximum timeout period on all platforms.
> > >
> > > > Also - with the timeouts we're seeing, does the LTTPR return NAKs
> > > > at all? That's
> > > > still another thing I had suggested alternate workarounds for so
> > > > that we could
> > > > terminate transactions immediately on NAKs, so I wonder if that
> > > > could save time
> > > > here as well.
> > >
> > > There's not much LTTPR specific in that wrt. what sinks would do
> > > normally (no NAKs for read, only for writes) except LTTPRs may
> > > rewrite
> > > NAKs to ACKs to account for buggy monitors returning NAKs when
> > > reading
> > > the 0xf0000 -> range. But I'd suggest not dealing with this aspect
> > > now,
> > > just sanitize the above retry thing, as you suggested, remove the
> > > i915
> > > retry loop and make the drm retry loop configurable.
> > >
> > > (In any case I also had the idea to stop transactions early when
> > > HPD
> > >  gets deasserted, but not sure if that's completely robust.)
> > >
> > > > > > > > Thanks
> > > > > > > > Khaled
> > > > > > > >
> > > > > > > > > > > Anyways, this seems about the only thing we can do
> > > > > > > > > > > given the
> > > > > > > > > > > limited
> > > > > > > > > > > hw capabilities.
> > > > > > > > > > > Reviewed-by: Ville Syrjälä <
> > > > > > > > > > > ville.syrjala@linux.intel.com>
> > > > > > > > > > >
> > > > > > > > > > > > Accordingly disable LTTPR detection until GLK,
> > > > > > > > > > > > where the
> > > > > > > > > > > > maximum timeout
> > > > > > > > > > > > we can set is only 1.6ms.
> > > > > > > > > > > >
> > > > > > > > > > > > Link training in the non-transparent mode is
> > > > > > > > > > > > known to fail at
> > > > > > > > > > > > least on
> > > > > > > > > > > > some SKL systems with a WD19 dock on the link,
> > > > > > > > > > > > which exposes an
> > > > > > > > > > > > LTTPR
> > > > > > > > > > > > (see the References below). While this could have
> > > > > > > > > > > > different
> > > > > > > > > > > > reasons
> > > > > > > > > > > > besides the too short AUX timeout used, not
> > > > > > > > > > > > detecting LTTPRs
> > > > > > > > > > > > (and so not
> > > > > > > > > > > > using the non-transparent LT mode) fixes link
> > > > > > > > > > > > training on these
> > > > > > > > > > > > systems.
> > > > > > > > > > > >
> > > > > > > > > > > > While at it add a code comment about the platform
> > > > > > > > > > > > specific
> > > > > > > > > > > > maximum
> > > > > > > > > > > > timeout values.
> > > > > > > > > > > >
> > > > > > > > > > > > v2: Add a comment about the g4x maximum timeout
> > > > > > > > > > > > as well.
> > > > > > > > > > > > (Ville)
> > > > > > > > > > > >
> > > > > > > > > > > > Reported-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > > > Reported-and-tested-by: Santiago Zarate <
> > > > > > > > > > > > santiago.zarate@suse.com>
> > > > > > > > > > > > Reported-and-tested-by: Bodo Graumann <
> > > > > > > > > > > > mail@bodograumann.de>
> > > > > > > > > > > > References:
> > > > > > > > > > > > https://gitlab.freedesktop.org/drm/intel/-/issues/3166
> > > > > > > > > > > > Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR
> > > > > > > > > > > > non-transparent
> > > > > > > > > > > > mode link training")
> > > > > > > > > > > > Cc: <stable@vger.kernel.org> # v5.11
> > > > > > > > > > > > Cc: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > >  drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > >   |  7 +++++++
> > > > > > > > > > > >  .../gpu/drm/i915/display/intel_dp_link_training.
> > > > > > > > > > > > c | 15
> > > > > > > > > > > > ++++++++++++---
> > > > > > > > > > > >  2 files changed, 19 insertions(+), 3 deletions(-
> > > > > > > > > > > > )
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git
> > > > > > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > > index eaebf123310a..10fe17b7280d 100644
> > > > > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> > > > > > > > > > > > @@ -133,6 +133,7 @@ static u32
> > > > > > > > > > > > g4x_get_aux_send_ctl(struct
> > > > > > > > > > > > intel_dp *intel_dp,
> > > > > > > > > > > >  else
> > > > > > > > > > > >  precharge = 5;
> > > > > > > > > > > >
> > > > > > > > > > > > +/* Max timeout value on G4x-BDW: 1.6ms */
> > > > > > > > > > > >  if (IS_BROADWELL(dev_priv))
> > > > > > > > > > > >  timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
> > > > > > > > > > > >  else
> > > > > > > > > > > > @@ -159,6 +160,12 @@ static u32
> > > > > > > > > > > > skl_get_aux_send_ctl(struct
> > > > > > > > > > > > intel_dp *intel_dp,
> > > > > > > > > > > >  enum phy phy = intel_port_to_phy(i915, dig_port-
> > > > > > > > > > > > > base.port);
> > > > > > > > > > > >  u32 ret;
> > > > > > > > > > > >
> > > > > > > > > > > > +/*
> > > > > > > > > > > > + * Max timeout values:
> > > > > > > > > > > > + * SKL-GLK: 1.6ms
> > > > > > > > > > > > + * CNL: 3.2ms
> > > > > > > > > > > > + * ICL+: 4ms
> > > > > > > > > > > > + */
> > > > > > > > > > > >  ret = DP_AUX_CH_CTL_SEND_BUSY |
> > > > > > > > > > > >        DP_AUX_CH_CTL_DONE |
> > > > > > > > > > > >        DP_AUX_CH_CTL_INTERRUPT |
> > > > > > > > > > > > diff --git
> > > > > > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_trai
> > > > > > > > > > > > ning.c
> > > > > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_trai
> > > > > > > > > > > > ning.c
> > > > > > > > > > > > index 19ba7c7cbaab..c0e25c75c105 100644
> > > > > > > > > > > > ---
> > > > > > > > > > > > a/drivers/gpu/drm/i915/display/intel_dp_link_trai
> > > > > > > > > > > > ning.c
> > > > > > > > > > > > +++
> > > > > > > > > > > > b/drivers/gpu/drm/i915/display/intel_dp_link_trai
> > > > > > > > > > > > ning.c
> > > > > > > > > > > > @@ -82,6 +82,18 @@ static void
> > > > > > > > > > > > intel_dp_read_lttpr_phy_caps(struct intel_dp
> > > > > > > > > > > > *intel_dp,
> > > > > > > > > > > >
> > > > > > > > > > > >  static bool
> > > > > > > > > > > > intel_dp_read_lttpr_common_caps(struct intel_dp
> > > > > > > > > > > > *intel_dp)
> > > > > > > > > > > >  {
> > > > > > > > > > > > +struct drm_i915_private *i915 =
> > > > > > > > > > > > dp_to_i915(intel_dp);
> > > > > > > > > > > > +
> > > > > > > > > > > > +if (intel_dp_is_edp(intel_dp))
> > > > > > > > > > > > +return false;
> > > > > > > > > > > > +
> > > > > > > > > > > > +/*
> > > > > > > > > > > > + * Detecting LTTPRs must be avoided on platforms
> > > > > > > > > > > > with
> > > > > > > > > > > > an AUX timeout
> > > > > > > > > > > > + * period < 3.2ms. (see DP Standard v2.0,
> > > > > > > > > > > > 2.11.2,
> > > > > > > > > > > > 3.6.6.1).
> > > > > > > > > > > > + */
> > > > > > > > > > > > +if (INTEL_GEN(i915) < 10)
> > > > > > > > > > > > +return false;
> > > > > > > > > > > > +
> > > > > > > > > > > >  if (drm_dp_read_lttpr_common_caps(&intel_dp-
> > > > > > > > > > > > >aux,
> > > > > > > > > > > >    intel_dp-
> > > > > > > > > > > > > lttpr_common_caps) < 0) {
> > > > > > > > > > > >  memset(intel_dp->lttpr_common_caps, 0,
> > > > > > > > > > > > @@ -127,9 +139,6 @@ int
> > > > > > > > > > > > intel_dp_lttpr_init(struct intel_dp
> > > > > > > > > > > > *intel_dp)
> > > > > > > > > > > >  bool ret;
> > > > > > > > > > > >  int i;
> > > > > > > > > > > >
> > > > > > > > > > > > -if (intel_dp_is_edp(intel_dp))
> > > > > > > > > > > > -return 0;
> > > > > > > > > > > > -
> > > > > > > > > > > >  ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > > > > > > > > > > >  if (!ret)
> > > > > > > > > > > >  return 0;
> > > > > > > > > > > > --
> > > > > > > > > > > > 2.25.1
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Ville Syrjälä
> > > > > > > > > > > Intel
> > > > > >
> > > > > > --
> > > > > > Sincerely,
> > > > > >    Lyude Paul (she/her)
> > > > > >    Software Engineer at Red Hat
> > > > > >
> > > > > > Note: I deal with a lot of emails and have a lot of bugs on
> > > > > > my plate. If
> > > > > > you've
> > > > > > asked me a question, are waiting for a review/merge on a
> > > > > > patch, etc. and I
> > > > > > haven't responded in a while, please feel free to send me
> > > > > > another email to
> > > > > > check
> > > > > > on my status. I don't bite!
> > > > > >
> > > >
> > > > --
> > > > Sincerely,
> > > >    Lyude Paul (she/her)
> > > >    Software Engineer at Red Hat
> > > >
> > > > Note: I deal with a lot of emails and have a lot of bugs on my
> > > > plate. If you've
> > > > asked me a question, are waiting for a review/merge on a patch,
> > > > etc. and I
> > > > haven't responded in a while, please feel free to send me another
> > > > email to check
> > > > on my status. I don't bite!
> > > >

[PATCH v2 2/3] drm/i915: Disable LTTPR support when the DPCD rev < 1.4

[Imre Deak]

By the specification the 0xF0000-0xF02FF range is only valid when the
DPCD revision is 1.4 or higher. Disable LTTPR support if this isn't so.

Trying to detect LTTPRs returned corrupted values for the above DPCD
range at least on a Skylake host with an LG 43UD79-B monitor with a DPCD
revision 1.2 connected.

Fixes: 7b2a4ab8b0ef ("drm/i915: Switch to LTTPR transparent mode link training")
Cc: <stable@vger.kernel.org> # v5.11
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
 .../drm/i915/display/intel_dp_link_training.c | 39 +++++++++++++------
 .../drm/i915/display/intel_dp_link_training.h |  2 +-
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index b6b5776f5a66..873684da0cd4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3711,9 +3711,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
 	int ret;
 
-	intel_dp_lttpr_init(intel_dp);
-
-	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd))
+	if (intel_dp_init_lttpr_and_dprx_caps(intel_dp) < 0)
 		return false;
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index c0e25c75c105..d8d90903226f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -35,6 +35,11 @@ intel_dp_dump_link_status(struct drm_device *drm,
 		    link_status[3], link_status[4], link_status[5]);
 }
 
+static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp)
+{
+	memset(&intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps));
+}
+
 static void intel_dp_reset_lttpr_count(struct intel_dp *intel_dp)
 {
 	intel_dp->lttpr_common_caps[DP_PHY_REPEATER_CNT -
@@ -96,8 +101,7 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
 
 	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
 					  intel_dp->lttpr_common_caps) < 0) {
-		memset(intel_dp->lttpr_common_caps, 0,
-		       sizeof(intel_dp->lttpr_common_caps));
+		intel_dp_reset_lttpr_common_caps(intel_dp);
 		return false;
 	}
 
@@ -119,27 +123,37 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
 }
 
 /**
- * intel_dp_lttpr_init - detect LTTPRs and init the LTTPR link training mode
+ * intel_dp_init_lttpr_and_dprx_caps - detect LTTPR and DPRX caps, init the LTTPR link training mode
  * @intel_dp: Intel DP struct
  *
- * Read the LTTPR common capabilities, switch to non-transparent link training
- * mode if any is detected and read the PHY capabilities for all detected
- * LTTPRs. In case of an LTTPR detection error or if the number of
+ * Read the LTTPR common and DPRX capabilities and switch to non-transparent
+ * link training mode if any is detected and read the PHY capabilities for all
+ * detected LTTPRs. In case of an LTTPR detection error or if the number of
  * LTTPRs is more than is supported (8), fall back to the no-LTTPR,
  * transparent mode link training mode.
  *
  * Returns:
- *   >0  if LTTPRs were detected and the non-transparent LT mode was set
+ *   >0  if LTTPRs were detected and the non-transparent LT mode was set. The
+ *       DPRX capabilities are read out.
  *    0  if no LTTPRs or more than 8 LTTPRs were detected or in case of a
- *       detection failure and the transparent LT mode was set
+ *       detection failure and the transparent LT mode was set. The DPRX
+ *       capabilities are read out.
+ *   <0  Reading out the DPRX capabilities failed.
  */
-int intel_dp_lttpr_init(struct intel_dp *intel_dp)
+int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
 {
 	int lttpr_count;
 	bool ret;
 	int i;
 
 	ret = intel_dp_read_lttpr_common_caps(intel_dp);
+
+	/* The DPTX shall read the DRPX caps after LTTPR detection. */
+	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd)) {
+		intel_dp_reset_lttpr_common_caps(intel_dp);
+		return -EIO;
+	}
+
 	if (!ret)
 		return 0;
 
@@ -182,7 +196,7 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
 
 	return lttpr_count;
 }
-EXPORT_SYMBOL(intel_dp_lttpr_init);
+EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);
 
 static u8 dp_voltage_max(u8 preemph)
 {
@@ -817,7 +831,10 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
 	 * TODO: Reiniting LTTPRs here won't be needed once proper connector
 	 * HW state readout is added.
 	 */
-	int lttpr_count = intel_dp_lttpr_init(intel_dp);
+	int lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp);
+
+	if (lttpr_count < 0)
+		return;
 
 	if (!intel_dp_link_train_all_phys(intel_dp, crtc_state, lttpr_count))
 		intel_dp_schedule_fallback_link_training(intel_dp, crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
index 6a1f76bd8c75..9cb7c28027f0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
@@ -11,7 +11,7 @@
 struct intel_crtc_state;
 struct intel_dp;
 
-int intel_dp_lttpr_init(struct intel_dp *intel_dp);
+int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp);
 
 void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
 			       const struct intel_crtc_state *crtc_state,
-- 
2.25.1

[PATCH v2 2/3] drm/i915: Disable LTTPR support when the DPCD rev < 1.4

[Imre Deak]

By the specification the 0xF0000-0xF02FF range is only valid when the
DPCD revision is 1.4 or higher. Disable LTTPR support if this isn't so.

Trying to detect LTTPRs returned corrupted values for the above DPCD
range at least on a Skylake host with an LG 43UD79-B monitor with a DPCD
revision 1.2 connected.

v2: Add the actual version check.

Fixes: 7b2a4ab8b0ef ("drm/i915: Switch to LTTPR transparent mode link training")
Cc: <stable@vger.kernel.org> # v5.11
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
 .../drm/i915/display/intel_dp_link_training.c | 48 ++++++++++++++-----
 .../drm/i915/display/intel_dp_link_training.h |  2 +-
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index b6b5776f5a66..873684da0cd4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3711,9 +3711,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
 	int ret;
 
-	intel_dp_lttpr_init(intel_dp);
-
-	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd))
+	if (intel_dp_init_lttpr_and_dprx_caps(intel_dp) < 0)
 		return false;
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index c0e25c75c105..5a821d644e9c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -35,6 +35,11 @@ intel_dp_dump_link_status(struct drm_device *drm,
 		    link_status[3], link_status[4], link_status[5]);
 }
 
+static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp)
+{
+	memset(&intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps));
+}
+
 static void intel_dp_reset_lttpr_count(struct intel_dp *intel_dp)
 {
 	intel_dp->lttpr_common_caps[DP_PHY_REPEATER_CNT -
@@ -96,8 +101,7 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
 
 	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
 					  intel_dp->lttpr_common_caps) < 0) {
-		memset(intel_dp->lttpr_common_caps, 0,
-		       sizeof(intel_dp->lttpr_common_caps));
+		intel_dp_reset_lttpr_common_caps(intel_dp);
 		return false;
 	}
 
@@ -119,30 +123,49 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
 }
 
 /**
- * intel_dp_lttpr_init - detect LTTPRs and init the LTTPR link training mode
+ * intel_dp_init_lttpr_and_dprx_caps - detect LTTPR and DPRX caps, init the LTTPR link training mode
  * @intel_dp: Intel DP struct
  *
- * Read the LTTPR common capabilities, switch to non-transparent link training
- * mode if any is detected and read the PHY capabilities for all detected
- * LTTPRs. In case of an LTTPR detection error or if the number of
+ * Read the LTTPR common and DPRX capabilities and switch to non-transparent
+ * link training mode if any is detected and read the PHY capabilities for all
+ * detected LTTPRs. In case of an LTTPR detection error or if the number of
  * LTTPRs is more than is supported (8), fall back to the no-LTTPR,
  * transparent mode link training mode.
  *
  * Returns:
- *   >0  if LTTPRs were detected and the non-transparent LT mode was set
+ *   >0  if LTTPRs were detected and the non-transparent LT mode was set. The
+ *       DPRX capabilities are read out.
  *    0  if no LTTPRs or more than 8 LTTPRs were detected or in case of a
- *       detection failure and the transparent LT mode was set
+ *       detection failure and the transparent LT mode was set. The DPRX
+ *       capabilities are read out.
+ *   <0  Reading out the DPRX capabilities failed.
  */
-int intel_dp_lttpr_init(struct intel_dp *intel_dp)
+int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
 {
 	int lttpr_count;
 	bool ret;
 	int i;
 
 	ret = intel_dp_read_lttpr_common_caps(intel_dp);
+
+	/* The DPTX shall read the DRPX caps after LTTPR detection. */
+	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd)) {
+		intel_dp_reset_lttpr_common_caps(intel_dp);
+		return -EIO;
+	}
+
 	if (!ret)
 		return 0;
 
+	/*
+	 * The 0xF0000-0xF02FF range is only valid if the DPCD revision is
+	 * at least 1.4.
+	 */
+	if (intel_dp->dpcd[DP_DPCD_REV] < 0x14) {
+		intel_dp_reset_lttpr_common_caps(intel_dp);
+		return 0;
+	}
+
 	lttpr_count = drm_dp_lttpr_count(intel_dp->lttpr_common_caps);
 	/*
 	 * Prevent setting LTTPR transparent mode explicitly if no LTTPRs are
@@ -182,7 +205,7 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
 
 	return lttpr_count;
 }
-EXPORT_SYMBOL(intel_dp_lttpr_init);
+EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);
 
 static u8 dp_voltage_max(u8 preemph)
 {
@@ -817,7 +840,10 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
 	 * TODO: Reiniting LTTPRs here won't be needed once proper connector
 	 * HW state readout is added.
 	 */
-	int lttpr_count = intel_dp_lttpr_init(intel_dp);
+	int lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp);
+
+	if (lttpr_count < 0)
+		return;
 
 	if (!intel_dp_link_train_all_phys(intel_dp, crtc_state, lttpr_count))
 		intel_dp_schedule_fallback_link_training(intel_dp, crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
index 6a1f76bd8c75..9cb7c28027f0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
@@ -11,7 +11,7 @@
 struct intel_crtc_state;
 struct intel_dp;
 
-int intel_dp_lttpr_init(struct intel_dp *intel_dp);
+int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp);
 
 void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
 			       const struct intel_crtc_state *crtc_state,
-- 
2.25.1

Re: [PATCH v2 2/3] drm/i915: Disable LTTPR support when the DPCD rev < 1.4

[Ville Syrjälä]

On Wed, Mar 17, 2021 at 09:01:49PM +0200, Imre Deak wrote:
> By the specification the 0xF0000-0xF02FF range is only valid when the
> DPCD revision is 1.4 or higher. Disable LTTPR support if this isn't so.
> 
> Trying to detect LTTPRs returned corrupted values for the above DPCD
> range at least on a Skylake host with an LG 43UD79-B monitor with a DPCD
> revision 1.2 connected.
> 
> v2: Add the actual version check.
> 
> Fixes: 7b2a4ab8b0ef ("drm/i915: Switch to LTTPR transparent mode link training")
> Cc: <stable@vger.kernel.org> # v5.11
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
>  .../drm/i915/display/intel_dp_link_training.c | 48 ++++++++++++++-----
>  .../drm/i915/display/intel_dp_link_training.h |  2 +-
>  3 files changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index b6b5776f5a66..873684da0cd4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3711,9 +3711,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  {
>  	int ret;
>  
> -	intel_dp_lttpr_init(intel_dp);
> -
> -	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd))
> +	if (intel_dp_init_lttpr_and_dprx_caps(intel_dp) < 0)
>  		return false;
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index c0e25c75c105..5a821d644e9c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -35,6 +35,11 @@ intel_dp_dump_link_status(struct drm_device *drm,
>  		    link_status[3], link_status[4], link_status[5]);
>  }
>  
> +static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp)
> +{
> +	memset(&intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps));
> +}
> +
>  static void intel_dp_reset_lttpr_count(struct intel_dp *intel_dp)
>  {
>  	intel_dp->lttpr_common_caps[DP_PHY_REPEATER_CNT -
> @@ -96,8 +101,7 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
>  
>  	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
>  					  intel_dp->lttpr_common_caps) < 0) {
> -		memset(intel_dp->lttpr_common_caps, 0,
> -		       sizeof(intel_dp->lttpr_common_caps));
> +		intel_dp_reset_lttpr_common_caps(intel_dp);
>  		return false;
>  	}
>  
> @@ -119,30 +123,49 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
>  }
>  
>  /**
> - * intel_dp_lttpr_init - detect LTTPRs and init the LTTPR link training mode
> + * intel_dp_init_lttpr_and_dprx_caps - detect LTTPR and DPRX caps, init the LTTPR link training mode
>   * @intel_dp: Intel DP struct
>   *
> - * Read the LTTPR common capabilities, switch to non-transparent link training
> - * mode if any is detected and read the PHY capabilities for all detected
> - * LTTPRs. In case of an LTTPR detection error or if the number of
> + * Read the LTTPR common and DPRX capabilities and switch to non-transparent
> + * link training mode if any is detected and read the PHY capabilities for all
> + * detected LTTPRs. In case of an LTTPR detection error or if the number of
>   * LTTPRs is more than is supported (8), fall back to the no-LTTPR,
>   * transparent mode link training mode.
>   *
>   * Returns:
> - *   >0  if LTTPRs were detected and the non-transparent LT mode was set
> + *   >0  if LTTPRs were detected and the non-transparent LT mode was set. The
> + *       DPRX capabilities are read out.
>   *    0  if no LTTPRs or more than 8 LTTPRs were detected or in case of a
> - *       detection failure and the transparent LT mode was set
> + *       detection failure and the transparent LT mode was set. The DPRX
> + *       capabilities are read out.
> + *   <0  Reading out the DPRX capabilities failed.
>   */
> -int intel_dp_lttpr_init(struct intel_dp *intel_dp)
> +int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
>  {
>  	int lttpr_count;
>  	bool ret;
>  	int i;
>  
>  	ret = intel_dp_read_lttpr_common_caps(intel_dp);
> +
> +	/* The DPTX shall read the DRPX caps after LTTPR detection. */
> +	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd)) {
> +		intel_dp_reset_lttpr_common_caps(intel_dp);
> +		return -EIO;
> +	}
> +
>  	if (!ret)
>  		return 0;
>  
> +	/*
> +	 * The 0xF0000-0xF02FF range is only valid if the DPCD revision is
> +	 * at least 1.4.
> +	 */
> +	if (intel_dp->dpcd[DP_DPCD_REV] < 0x14) {
> +		intel_dp_reset_lttpr_common_caps(intel_dp);
> +		return 0;
> +	}

Slight chicken vs. egg I guess. Seems ok

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
>  	lttpr_count = drm_dp_lttpr_count(intel_dp->lttpr_common_caps);
>  	/*
>  	 * Prevent setting LTTPR transparent mode explicitly if no LTTPRs are
> @@ -182,7 +205,7 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
>  
>  	return lttpr_count;
>  }
> -EXPORT_SYMBOL(intel_dp_lttpr_init);
> +EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);
>  
>  static u8 dp_voltage_max(u8 preemph)
>  {
> @@ -817,7 +840,10 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
>  	 * TODO: Reiniting LTTPRs here won't be needed once proper connector
>  	 * HW state readout is added.
>  	 */
> -	int lttpr_count = intel_dp_lttpr_init(intel_dp);
> +	int lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp);
> +
> +	if (lttpr_count < 0)
> +		return;
>  
>  	if (!intel_dp_link_train_all_phys(intel_dp, crtc_state, lttpr_count))
>  		intel_dp_schedule_fallback_link_training(intel_dp, crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> index 6a1f76bd8c75..9cb7c28027f0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> @@ -11,7 +11,7 @@
>  struct intel_crtc_state;
>  struct intel_dp;
>  
> -int intel_dp_lttpr_init(struct intel_dp *intel_dp);
> +int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp);
>  
>  void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
>  			       const struct intel_crtc_state *crtc_state,
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 2/3] drm/i915: Disable LTTPR support when the DPCD rev < 1.4

[Imre Deak]

On Thu, Mar 18, 2021 at 07:57:22PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 17, 2021 at 09:01:49PM +0200, Imre Deak wrote:
> > By the specification the 0xF0000-0xF02FF range is only valid when the
> > DPCD revision is 1.4 or higher. Disable LTTPR support if this isn't so.
> > 
> > Trying to detect LTTPRs returned corrupted values for the above DPCD
> > range at least on a Skylake host with an LG 43UD79-B monitor with a DPCD
> > revision 1.2 connected.
> > 
> > v2: Add the actual version check.
> > 
> > Fixes: 7b2a4ab8b0ef ("drm/i915: Switch to LTTPR transparent mode link training")
> > Cc: <stable@vger.kernel.org> # v5.11
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
> >  .../drm/i915/display/intel_dp_link_training.c | 48 ++++++++++++++-----
> >  .../drm/i915/display/intel_dp_link_training.h |  2 +-
> >  3 files changed, 39 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index b6b5776f5a66..873684da0cd4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -3711,9 +3711,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> >  {
> >  	int ret;
> >  
> > -	intel_dp_lttpr_init(intel_dp);
> > -
> > -	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd))
> > +	if (intel_dp_init_lttpr_and_dprx_caps(intel_dp) < 0)
> >  		return false;
> >  
> >  	/*
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > index c0e25c75c105..5a821d644e9c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -35,6 +35,11 @@ intel_dp_dump_link_status(struct drm_device *drm,
> >  		    link_status[3], link_status[4], link_status[5]);
> >  }
> >  
> > +static void intel_dp_reset_lttpr_common_caps(struct intel_dp *intel_dp)
> > +{
> > +	memset(&intel_dp->lttpr_common_caps, 0, sizeof(intel_dp->lttpr_common_caps));
> > +}
> > +
> >  static void intel_dp_reset_lttpr_count(struct intel_dp *intel_dp)
> >  {
> >  	intel_dp->lttpr_common_caps[DP_PHY_REPEATER_CNT -
> > @@ -96,8 +101,7 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
> >  
> >  	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> >  					  intel_dp->lttpr_common_caps) < 0) {
> > -		memset(intel_dp->lttpr_common_caps, 0,
> > -		       sizeof(intel_dp->lttpr_common_caps));
> > +		intel_dp_reset_lttpr_common_caps(intel_dp);
> >  		return false;
> >  	}
> >  
> > @@ -119,30 +123,49 @@ intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
> >  }
> >  
> >  /**
> > - * intel_dp_lttpr_init - detect LTTPRs and init the LTTPR link training mode
> > + * intel_dp_init_lttpr_and_dprx_caps - detect LTTPR and DPRX caps, init the LTTPR link training mode
> >   * @intel_dp: Intel DP struct
> >   *
> > - * Read the LTTPR common capabilities, switch to non-transparent link training
> > - * mode if any is detected and read the PHY capabilities for all detected
> > - * LTTPRs. In case of an LTTPR detection error or if the number of
> > + * Read the LTTPR common and DPRX capabilities and switch to non-transparent
> > + * link training mode if any is detected and read the PHY capabilities for all
> > + * detected LTTPRs. In case of an LTTPR detection error or if the number of
> >   * LTTPRs is more than is supported (8), fall back to the no-LTTPR,
> >   * transparent mode link training mode.
> >   *
> >   * Returns:
> > - *   >0  if LTTPRs were detected and the non-transparent LT mode was set
> > + *   >0  if LTTPRs were detected and the non-transparent LT mode was set. The
> > + *       DPRX capabilities are read out.
> >   *    0  if no LTTPRs or more than 8 LTTPRs were detected or in case of a
> > - *       detection failure and the transparent LT mode was set
> > + *       detection failure and the transparent LT mode was set. The DPRX
> > + *       capabilities are read out.
> > + *   <0  Reading out the DPRX capabilities failed.
> >   */
> > -int intel_dp_lttpr_init(struct intel_dp *intel_dp)
> > +int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
> >  {
> >  	int lttpr_count;
> >  	bool ret;
> >  	int i;
> >  
> >  	ret = intel_dp_read_lttpr_common_caps(intel_dp);
> > +
> > +	/* The DPTX shall read the DRPX caps after LTTPR detection. */
> > +	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd)) {
> > +		intel_dp_reset_lttpr_common_caps(intel_dp);
> > +		return -EIO;
> > +	}
> > +
> >  	if (!ret)
> >  		return 0;
> >  
> > +	/*
> > +	 * The 0xF0000-0xF02FF range is only valid if the DPCD revision is
> > +	 * at least 1.4.
> > +	 */
> > +	if (intel_dp->dpcd[DP_DPCD_REV] < 0x14) {
> > +		intel_dp_reset_lttpr_common_caps(intel_dp);
> > +		return 0;
> > +	}
> 
> Slight chicken vs. egg I guess. Seems ok

Yes, reading 0xF0000-0xF0007 has a side effect and I suppose the LTTPRs
could change something in the returned DPRX caps, depending on whether
the read happened or not.

> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +
> >  	lttpr_count = drm_dp_lttpr_count(intel_dp->lttpr_common_caps);
> >  	/*
> >  	 * Prevent setting LTTPR transparent mode explicitly if no LTTPRs are
> > @@ -182,7 +205,7 @@ int intel_dp_lttpr_init(struct intel_dp *intel_dp)
> >  
> >  	return lttpr_count;
> >  }
> > -EXPORT_SYMBOL(intel_dp_lttpr_init);
> > +EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);
> >  
> >  static u8 dp_voltage_max(u8 preemph)
> >  {
> > @@ -817,7 +840,10 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
> >  	 * TODO: Reiniting LTTPRs here won't be needed once proper connector
> >  	 * HW state readout is added.
> >  	 */
> > -	int lttpr_count = intel_dp_lttpr_init(intel_dp);
> > +	int lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp);
> > +
> > +	if (lttpr_count < 0)
> > +		return;
> >  
> >  	if (!intel_dp_link_train_all_phys(intel_dp, crtc_state, lttpr_count))
> >  		intel_dp_schedule_fallback_link_training(intel_dp, crtc_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> > index 6a1f76bd8c75..9cb7c28027f0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> > @@ -11,7 +11,7 @@
> >  struct intel_crtc_state;
> >  struct intel_dp;
> >  
> > -int intel_dp_lttpr_init(struct intel_dp *intel_dp);
> > +int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp);
> >  
> >  void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> >  			       const struct intel_crtc_state *crtc_state,
> > -- 
> > 2.25.1
> 
> -- 
> Ville Syrjälä
> Intel

[PATCH v2 3/3] drm/i915: Disable LTTPR support when the LTTPR rev < 1.4

[Imre Deak]

By the specification the 0xF0000 - 0xF02FF range is only valid if the
LTTPR revision at 0xF0000 is at least 1.4. Disable the LTTPR support
otherwise.

Fixes: 7b2a4ab8b0ef ("drm/i915: Switch to LTTPR transparent mode link training")
Cc: <stable@vger.kernel.org> # v5.11
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../gpu/drm/i915/display/intel_dp_link_training.c  | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index d8d90903226f..d92eb192c89d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -100,17 +100,23 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
 		return false;
 
 	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
-					  intel_dp->lttpr_common_caps) < 0) {
-		intel_dp_reset_lttpr_common_caps(intel_dp);
-		return false;
-	}
+					  intel_dp->lttpr_common_caps) < 0)
+		goto reset_caps;
 
 	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
 		    "LTTPR common capabilities: %*ph\n",
 		    (int)sizeof(intel_dp->lttpr_common_caps),
 		    intel_dp->lttpr_common_caps);
 
+	/* The minimum value of LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV is 1.4 */
+	if (intel_dp->lttpr_common_caps[0] < 0x14)
+		goto reset_caps;
+
 	return true;
+
+reset_caps:
+	intel_dp_reset_lttpr_common_caps(intel_dp);
+	return false;
 }
 
 static bool
-- 
2.25.1

Re: [PATCH v2 3/3] drm/i915: Disable LTTPR support when the LTTPR rev < 1.4

[Ville Syrjälä]

On Wed, Mar 17, 2021 at 08:49:01PM +0200, Imre Deak wrote:
> By the specification the 0xF0000 - 0xF02FF range is only valid if the
> LTTPR revision at 0xF0000 is at least 1.4. Disable the LTTPR support
> otherwise.
> 
> Fixes: 7b2a4ab8b0ef ("drm/i915: Switch to LTTPR transparent mode link training")
> Cc: <stable@vger.kernel.org> # v5.11
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_dp_link_training.c  | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index d8d90903226f..d92eb192c89d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -100,17 +100,23 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
>  		return false;
>  
>  	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> -					  intel_dp->lttpr_common_caps) < 0) {
> -		intel_dp_reset_lttpr_common_caps(intel_dp);
> -		return false;
> -	}
> +					  intel_dp->lttpr_common_caps) < 0)
> +		goto reset_caps;

BTW just noticed this oddball thing in the spec:
"DPTX shall read specific registers within the LTTPR field (DPCD
 Addresses F0000h through F0004h; see Table 2-198) to determine
 whether any LTTPR(s) are present and if so, how many. This read
 shall be in the form of a 5-byte native AUX Read transaction."

Why exactly 5 bytes? I have no idea. Doesn't really make sense.
Just wondering if we really need to respect that and some LTTPRs
would fsck things up if we read more...

Anyways
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
>  		    "LTTPR common capabilities: %*ph\n",
>  		    (int)sizeof(intel_dp->lttpr_common_caps),
>  		    intel_dp->lttpr_common_caps);
>  
> +	/* The minimum value of LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV is 1.4 */
> +	if (intel_dp->lttpr_common_caps[0] < 0x14)
> +		goto reset_caps;
> +
>  	return true;
> +
> +reset_caps:
> +	intel_dp_reset_lttpr_common_caps(intel_dp);
> +	return false;
>  }
>  
>  static bool
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 3/3] drm/i915: Disable LTTPR support when the LTTPR rev < 1.4

[Imre Deak]

On Thu, Mar 18, 2021 at 08:00:10PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 17, 2021 at 08:49:01PM +0200, Imre Deak wrote:
> > By the specification the 0xF0000 - 0xF02FF range is only valid if the
> > LTTPR revision at 0xF0000 is at least 1.4. Disable the LTTPR support
> > otherwise.
> > 
> > Fixes: 7b2a4ab8b0ef ("drm/i915: Switch to LTTPR transparent mode link training")
> > Cc: <stable@vger.kernel.org> # v5.11
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  .../gpu/drm/i915/display/intel_dp_link_training.c  | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > index d8d90903226f..d92eb192c89d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -100,17 +100,23 @@ static bool intel_dp_read_lttpr_common_caps(struct intel_dp *intel_dp)
> >  		return false;
> >  
> >  	if (drm_dp_read_lttpr_common_caps(&intel_dp->aux,
> > -					  intel_dp->lttpr_common_caps) < 0) {
> > -		intel_dp_reset_lttpr_common_caps(intel_dp);
> > -		return false;
> > -	}
> > +					  intel_dp->lttpr_common_caps) < 0)
> > +		goto reset_caps;
> 
> BTW just noticed this oddball thing in the spec:
> "DPTX shall read specific registers within the LTTPR field (DPCD
>  Addresses F0000h through F0004h; see Table 2-198) to determine
>  whether any LTTPR(s) are present and if so, how many. This read
>  shall be in the form of a 5-byte native AUX Read transaction."
> 
> Why exactly 5 bytes? I have no idea. Doesn't really make sense.
> Just wondering if we really need to respect that and some LTTPRs
> would fsck things up if we read more...

I pointed this out to spec people and the new version (2.1) will require
this to be 8 bytes. But yes, I do hope no existing ones depend on this
being 5 bytes.

> Anyways
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> >  
> >  	drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> >  		    "LTTPR common capabilities: %*ph\n",
> >  		    (int)sizeof(intel_dp->lttpr_common_caps),
> >  		    intel_dp->lttpr_common_caps);
> >  
> > +	/* The minimum value of LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV is 1.4 */
> > +	if (intel_dp->lttpr_common_caps[0] < 0x14)
> > +		goto reset_caps;
> > +
> >  	return true;
> > +
> > +reset_caps:
> > +	intel_dp_reset_lttpr_common_caps(intel_dp);
> > +	return false;
> >  }
> >  
> >  static bool
> > -- 
> > 2.25.1
> 
> -- 
> Ville Syrjälä
> Intel

Re: ✓ Fi.CI.IGT: success for drm/i915: Fix DP LTTPR link training mode initialization (rev2)

[Imre Deak]

On Wed, Mar 17, 2021 at 10:34:24PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Fix DP LTTPR link training mode initialization (rev2)
> URL   : https://patchwork.freedesktop.org/series/88070/
> State : success

Patchset pushed to drm-intel-next, thanks for the reports, testing and review.

I'll send a separate backport to the 5.11 stable tree if needed, since
patch 1 will not apply there due to a trivial conflict (intel_dp.c ->
intel_dp_aux.c code movement in drm-intel-next).

> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_9866_full -> Patchwork_19802_full
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.
> 
>   
> 
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_19802_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_create@create-massive:
>     - shard-snb:          NOTRUN -> [DMESG-WARN][1] ([i915#3002])
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-snb7/igt@gem_create@create-massive.html
> 
>   * igt@gem_ctx_isolation@preservation-s3@vcs0:
>     - shard-apl:          [PASS][2] -> [DMESG-WARN][3] ([i915#180]) +1 similar issue
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-apl8/igt@gem_ctx_isolation@preservation-s3@vcs0.html
>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl3/igt@gem_ctx_isolation@preservation-s3@vcs0.html
> 
>   * igt@gem_ctx_persistence@legacy-engines-hang@blt:
>     - shard-skl:          NOTRUN -> [SKIP][4] ([fdo#109271]) +159 similar issues
>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl9/igt@gem_ctx_persistence@legacy-engines-hang@blt.html
> 
>   * igt@gem_ctx_persistence@legacy-engines-mixed:
>     - shard-snb:          NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#1099]) +5 similar issues
>    [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-snb7/igt@gem_ctx_persistence@legacy-engines-mixed.html
> 
>   * igt@gem_exec_fair@basic-flow@rcs0:
>     - shard-tglb:         [PASS][6] -> [FAIL][7] ([i915#2842])
>    [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-tglb1/igt@gem_exec_fair@basic-flow@rcs0.html
>    [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-tglb2/igt@gem_exec_fair@basic-flow@rcs0.html
> 
>   * igt@gem_exec_fair@basic-none@rcs0:
>     - shard-glk:          [PASS][8] -> [FAIL][9] ([i915#2842])
>    [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-glk5/igt@gem_exec_fair@basic-none@rcs0.html
>    [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-glk7/igt@gem_exec_fair@basic-none@rcs0.html
> 
>   * igt@gem_exec_fair@basic-pace@vcs0:
>     - shard-kbl:          [PASS][10] -> [FAIL][11] ([i915#2842])
>    [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-kbl6/igt@gem_exec_fair@basic-pace@vcs0.html
>    [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl2/igt@gem_exec_fair@basic-pace@vcs0.html
> 
>   * igt@gem_exec_fair@basic-pace@vcs1:
>     - shard-iclb:         NOTRUN -> [FAIL][12] ([i915#2842])
>    [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb1/igt@gem_exec_fair@basic-pace@vcs1.html
> 
>   * igt@gem_exec_reloc@basic-many-active@rcs0:
>     - shard-snb:          NOTRUN -> [FAIL][13] ([i915#2389]) +2 similar issues
>    [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-snb7/igt@gem_exec_reloc@basic-many-active@rcs0.html
> 
>   * igt@gem_exec_schedule@u-fairslice@rcs0:
>     - shard-skl:          [PASS][14] -> [DMESG-WARN][15] ([i915#1610] / [i915#2803])
>    [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl9/igt@gem_exec_schedule@u-fairslice@rcs0.html
>    [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@gem_exec_schedule@u-fairslice@rcs0.html
> 
>   * igt@gem_huc_copy@huc-copy:
>     - shard-kbl:          NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#2190])
>    [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl7/igt@gem_huc_copy@huc-copy.html
> 
>   * igt@gem_mmap_gtt@cpuset-medium-copy-odd:
>     - shard-iclb:         [PASS][17] -> [FAIL][18] ([i915#307]) +1 similar issue
>    [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-iclb5/igt@gem_mmap_gtt@cpuset-medium-copy-odd.html
>    [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb1/igt@gem_mmap_gtt@cpuset-medium-copy-odd.html
> 
>   * igt@gem_pwrite@basic-exhaustion:
>     - shard-skl:          NOTRUN -> [WARN][19] ([i915#2658])
>    [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl6/igt@gem_pwrite@basic-exhaustion.html
>     - shard-snb:          NOTRUN -> [WARN][20] ([i915#2658])
>    [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-snb2/igt@gem_pwrite@basic-exhaustion.html
>     - shard-apl:          NOTRUN -> [WARN][21] ([i915#2658])
>    [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl2/igt@gem_pwrite@basic-exhaustion.html
> 
>   * igt@gem_userptr_blits@process-exit-mmap@wb:
>     - shard-apl:          NOTRUN -> [SKIP][22] ([fdo#109271] / [i915#1699]) +7 similar issues
>    [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl2/igt@gem_userptr_blits@process-exit-mmap@wb.html
>     - shard-skl:          NOTRUN -> [SKIP][23] ([fdo#109271] / [i915#1699]) +3 similar issues
>    [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl6/igt@gem_userptr_blits@process-exit-mmap@wb.html
> 
>   * igt@gem_vm_create@destroy-race:
>     - shard-tglb:         [PASS][24] -> [TIMEOUT][25] ([i915#2795])
>    [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-tglb8/igt@gem_vm_create@destroy-race.html
>    [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-tglb5/igt@gem_vm_create@destroy-race.html
> 
>   * igt@gen9_exec_parse@allowed-single:
>     - shard-skl:          [PASS][26] -> [DMESG-WARN][27] ([i915#1436] / [i915#716])
>    [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl7/igt@gen9_exec_parse@allowed-single.html
>    [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl9/igt@gen9_exec_parse@allowed-single.html
> 
>   * igt@gen9_exec_parse@bb-chained:
>     - shard-iclb:         NOTRUN -> [SKIP][28] ([fdo#112306])
>    [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@gen9_exec_parse@bb-chained.html
> 
>   * igt@i915_module_load@reload-with-fault-injection:
>     - shard-skl:          NOTRUN -> [DMESG-WARN][29] ([i915#1982])
>    [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl4/igt@i915_module_load@reload-with-fault-injection.html
> 
>   * igt@i915_pm_dc@dc6-dpms:
>     - shard-kbl:          NOTRUN -> [FAIL][30] ([i915#454])
>    [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl6/igt@i915_pm_dc@dc6-dpms.html
> 
>   * igt@i915_pm_dc@dc6-psr:
>     - shard-skl:          NOTRUN -> [FAIL][31] ([i915#454])
>    [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@i915_pm_dc@dc6-psr.html
> 
>   * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp:
>     - shard-apl:          NOTRUN -> [SKIP][32] ([fdo#109271] / [i915#1937])
>    [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl2/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp.html
> 
>   * igt@i915_pm_rc6_residency@rc6-idle:
>     - shard-iclb:         NOTRUN -> [WARN][33] ([i915#1804] / [i915#2684])
>    [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@i915_pm_rc6_residency@rc6-idle.html
> 
>   * igt@i915_query@query-topology-unsupported:
>     - shard-iclb:         NOTRUN -> [SKIP][34] ([fdo#109302])
>    [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@i915_query@query-topology-unsupported.html
> 
>   * igt@i915_selftest@live@hangcheck:
>     - shard-snb:          NOTRUN -> [INCOMPLETE][35] ([i915#2782])
>    [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-snb6/igt@i915_selftest@live@hangcheck.html
> 
>   * igt@kms_big_fb@yf-tiled-8bpp-rotate-270:
>     - shard-iclb:         NOTRUN -> [SKIP][36] ([fdo#110723])
>    [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@kms_big_fb@yf-tiled-8bpp-rotate-270.html
> 
>   * igt@kms_big_joiner@invalid-modeset:
>     - shard-skl:          NOTRUN -> [SKIP][37] ([fdo#109271] / [i915#2705])
>    [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl10/igt@kms_big_joiner@invalid-modeset.html
>     - shard-kbl:          NOTRUN -> [SKIP][38] ([fdo#109271] / [i915#2705])
>    [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl2/igt@kms_big_joiner@invalid-modeset.html
> 
>   * igt@kms_ccs@pipe-c-crc-primary-rotation-180:
>     - shard-skl:          NOTRUN -> [SKIP][39] ([fdo#109271] / [fdo#111304])
>    [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl10/igt@kms_ccs@pipe-c-crc-primary-rotation-180.html
> 
>   * igt@kms_chamelium@vga-hpd:
>     - shard-apl:          NOTRUN -> [SKIP][40] ([fdo#109271] / [fdo#111827]) +19 similar issues
>    [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl6/igt@kms_chamelium@vga-hpd.html
> 
>   * igt@kms_chamelium@vga-hpd-after-suspend:
>     - shard-skl:          NOTRUN -> [SKIP][41] ([fdo#109271] / [fdo#111827]) +15 similar issues
>    [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@kms_chamelium@vga-hpd-after-suspend.html
> 
>   * igt@kms_color@pipe-a-ctm-0-25:
>     - shard-iclb:         NOTRUN -> [FAIL][42] ([i915#1149] / [i915#315])
>    [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@kms_color@pipe-a-ctm-0-25.html
> 
>   * igt@kms_color@pipe-b-ctm-0-75:
>     - shard-skl:          [PASS][43] -> [DMESG-WARN][44] ([i915#1982])
>    [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl4/igt@kms_color@pipe-b-ctm-0-75.html
>    [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl5/igt@kms_color@pipe-b-ctm-0-75.html
> 
>   * igt@kms_color_chamelium@pipe-a-ctm-blue-to-red:
>     - shard-kbl:          NOTRUN -> [SKIP][45] ([fdo#109271] / [fdo#111827]) +7 similar issues
>    [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl6/igt@kms_color_chamelium@pipe-a-ctm-blue-to-red.html
>     - shard-iclb:         NOTRUN -> [SKIP][46] ([fdo#109284] / [fdo#111827]) +4 similar issues
>    [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@kms_color_chamelium@pipe-a-ctm-blue-to-red.html
> 
>   * igt@kms_color_chamelium@pipe-c-ctm-red-to-blue:
>     - shard-snb:          NOTRUN -> [SKIP][47] ([fdo#109271] / [fdo#111827]) +24 similar issues
>    [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-snb7/igt@kms_color_chamelium@pipe-c-ctm-red-to-blue.html
> 
>   * igt@kms_color_chamelium@pipe-d-ctm-blue-to-red:
>     - shard-iclb:         NOTRUN -> [SKIP][48] ([fdo#109278] / [fdo#109284] / [fdo#111827]) +1 similar issue
>    [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@kms_color_chamelium@pipe-d-ctm-blue-to-red.html
> 
>   * igt@kms_content_protection@dp-mst-type-1:
>     - shard-iclb:         NOTRUN -> [SKIP][49] ([i915#3116])
>    [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@kms_content_protection@dp-mst-type-1.html
> 
>   * igt@kms_content_protection@srm:
>     - shard-apl:          NOTRUN -> [TIMEOUT][50] ([i915#1319])
>    [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl7/igt@kms_content_protection@srm.html
> 
>   * igt@kms_cursor_crc@pipe-a-cursor-suspend:
>     - shard-skl:          [PASS][51] -> [INCOMPLETE][52] ([i915#300])
>    [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
>    [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
> 
>   * igt@kms_cursor_crc@pipe-b-cursor-512x512-rapid-movement:
>     - shard-iclb:         NOTRUN -> [SKIP][53] ([fdo#109278] / [fdo#109279])
>    [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@kms_cursor_crc@pipe-b-cursor-512x512-rapid-movement.html
> 
>   * igt@kms_cursor_crc@pipe-d-cursor-256x256-rapid-movement:
>     - shard-iclb:         NOTRUN -> [SKIP][54] ([fdo#109278]) +4 similar issues
>    [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@kms_cursor_crc@pipe-d-cursor-256x256-rapid-movement.html
> 
>   * igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
>     - shard-iclb:         NOTRUN -> [SKIP][55] ([fdo#109274] / [fdo#109278])
>    [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic.html
> 
>   * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
>     - shard-skl:          NOTRUN -> [FAIL][56] ([i915#2346] / [i915#533])
>    [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
> 
>   * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic:
>     - shard-apl:          NOTRUN -> [DMESG-FAIL][57] ([IGT#6])
>    [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl8/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic.html
> 
>   * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1:
>     - shard-skl:          NOTRUN -> [FAIL][58] ([i915#79])
>    [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
> 
>   * igt@kms_flip@flip-vs-expired-vblank@a-edp1:
>     - shard-skl:          [PASS][59] -> [FAIL][60] ([i915#79])
>    [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl5/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
>    [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl3/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
> 
>   * igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2:
>     - shard-glk:          [PASS][61] -> [FAIL][62] ([i915#79]) +2 similar issues
>    [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-glk4/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2.html
>    [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-glk5/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2.html
> 
>   * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
>     - shard-kbl:          [PASS][63] -> [DMESG-WARN][64] ([i915#180]) +5 similar issues
>    [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
>    [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
> 
>   * igt@kms_flip@flip-vs-suspend@c-dp1:
>     - shard-apl:          NOTRUN -> [DMESG-WARN][65] ([i915#180]) +1 similar issue
>    [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl2/igt@kms_flip@flip-vs-suspend@c-dp1.html
> 
>   * igt@kms_flip@plain-flip-fb-recreate@b-edp1:
>     - shard-skl:          NOTRUN -> [FAIL][66] ([i915#2122])
>    [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html
> 
>   * igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1:
>     - shard-skl:          [PASS][67] -> [FAIL][68] ([i915#2122])
>    [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl5/igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1.html
>    [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl8/igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1.html
> 
>   * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs:
>     - shard-skl:          NOTRUN -> [SKIP][69] ([fdo#109271] / [i915#2672])
>    [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl9/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs.html
>     - shard-apl:          NOTRUN -> [SKIP][70] ([fdo#109271] / [i915#2672]) +1 similar issue
>    [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl6/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs.html
> 
>   * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile:
>     - shard-skl:          NOTRUN -> [SKIP][71] ([fdo#109271] / [i915#2642])
>    [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile.html
> 
>   * igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile:
>     - shard-kbl:          NOTRUN -> [SKIP][72] ([fdo#109271] / [i915#2642])
>    [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl7/igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile.html
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-cpu:
>     - shard-iclb:         NOTRUN -> [SKIP][73] ([fdo#109280]) +9 similar issues
>    [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-cpu.html
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-blt:
>     - shard-skl:          [PASS][74] -> [FAIL][75] ([i915#49])
>    [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-blt.html
>    [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-blt.html
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-onoff:
>     - shard-snb:          NOTRUN -> [SKIP][76] ([fdo#109271]) +421 similar issues
>    [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-snb7/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-onoff.html
> 
>   * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc:
>     - shard-kbl:          NOTRUN -> [SKIP][77] ([fdo#109271]) +61 similar issues
>    [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl7/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc.html
> 
>   * igt@kms_hdr@bpc-switch-dpms:
>     - shard-skl:          [PASS][78] -> [FAIL][79] ([i915#1188])
>    [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html
>    [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl4/igt@kms_hdr@bpc-switch-dpms.html
> 
>   * igt@kms_hdr@static-swap:
>     - shard-iclb:         NOTRUN -> [SKIP][80] ([i915#1187])
>    [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@kms_hdr@static-swap.html
> 
>   * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence:
>     - shard-skl:          NOTRUN -> [SKIP][81] ([fdo#109271] / [i915#533]) +1 similar issue
>    [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence.html
> 
>   * igt@kms_pipe_crc_basic@read-crc-pipe-d:
>     - shard-apl:          NOTRUN -> [SKIP][82] ([fdo#109271] / [i915#533]) +1 similar issue
>    [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl7/igt@kms_pipe_crc_basic@read-crc-pipe-d.html
> 
>   * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
>     - shard-skl:          [PASS][83] -> [FAIL][84] ([i915#1036] / [i915#533])
>    [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
>    [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl9/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
> 
>   * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
>     - shard-kbl:          NOTRUN -> [FAIL][85] ([fdo#108145] / [i915#265])
>    [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl7/igt@kms_plane_alpha_blend@pipe-a-alpha-7efc.html
> 
>   * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
>     - shard-apl:          NOTRUN -> [FAIL][86] ([fdo#108145] / [i915#265]) +2 similar issues
>    [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl8/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
> 
>   * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
>     - shard-skl:          NOTRUN -> [FAIL][87] ([fdo#108145] / [i915#265]) +4 similar issues
>    [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb.html
> 
>   * igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb:
>     - shard-apl:          NOTRUN -> [FAIL][88] ([i915#265])
>    [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl8/igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb.html
> 
>   * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
>     - shard-skl:          [PASS][89] -> [FAIL][90] ([fdo#108145] / [i915#265])
>    [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
>    [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
> 
>   * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-1:
>     - shard-skl:          NOTRUN -> [SKIP][91] ([fdo#109271] / [i915#658]) +1 similar issue
>    [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl10/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-1.html
> 
>   * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-3:
>     - shard-kbl:          NOTRUN -> [SKIP][92] ([fdo#109271] / [i915#658]) +2 similar issues
>    [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl7/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-3.html
> 
>   * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-5:
>     - shard-iclb:         NOTRUN -> [SKIP][93] ([i915#658]) +1 similar issue
>    [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-5.html
> 
>   * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-4:
>     - shard-apl:          NOTRUN -> [SKIP][94] ([fdo#109271] / [i915#658]) +7 similar issues
>    [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl8/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-4.html
> 
>   * igt@kms_psr@psr2_cursor_plane_onoff:
>     - shard-iclb:         [PASS][95] -> [SKIP][96] ([fdo#109441])
>    [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
>    [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@kms_psr@psr2_cursor_plane_onoff.html
> 
>   * igt@kms_psr@psr2_primary_mmap_cpu:
>     - shard-iclb:         NOTRUN -> [SKIP][97] ([fdo#109441]) +1 similar issue
>    [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@kms_psr@psr2_primary_mmap_cpu.html
> 
>   * igt@kms_sysfs_edid_timing:
>     - shard-skl:          NOTRUN -> [FAIL][98] ([IGT#2])
>    [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@kms_sysfs_edid_timing.html
> 
>   * igt@kms_vblank@pipe-d-wait-forked-hang:
>     - shard-apl:          NOTRUN -> [SKIP][99] ([fdo#109271]) +203 similar issues
>    [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl2/igt@kms_vblank@pipe-d-wait-forked-hang.html
> 
>   * igt@kms_writeback@writeback-fb-id:
>     - shard-apl:          NOTRUN -> [SKIP][100] ([fdo#109271] / [i915#2437]) +1 similar issue
>    [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-apl8/igt@kms_writeback@writeback-fb-id.html
> 
>   * igt@nouveau_crc@pipe-a-ctx-flip-skip-current-frame:
>     - shard-iclb:         NOTRUN -> [SKIP][101] ([i915#2530])
>    [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@nouveau_crc@pipe-a-ctx-flip-skip-current-frame.html
> 
>   * igt@perf@gen12-mi-rpc:
>     - shard-iclb:         NOTRUN -> [SKIP][102] ([fdo#109289])
>    [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@perf@gen12-mi-rpc.html
> 
>   * igt@perf@polling-parameterized:
>     - shard-kbl:          [PASS][103] -> [FAIL][104] ([i915#1542])
>    [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-kbl3/igt@perf@polling-parameterized.html
>    [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl4/igt@perf@polling-parameterized.html
> 
>   * igt@sysfs_clients@recycle:
>     - shard-kbl:          [PASS][105] -> [FAIL][106] ([i915#3028])
>    [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-kbl3/igt@sysfs_clients@recycle.html
>    [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl6/igt@sysfs_clients@recycle.html
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_create@create-clear:
>     - shard-glk:          [FAIL][107] ([i915#3160]) -> [PASS][108]
>    [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-glk6/igt@gem_create@create-clear.html
>    [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-glk2/igt@gem_create@create-clear.html
> 
>   * igt@gem_ctx_persistence@many-contexts:
>     - shard-iclb:         [INCOMPLETE][109] ([i915#3057]) -> [PASS][110]
>    [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-iclb6/igt@gem_ctx_persistence@many-contexts.html
>    [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb8/igt@gem_ctx_persistence@many-contexts.html
> 
>   * igt@gem_eio@in-flight-contexts-10ms:
>     - shard-tglb:         [TIMEOUT][111] ([i915#3063]) -> [PASS][112]
>    [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-tglb3/igt@gem_eio@in-flight-contexts-10ms.html
>    [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-tglb8/igt@gem_eio@in-flight-contexts-10ms.html
>     - shard-iclb:         [TIMEOUT][113] -> [PASS][114]
>    [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-iclb7/igt@gem_eio@in-flight-contexts-10ms.html
>    [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb1/igt@gem_eio@in-flight-contexts-10ms.html
> 
>   * igt@gem_exec_fair@basic-none@vcs0:
>     - shard-kbl:          [FAIL][115] ([i915#2842]) -> [PASS][116] +1 similar issue
>    [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-kbl2/igt@gem_exec_fair@basic-none@vcs0.html
>    [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl7/igt@gem_exec_fair@basic-none@vcs0.html
> 
>   * igt@gem_exec_fair@basic-pace-share@rcs0:
>     - shard-glk:          [FAIL][117] ([i915#2842]) -> [PASS][118]
>    [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-glk9/igt@gem_exec_fair@basic-pace-share@rcs0.html
>    [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-glk8/igt@gem_exec_fair@basic-pace-share@rcs0.html
> 
>   * igt@gem_exec_fair@basic-pace@rcs0:
>     - shard-kbl:          [SKIP][119] ([fdo#109271]) -> [PASS][120]
>    [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-kbl6/igt@gem_exec_fair@basic-pace@rcs0.html
>    [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl2/igt@gem_exec_fair@basic-pace@rcs0.html
> 
>   * igt@gem_exec_schedule@u-fairslice@rcs0:
>     - shard-iclb:         [DMESG-WARN][121] ([i915#2803]) -> [PASS][122]
>    [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-iclb4/igt@gem_exec_schedule@u-fairslice@rcs0.html
>    [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb7/igt@gem_exec_schedule@u-fairslice@rcs0.html
> 
>   * igt@gem_exec_schedule@u-fairslice@vecs0:
>     - shard-skl:          [DMESG-WARN][123] ([i915#1610] / [i915#2803]) -> [PASS][124]
>    [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl9/igt@gem_exec_schedule@u-fairslice@vecs0.html
>    [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl7/igt@gem_exec_schedule@u-fairslice@vecs0.html
> 
>   * igt@i915_suspend@debugfs-reader:
>     - shard-skl:          [INCOMPLETE][125] ([i915#198]) -> [PASS][126] +1 similar issue
>    [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl2/igt@i915_suspend@debugfs-reader.html
>    [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl8/igt@i915_suspend@debugfs-reader.html
> 
>   * igt@kms_color@pipe-c-ctm-0-75:
>     - shard-skl:          [DMESG-WARN][127] ([i915#1982]) -> [PASS][128]
>    [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl8/igt@kms_color@pipe-c-ctm-0-75.html
>    [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl6/igt@kms_color@pipe-c-ctm-0-75.html
> 
>   * igt@kms_flip@flip-vs-expired-vblank@c-edp1:
>     - shard-tglb:         [FAIL][129] ([i915#79]) -> [PASS][130]
>    [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-tglb3/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
>    [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-tglb5/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
> 
>   * igt@kms_flip@plain-flip-ts-check-interruptible@a-hdmi-a1:
>     - shard-glk:          [FAIL][131] ([i915#2122]) -> [PASS][132]
>    [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-glk5/igt@kms_flip@plain-flip-ts-check-interruptible@a-hdmi-a1.html
>    [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-glk1/igt@kms_flip@plain-flip-ts-check-interruptible@a-hdmi-a1.html
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt:
>     - shard-skl:          [FAIL][133] ([i915#49]) -> [PASS][134]
>    [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt.html
>    [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-skl10/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt.html
> 
>   * igt@kms_hdr@bpc-switch-suspend:
>     - shard-kbl:          [DMESG-WARN][135] ([i915#180]) -> [PASS][136] +3 similar issues
>    [135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-kbl7/igt@kms_hdr@bpc-switch-suspend.html
>    [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-kbl7/igt@kms_hdr@bpc-switch-suspend.html
> 
>   * igt@kms_psr@psr2_sprite_plane_move:
>     - shard-iclb:         [SKIP][137] ([fdo#109441]) -> [PASS][138] +2 similar issues
>    [137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-iclb3/igt@kms_psr@psr2_sprite_plane_move.html
>    [138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
> 
>   * igt@sysfs_clients@recycle:
>     - shard-iclb:         [FAIL][139] ([i915#3028]) -> [PASS][140]
>    [139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-iclb6/igt@sysfs_clients@recycle.html
>    [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/shard-iclb6/igt@sysfs_clients@recycle.html
> 
>   * igt@sysfs_clients@recycle-many:
>     - shard-tglb:         [FAIL][141] ([i915#3028]) -> [PASS][142] +1 similar issue
>    [141]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9866/shard-tglb7/ig
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19802/index.html