Re: [PATCH 02/40] drm/i915: Use the cached min/min/rpe values in the vlv debugfs code

[Daniel Vetter <daniel@ffwll.ch>]

On Sat, Jul 12, 2014 at 07:00:05PM +0530, Deepak S wrote:
> 
> On Saturday 28 June 2014 04:33 AM, ville.syrjala@linux.intel.com wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >No need to re-read the hardware rps fuses when we already have all the
> >values tucked away in dev_priv->rps.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++++---------
> >  drivers/gpu/drm/i915/i915_drv.h     |  2 --
> >  drivers/gpu/drm/i915/intel_pm.c     |  8 ++++----
> >  3 files changed, 14 insertions(+), 15 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >index a93b3bf..415010e 100644
> >--- a/drivers/gpu/drm/i915/i915_debugfs.c
> >+++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >@@ -1131,20 +1131,21 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
> >  		seq_printf(m, "Max overclocked frequency: %dMHz\n",
> >  			   dev_priv->rps.max_freq * GT_FREQUENCY_MULTIPLIER);
> >  	} else if (IS_VALLEYVIEW(dev)) {
> >-		u32 freq_sts, val;
> >+		u32 freq_sts;
> >  		mutex_lock(&dev_priv->rps.hw_lock);
> >  		freq_sts = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> >  		seq_printf(m, "PUNIT_REG_GPU_FREQ_STS: 0x%08x\n", freq_sts);
> >  		seq_printf(m, "DDR freq: %d MHz\n", dev_priv->mem_freq);
> >-		val = valleyview_rps_max_freq(dev_priv);
> >  		seq_printf(m, "max GPU freq: %d MHz\n",
> >-			   vlv_gpu_freq(dev_priv, val));
> >+			   dev_priv->rps.max_freq);
> >-		val = valleyview_rps_min_freq(dev_priv);
> >  		seq_printf(m, "min GPU freq: %d MHz\n",
> >-			   vlv_gpu_freq(dev_priv, val));
> >+			   dev_priv->rps.min_freq);
> >+
> >+		seq_printf(m, "efficient (RPe) frequency: %d MHz\n",
> >+			   dev_priv->rps.efficient_freq);
> >  		seq_printf(m, "current GPU freq: %d MHz\n",
> >  			   vlv_gpu_freq(dev_priv, (freq_sts >> 8) & 0xff));
> >@@ -3565,8 +3566,8 @@ i915_max_freq_set(void *data, u64 val)
> >  	if (IS_VALLEYVIEW(dev)) {
> >  		val = vlv_freq_opcode(dev_priv, val);
> >-		hw_max = valleyview_rps_max_freq(dev_priv);
> >-		hw_min = valleyview_rps_min_freq(dev_priv);
> >+		hw_max = dev_priv->rps.max_freq;
> >+		hw_min = dev_priv->rps.min_freq;
> >  	} else {
> >  		do_div(val, GT_FREQUENCY_MULTIPLIER);
> >@@ -3646,8 +3647,8 @@ i915_min_freq_set(void *data, u64 val)
> >  	if (IS_VALLEYVIEW(dev)) {
> >  		val = vlv_freq_opcode(dev_priv, val);
> >-		hw_max = valleyview_rps_max_freq(dev_priv);
> >-		hw_min = valleyview_rps_min_freq(dev_priv);
> >+		hw_max = dev_priv->rps.max_freq;
> >+		hw_min = dev_priv->rps.min_freq;
> >  	} else {
> >  		do_div(val, GT_FREQUENCY_MULTIPLIER);
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index 8cea596..38859d1 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -2646,8 +2646,6 @@ extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
> >  extern void intel_init_pch_refclk(struct drm_device *dev);
> >  extern void gen6_set_rps(struct drm_device *dev, u8 val);
> >  extern void valleyview_set_rps(struct drm_device *dev, u8 val);
> >-extern int valleyview_rps_max_freq(struct drm_i915_private *dev_priv);
> >-extern int valleyview_rps_min_freq(struct drm_i915_private *dev_priv);
> >  extern void intel_detect_pch(struct drm_device *dev);
> >  extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
> >  extern int intel_enable_rc6(const struct drm_device *dev);
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >index ef00756..10c9c02 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -3729,7 +3729,7 @@ void gen6_update_ring_freq(struct drm_device *dev)
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  }
> >-int cherryview_rps_max_freq(struct drm_i915_private *dev_priv)
> >+static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 val, rp0;
> >@@ -3749,7 +3749,7 @@ static int cherryview_rps_rpe_freq(struct drm_i915_private *dev_priv)
> >  	return rpe;
> >  }
> >-int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> >+static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 val, rpn;
> >@@ -3758,7 +3758,7 @@ int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
> >  	return rpn;
> >  }
> >-int valleyview_rps_max_freq(struct drm_i915_private *dev_priv)
> >+static int valleyview_rps_max_freq(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 val, rp0;
> >@@ -3783,7 +3783,7 @@ static int valleyview_rps_rpe_freq(struct drm_i915_private *dev_priv)
> >  	return rpe;
> >  }
> >-int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
> >+static int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
> >  {
> >  	return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
> >  }
> 
> Looks good. Reviewed-by: Deepak S <deepak.s@linux.intel.com>

Queued for -next, thanks for the patch. Since Ville is on vacation can you
perhaps pick up the rps patches that lack some polish and rebase them on
top of your vlv rps work? That way we also avoid conflicts when merging.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch