Re: [PATCH] i915: Modify for pineview clock source and lvds sync polarity

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, 8 Dec 2010 13:46:38 -0800, Bryan Freed <bfreed@chromium.org> wrote:
> The i915 driver normally assumes the video bios has configured several
> of the LVDS panel registers, and it just inherits the values. If the
> vbios has not run, several of these will need to be setup.
> 
> intel_bios.c: default clock source selection on pineview to use the SSC
> source
> intel_display.c: ensures the sync polarity to the panel is correct.

Once again I am astounded that we haven't set the polarities correctly.
Can you split this up into two patches (adjusting the defaults and setting
the sync flags).

> + /* Only tested on pineview, should work on IS_9XX */
> + if (IS_PINEVIEW(dev)) {
> + /* Set sync polarity for case when vbios did not */
> + if (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC)
> + lvds |= LVDS_HSYNC_POLARITY;
> + if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
> + lvds &= ~LVDS_HSYNC_POLARITY;
> + if (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC)
> + lvds |= LVDS_VSYNC_POLARITY;
> + if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
> + lvds &= ~LVDS_VSYNC_POLARITY;
> + }

Aside from being white-space mangled, experience suggests that if we are
going to set some flags, then we should just unconditionally clear them
first. Sooner or later there will be a BIOS that sets conflicting flags.
Furthermore, we should record what the BIOS (as we are currently trusting
the BIOS to set up the internal graphics correctly) set them to and print
a warning if they differ. I forsee bug reports... ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre