[Intel-gfx] [PATCH 2/3] drm/i915: move VLV DDR freq fetch into init_clock_gating

[Jesse Barnes]

On Tue, 5 Nov 2013 00:49:57 +0200
Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:

> On Mon, Nov 04, 2013 at 11:52:45AM -0800, Jesse Barnes wrote:
> > We don't want it delayed with the RPS work.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 31 ++++++++++++++++++-------------
> >  1 file changed, 18 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index a0c907f..2e7072e 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4064,19 +4064,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
> >  	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
> >  
> >  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> > -	switch ((val >> 6) & 3) {
> > -	case 0:
> > -	case 1:
> > -		dev_priv->mem_freq = 800;
> > -		break;
> > -	case 2:
> > -		dev_priv->mem_freq = 1066;
> > -		break;
> > -	case 3:
> > -		dev_priv->mem_freq = 1333;
> > -		break;
> > -	}
> > -	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
> >  
> >  	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
> >  	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
> > @@ -5325,6 +5312,24 @@ static void ivybridge_init_clock_gating(struct drm_device *dev)
> >  static void valleyview_init_clock_gating(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	u32 val;
> > +
> > +	mutex_lock(&dev_priv->rps.hw_lock);
> > +	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> > +	mutex_unlock(&dev_priv->rps.hw_lock);
> > +	switch ((val >> 6) & 3) {
> > +	case 0:
> > +	case 1:
> > +		dev_priv->mem_freq = 800;
> > +		break;
> > +	case 2:
> > +		dev_priv->mem_freq = 1066;
> > +		break;
> > +	case 3:
> > +		dev_priv->mem_freq = 1333;
> > +		break;
> > +	}
> 
> This doesn't actually match the punit HAS I have. What I see
> is 0=800, 1=1066, 2=1333, 3=invalid.
> 
> But using the DDR rate to determine the CCK clock isn't the best idea
> I think. It seems there's an option of running CCK at 800MHz even for
> the DDR 1066 case. So I think we should just use CCK_FUSE_REG to
> figure it out, just like gmbus_set_freq() already does.
> 

Hm this is pretty old code, so the docs may have changed.  IIRC there
were 3 places with this info, but the Punit turbo HAS no longer seems
to have it.  My current Punit HAS matches yours though, so I guess we
should fix that with a separate patch (though it makes me a bit nervous
changing this old code w/o a bug report; it will affect which turbo
freqs are used on current machines).

If the CCK and DDR freqs can really mismatch then we should definitely
use the CCK read directly.  I'll export that function from
intel_display.c and  use it in both places.

So I'll send follow ups for the VCO calc (including an i2c change) and
another patch to fixup the DDR rate detection.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center