On Wed, Oct 10, 2012 at 1:47 PM, Lespiau, Damien <damien.lespiau at intel.com> wrote: > On Fri, Oct 5, 2012 at 4:05 PM, Paulo Zanoni <przanoni at gmail.com> wrote: > >> +static int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv) >> +{ >> + if (I915_READ(HSW_FUSE_STRAP) & HSW_CDCLK_LIMIT) >> + return 450; > > I don't think reading this fused register is necessary here, it only > really matters when you try to set the LCPLL alternate frequency to > check if the hardware supports it. I'd just rely on the frequency set > bits in LCPLL (if you remove this, it means that the hunk adding the > register definition is not needed any more). > > If you really want to check for consistency, then you could warn if > the LCPLL freq has been programmed with 0x1 and HSW_CDCLK_LIMIT is set > (in which case the setting will be ignored). But then, there's nothing > we can do anyway, so well... Hum, I meant to add, it's just bike-shedding really, it's totally fine if you want to return 450 if HSW_CDCLK_LIMIT is set without checking for the programmed value in LCPLL. So really: Reviewed-by: Damien Lespiau <damien.lespiau at intel.com> -- Damien