diff mbox series

drm/i915: Reenable LTTPR non-transparent LT mode for DPCD_REV<1.4

Message ID 20210512212809.1234701-1-imre.deak@intel.com
State Accepted
Commit e11851429fdc23524aa244f76508c3c7aeaefdf6
Headers show
Series drm/i915: Reenable LTTPR non-transparent LT mode for DPCD_REV<1.4 | expand

Commit Message

Imre Deak May 12, 2021, 9:28 p.m. UTC
The driver currently disables the LTTPR non-transparent link training
mode for sinks with a DPCD_REV<1.4, based on the following description
of the LTTPR DPCD register range in DP standard 2.0 (at the 0xF0000
register description):

""
LTTPR-related registers at DPCD Addresses F0000h through F02FFh are valid
only for DPCD r1.4 (or higher).
"""

The transparent link training mode should still work fine, however the
implementation for this in some retimer FWs seems to be broken, see the
References: link below.

After discussions with DP standard authors the above "DPCD r1.4" does
not refer to the DPCD revision (stored in the DPCD_REV reg at 0x00000),
rather to the "LTTPR field data structure revision" stored in the
0xF0000 reg. An update request has been filed at vesa.org (see
wg/Link/documentComment/3746) for the upcoming v2.1 specification to
clarify the above description along the following lines:

"""
LTTPR-related registers at DPCD Addresses F0000h through F02FFh are
valid only for LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV 1.4 (or
higher)
"""

Based on my tests Windows uses the non-transparent link training mode
for DPCD_REV==1.2 sinks as well (so presumably for all DPCD_REVs), and
forcing it to use transparent mode on ICL/TGL platforms leads to the
same LT failure as reported at the References: link.

Based on the above let's assume that the transparent link training mode
is not well tested/supported and align the code to the correct
interpretation of what the r1.4 version refers to.

References: https://gitlab.freedesktop.org/drm/intel/-/issues/3415
Fixes: 264613b406eb ("drm/i915: Disable LTTPR support when the DPCD rev < 1.4")
Cc: <stable@vger.kernel.org> # v5.11+
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../drm/i915/display/intel_dp_link_training.c | 71 +++++++++----------
 1 file changed, 33 insertions(+), 38 deletions(-)

Comments

Khaled Almahallawy May 17, 2021, 10:23 p.m. UTC | #1
Tested on latest drm-tip with DPCD=1.2 Sink and LTTPR set to non-
transparent mode:

[  706.966375] i915 0000:00:02.0: [drm:drm_dp_dpcd_read] AUX USBC2/DDI
TC2/PHY TC2: 0xf0000 AUX -> (ret=  8) 14 1e 80 55 04 00 00 00
[  706.966383] i915 0000:00:02.0:
[drm:intel_dp_init_lttpr_and_dprx_caps [i915]] LTTPR common
capabilities: 14 1e 80 55 04 00 00 00
[  706.966900] i915 0000:00:02.0: [drm:drm_dp_dpcd_write] AUX USBC2/DDI
TC2/PHY TC2: 0xf0003 AUX <- (ret=  1) 55
[  706.967397] i915 0000:00:02.0: [drm:drm_dp_dpcd_write] AUX USBC2/DDI
TC2/PHY TC2: 0xf0003 AUX <- (ret=  1) aa
[  706.968384] i915 0000:00:02.0: [drm:drm_dp_dpcd_read] AUX USBC2/DDI
TC2/PHY TC2: 0xf0020 AUX -> (ret=  3) 03 00 00
[  706.968388] i915 0000:00:02.0:
[drm:intel_dp_init_lttpr_and_dprx_caps [i915]] LTTPR 1 PHY
capabilities: 03 00 00
[  706.969913] i915 0000:00:02.0: [drm:drm_dp_dpcd_read] AUX USBC2/DDI
TC2/PHY TC2: 0x00000 AUX -> (ret= 15) 12 14 c4 01 01 00 01 00 02 02 06
00 00 00 00
[  706.969917] i915 0000:00:02.0: [drm:drm_dp_read_dpcd_caps] AUX
USBC2/DDI TC2/PHY TC2: DPCD: 12 14 c4 01 01 00 01 00 02 02 06 00 00 00
00

Tested-by: Khaled Almahallawy <khaled.almahallawy@intel.com>

Reviewed-by: Khaled Almahallawy <khaled.almahallawy@intel.com>


Thank you for the patch
~Khaled

On Thu, 2021-05-13 at 00:28 +0300, Imre Deak wrote:
> The driver currently disables the LTTPR non-transparent link training

> mode for sinks with a DPCD_REV<1.4, based on the following

> description

> of the LTTPR DPCD register range in DP standard 2.0 (at the 0xF0000

> register description):

> 

> ""

> LTTPR-related registers at DPCD Addresses F0000h through F02FFh are

> valid

> only for DPCD r1.4 (or higher).

> """

