On Wed, 12 Apr 2017, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Thu, Apr 06, 2017 at 02:00:12PM -0700, Manasi Navare wrote: >> Currently intel_dp_check_link_status() tries to retrain the link if >> Clock recovery or Channel EQ for any of the lanes indicated by >> intel_dp->lane_count is not set. However these values cached in intel_dp >> structure can be stale if link training has failed for these values >> during previous modeset. Or these values can get stale since we have >> now re read the DPCD registers or it can be 0 in case of connected boot >> case. >> >> This patch validates these values against the max link rate and max lane >> count values. >> >> This is absolutely required incase the common_rates or max lane count >> are now different due to link fallback. >> >> v2: >> * Include the FIXME commnet inside the function (Ville Syrjala) >> * Remove the redundant parenthesis (Ville Syrjala) >> >> v3 by Jani: >> * rebase on the DP refactoring series >> * rename intel_dp_link_params_is_valid to intel_dp_link_params_valid >> * minor stylistic changes >> >> v4: >> * Compare the link rate against max link rate not the >> common_rates since common_rates does not account for the >> lowered fallback link rate value. (Ville Syrjala) >> >> v5: >> * Fixed a warning for unused variable (Manasi) >> >> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> >> Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 26 +++++++++++++++++++++++--- >> 1 file changed, 23 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 3c5c80d..36485be 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -322,6 +322,24 @@ static int intel_dp_common_len_rate_limit(struct intel_dp *intel_dp, >> return 0; >> } >> >> +static bool intel_dp_link_params_valid(struct intel_dp *intel_dp) >> +{ >> + /* >> + * FIXME: we need to synchronize the current link parameters with >> + * hardware readout. Currently fast link training doesn't work on >> + * boot-up. >> + */ >> + if (intel_dp->link_rate == 0 || >> + intel_dp->link_rate > intel_dp->max_link_rate) >> + return false; >> + >> + if (intel_dp->lane_count == 0 || >> + intel_dp->lane_count > intel_dp_max_lane_count(intel_dp)) >> + return false; >> + >> + return true; >> +} > > This lgtm > > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Pushed to drm-intel-next-queued, thanks for the patch and review. BR, Jani. > >> + >> int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, >> int link_rate, uint8_t lane_count) >> { >> @@ -4253,9 +4271,11 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >> if (!to_intel_crtc(intel_encoder->base.crtc)->active) >> return; >> >> - /* FIXME: we need to synchronize this sort of stuff with hardware >> - * readout. Currently fast link training doesn't work on boot-up. */ >> - if (!intel_dp->lane_count) >> + /* >> + * Validate the cached values of intel_dp->link_rate and >> + * intel_dp->lane_count before attempting to retrain. >> + */ >> + if (!intel_dp_link_params_valid(intel_dp)) >> return; >> >> /* Retrain if Channel EQ or CR not ok */ >> -- >> 2.1.4 -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx