[Intel-gfx] [PATCH 2/8] drm/i915: replace ad-hoc dual-link lvds checks

[Daniel Vetter]

On Fri, Nov 16, 2012 at 5:37 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 1ad6d34..0973391 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -690,8 +690,7 @@ intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
>>         intel_clock_t clock;
>>         int err = target;
>>
>> -       if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
>> -           (I915_READ(LVDS)) != 0) {
>> +       if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
>
> This chunk doesn't seem to do exactly what the commit message says. I
> tried to git-blame the lines and got a little confused. Maybe this
> chunk deserves its own commit with an explanation. Maybe what you
> really want to do is to revert commit
> 832cc28d5bc676331e6376d940ae45d5937aa688 instead of removing the line?
> If you really want to remove the line, you may also update the comment
> immediately below?

Afaict the LVDS register check is only to make sure that we don't read
garbage. Iow the code doesn't handle the more robust dual-link mode
checking introduced in b03543857fd75876b96e10d4320b77

If we switch over to that method to check for dual-link, we also don't
need to do the (rather bogus imo) register check and hence can just
drop it.

I've dug around in the commit history, and the last patch to touch
this code predates b03543857fd75876b, hence why I think we should just
drop the register check and use the new is-dual-link function,
assuming that this has simple been overlooked in the conversion.

Does that make sense?
-Daniel


>
> The chunks below look correct.
>
>>                 /*
>>                  * For LVDS, if the panel is on, just rely on its current
>>                  * settings for dual-channel.  We haven't figured out how to
>> @@ -766,8 +765,7 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
>>                         lvds_reg = PCH_LVDS;
>>                 else
>>                         lvds_reg = LVDS;
>> -               if ((I915_READ(lvds_reg) & LVDS_CLKB_POWER_MASK) ==
>> -                   LVDS_CLKB_POWER_UP)
>> +               if (is_dual_link_lvds(dev_priv, lvds_reg))
>>                         clock.p2 = limit->p2.p2_fast;



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch