> -----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: [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
