[Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts

[Daniel Vetter]

On Wed, Aug 14, 2013 at 09:01:22PM +0000, Azad, Vinit wrote:
> Our interrupt handler isn't being fired since we do set the IER bits
> properly (IIR bits aren't set). The overhead isn't because our driver is
> reacting to these interrupts, but because hardware keeps generating
> internal messages when PMINTRMSK doesn't mask out the up/down EI
> interrupts (which happen periodically).

Ah, that makes more sense. Added to the commit message, please elaborate
more precisely for the next patch what exactly the effects are.

> Unfortunately, the default value of PMINTRMSK register is 0 instead of
> 0xFFFFFFFF as it is for other IMR, so we have to mask out the interrupts
> manually.
> 
> -----Original Message-----
> From: daniel.vetter at ffwll.ch [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, August 14, 2013 13:47
> To: Azad, Vinit
> Cc: intel-gfx
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Only unmask required PM interrupts
> 
> On Wed, Aug 14, 2013 at 10:34 PM, Vinit Azad <vinit.azad at intel.com> wrote:
> > Un-masking all PM interrupts causes hardware to generate interrupts 
> > regardless of whether the interrupts are enabled on the DE side. Since 
> > turbo only need up/down threshold and
> > rc6 timeout interrupt, mask all other interrupts bits to avoid 
> > unnecessary overhead/wake up.
> 
> Just to clarify since I can't really believe this yet: Even though we
> disable all other interrupt sources in PMIER and mask them in PMIMR hw
> still manages to fire off our interrupt handler? Do those interrupts end
> up setting PMIIR?
> 
> Thanks, Daniel
> >
> > Change-Id: I6c947df6fd5f60584d39b9e8b8c89faa51a5e827
> > Signed-off-by: Vinit Azad <vinit.azad at intel.com>

Patch merged to dinq with a bit of manual fixup. Please base patches on
top of my drm-intel-nightly tree if possible, only really critical fixes
(for hangs, black screens, ...) should be pased on -fixes.

Thanks for the patch,
-Daniel

> > ---
> >  drivers/gpu/drm/i915/intel_pm.c |    8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> > b/drivers/gpu/drm/i915/intel_pm.c index 8e9ce07..17a0dae 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3328,8 +3328,8 @@ static void gen6_enable_rps(struct drm_device *dev)
> >         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);
> > -       /* unmask all PM interrupts */
> > -       I915_WRITE(GEN6_PMINTRMSK, 0);
> > +       /* only unmask PM interrupts we need. Mask all others. */
> > +       I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
> >
> >         rc6vids = 0;
> >         ret = sandybridge_pcode_read(dev_priv, 
> > GEN6_PCODE_READ_RC6VIDS, &rc6vids); @@ -3596,8 +3596,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
> >         WARN_ON(dev_priv->rps.pm_iir != 0);
> >         I915_WRITE(GEN6_PMIMR, 0);
> >         spin_unlock_irq(&dev_priv->rps.lock);
> > -       /* enable all PM interrupts */
> > -       I915_WRITE(GEN6_PMINTRMSK, 0);
> > +       /* enable only the PM interrupts we need. Mask everything else */
> > +       I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
> >
> >         gen6_gt_force_wake_put(dev_priv);  }
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch