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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
