[Intel-gfx] [PATCH v3 4/4] drm/i915/edp: Be less aggressive about changing link config on eDP

Chris Wilson chris at chris-wilson.co.uk
Wed Jul 12 21:38:03 UTC 2017


Quoting Manasi Navare (2017-07-12 22:36:49)
> On Wed, Jul 12, 2017 at 12:16:13AM +0100, Chris Wilson wrote:
> > Quoting Jim Bride (2017-07-11 23:19:56)
> > > @@ -174,21 +176,25 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > >  
> > >                 if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > >                         DRM_ERROR("failed to get link status\n");
> > > +                       intel_dp->train_set_valid = false;
> > >                         return false;
> > >                 }
> > >  
> > >                 if (drm_dp_clock_recovery_ok(link_status, intel_dp->lane_count)) {
> > >                         DRM_DEBUG_KMS("clock recovery OK\n");
> > > +                       intel_dp->train_set_valid = is_edp(intel_dp);
> > 
> > Ouch, that was hard to spot amongst the decoys. How about setting
> > intel_dp->train_set_valid = false at the very start of training, and
> > only on success set it to true, something like
> >
> 
> Or like I suggested, just set train_set_valid to false in the
> failure_handling and set it to true only on success.

It just looked a little crowded in the failure_handling: whereas at the
start of the function, there was plenty of whitespace for it to stand
out. That was all I was thinking.
-Chris


More information about the Intel-gfx mailing list