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

[Daniel Vetter]

On Fri, May 16, 2014 at 12:46:00PM +0300, Ville Syrjälä wrote:
> 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.

Imo better as a follow-up patch with the Bspec quote above in the commit
message.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch