On Wed, May 29, 2013 at 09:19:25PM +0200, Daniel Vetter wrote: > On Tue, May 28, 2013 at 07:22:24PM -0700, Ben Widawsky wrote: > > HSW has some special requirements for the VEBOX. Splitting out the > > interrupt handler will make the code a bit nicer and less error prone > > when we begin to handle those. > > > > The slight functional change in this patch (queueing work while holding > > the spinlock) is intentional as it makes a subsequent patch a bit nicer. > > The change should also only effect HSW platforms. > > > > Reviewed-by: Damien Lespiau <damien.lesp...@intel.com> > > Signed-off-by: Ben Widawsky <b...@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 30 +++++++++++++++++++++++++++++- > > 1 file changed, 29 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 557acd3..c7b51c2 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -842,6 +842,7 @@ static void snb_gt_irq_handler(struct drm_device *dev, > > ivybridge_handle_parity_error(dev); > > } > > > > +/* Legacy way of handling PM interrupts */ > > static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, > > u32 pm_iir) > > { > > @@ -921,6 +922,31 @@ static void dp_aux_irq_handler(struct drm_device *dev) > > wake_up_all(&dev_priv->gmbus_wait_queue); > > } > > > > +/* Unlike gen6_queue_rps_work() from which this function is originally > > derived, > > + * we must be able to deal with other PM interrupts. This is complicated > > because > > + * of the way in which we use the masks to defer the RPS work (which for > > + * posterity is necessary because of forcewake). > > + */ > > +static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv, > > + u32 pm_iir) > > +{ > > + unsigned long flags; > > + > > + spin_lock_irqsave(&dev_priv->rps.lock, flags); > > + dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_DEFERRED_EVENTS; > > + if (dev_priv->rps.pm_iir) { > > + I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir); > > + /* never want to mask useful interrupts. (also posting read) */ > > + WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & > > ~GEN6_PM_DEFERRED_EVENTS); > > + /* TODO: if queue_work is slow, move it out of the spinlock */ > > + queue_work(dev_priv->wq, &dev_priv->rps.work); > > Not happy how hsw and gen6 rps queueing now differs ever so slightly. It's > rather completely irrelevant where we actually queue the work since: > - interrupt handlers are non-reentrant > - queue_work is irq safe anyway. > So smells like cargo-culting too me. > > So I've killed your TODO and moved it out. If there's indeed a good reason > later on we can reconsider.
Dropped this change again since it affects correctness later on. -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