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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
