On Sat, 28 Apr 2012 08:56:39 +0100 Chris Wilson <chris at chris-wilson.co.uk> wrote: > In order to avoid missed down-interrupts when coming out of RC6, it is > advised that we always reset the down-threshold upon a PM event. This > is due to that the PM unit goes through a little dance when coming > out of RC6, it first brings the GPU up at the lowest frequency then a > short time later it restores the thresholds. During that interval, the > down-interval may expire and the interrupt be suppressed. > > Now aware of the dance taking place within the GPU when coming out of > RC6, one wonders what other writes need to be queued in the fifo > buffer in order to be properly sequenced; setting the RP state > appears to be one. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44006 > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> I tried really hard to review this, but it was too much. The code definitely is cleaner as a result, so this is: Acked-by: Ben Widawsky <ben at bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_irq.c | 37 +++++-------------------- > drivers/gpu/drm/i915/intel_pm.c | 57 > +++++++++++++++++++++++++++------------ 2 files changed, 47 > insertions(+), 47 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > b/drivers/gpu/drm/i915/i915_irq.c index c2511fe..1ea39e4 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -353,8 +353,8 @@ static void gen6_pm_rps_work(struct work_struct > *work) { > drm_i915_private_t *dev_priv = container_of(work, > drm_i915_private_t, rps_work); > - u8 new_delay = dev_priv->cur_delay; > u32 pm_iir, pm_imr; > + u8 new_delay; > > spin_lock_irq(&dev_priv->rps_lock); > pm_iir = dev_priv->pm_iir; > @@ -363,41 +363,18 @@ static void gen6_pm_rps_work(struct work_struct > *work) I915_WRITE(GEN6_PMIMR, 0); > spin_unlock_irq(&dev_priv->rps_lock); > > - if (!pm_iir) > + if ((pm_iir & GEN6_PM_DEFERRED_EVENTS) == 0) > return; > > mutex_lock(&dev_priv->dev->struct_mutex); > - if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) { > - if (dev_priv->cur_delay != dev_priv->max_delay) > - new_delay = dev_priv->cur_delay + 1; > - if (new_delay > dev_priv->max_delay) > - new_delay = dev_priv->max_delay; > - } else if (pm_iir & (GEN6_PM_RP_DOWN_THRESHOLD | > GEN6_PM_RP_DOWN_TIMEOUT)) { > - gen6_gt_force_wake_get(dev_priv); > - if (dev_priv->cur_delay != dev_priv->min_delay) > - new_delay = dev_priv->cur_delay - 1; > - if (new_delay < dev_priv->min_delay) { > - new_delay = dev_priv->min_delay; > - I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, > - > I915_READ(GEN6_RP_INTERRUPT_LIMITS) | > - ((new_delay << 16) & 0x3f0000)); > - } else { > - /* Make sure we continue to get down > interrupts > - * until we hit the minimum frequency */ > - I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, > - > I915_READ(GEN6_RP_INTERRUPT_LIMITS) & ~0x3f0000); > - } > - gen6_gt_force_wake_put(dev_priv); > - } > + > + if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) > + new_delay = dev_priv->cur_delay + 1; > + else > + new_delay = dev_priv->cur_delay - 1; > > gen6_set_rps(dev_priv->dev, new_delay); > - dev_priv->cur_delay = new_delay; > > - /* > - * rps_lock not held here because clearing is > non-destructive. There is > - * an *extremely* unlikely race with gen6_rps_enable() that > is prevented > - * by holding struct_mutex for the duration of the write. > - */ > mutex_unlock(&dev_priv->dev->struct_mutex); > } > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c index 3b05ba3..5e6b0c8 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2127,10 +2127,33 @@ void ironlake_disable_drps(struct drm_device > *dev) void gen6_set_rps(struct drm_device *dev, u8 val) > { > struct drm_i915_private *dev_priv = dev->dev_private; > - u32 swreq; > + u32 limits; > > - swreq = (val & 0x3ff) << 25; > - I915_WRITE(GEN6_RPNSWREQ, swreq); > + limits = 0; > + if (val >= dev_priv->max_delay) > + val = dev_priv->max_delay; > + else > + limits |= dev_priv->max_delay << 24; > + > + if (val <= dev_priv->min_delay) > + val = dev_priv->min_delay; > + else > + limits |= dev_priv->min_delay << 16; > + > + if (val == dev_priv->cur_delay) > + return; > + > + I915_WRITE(GEN6_RPNSWREQ, > + GEN6_FREQUENCY(val) | > + GEN6_OFFSET(0) | > + GEN6_AGGRESSIVE_TURBO); > + > + /* Make sure we continue to get interrupts > + * until we hit the minimum or maximum frequencies. > + */ > + I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, limits); > + > + dev_priv->cur_delay = val; > } > > void gen6_disable_rps(struct drm_device *dev) > @@ -2183,11 +2206,10 @@ int intel_enable_rc6(const struct drm_device > *dev) > void gen6_enable_rps(struct drm_i915_private *dev_priv) > { > - u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); > - u32 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS); > + u32 rp_state_cap; > + u32 gt_perf_status; > u32 pcu_mbox, rc6_mask = 0; > u32 gtfifodbg; > - int cur_freq, min_freq, max_freq; > int rc6_mode; > int i; > > @@ -2208,6 +2230,14 @@ void gen6_enable_rps(struct drm_i915_private > *dev_priv) > gen6_gt_force_wake_get(dev_priv); > > + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); > + gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS); > + > + /* In units of 100MHz */ > + dev_priv->max_delay = rp_state_cap & 0xff; > + dev_priv->min_delay = (rp_state_cap & 0xff0000) >> 16; > + dev_priv->cur_delay = 0; > + > /* disable the counters and set deterministic thresholds */ > I915_WRITE(GEN6_RC_CONTROL, 0); > > @@ -2255,8 +2285,8 @@ void gen6_enable_rps(struct drm_i915_private > *dev_priv) > I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000); > I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, > - 18 << 24 | > - 6 << 16); > + dev_priv->max_delay << 24 | > + dev_priv->min_delay << 16); > I915_WRITE(GEN6_RP_UP_THRESHOLD, 10000); > I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 1000000); > I915_WRITE(GEN6_RP_UP_EI, 100000); > @@ -2282,10 +2312,6 @@ void gen6_enable_rps(struct drm_i915_private > *dev_priv) 500)) > DRM_ERROR("timeout waiting for pcode mailbox to > finish\n"); > - min_freq = (rp_state_cap & 0xff0000) >> 16; > - max_freq = rp_state_cap & 0xff; > - cur_freq = (gt_perf_status & 0xff00) >> 8; > - > /* Check for overclock support */ > if (wait_for((I915_READ(GEN6_PCODE_MAILBOX) & > GEN6_PCODE_READY) == 0, 500)) > @@ -2296,14 +2322,11 @@ void gen6_enable_rps(struct drm_i915_private > *dev_priv) 500)) > DRM_ERROR("timeout waiting for pcode mailbox to > finish\n"); if (pcu_mbox & (1<<31)) { /* OC supported */ > - max_freq = pcu_mbox & 0xff; > + dev_priv->max_delay = pcu_mbox & 0xff; > DRM_DEBUG_DRIVER("overclocking supported, adjusting > frequency max to %dMHz\n", pcu_mbox * 50); } > > - /* In units of 100MHz */ > - dev_priv->max_delay = max_freq; > - dev_priv->min_delay = min_freq; > - dev_priv->cur_delay = cur_freq; > + gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); > > /* requires MSI enabled */ > I915_WRITE(GEN6_PMIER,