Re: [PATCH 1/7] drm/i915: Relax RPS contraints to allows setting minfreq on idle

[Deepak S <deepak.s@linux.intel.com>]

On Friday 06 March 2015 08:36 PM, Chris Wilson wrote:
> When we idle, we set the GPU frequency to the hardware minimum (not user
> minimum). We introduce a new variable to distinguish between the
> different roles, and to allow easy tuning of the idle frequency without
> impacting over aspects of RPS. Setting the minimum frequency should be a
> safety blanket as the pcu on the GPU should be power gating itself
> anyway. However, in order for us to do set the absolute minimum
> frequency, we need to relax a few of our assertions that we do not
> exceed the user limits.
>
> v2: Add idle_freq
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c |  6 ++++++
>   drivers/gpu/drm/i915/i915_drv.h     |  1 +
>   drivers/gpu/drm/i915/intel_pm.c     | 35 +++++++++++++++++++----------------
>   3 files changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c3a5322bc7f4..cc83cc0ff823 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1219,6 +1219,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>   
>   		seq_printf(m, "Max overclocked frequency: %dMHz\n",
>   			   intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
> +
> +		seq_printf(m, "Idle freq: %d MHz\n",
> +			   intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
>   	} else if (IS_VALLEYVIEW(dev)) {
>   		u32 freq_sts;
>   
> @@ -1233,6 +1236,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>   		seq_printf(m, "min GPU freq: %d MHz\n",
>   			   intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
>   
> +		seq_printf(m, "idle GPU freq: %d MHz\n",
> +			   intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
> +
>   		seq_printf(m,
>   			   "efficient (RPe) frequency: %d MHz\n",
>   			   intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 18a6e56c7c94..73292a183492 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1022,6 +1022,7 @@ struct intel_gen6_power_mgmt {
>   	u8 max_freq_softlimit;	/* Max frequency permitted by the driver */
>   	u8 max_freq;		/* Maximum frequency, RP0 if not overclocking */
>   	u8 min_freq;		/* AKA RPn. Minimum frequency */
> +	u8 idle_freq;		/* Frequency to request when we are idle */
>   	u8 efficient_freq;	/* AKA RPe. Pre-determined balanced frequency */
>   	u8 rp1_freq;		/* "less than" RP0 power/freqency */
>   	u8 rp0_freq;		/* Non-overclocked max frequency. */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6b277f9f7ffa..e53724516852 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3711,9 +3711,9 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>   		break;
>   	}
>   	/* Max/min bins are special */
> -	if (val == dev_priv->rps.min_freq_softlimit)
> +	if (val <= dev_priv->rps.min_freq_softlimit)
>   		new_power = LOW_POWER;
> -	if (val == dev_priv->rps.max_freq_softlimit)
> +	if (val >= dev_priv->rps.max_freq_softlimit)
>   		new_power = HIGH_POWER;
>   	if (new_power == dev_priv->rps.power)
>   		return;
> @@ -3802,8 +3802,8 @@ static void gen6_set_rps(struct drm_device *dev, u8 val)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   
>   	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> -	WARN_ON(val > dev_priv->rps.max_freq_softlimit);
> -	WARN_ON(val < dev_priv->rps.min_freq_softlimit);
> +	WARN_ON(val > dev_priv->rps.max_freq);
> +	WARN_ON(val < dev_priv->rps.min_freq);
>   
>   	/* min/max delay may still have been modified so be sure to
>   	 * write the limits value.
> @@ -3836,8 +3836,8 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   
>   	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> -	WARN_ON(val > dev_priv->rps.max_freq_softlimit);
> -	WARN_ON(val < dev_priv->rps.min_freq_softlimit);
> +	WARN_ON(val > dev_priv->rps.max_freq);
> +	WARN_ON(val < dev_priv->rps.min_freq);
>   
>   	if (WARN_ONCE(IS_CHERRYVIEW(dev) && (val & 1),
>   		      "Odd GPU freq value\n"))
> @@ -3864,10 +3864,11 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val)
>   static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>   {
>   	struct drm_device *dev = dev_priv->dev;
> +	u32 val = dev_priv->rps.idle_freq;
>   
>   	/* CHV and latest VLV don't need to force the gfx clock */
>   	if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
> -		valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> +		valleyview_set_rps(dev_priv->dev, val);
>   		return;
>   	}
>   
> @@ -3875,7 +3876,7 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>   	 * When we are idle.  Drop to min voltage state.
>   	 */
>   
> -	if (dev_priv->rps.cur_freq <= dev_priv->rps.min_freq_softlimit)
> +	if (dev_priv->rps.cur_freq <= val)
>   		return;
>   
>   	/* Mask turbo interrupt so that they will not come in between */
> @@ -3884,10 +3885,9 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>   
>   	vlv_force_gfx_clock(dev_priv, true);
>   
> -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
> +	dev_priv->rps.cur_freq = val;
>   
> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
> -					dev_priv->rps.min_freq_softlimit);
> +	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
>   
>   	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
>   				& GENFREQSTATUS) == 0, 100))
> @@ -3895,8 +3895,7 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>   
>   	vlv_force_gfx_clock(dev_priv, false);
>   
> -	I915_WRITE(GEN6_PMINTRMSK,
> -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
> +	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
>   }
>   
>   void gen6_rps_idle(struct drm_i915_private *dev_priv)
> @@ -3908,7 +3907,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
>   		if (IS_VALLEYVIEW(dev))
>   			vlv_set_rps_idle(dev_priv);
>   		else
> -			gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> +			gen6_set_rps(dev_priv->dev, dev_priv->rps.idle_freq);
>   		dev_priv->rps.last_adj = 0;
>   	}
>   	mutex_unlock(&dev_priv->rps.hw_lock);
> @@ -4059,6 +4058,8 @@ static void gen6_init_rps_frequencies(struct drm_device *dev)
>   					dev_priv->rps.max_freq);
>   	}
>   
> +	dev_priv->rps.idle_freq = dev_priv->rps.min_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;
> @@ -4227,7 +4228,7 @@ static void gen8_enable_rps(struct drm_device *dev)
>   	/* 6: Ring frequency + overclocking (our driver does this later */
>   
>   	dev_priv->rps.power = HIGH_POWER; /* force a reset */
> -	gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> +	gen6_set_rps(dev_priv->dev, dev_priv->rps.idle_freq);
>   
>   	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>   }
> @@ -4321,7 +4322,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>   	}
>   
>   	dev_priv->rps.power = HIGH_POWER; /* force a reset */
> -	gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> +	gen6_set_rps(dev_priv->dev, dev_priv->rps.idle_freq);
>   
>   	rc6vids = 0;
>   	ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
> @@ -4761,6 +4762,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)

I think we missed initializing idle_freq for valleyview platform.

>   		   dev_priv->rps.min_freq) & 1,
>   		  "Odd GPU freq values\n");
>   
> +	dev_priv->rps.idle_freq = dev_priv->rps.min_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;




_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx