On Thu, 2016-09-01 at 15:08 -0700, Manasi Navare wrote:
> From: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
>
> This function cleans up clock recovery loop in link training
> compliant
> tp Dp Spec 1.2. It tries the clock recovery 5 times for the same
> voltage
> or until max voltage swing is reached and removes the additional non
> compliant retries. This function now returns a boolean values based
> on
> if clock recovery passed or failed.
>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dp_link_training.c | 59 ++++++++++++-----
> ----------
> 1 file changed, 26 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 9145e5a..13a0341 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -117,19 +117,19 @@ static bool
> intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> int lane;
>
> for (lane = 0; lane < intel_dp->lane_count; lane++)
> - if (intel_dp->train_set[lane] &
> DP_TRAIN_MAX_SWING_REACHED == 0)
> + if ((intel_dp->train_set[lane] &
> + DP_TRAIN_MAX_SWING_REACHED) == 0)
> return false;
>
This function was defined in the previous patch. Do we need to split
this if(...) line into separate lines in this patch?
> return true;
> }
>
> /* Enable corresponding port and start training pattern 1 */
> -static void
> +static bool
> intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> {
> - int i;
> uint8_t voltage;
> - int voltage_tries, loop_tries;
> + int voltage_tries, max_vswing_tries;
> uint8_t link_config[2];
> uint8_t link_bw, rate_select;
>
> @@ -145,6 +145,7 @@ intel_dp_link_training_clock_recovery(struct
> intel_dp *intel_dp)
> if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET,
> link_config, 2);
> +
> if (intel_dp->num_sink_rates)
> drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_RATE_SET,
> &rate_select, 1);
> @@ -160,58 +161,50 @@ intel_dp_link_training_clock_recovery(struct
> intel_dp *intel_dp)
> DP_TRAINING_PATTERN_1 |
> DP_LINK_SCRAMBLING_DISABLE))
> {
> DRM_ERROR("failed to enable link training\n");
> - return;
> + return false;
> }
>
> - voltage = 0xff;
> - voltage_tries = 0;
> - loop_tries = 0;
> + voltage_tries = 1;
> + max_vswing_tries = 0;
> for (;;) {
> uint8_t link_status[DP_LINK_STATUS_SIZE];
>
> drm_dp_link_train_clock_recovery_delay(intel_dp-
> >dpcd);
> +
> if (!intel_dp_get_link_status(intel_dp,
> link_status)) {
> DRM_ERROR("failed to get link status\n");
> - break;
> + return false;
> }
>
> if (drm_dp_clock_recovery_ok(link_status, intel_dp-
> >lane_count)) {
> DRM_DEBUG_KMS("clock recovery OK\n");
> - break;
> + return true;
> }
>
> - /* Check to see if we've tried the max voltage */
> - if (intel_dp_link_max_vswing_reached(intel_dp)) {
> - ++loop_tries;
> - if (loop_tries == 5) {
> - DRM_ERROR("too many full retries,
> give up\n");
> - intel_dp_dump_link_status(link_statu
> s);
> - break;
> - }
> - intel_dp_reset_link_train(intel_dp,
> - DP_TRAINING_PATTER
> N_1 |
> - DP_LINK_SCRAMBLING
> _DISABLE);
> - voltage_tries = 0;
> - continue;
> + if (voltage_tries == 5 || max_vswing_tries == 1) {
> + DRM_DEBUG_KMS("Max. vswing reached or same
> voltage "
> + "tried 5 times\n");
> + return false;
> }
I think it would be beneficial to know which one we hit if the clock
recovery fails. Therefore, I would split these debug messages so that
there would be separate debug messages when reaching max vswing or
trying out the same voltage for five times.
>
> - /* Check to see if we've tried the same voltage 5
> times */
> - if ((intel_dp->train_set[0] &
> DP_TRAIN_VOLTAGE_SWING_MASK) == voltage) {
> - ++voltage_tries;
> - if (voltage_tries == 5) {
> - DRM_ERROR("too many voltage retries,
> give up\n");
> - break;
> - }
> - } else
> - voltage_tries = 0;
> voltage = intel_dp->train_set[0] &
> DP_TRAIN_VOLTAGE_SWING_MASK;
>
> /* Update training set as requested by target */
> intel_get_adjust_train(intel_dp, link_status);
> if (!intel_dp_update_link_train(intel_dp)) {
> DRM_ERROR("failed to update link
> training\n");
> - break;
> + return false;
> }
> +
> + if ((intel_dp->train_set[0] &
> DP_TRAIN_VOLTAGE_SWING_MASK) ==
> + voltage)
> + ++voltage_tries;
> + else
> + voltage_tries = 1;
> +
> + if (intel_dp_link_max_vswing_reached(intel_dp))
> + ++max_vswing_tries;
> +
> }
> }
>
--
Mika Kahola - Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
