Re: [PATCH 01/10] drm/i915: rewrite the LCPLL code

Lespiau, Damien
2012-10-10T12:53:07Z

On Wed, Oct 10, 2012 at 1:47 PM, Lespiau, Damien
<damien.lespiau@intel.com> wrote:
> On Fri, Oct 5, 2012 at 4:05 PM, Paulo Zanoni <przanoni@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@intel.com>

--
Damien