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@xxxxxxxxx>
> Cc : Imre Deak <imre.deak@xxxxxxxxx>
> Cc : Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx>
> Cc : Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: William Tseng <william.tseng@xxxxxxxxx>
> ---
> .../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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
