Re: [PATCH 03/40] drm/i915: Align chv rps min/max/rpe values

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sat, Jul 12, 2014 at 07:16:15PM +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>
> >
> > CHV wants even rps opcodes so make sure the min/max/rpe values are also
> > even.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_debugfs.c |  8 ++++++++
> >   drivers/gpu/drm/i915/intel_pm.c     | 19 ++++++++++++++-----
> >   2 files changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 415010e..9b01e7c 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3566,6 +3566,10 @@ i915_max_freq_set(void *data, u64 val)
> >   	if (IS_VALLEYVIEW(dev)) {
> >   		val = vlv_freq_opcode(dev_priv, val);
> >   
> > +		/* CHV needs even encode values */
> > +		if (IS_CHERRYVIEW(dev))
> > +			val &= ~1;
> > +
> >   		hw_max = dev_priv->rps.max_freq;
> >   		hw_min = dev_priv->rps.min_freq;
> >   	} else {
> > @@ -3647,6 +3651,10 @@ i915_min_freq_set(void *data, u64 val)
> >   	if (IS_VALLEYVIEW(dev)) {
> >   		val = vlv_freq_opcode(dev_priv, val);
> >   
> > +		/* CHV needs even encode values */
> > +		if (IS_CHERRYVIEW(dev))
> > +			val = ALIGN(val, 2);
> > +
> >   		hw_max = dev_priv->rps.max_freq;
> >   		hw_min = dev_priv->rps.min_freq;
> >   	} else {
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 10c9c02..e3f23c2 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3924,21 +3924,30 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
> >   	mutex_lock(&dev_priv->rps.hw_lock);
> >   
> >   	dev_priv->rps.max_freq = cherryview_rps_max_freq(dev_priv);
> > +	if (WARN_ON_ONCE(dev_priv->rps.max_freq & 1))
> > +		dev_priv->rps.max_freq &= ~1;
> 
> Cannot we use ALIGN Here?

The idea was to round max freq down and min freq up.

> 
> Other than this it looks fine
> 
> Reviewed-by: Deepak S <deepak.s@linux.intel.com>
> 
> 
> >   	dev_priv->rps.rp0_freq = dev_priv->rps.max_freq;
> >   	DRM_DEBUG_DRIVER("max GPU freq: %d MHz (%u)\n",
> >   			 vlv_gpu_freq(dev_priv, dev_priv->rps.max_freq),
> >   			 dev_priv->rps.max_freq);
> >   
> > -	dev_priv->rps.efficient_freq = cherryview_rps_rpe_freq(dev_priv);
> > -	DRM_DEBUG_DRIVER("RPe GPU freq: %d MHz (%u)\n",
> > -			 vlv_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
> > -			 dev_priv->rps.efficient_freq);
> > -
> >   	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
> > +	if (WARN_ON_ONCE(dev_priv->rps.min_freq & 1))
> > +		dev_priv->rps.min_freq = ALIGN(dev_priv->rps.min_freq, 2);
> >   	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
> >   			 vlv_gpu_freq(dev_priv, dev_priv->rps.min_freq),
> >   			 dev_priv->rps.min_freq);
> >   
> > +	dev_priv->rps.efficient_freq = cherryview_rps_rpe_freq(dev_priv);
> > +	if (WARN_ON_ONCE(dev_priv->rps.min_freq & 1))
> > +		dev_priv->rps.efficient_freq &= ~1;
> > +	dev_priv->rps.efficient_freq = clamp(dev_priv->rps.efficient_freq,
> > +					     dev_priv->rps.min_freq,
> > +					     dev_priv->rps.max_freq);
> > +	DRM_DEBUG_DRIVER("RPe GPU freq: %d MHz (%u)\n",
> > +			 vlv_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
> > +			 dev_priv->rps.efficient_freq);
> > +
> >   	/* Preserve min/max settings in case of re-init */
> >   	if (dev_priv->rps.max_freq_softlimit == 0)
> >   		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;

-- 
Ville Syrjälä
Intel OTC