On Fri, Oct 01, 2021 at 07:08:26PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> The DP spec says:
> "If the receiver keeps the same value in the ADJUST_REQUEST_LANEx_y
> register(s) while the LANEx_CR_DONE bits remain unset, the transmitter
> must loop four times with the same voltage swing. On the fifth time,
> the transmitter must down-shift to the lower bit rate and must repeat
> the CR-lock training sequence as described below."
>
> Lets fix the code to follow that instead of terminating after five
> times of transmitting the same signal levels. The text in spec feels
> a little bit ambiguous still, but this is my best guess at its meaning.
>
> As a bonus this also gets rid of the train_set[0] stuff which
> would not work for per-lane drive settings anyway.
>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> CC: Jani Nikula <jani.nikula@xxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
It's likely that the receiver is still trying new settings and didn't
abort yet, if it changes anything in the request.
> ---
> .../drm/i915/display/intel_dp_link_training.c | 29 +++++++++++++++----
> 1 file changed, 24 insertions(+), 5 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 053ed9302cda..73a823f1ec22 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -511,6 +511,25 @@ static void intel_dp_link_training_clock_recovery_delay(struct intel_dp *intel_d
> drm_dp_lttpr_link_train_clock_recovery_delay();
> }
>
> +static bool intel_dp_adjust_request_changed(int lane_count,
> + const u8 old_link_status[DP_LINK_STATUS_SIZE],
> + const u8 new_link_status[DP_LINK_STATUS_SIZE])
> +{
> + int lane;
> +
> + for (lane = 0; lane < lane_count; lane++) {
> + u8 old = drm_dp_get_adjust_request_voltage(old_link_status, lane) |
> + drm_dp_get_adjust_request_pre_emphasis(old_link_status, lane);
> + u8 new = drm_dp_get_adjust_request_voltage(new_link_status, lane) |
> + drm_dp_get_adjust_request_pre_emphasis(new_link_status, lane);
> +
> + if (old != new)
> + return true;
> + }
> +
> + return false;
> +}
> +
> /*
> * Perform the link training clock recovery phase on the given DP PHY using
> * training pattern 1.
> @@ -521,7 +540,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> enum drm_dp_phy dp_phy)
> {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> - u8 voltage;
> + u8 old_link_status[DP_LINK_STATUS_SIZE] = {};
> int voltage_tries, cr_tries, max_cr_tries;
> bool max_vswing_reached = false;
>
> @@ -574,8 +593,6 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> return false;
> }
>
> - voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
> -
> /* Update training set as requested by target */
> intel_dp_get_adjust_train(intel_dp, crtc_state, dp_phy,
> link_status);
> @@ -585,12 +602,14 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp,
> return false;
> }
>
> - if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
> - voltage)
> + if (!intel_dp_adjust_request_changed(crtc_state->lane_count,
> + old_link_status, link_status))
> ++voltage_tries;
> else
> voltage_tries = 1;
>
> + memcpy(old_link_status, link_status, sizeof(link_status));
> +
> if (intel_dp_link_max_vswing_reached(intel_dp, crtc_state))
> max_vswing_reached = true;
>
> --
> 2.32.0
>
