On Wed, 6 Mar 2013 16:21:03 +0530 (IST)
Rohit Jain <rohit at intel.com> wrote:
>
>
> On Sat, 2 Mar 2013, Jesse Barnes wrote:
>
> > From: Ben Widawsky <ben at bwidawsk.net>
> >
> > Uses slightly different interfaces than other platforms.
> >
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 148 +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 144 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index e3947cb..d16f4f40 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2477,6 +2477,47 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> > trace_intel_gpu_freq_change(val * 50);
> > }
> >
> > +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(100);
> > + 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);
> > +
> > + if (val == dev_priv->rps.cur_delay)
> > + return;
> > +
> > + valleyview_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> > +
> > + do {
> > + valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
> > + if (time_after(jiffies, timeout)) {
> > + DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
> > + break;
> > + }
> > + udelay(10);
> > + } while (pval & 1);
> > +
> > + valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, &pval);
> > + if ((pval >> 8) != val)
> > + DRM_DEBUG_DRIVER("punit overrode freq: %d requested, but got %d\n",
> > + val, 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 = val;
>
> Shouldn't we store pval >> 8 instead of val in cur_delay in order to
> reclaim the rps state? If we store val here, the requested frequency will
> eventually exceed max_delay if punit overrides with a lower frequency.
>
Yeah we should track the current freq here instead. But we clamp to
max_delay in the caller right? And yeah I missed the update to
i915_irq.c, I fixed that too.
--
Jesse Barnes, Intel Open Source Technology Center
