On Wed, Jun 12, 2013 at 01:37:19PM +0200, Daniel Vetter wrote: > Now that the rps interrupt locking isn't clearly separated (at elast > conceptually) from all the other interrupt locking having a different > lock stopped making sense. With this we can (again) unifiy the > ringbuffer irq refcounts without causing a massive confusion, but > that's for the next patch. > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> Patches 14-17 are: Reviewed-by: Ben Widawsky <ben at bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_dma.c | 1 - > drivers/gpu/drm/i915/i915_drv.h | 8 ++++---- > drivers/gpu/drm/i915/i915_irq.c | 12 ++++++------ > drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++-------- > drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++---- > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- > 6 files changed, 23 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index b913b3d..14c3e9c 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1612,7 +1612,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > spin_lock_init(&dev_priv->irq_lock); > spin_lock_init(&dev_priv->gpu_error.lock); > - spin_lock_init(&dev_priv->rps.lock); > spin_lock_init(&dev_priv->backlight.lock); > mutex_init(&dev_priv->dpio_lock); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 07954b2..42d1363 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -691,12 +691,12 @@ struct i915_suspend_saved_registers { > }; > > struct intel_gen6_power_mgmt { > + /* work and pm_iir are protected by dev_priv->irq_lock */ > struct work_struct work; > - struct delayed_work vlv_work; > u32 pm_iir; > - /* lock - irqsave spinlock that protectects the work_struct and > - * pm_iir. */ > - spinlock_t lock; > + > + /* On vlv we need to manually drop to Vmin with a delayed work. */ > + struct delayed_work vlv_work; > > /* The below variables an all the rps hw state are protected by > * dev->struct mutext. */ > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 752b98d..cd7135d 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -696,13 +696,13 @@ static void gen6_pm_rps_work(struct work_struct *work) > u32 pm_iir, pm_imr; > u8 new_delay; > > - spin_lock_irq(&dev_priv->rps.lock); > + spin_lock_irq(&dev_priv->irq_lock); > pm_iir = dev_priv->rps.pm_iir; > dev_priv->rps.pm_iir = 0; > pm_imr = I915_READ(GEN6_PMIMR); > /* Make sure not to corrupt PMIMR state used by ringbuffer code */ > I915_WRITE(GEN6_PMIMR, pm_imr & ~GEN6_PM_RPS_EVENTS); > - spin_unlock_irq(&dev_priv->rps.lock); > + spin_unlock_irq(&dev_priv->irq_lock); > > if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0) > return; > @@ -856,11 +856,11 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, > * The mask bit in IMR is cleared by dev_priv->rps.work. > */ > > - spin_lock(&dev_priv->rps.lock); > + spin_lock(&dev_priv->irq_lock); > dev_priv->rps.pm_iir |= pm_iir; > I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir); > POSTING_READ(GEN6_PMIMR); > - spin_unlock(&dev_priv->rps.lock); > + spin_unlock(&dev_priv->irq_lock); > > queue_work(dev_priv->wq, &dev_priv->rps.work); > } > @@ -933,12 +933,12 @@ static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv, > u32 pm_iir) > { > if (pm_iir & GEN6_PM_RPS_EVENTS) { > - spin_lock(&dev_priv->rps.lock); > + spin_lock(&dev_priv->irq_lock); > dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS; > 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_RPS_EVENTS); > - spin_unlock(&dev_priv->rps.lock); > + spin_unlock(&dev_priv->irq_lock); > > queue_work(dev_priv->wq, &dev_priv->rps.work); > } > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index adc44e4..6a5cfb5 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3128,9 +3128,9 @@ static void gen6_disable_rps(struct drm_device *dev) > * register (PMIMR) to mask PM interrupts. The only risk is in leaving > * stale bits in PMIIR and PMIMR which gen6_enable_rps will clean up. */ > > - spin_lock_irq(&dev_priv->rps.lock); > + spin_lock_irq(&dev_priv->irq_lock); > dev_priv->rps.pm_iir = 0; > - spin_unlock_irq(&dev_priv->rps.lock); > + spin_unlock_irq(&dev_priv->irq_lock); > > I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS); > } > @@ -3147,9 +3147,9 @@ static void valleyview_disable_rps(struct drm_device *dev) > * register (PMIMR) to mask PM interrupts. The only risk is in leaving > * stale bits in PMIIR and PMIMR which gen6_enable_rps will clean up. */ > > - spin_lock_irq(&dev_priv->rps.lock); > + spin_lock_irq(&dev_priv->irq_lock); > dev_priv->rps.pm_iir = 0; > - spin_unlock_irq(&dev_priv->rps.lock); > + spin_unlock_irq(&dev_priv->irq_lock); > > I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR)); > > @@ -3314,13 +3314,13 @@ static void gen6_enable_rps(struct drm_device *dev) > > /* requires MSI enabled */ > I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS); > - spin_lock_irq(&dev_priv->rps.lock); > + spin_lock_irq(&dev_priv->irq_lock); > /* FIXME: Our interrupt enabling sequence is bonghits. > * dev_priv->rps.pm_iir really should be 0 here. */ > dev_priv->rps.pm_iir = 0; > I915_WRITE(GEN6_PMIMR, I915_READ(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS); > I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS); > - spin_unlock_irq(&dev_priv->rps.lock); > + spin_unlock_irq(&dev_priv->irq_lock); > /* unmask all PM interrupts */ > I915_WRITE(GEN6_PMINTRMSK, 0); > > @@ -3585,10 +3585,10 @@ static void valleyview_enable_rps(struct drm_device *dev) > > /* requires MSI enabled */ > I915_WRITE(GEN6_PMIER, GEN6_PM_RPS_EVENTS); > - spin_lock_irq(&dev_priv->rps.lock); > + spin_lock_irq(&dev_priv->irq_lock); > WARN_ON(dev_priv->rps.pm_iir != 0); > I915_WRITE(GEN6_PMIMR, 0); > - spin_unlock_irq(&dev_priv->rps.lock); > + spin_unlock_irq(&dev_priv->irq_lock); > /* enable all PM interrupts */ > I915_WRITE(GEN6_PMINTRMSK, 0); > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 1ef081c..a7c9934 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1053,14 +1053,14 @@ hsw_vebox_get_irq(struct intel_ring_buffer *ring) > if (!dev->irq_enabled) > return false; > > - spin_lock_irqsave(&dev_priv->rps.lock, flags); > + spin_lock_irqsave(&dev_priv->irq_lock, flags); > if (ring->irq_refcount.pm++ == 0) { > u32 pm_imr = I915_READ(GEN6_PMIMR); > I915_WRITE_IMR(ring, ~ring->irq_enable_mask); > I915_WRITE(GEN6_PMIMR, pm_imr & ~ring->irq_enable_mask); > POSTING_READ(GEN6_PMIMR); > } > - spin_unlock_irqrestore(&dev_priv->rps.lock, flags); > + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > > return true; > } > @@ -1075,14 +1075,14 @@ hsw_vebox_put_irq(struct intel_ring_buffer *ring) > if (!dev->irq_enabled) > return; > > - spin_lock_irqsave(&dev_priv->rps.lock, flags); > + spin_lock_irqsave(&dev_priv->irq_lock, flags); > if (--ring->irq_refcount.pm == 0) { > u32 pm_imr = I915_READ(GEN6_PMIMR); > I915_WRITE_IMR(ring, ~0); > I915_WRITE(GEN6_PMIMR, pm_imr | ring->irq_enable_mask); > POSTING_READ(GEN6_PMIMR); > } > - spin_unlock_irqrestore(&dev_priv->rps.lock, flags); > + spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > } > > static int > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index efc403d..f960805 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -76,7 +76,7 @@ struct intel_ring_buffer { > > struct { > u32 gt; /* protected by dev_priv->irq_lock */ > - u32 pm; /* protected by dev_priv->rps.lock (sucks) */ > + u32 pm; /* protected by dev_priv->irq_lock */ > } irq_refcount; > u32 irq_enable_mask; /* bitmask to enable ring interrupt */ > u32 trace_irq_seqno; > -- > 1.8.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center