[Intel-gfx] [PATCH 08/14] drm/i915: fix Haswell DP M/N registers

[Paulo Zanoni]

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