Hi 2012/10/15 Daniel Vetter <daniel at ffwll.ch>: > On Mon, Oct 15, 2012 at 10:29 PM, Adam Jackson <ajax at redhat.com> wrote: >> On 10/15/12 2:51 PM, Paulo Zanoni wrote: >> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c >>> b/drivers/gpu/drm/i915/intel_display.c >>> index f48986b9..ba40aa7 100644 >>> --- a/drivers/gpu/drm/i915/intel_display.c >>> +++ b/drivers/gpu/drm/i915/intel_display.c >>> @@ -5356,7 +5356,8 @@ static int haswell_crtc_mode_set(struct drm_crtc >>> *crtc, >>> >>> intel_set_pipe_timings(intel_crtc, mode, adjusted_mode); >>> >>> - ironlake_set_m_n(crtc, mode, adjusted_mode); >>> + if (!(is_dp && !is_cpu_edp)) >>> + ironlake_set_m_n(crtc, mode, adjusted_mode); >> >> >> The double-negation here hurts my brain. I think this would be clearer and >> equivalent phrased positively: >> >> if (!is_dp || is_pch_edp) >> ironlake_set_m_n(crtc, mode, adjusted_mode); > > pch_edp doesn't really exist (since nothing much is still on the pch, > only the vga port is left), I believe Adam wanted to say "if (!is_dp || is_cpu_edp)", there's no pch_edp check :) The check is in its current form because a few lines above there's a "if (is_dp && !is_cpu_edp)", so this one just added a negation to the previous test. I also agree that double-negation hurts brains, so no problem in changing that. > so not really clearer. I suspect we > actually want a !is_dp check in there - since the eDP enabling is > later in the series all edp checks don't really matter anyway. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Paulo Zanoni
