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 <[email protected]> > Cc : Imre Deak <[email protected]> > Cc : Tejas Upadhyay <[email protected]> > Cc : Ville Syrjälä <[email protected]> > Signed-off-by: William Tseng <[email protected]> > --- > .../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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
