On Thu, 07 Oct 2021, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Thu, Oct 07, 2021 at 04:39:07PM +0300, Jani Nikula wrote: >> Add per-lane abstraction for max vswing reached to make follow-up >> cleaner, as this one reverses the conditions. >> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> .../drm/i915/display/intel_dp_link_training.c | 42 +++++++++++-------- >> 1 file changed, 25 insertions(+), 17 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 1a943ae38a6b..d239d72bfcf2 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c >> @@ -515,29 +515,37 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, >> return ret == crtc_state->lane_count; >> } >> >> +/* >> + * FIXME: The DP spec is very confusing here, also the Link CTS spec seems to >> + * have self contradicting tests around this area. >> + * >> + * In lieu of better ideas let's just stop when we've reached the max supported >> + * vswing with its max pre-emphasis, which is either 2+1 or 3+0 depending on >> + * whether vswing level 3 is supported or not. >> + */ >> +static bool intel_dp_lane_max_vswing_reached(u8 train_set_lane) >> +{ >> + u8 v = (train_set_lane & DP_TRAIN_VOLTAGE_SWING_MASK) >> >> + DP_TRAIN_VOLTAGE_SWING_SHIFT; >> + u8 p = (train_set_lane & DP_TRAIN_PRE_EMPHASIS_MASK) >> >> + DP_TRAIN_PRE_EMPHASIS_SHIFT; >> + >> + if (train_set_lane & DP_TRAIN_MAX_SWING_REACHED) >> + return true; >> + >> + if (v + p == 3) >> + return true; > > We want both to be true at the same time. D'oh! See, that's why I split it up from the other patch, but fumbled it anyway. Thanks. BR, Jani. > >> + >> + return false; >> +} >> + >> static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp, >> const struct intel_crtc_state *crtc_state) >> { >> int lane; >> >> - /* >> - * FIXME: The DP spec is very confusing here, also the Link CTS >> - * spec seems to have self contradicting tests around this area. >> - * >> - * In lieu of better ideas let's just stop when we've reached the >> - * max supported vswing with its max pre-emphasis, which is either >> - * 2+1 or 3+0 depending on whether vswing level 3 is supported or not. >> - */ >> for (lane = 0; lane < crtc_state->lane_count; lane++) { >> - u8 v = (intel_dp->train_set[lane] & DP_TRAIN_VOLTAGE_SWING_MASK) >> >> - DP_TRAIN_VOLTAGE_SWING_SHIFT; >> - u8 p = (intel_dp->train_set[lane] & DP_TRAIN_PRE_EMPHASIS_MASK) >> >> - DP_TRAIN_PRE_EMPHASIS_SHIFT; >> - >> - if ((intel_dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) == 0) >> - return false; >> - >> - if (v + p != 3) >> + if (!intel_dp_lane_max_vswing_reached(intel_dp->train_set[lane])) >> return false; >> } >> >> -- >> 2.30.2 -- Jani Nikula, Intel Open Source Graphics Center