Re: [PATCH 2/5] drm/i915/bdw: Implement a basic PM interrupt handler

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, May 16, 2014 at 11:09:35AM +0200, Daniel Vetter wrote:
> On Fri, May 16, 2014 at 01:38:18AM +0000, O'Rourke, Tom wrote:
> > >+static void gen8_disable_rps_interrupts(struct drm_device *dev) {
> > >+	struct drm_i915_private *dev_priv = dev->dev_private;
> > >+
> > >+	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
> > 
> > [TOR:] Please note that for Broadwell, bit 31 in GEN6_PMINTRMSK is not an interrupt disable bit.
> > In "drm/i915: Enable PM Interrupts target via	Display Interface." this bit is defined as:
> > +#define GEN8_PMINTR_REDIRECT_TO_NON_DISP	(1<<31)
> > 
> > Writing this bit here could have unintended consequences.
> 
> Hm, we seem to unconditionally set this bit on gen8 anyway. Still harmful?
> Mika, Ville?

I was thinking that since we mask all the interrupts anyway it
doesn't really matter which way we set the bit here. But I'm
not actually sure what happens if there's an already pending
interrupt when we flip the bit. That is I have no idea if it
could raise an interrupt on the non-disp side or not.

So leaving the bit as zero here might be the safer option, and
at least it would be more consistent with the rest of the gen8
pm irq code. So if someone wants to make that change, my r-b
will still stand.

-- 
Ville Syrjälä
Intel OTC