On Tue, May 28, 2013 at 07:22:27PM -0700, Ben Widawsky wrote:
> PM interrupts have an expanded role on HSW. It helps route the EBOX
> interrupts. This patch is necessary to make the existing code which
> touches the mask, and enable registers more friendly to other code paths
> that also will need these registers.
>
> To be more explicit:
> At preinstall all interrupts are masked and disabled. This implies that
> preinstall should always happen before any enabling/disabling of RPS or
> other interrupts.
>
> The PMIMR is touched by the workqueue, so enable/disable touch IER and
> IIR. Similarly, the code currently expects IMR has no use outside of the
> RPS related interrupts so they unconditionally set 0, or ~0. We could
> use IER in the workqueue, and IMR elsewhere, but since the workqueue
> use-case is more transient the existing usage makes sense.
>
> Disable RPS events:
> IER := IER & ~GEN6_PM_RPS_EVENTS // Disable RPS related interrupts
> IIR := GEN6_PM_RPS_EVENTS // Disable any outstanding interrupts
>
> Enable RPS events:
> IER := IER | GEN6_PM_RPS_EVENTS // Enable the RPS related interrupts
> IIR := GEN6_PM_RPS_EVENTS // Make sure there were no leftover events
> (really shouldn't happen)
>
> v2: Shouldn't destroy PMIIR or PMIMR VEBOX interrupt state in
> enable/disable rps functions (Haihao)
>
> v3: Bug found by Chris where we were clearing the wrong bits at rps
> disable.
> expanded commit message
>
> v4: v3 was based off the wrong branch
>
> v5: Added the setting of PMIMR because of previous patch update
>
> CC: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
Reviewed-by: Damien Lespiau <damien.lespiau at intel.com>
--
Damien
> drivers/gpu/drm/i915/i915_irq.c | 21 +++++++++++----------
> drivers/gpu/drm/i915/i915_reg.h | 2 +-
> drivers/gpu/drm/i915/intel_pm.c | 13 +++++++------
> 3 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9c66fcf..8da936d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -700,10 +700,11 @@ static void gen6_pm_rps_work(struct work_struct *work)
> pm_iir = dev_priv->rps.pm_iir;
> dev_priv->rps.pm_iir = 0;
> pm_imr = I915_READ(GEN6_PMIMR);
> - I915_WRITE(GEN6_PMIMR, 0);
> + /* Make sure not to corrupt PMIMR state used by ringbuffer code */
> + I915_WRITE(GEN6_PMIMR, pm_imr & ~GEN6_PM_RPS_EVENTS);
> spin_unlock_irq(&dev_priv->rps.lock);
>
> - if ((pm_iir & GEN6_PM_DEFERRED_EVENTS) == 0)
> + if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0)
> return;
>
> mutex_lock(&dev_priv->rps.hw_lock);
> @@ -933,17 +934,17 @@ static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv,
> unsigned long flags;
>
> spin_lock_irqsave(&dev_priv->rps.lock, flags);
> - dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_DEFERRED_EVENTS;
> + dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS;
> if (dev_priv->rps.pm_iir) {
> I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir);
> /* never want to mask useful interrupts. (also posting read) */
> - WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_DEFERRED_EVENTS);
> + WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
> /* TODO: if queue_work is slow, move it out of the spinlock */
> queue_work(dev_priv->wq, &dev_priv->rps.work);
> }
> spin_unlock_irqrestore(&dev_priv->rps.lock, flags);
>
> - if (pm_iir & ~GEN6_PM_DEFERRED_EVENTS)
> + if (pm_iir & ~GEN6_PM_RPS_EVENTS)
> DRM_ERROR("Unexpected PM interrupted\n");
> }
>
> @@ -1018,7 +1019,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> if (pipe_stats[0] & PIPE_GMBUS_INTERRUPT_STATUS)
> gmbus_irq_handler(dev);
>
> - if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
> + if (pm_iir & GEN6_PM_RPS_EVENTS)
> gen6_queue_rps_work(dev_priv, pm_iir);
>
> I915_WRITE(GTIIR, gt_iir);
> @@ -1259,7 +1260,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
> if (pm_iir) {
> if (IS_HASWELL(dev))
> hsw_pm_irq_handler(dev_priv, pm_iir);
> - else if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
> + else if (pm_iir & GEN6_PM_RPS_EVENTS)
> gen6_queue_rps_work(dev_priv, pm_iir);
> I915_WRITE(GEN6_PMIIR, pm_iir);
> ret = IRQ_HANDLED;
> @@ -1374,7 +1375,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
> if (IS_GEN5(dev) && de_iir & DE_PCU_EVENT)
> ironlake_handle_rps_change(dev);
>
> - if (IS_GEN6(dev) && pm_iir & GEN6_PM_DEFERRED_EVENTS)
> + if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
> gen6_queue_rps_work(dev_priv, pm_iir);
>
> I915_WRITE(GTIIR, gt_iir);
> @@ -2716,8 +2717,8 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
> POSTING_READ(GTIER);
>
> /* Power management */
> - I915_WRITE(GEN6_PMIMR, ~GEN6_PM_DEFERRED_EVENTS);
> - I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS);
> + I915_WRITE(GEN6_PMIMR, ~GEN6_PM_RPS_EVENTS);
> + I915_WRITE(GEN6_PMIER, GEN6_PM_RPS_EVENTS);
> POSTING_READ(GEN6_PMIMR);
>
> ibx_irq_postinstall(dev);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a5717f1..3a29f99 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4562,7 +4562,7 @@
> #define GEN6_PM_RP_DOWN_THRESHOLD (1<<4)
> #define GEN6_PM_RP_UP_EI_EXPIRED (1<<2)
> #define GEN6_PM_RP_DOWN_EI_EXPIRED (1<<1)
> -#define GEN6_PM_DEFERRED_EVENTS (GEN6_PM_RP_UP_THRESHOLD | \
> +#define GEN6_PM_RPS_EVENTS (GEN6_PM_RP_UP_THRESHOLD | \
> GEN6_PM_RP_DOWN_THRESHOLD | \
> GEN6_PM_RP_DOWN_TIMEOUT)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 66750fe..09a2f90 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2625,7 +2625,7 @@ static void gen6_disable_rps(struct drm_device *dev)
> I915_WRITE(GEN6_RC_CONTROL, 0);
> I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
> I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
> - I915_WRITE(GEN6_PMIER, 0);
> + I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) & ~GEN6_PM_RPS_EVENTS);
> /* Complete PM interrupt masking here doesn't race with the rps work
> * item again unmasking PM interrupts because that is using a different
> * register (PMIMR) to mask PM interrupts. The only risk is in leaving
> @@ -2635,7 +2635,7 @@ static void gen6_disable_rps(struct drm_device *dev)
> dev_priv->rps.pm_iir = 0;
> spin_unlock_irq(&dev_priv->rps.lock);
>
> - I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
> + I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
> }
>
> static void valleyview_disable_rps(struct drm_device *dev)
> @@ -2816,12 +2816,13 @@ static void gen6_enable_rps(struct drm_device *dev)
> gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
>
> /* requires MSI enabled */
> - I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS);
> + I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS);
> spin_lock_irq(&dev_priv->rps.lock);
> dev_priv->rps.pm_iir = 0;
> - I915_WRITE(GEN6_PMIMR, 0);
> + I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
> + I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
> spin_unlock_irq(&dev_priv->rps.lock);
> - /* enable all PM interrupts */
> + /* unmask all PM interrupts */
> I915_WRITE(GEN6_PMINTRMSK, 0);
>
> rc6vids = 0;
> @@ -3084,7 +3085,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
> valleyview_set_rps(dev_priv->dev, rpe);
>
> /* requires MSI enabled */
> - I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS);
> + I915_WRITE(GEN6_PMIER, GEN6_PM_RPS_EVENTS);
> spin_lock_irq(&dev_priv->rps.lock);
> WARN_ON(dev_priv->rps.pm_iir != 0);
> I915_WRITE(GEN6_PMIMR, 0);
> --
> 1.8.3
>
