| Message ID | 1371037046-3732-18-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. I think the original reason for the RPS lock still makes sense, so I'd say your assertion should be that it's not worth the extra complexity, which I can agree with. (The original use of the rps_lock was just for the rps workqueue, it expanded a bunch since then). > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > 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; A bit unrelated, but meh. > > /* 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@ffwll.ch> Patches 14-17 are: Reviewed-by: Ben Widawsky <ben@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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;
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@ffwll.ch> --- 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(-)