[Intel-gfx] [PATCH 4/5] drm/i915: move gen6 rps handling to workqueue

Ben Widawsky ben at bwidawsk.net
Fri Apr 22 18:12:26 CEST 2011


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



More information about the Intel-gfx mailing list