Re: [PATCH 2/7] drm/i915: Fix computation of last_adjustment for RPS autotuning

Daniel Vetter
2015-03-06T17:30:49Z

On Fri, Mar 06, 2015 at 03:06:26PM +0000, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I might have impending w/e syndrome, but I can't see what you're fixing
any more. Can you please augment the commit message a bit?

Thanks, Daniel

> ---
> drivers/gpu/drm/i915/i915_irq.c | 27 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9baecb79de8c..1296ce37e435 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1150,21 +1150,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
> mutex_lock(&dev_priv->rps.hw_lock);
>
> adj = dev_priv->rps.last_adj;
> + new_delay = dev_priv->rps.cur_freq;
> if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
> if (adj > 0)
> adj *= 2;
> - else {
> - /* CHV needs even encode values */
> - adj = IS_CHERRYVIEW(dev_priv->dev) ? 2 : 1;
> - }
> - new_delay = dev_priv->rps.cur_freq + adj;
> -
> + else /* CHV needs even encode values */
> + adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1;
> /*
> * For better performance, jump directly
> * to RPe if we're below it.
> */
> - if (new_delay < dev_priv->rps.efficient_freq)
> + if (new_delay < dev_priv->rps.efficient_freq - adj) {
> new_delay = dev_priv->rps.efficient_freq;
> + adj = 0;
> + }
> } else if (pm_iir & GEN6_PM_RP_DOWN_TIMEOUT) {
> if (dev_priv->rps.cur_freq > dev_priv->rps.efficient_freq)
> new_delay = dev_priv->rps.efficient_freq;
> @@ -1176,24 +1175,22 @@ static void gen6_pm_rps_work(struct work_struct *work)
> } else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) {
> if (adj < 0)
> adj *= 2;
> - else {
> - /* CHV needs even encode values */
> - adj = IS_CHERRYVIEW(dev_priv->dev) ? -2 : -1;
> - }
> - new_delay = dev_priv->rps.cur_freq + adj;
> + else /* CHV needs even encode values */
> + adj = IS_CHERRYVIEW(dev_priv) ? -2 : -1;
> } else { /* unknown event */
> - new_delay = dev_priv->rps.cur_freq;
> + adj = 0;
> }
>
> + dev_priv->rps.last_adj = adj;
> +
> /* sysfs frequency interfaces may have snuck in while servicing the
> * interrupt
> */
> + new_delay += adj;
> new_delay = clamp_t(int, new_delay,
> dev_priv->rps.min_freq_softlimit,
> dev_priv->rps.max_freq_softlimit);
>
> - dev_priv->rps.last_adj = new_delay - dev_priv->rps.cur_freq;
> -
> intel_set_rps(dev_priv->dev, new_delay);
>
> mutex_unlock(&dev_priv->rps.hw_lock);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch