[Intel-gfx] [PATCH v2] drm/i915: Reading DPRX caps in LTTPR transparent mode after LTTPR detection

[Almahallawy, Khaled]

On Sat, 2021-05-29 at 13:17 +0800, William Tseng wrote:
> In some cases, the MAX_LANE_COUNT in the register at DCPD Address
> 0002h
> may be updated by LTTPR in non-transparent mode while reading DPRX
> Caps
> registers, e.g., the lane count is changed from 2 to 4. This may
> cause
> Link Training failure because of the updated lane count, which might
> not
> be supported by the DPRX.
> 
Hi William,
 
Imre is better equipped to reply here. 
 
However, I believe this patch is violating the specs. Quoting the
following from DP Specs Sec “3.6.6.6.1 LTTPR Recognition”:
 
1-  “After HPD is propagated from the DPRX to the DPTX, a DP Source
device with a 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. If one or more
LTTPRs are present in the link (as indicated by the DPTX receiving a
response with a value of 80h, 40h, 20h, 10h, 08h, 04h, 02h, or 01h for
the PHY_REPEATER_CNT register (DPCD Address F0002h)), the LTTPR-aware
DPTX may place the LTTPR(s) in Non-transparent mode.
”
 
2-  “After LTTPR recognition, a DP Source device with a DPTX shall read
the DP Sink device with a DPRX’s capability by reading DisplayID or
legacy EDID and the DPRX’s Receiver Capability field (DPCD Addresses
00000h through 000FFh; see Table 2-183).”
 
So your patch is setting non-transparent mode after reading DPCD_CAP
which is not according to specs. 
 
If this patch addresses the problem we discussed before it is actually
the other way around.  LTTPR in non-transparent mode was not setting
lane count from 2 to 4. It is LTTPR in non-LTTPR and Transparent-Mode
was setting the lane count from 4 to 2. 
 
I believe the best solution for this issue is what Imre plans to
implement where once we fail LT on 4 lanes, we fallback to 2 lanes. 
 
Thank You
Khaled

> This change may work around the problem, which LTTPR updates the DCPD
> register not liseted in the table, i.e., Table 3-65, mentioned in the
> DP standard, Section 3.6.3, Version 2.0.
> 
> ""
> Upon discovering its location between the DPTX and DPRX, the LTTPR
> replies to AUX request transactions to its DPCD address range within
> the
> LTTPR field. The LTTPR passes through all other AUX request
> transactions
> with one exception – an LTTPR shall snoop AUX request transactions to
> those DPCD Addresses listed in Table 3-65 and take necessary actions
> as
> specified in the table. (For complete register descriptions,
> see Table 2-184.)
> ""
> 
> Cc : Khaled Almahallawy <khaled.almahallawy at intel.com>
> Cc : Imre Deak <imre.deak at intel.com>
> Cc : Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay at intel.com>
> Cc : Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: William Tseng <william.tseng at intel.com>
> ---
>  .../drm/i915/display/intel_dp_link_training.c | 44 +++++++++------
> ----
>  1 file changed, 22 insertions(+), 22 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 50cae0198a3d..3658deb9da1c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -131,7 +131,6 @@ intel_dp_set_lttpr_transparent_mode(struct
> intel_dp *intel_dp, bool enable)
>  static int intel_dp_init_lttpr(struct intel_dp *intel_dp)
>  {
>  	int lttpr_count;
> -	int i;
>  
>  	if (!intel_dp_read_lttpr_common_caps(intel_dp))
>  		return 0;
> @@ -152,27 +151,6 @@ static int intel_dp_init_lttpr(struct intel_dp
> *intel_dp)
>  	 */
>  	intel_dp_set_lttpr_transparent_mode(intel_dp, true);
>  
> -	/*
> -	 * In case of unsupported number of LTTPRs or failing to switch
> to
> -	 * non-transparent mode fall-back to transparent link training
> mode,
> -	 * still taking into account any LTTPR common lane- rate/count
> limits.
> -	 */
> -	if (lttpr_count < 0)
> -		return 0;
> -
> -	if (!intel_dp_set_lttpr_transparent_mode(intel_dp, false)) {
> -		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> -			    "Switching to LTTPR non-transparent LT mode
> failed, fall-back to transparent mode\n");
> -
> -		intel_dp_set_lttpr_transparent_mode(intel_dp, true);
> -		intel_dp_reset_lttpr_count(intel_dp);
> -
> -		return 0;
> -	}
> -
> -	for (i = 0; i < lttpr_count; i++)
> -		intel_dp_read_lttpr_phy_caps(intel_dp,
> DP_PHY_LTTPR(i));
> -
>  	return lttpr_count;
>  }
>  
> @@ -197,6 +175,7 @@ static int intel_dp_init_lttpr(struct intel_dp
> *intel_dp)
>  int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp *intel_dp)
>  {
>  	int lttpr_count = intel_dp_init_lttpr(intel_dp);
> +	int i;
>  
>  	/* The DPTX shall read the DPRX caps after LTTPR detection. */
>  	if (drm_dp_read_dpcd_caps(&intel_dp->aux, intel_dp->dpcd)) {
> @@ -204,6 +183,27 @@ int intel_dp_init_lttpr_and_dprx_caps(struct
> intel_dp *intel_dp)
>  		return -EIO;
>  	}
>  
> +	/*
> +	 * In case of unsupported number of LTTPRs or failing to switch
> to
> +	 * non-transparent mode fall-back to transparent link training
> mode,
> +	 * still taking into account any LTTPR common lane- rate/count
> limits.
> +	 */
> +	if (lttpr_count <= 0)
> +		return 0;
> +
> +	if (!intel_dp_set_lttpr_transparent_mode(intel_dp, false)) {
> +		drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> +		"Switching to LTTPR non-transparent LT mode failed,
> fall-back to transparent mode\n");
> +
> +		intel_dp_set_lttpr_transparent_mode(intel_dp, true);
> +		intel_dp_reset_lttpr_count(intel_dp);
> +
> +		return 0;
> +	}
> +
> +	for (i = 0; i < lttpr_count; i++)
> +		intel_dp_read_lttpr_phy_caps(intel_dp,
> DP_PHY_LTTPR(i));
> +
>  	return lttpr_count;
>  }
>  EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);