> 

> The transparent link training mode should still work fine, however

> the

> implementation for this in some retimer FWs seems to be broken, see

> the

> References: link below.

> 

> After discussions with DP standard authors the above "DPCD r1.4" does

> not refer to the DPCD revision (stored in the DPCD_REV reg at

> 0x00000),

> rather to the "LTTPR field data structure revision" stored in the

> 0xF0000 reg. An update request has been filed at vesa.org (see

> wg/Link/documentComment/3746) for the upcoming v2.1 specification to

> clarify the above description along the following lines:

> 

> """

> LTTPR-related registers at DPCD Addresses F0000h through F02FFh are

> valid only for LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV 1.4

> (or

> higher)

> """

> 

> Based on my tests Windows uses the non-transparent link training mode

> for DPCD_REV==1.2 sinks as well (so presumably for all DPCD_REVs),

> and

> forcing it to use transparent mode on ICL/TGL platforms leads to the

> same LT failure as reported at the References: link.

> 

> Based on the above let's assume that the transparent link training

> mode

> is not well tested/supported and align the code to the correct

> interpretation of what the r1.4 version refers to.

> 

> References: https://gitlab.freedesktop.org/drm/intel/-/issues/3415

> Fixes: 264613b406eb ("drm/i915: Disable LTTPR support when the DPCD

> rev < 1.4")

> Cc: <stable@vger.kernel.org> # v5.11+

> Signed-off-by: Imre Deak <imre.deak@intel.com>

> ---

>  .../drm/i915/display/intel_dp_link_training.c | 71 +++++++++------

> ----

>  1 file changed, 33 insertions(+), 38 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 6bf6f1ec13ed8..08bceae40aa8d 100644

> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c

> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c

> @@ -128,50 +128,14 @@ intel_dp_set_lttpr_transparent_mode(struct

> intel_dp *intel_dp, bool enable)

>  	return drm_dp_dpcd_write(&intel_dp->aux, DP_PHY_REPEATER_MODE,

> &val, 1) == 1;

>  }

>  

> -/**

> - * 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 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. 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. The

> DPRX

> - *       capabilities are read out.

> - *   <0  Reading out the DPRX capabilities failed.

> - */

> -int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)

> +static int intel_dp_init_lttpr(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 DPRX 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)

> +	if (!intel_dp_read_lttpr_common_caps(intel_dp))

>  		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

> @@ -211,6 +175,37 @@ int intel_dp_init_lttpr_and_dprx_caps(struct

> intel_dp *intel_dp)

>  

>  	return lttpr_count;

>  }

> +

> +/**

> + * 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 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. 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. The

> DPRX

> + *       capabilities are read out.

> + *   <0  Reading out the DPRX capabilities failed.

> + */

> +int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)

> +{

> +	int lttpr_count = intel_dp_init_lttpr(intel_dp);

> +

> +	/* The DPTX shall read the DPRX 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;

> +	}

> +

> +	return lttpr_count;

> +}

>  EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);

>  

>  static u8 dp_voltage_max(u8 preemph)
diff mbox series

Patch

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 6bf6f1ec13ed8..08bceae40aa8d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -128,50 +128,14 @@  intel_dp_set_lttpr_transparent_mode(struct intel_dp *intel_dp, bool enable)
 	return drm_dp_dpcd_write(&intel_dp->aux, DP_PHY_REPEATER_MODE, &val, 1) == 1;
 }
 
-/**
- * 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 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. 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. The DPRX
- *       capabilities are read out.
- *   <0  Reading out the DPRX capabilities failed.
- */
-int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
+static int intel_dp_init_lttpr(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 DPRX 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)
+	if (!intel_dp_read_lttpr_common_caps(intel_dp))
 		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
@@ -211,6 +175,37 @@  int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
 
 	return lttpr_count;
 }
+
+/**
+ * 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 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. 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. The DPRX
+ *       capabilities are read out.
+ *   <0  Reading out the DPRX capabilities failed.
+ */
+int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
+{
+	int lttpr_count = intel_dp_init_lttpr(intel_dp);
+
+	/* The DPTX shall read the DPRX 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;
+	}
+
+	return lttpr_count;
+}
 EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);
 
 static u8 dp_voltage_max(u8 preemph)