On Thu, Apr 21, 2011 at 07:34:08AM +0100, Chris Wilson wrote: > > + /* > > + * Lock not held here, because clearing is non-destructive, and > > + * the interrupt handler is the only other place where it is written. > > + */ > > + I915_WRITE(GEN6_PMIMR, pm_imr & ~pm_iir); } > > But does this register use __gen6_gt_wait_for_fifo? *That* requires a > lock. I'm starting to see a need for I915_READ_IRQ, I915_WRITE_IRQ, so > that the circumstances are explicit and we acknowledge them when modifying > the read/write routines.
GEN6_PMIMR is over 0x40000, so it should be safe. The write to it in the interrupt handler would also not be safe otherwise. > > > + unsigned long flags; > > + /* Make sure no new interrupts come in */ > > + spin_lock_irqsave(&dev_priv->rps_lock, flags); > > + I915_WRITE(GEN6_PMIMR, pm_iir); > > + > > + /* Add the new ones */ > > + BUG_ON(dev_priv->pm_iir & pm_iir); > > Bah. The comments are absolutely useless. Really. What you need to > describe is how the use of IMR and IIR is split between the interrupt > handler and the tasklet. > > Or maybe they did their job, because I had to go back and read much more > carefully what you were doing... > Coming up... Ben _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
