Reviewed-by: Mika Kahola <[email protected]> On Fri, 2016-09-02 at 22:05 +0300, Pandiyan, Dhinakaran wrote: > On Fri, 2016-09-02 at 14:20 +0300, Mika Kahola wrote: > > > > On Thu, 2016-09-01 at 15:08 -0700, Manasi Navare wrote: > > > > > > Fix the number of tries in channel euqalization link training > > > sequence > > > according to DP 1.2 Spec. It returns a boolean depending on > > > channel > > > equalization pass or failure. > > > > > > Signed-off-by: Dhinakaran Pandiyan <[email protected] > > > > > > > Signed-off-by: Manasi Navare <[email protected]> > > > --- > > > drivers/gpu/drm/i915/intel_dp_link_training.c | 57 ++++++++++--- > > > ---- > > > ---------- > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > 2 files changed, 22 insertions(+), 36 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c > > > b/drivers/gpu/drm/i915/intel_dp_link_training.c > > > index 13a0341..07f0159 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > > > @@ -240,12 +240,12 @@ static u32 intel_dp_training_pattern(struct > > > intel_dp *intel_dp) > > > return training_pattern; > > > } > > > > > > -static void > > > +static bool > > > intel_dp_link_training_channel_equalization(struct intel_dp > > > *intel_dp) > > > { > > > - bool channel_eq = false; > > > - int tries, cr_tries; > > > + int tries; > > > u32 training_pattern; > > > + uint8_t link_status[DP_LINK_STATUS_SIZE]; > > > > > > training_pattern = intel_dp_training_pattern(intel_dp); > > > > > > @@ -254,20 +254,11 @@ > > > intel_dp_link_training_channel_equalization(struct intel_dp > > > *intel_dp) > > > training_pattern | > > > DP_LINK_SCRAMBLING_DISABLE) > > > ) { > > > DRM_ERROR("failed to start channel > > > equalization\n"); > > > - return; > > > + return false; > > > } > > > > > > - tries = 0; > > > - cr_tries = 0; > > > - channel_eq = false; > > > - for (;;) { > > > - uint8_t link_status[DP_LINK_STATUS_SIZE]; > > > - > > > - if (cr_tries > 5) { > > > - DRM_ERROR("failed to train DP, > > > aborting\n"); > > > - intel_dp_dump_link_status(link_status); > > > - break; > > > - } > > > + intel_dp->channel_eq_status = false; > > > + for (tries = 0; tries < 5; tries++) { > > > > > > drm_dp_link_train_channel_eq_delay(intel_dp- > > > >dpcd); > > > if (!intel_dp_get_link_status(intel_dp, > > > link_status)) { > > > @@ -278,44 +269,38 @@ > > > intel_dp_link_training_channel_equalization(struct intel_dp > > > *intel_dp) > > > /* Make sure clock is still ok */ > > > if (!drm_dp_clock_recovery_ok(link_status, > > > intel_dp- > > > >lane_count)) > > > { > > > - intel_dp_link_training_clock_recovery(in > > > tel_ > > > dp); > > > - intel_dp_set_link_train(intel_dp, > > > - training_pattern > > > | > > > - DP_LINK_SCRAMBLI > > > NG_D > > > ISABLE); > > > - cr_tries++; > > > - continue; > > > + intel_dp_dump_link_status(link_status); > > > + DRM_DEBUG_KMS("Clock recovery check > > > failed, > > > cannot " > > > + "continue channel > > > equalization\n"); > > > + break; > > > } > > This clock recovery check got me thinking. Do we really need to > > check > > if clock recovery is still ok within a loop? Could we move this > > outside > > the loop and return early if we have failed in clock recovery? One > > idea > > that I have in mind is that we wouldn't need to enter in channel > > equalization if we have failed with clock recovery earlier. > > > Looks like we do. This check helps us to break out of the loop for > link > rate reduction after adjusting drive setting. You're right we do that. > > > > > > > > > > > > > if (drm_dp_channel_eq_ok(link_status, > > > intel_dp->lane_count)) > > > { > > > - channel_eq = true; > > > + intel_dp->channel_eq_status = true; > > > + DRM_DEBUG_KMS("Channel EQ done. DP > > > Training > > > " > > > + "successful\n"); > > > break; > > > } > > > > > > - /* Try 5 times, then try clock recovery if that > > > fails */ > > > - if (tries > 5) { > > > - intel_dp_link_training_clock_recovery(in > > > tel_ > > > dp); > > > - intel_dp_set_link_train(intel_dp, > > > - training_pattern > > > | > > > - DP_LINK_SCRAMBLI > > > NG_D > > > ISABLE); > > > - tries = 0; > > > - cr_tries++; > > > - continue; > > > - } > > > - > > > /* 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; > > > } > > > - ++tries; > > > + } > > > + > > > + /* Try 5 times, else fail and try at lower BW */ > > > + if (tries == 5) { > > > + intel_dp_dump_link_status(link_status); > > > + DRM_DEBUG_KMS("Channel equalization failed 5 > > > times\n"); > > > } > > > > > > intel_dp_set_idle_link_train(intel_dp); > > > > > > - if (channel_eq) > > > - DRM_DEBUG_KMS("Channel EQ done. DP Training > > > successful\n"); > > > + return intel_dp->channel_eq_status; > > > + > > > } > > > > > > void intel_dp_stop_link_train(struct intel_dp *intel_dp) > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > > b/drivers/gpu/drm/i915/intel_drv.h > > > index efcd80b..e5bc976 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -878,6 +878,7 @@ struct intel_dp { > > > bool link_mst; > > > bool has_audio; > > > bool detect_done; > > > + bool channel_eq_status; > > > enum hdmi_force_audio force_audio; > > > bool limited_color_range; > > > bool color_range_auto; -- Mika Kahola - Intel OTC
_______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
