On Tue, 25 Jun 2013 19:21:02 +0300
ville.syrjala at linux.intel.com wrote:
> From: Ville Syrj?l? <ville.syrjala at linux.intel.com>
>
> It seems that even though Punit reports the frequency change to have
> been completed, it still reports the old frequency in the status
> register for some time.
>
> So rather than polling for Punit to complete the frequency change after
> each request, poll before. This gets rid of the spurious "Punit overrode
> GPU freq" messages.
>
> This also lets us continue working while Punit is performing the actual
> frequency change. As a result, openarena demo088-test1 timedemo average
> fps is increased by ~5 fps, and the slowest frame duration is reduced
> by ~25%.
>
> The sysfs cur_freq file always reads the current frequency from Punit
> anyway, so having rps.cur_delay be slightly off at times doesn't matter.
>
> Signed-off-by: Ville Syrj?l? <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 53 +++++++++++++++++++++++++++--------------
> 1 file changed, 35 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 48a3162..6dbcad7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3066,17 +3066,49 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> trace_intel_gpu_freq_change(val * 50);
> }
>
> +/*
> + * Wait until the previous freq change has completed,
> + * or the timeout elapsed, and then update our notion
> + * of the current GPU frequency.
> + */
> +static void vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv)
> +{
> + unsigned long timeout = jiffies + msecs_to_jiffies(10);
> + u32 pval;
> +
> + WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> +
> + do {
> + pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> + if (time_after(jiffies, timeout)) {
> + DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
> + break;
> + }
> + udelay(10);
> + } while (pval & 1);
> +
> + pval >>= 8;
> +
> + if (pval != dev_priv->rps.cur_delay)
> + DRM_DEBUG_DRIVER("Punit overrode GPU freq: %d MHz (%u) requested, but got %d Mhz (%u)\n",
> + vlv_gpu_freq(dev_priv->mem_freq, dev_priv->rps.cur_delay),
> + dev_priv->rps.cur_delay,
> + vlv_gpu_freq(dev_priv->mem_freq, pval), pval);
> +
> + dev_priv->rps.cur_delay = pval;
> +}
> +
> void valleyview_set_rps(struct drm_device *dev, u8 val)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - unsigned long timeout = jiffies + msecs_to_jiffies(10);
> u32 limits = gen6_rps_limits(dev_priv, &val);
> - u32 pval;
>
> WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> WARN_ON(val > dev_priv->rps.max_delay);
> WARN_ON(val < dev_priv->rps.min_delay);
>
> + vlv_update_rps_cur_delay(dev_priv);
> +
> DRM_DEBUG_DRIVER("GPU freq request from %d MHz (%u) to %d MHz (%u)\n",
> vlv_gpu_freq(dev_priv->mem_freq,
> dev_priv->rps.cur_delay),
> @@ -3088,27 +3120,12 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
>
> vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
>
> - do {
> - pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> - if (time_after(jiffies, timeout)) {
> - DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
> - break;
> - }
> - udelay(10);
> - } while (pval & 1);
> -
> - pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> - if ((pval >> 8) != val)
> - DRM_DEBUG_DRIVER("Punit overrode GPU freq: %d MHz (%u) requested, but got %d Mhz (%u)\n",
> - vlv_gpu_freq(dev_priv->mem_freq, val), val,
> - vlv_gpu_freq(dev_priv->mem_freq, pval >> 8), pval >> 8);
> -
> /* Make sure we continue to get interrupts
> * until we hit the minimum or maximum frequencies.
> */
> I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, limits);
>
> - dev_priv->rps.cur_delay = pval >> 8;
> + dev_priv->rps.cur_delay = val;
>
> trace_intel_gpu_freq_change(vlv_gpu_freq(dev_priv->mem_freq, val));
> }
Hiding some of the Punit latency is nice...
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
