[Intel-gfx] [PATCH 16/26] drm/i915: turbo & RC6 support for VLV
Jain, Rohit
rohit.jain at intel.com
Mon Mar 11 07:21:40 CET 2013
> -----Original Message-----
> From: intel-gfx-bounces+rohit.jain=intel.com at lists.freedesktop.org
> [mailto:intel-gfx-bounces+rohit.jain=intel.com at lists.freedesktop.org]
> On Behalf Of Jesse Barnes
> Sent: Friday, March 08, 2013 3:58 AM
> To: Rohit Jain
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 16/26] drm/i915: turbo & RC6 support
> for VLV
>
> 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.
Cool! On my board, max_delay gets set to 255 while the punit refuses to go
above 222 in practice. In this case, before we can clamp to max_delay, cur_delay
overflows and gets set to min_delay instead :)
Fixing it like this solves this problem neatly.
Cheers,
Rohit
More information about the Intel-gfx
mailing list