drm/i915: Move the get/put irq locking into the caller

With only a single callsite for intel_engine_cs->irq_get and ->irq_put,
we can reduce the code size by moving the common preamble into the
caller, and we can also eliminate the reference counting.

For completeness, as we are no longer doing reference counting on irq,
rename the get/put vfunctions to enable/disable respectively and are
able to review the use of posting reads. We only require the
serialisation with hardware when enabling the interrupt (i.e. so we
cannot miss an interrupt by going to sleep before the hardware truly
enables it).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-18-git-send-email-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7724bae27bcf0ca35e89ac3ea6aacad4dbdcb60f..be25b7bdacfed5011c5c56d111a0167ce07d140c 100644 (file)
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -259,12 +259,12 @@ static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
        dev_priv->gt_irq_mask &= ~interrupt_mask;
        dev_priv->gt_irq_mask |= (~enabled_irq_mask & interrupt_mask);
        I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
-       POSTING_READ(GTIMR);
 }
 
 void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask)
 {
        ilk_update_gt_irq(dev_priv, mask, mask);
+       POSTING_READ_FW(GTIMR);
 }
 
 void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask)
@@ -2819,9 +2819,9 @@ ring_idle(struct intel_engine_cs *engine, u32 seqno)
 }
 
 static bool
-ipehr_is_semaphore_wait(struct drm_i915_private *dev_priv, u32 ipehr)
+ipehr_is_semaphore_wait(struct intel_engine_cs *engine, u32 ipehr)
 {
-       if (INTEL_GEN(dev_priv) >= 8) {
+       if (INTEL_GEN(engine->i915) >= 8) {
                return (ipehr >> 23) == 0x1c;
        } else {
                ipehr &= ~MI_SEMAPHORE_SYNC_MASK;
@@ -2892,7 +2892,7 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
                return NULL;
 
        ipehr = I915_READ(RING_IPEHR(engine->mmio_base));
-       if (!ipehr_is_semaphore_wait(engine->i915, ipehr))
+       if (!ipehr_is_semaphore_wait(engine, ipehr))
                return NULL;
 
        /*

diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index f20ccacde81e6721e599b7486b8b4d3a0229522e..009d6e121767b1a70ac1c24eb3e3134854175645 100644 (file)
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -50,12 +50,18 @@ static void irq_enable(struct intel_engine_cs *engine)
         * just in case.
         */
        engine->irq_posted = true;
-       WARN_ON(!engine->irq_get(engine));
+
+       spin_lock_irq(&engine->i915->irq_lock);
+       engine->irq_enable(engine);
+       spin_unlock_irq(&engine->i915->irq_lock);
 }
 
 static void irq_disable(struct intel_engine_cs *engine)
 {
-       engine->irq_put(engine);
+       spin_lock_irq(&engine->i915->irq_lock);
+       engine->irq_disable(engine);
+       spin_unlock_irq(&engine->i915->irq_lock);
+
        engine->irq_posted = false;
 }
 

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f1a01137334c971ee311b7035fc9cfa0ddb65e5b..380175149916cabc06cb94e6430a5037a0bd418c 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1617,36 +1617,18 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
        return 0;
 }
 
-static bool gen8_logical_ring_get_irq(struct intel_engine_cs *engine)
+static void gen8_logical_ring_enable_irq(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
-
-       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
-               return false;
-
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (engine->irq_refcount++ == 0) {
-               I915_WRITE_IMR(engine,
-                              ~(engine->irq_enable_mask | engine->irq_keep_mask));
-               POSTING_READ(RING_IMR(engine->mmio_base));
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
-
-       return true;
+       I915_WRITE_IMR(engine,
+                      ~(engine->irq_enable_mask | engine->irq_keep_mask));
+       POSTING_READ_FW(RING_IMR(engine->mmio_base));
 }
 
-static void gen8_logical_ring_put_irq(struct intel_engine_cs *engine)
+static void gen8_logical_ring_disable_irq(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
-
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (--engine->irq_refcount == 0) {
-               I915_WRITE_IMR(engine, ~engine->irq_keep_mask);
-               POSTING_READ(RING_IMR(engine->mmio_base));
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+       I915_WRITE_IMR(engine, ~engine->irq_keep_mask);
 }
 
 static int gen8_emit_flush(struct drm_i915_gem_request *request,
@@ -1966,8 +1948,8 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
        engine->init_hw = gen8_init_common_ring;
        engine->emit_request = gen8_emit_request;
        engine->emit_flush = gen8_emit_flush;
-       engine->irq_get = gen8_logical_ring_get_irq;
-       engine->irq_put = gen8_logical_ring_put_irq;
+       engine->irq_enable = gen8_logical_ring_enable_irq;
+       engine->irq_disable = gen8_logical_ring_disable_irq;
        engine->emit_bb_start = gen8_emit_bb_start;
        if (IS_BXT_REVID(engine->i915, 0, BXT_REVID_A1))
                engine->irq_seqno_barrier = bxt_a_seqno_barrier;

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3bd598673d8824c5a4051a3ebbede96a3740e91b..994f7616a58b8a3e044734f6784310cb0a9a06f7 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1636,103 +1636,54 @@ gen6_seqno_barrier(struct intel_engine_cs *engine)
        spin_unlock_irq(&dev_priv->uncore.lock);
 }
 
-static bool
-gen5_ring_get_irq(struct intel_engine_cs *engine)
+static void
+gen5_irq_enable(struct intel_engine_cs *engine)
 {
-       struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
-
-       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
-               return false;
-
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (engine->irq_refcount++ == 0)
-               gen5_enable_gt_irq(dev_priv, engine->irq_enable_mask);
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
-
-       return true;
+       gen5_enable_gt_irq(engine->i915, engine->irq_enable_mask);
 }
 
 static void
-gen5_ring_put_irq(struct intel_engine_cs *engine)
+gen5_irq_disable(struct intel_engine_cs *engine)
 {
-       struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
-
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (--engine->irq_refcount == 0)
-               gen5_disable_gt_irq(dev_priv, engine->irq_enable_mask);
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+       gen5_disable_gt_irq(engine->i915, engine->irq_enable_mask);
 }
 
-static bool
-i9xx_ring_get_irq(struct intel_engine_cs *engine)
+static void
+i9xx_irq_enable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
-
-       if (!intel_irqs_enabled(dev_priv))
-               return false;
-
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (engine->irq_refcount++ == 0) {
-               dev_priv->irq_mask &= ~engine->irq_enable_mask;
-               I915_WRITE(IMR, dev_priv->irq_mask);
-               POSTING_READ(IMR);
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
 
-       return true;
+       dev_priv->irq_mask &= ~engine->irq_enable_mask;
+       I915_WRITE(IMR, dev_priv->irq_mask);
+       POSTING_READ_FW(RING_IMR(engine->mmio_base));
 }
 
 static void
-i9xx_ring_put_irq(struct intel_engine_cs *engine)
+i9xx_irq_disable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
 
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (--engine->irq_refcount == 0) {
-               dev_priv->irq_mask |= engine->irq_enable_mask;
-               I915_WRITE(IMR, dev_priv->irq_mask);
-               POSTING_READ(IMR);
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+       dev_priv->irq_mask |= engine->irq_enable_mask;
+       I915_WRITE(IMR, dev_priv->irq_mask);
 }
 
-static bool
-i8xx_ring_get_irq(struct intel_engine_cs *engine)
+static void
+i8xx_irq_enable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
 
-       if (!intel_irqs_enabled(dev_priv))
-               return false;
-
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (engine->irq_refcount++ == 0) {
-               dev_priv->irq_mask &= ~engine->irq_enable_mask;
-               I915_WRITE16(IMR, dev_priv->irq_mask);
-               POSTING_READ16(IMR);
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
-
-       return true;
+       dev_priv->irq_mask &= ~engine->irq_enable_mask;
+       I915_WRITE16(IMR, dev_priv->irq_mask);
+       POSTING_READ16(RING_IMR(engine->mmio_base));
 }
 
 static void
-i8xx_ring_put_irq(struct intel_engine_cs *engine)
+i8xx_irq_disable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
 
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (--engine->irq_refcount == 0) {
-               dev_priv->irq_mask |= engine->irq_enable_mask;
-               I915_WRITE16(IMR, dev_priv->irq_mask);
-               POSTING_READ16(IMR);
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+       dev_priv->irq_mask |= engine->irq_enable_mask;
+       I915_WRITE16(IMR, dev_priv->irq_mask);
 }
 
 static int
@@ -1773,122 +1724,74 @@ i9xx_add_request(struct drm_i915_gem_request *req)
        return 0;
 }
 
-static bool
-gen6_ring_get_irq(struct intel_engine_cs *engine)
+static void
+gen6_irq_enable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
-
-       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
-               return false;
 
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (engine->irq_refcount++ == 0) {
-               if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
-                       I915_WRITE_IMR(engine,
-                                      ~(engine->irq_enable_mask |
-                                        GT_PARITY_ERROR(dev_priv)));
-               else
-                       I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
-               gen5_enable_gt_irq(dev_priv, engine->irq_enable_mask);
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
-
-       return true;
+       if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
+               I915_WRITE_IMR(engine,
+                              ~(engine->irq_enable_mask |
+                                GT_PARITY_ERROR(dev_priv)));
+       else
+               I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
+       gen5_enable_gt_irq(dev_priv, engine->irq_enable_mask);
 }
 
 static void
-gen6_ring_put_irq(struct intel_engine_cs *engine)
+gen6_irq_disable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
 
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (--engine->irq_refcount == 0) {
-               if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
-                       I915_WRITE_IMR(engine, ~GT_PARITY_ERROR(dev_priv));
-               else
-                       I915_WRITE_IMR(engine, ~0);
-               gen5_disable_gt_irq(dev_priv, engine->irq_enable_mask);
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+       if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
+               I915_WRITE_IMR(engine, ~GT_PARITY_ERROR(dev_priv));
+       else
+               I915_WRITE_IMR(engine, ~0);
+       gen5_disable_gt_irq(dev_priv, engine->irq_enable_mask);
 }
 
-static bool
-hsw_vebox_get_irq(struct intel_engine_cs *engine)
+static void
+hsw_vebox_irq_enable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
-
-       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
-               return false;
 
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (engine->irq_refcount++ == 0) {
-               I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
-               gen6_enable_pm_irq(dev_priv, engine->irq_enable_mask);
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
-
-       return true;
+       I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
+       gen6_enable_pm_irq(dev_priv, engine->irq_enable_mask);
 }
 
 static void
-hsw_vebox_put_irq(struct intel_engine_cs *engine)
+hsw_vebox_irq_disable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
 
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (--engine->irq_refcount == 0) {
-               I915_WRITE_IMR(engine, ~0);
-               gen6_disable_pm_irq(dev_priv, engine->irq_enable_mask);
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+       I915_WRITE_IMR(engine, ~0);
+       gen6_disable_pm_irq(dev_priv, engine->irq_enable_mask);
 }
 
-static bool
-gen8_ring_get_irq(struct intel_engine_cs *engine)
+static void
+gen8_irq_enable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
 
-       if (WARN_ON(!intel_irqs_enabled(dev_priv)))
-               return false;
-
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (engine->irq_refcount++ == 0) {
-               if (HAS_L3_DPF(dev_priv) && engine->id == RCS) {
-                       I915_WRITE_IMR(engine,
-                                      ~(engine->irq_enable_mask |
-                                        GT_RENDER_L3_PARITY_ERROR_INTERRUPT));
-               } else {
-                       I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
-               }
-               POSTING_READ(RING_IMR(engine->mmio_base));
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
-
-       return true;
+       if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
+               I915_WRITE_IMR(engine,
+                              ~(engine->irq_enable_mask |
+                                GT_RENDER_L3_PARITY_ERROR_INTERRUPT));
+       else
+               I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
+       POSTING_READ_FW(RING_IMR(engine->mmio_base));
 }
 
 static void
-gen8_ring_put_irq(struct intel_engine_cs *engine)
+gen8_irq_disable(struct intel_engine_cs *engine)
 {
        struct drm_i915_private *dev_priv = engine->i915;
-       unsigned long flags;
 
-       spin_lock_irqsave(&dev_priv->irq_lock, flags);
-       if (--engine->irq_refcount == 0) {
-               if (HAS_L3_DPF(dev_priv) && engine->id == RCS) {
-                       I915_WRITE_IMR(engine,
-                                      ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
-               } else {
-                       I915_WRITE_IMR(engine, ~0);
-               }
-               POSTING_READ(RING_IMR(engine->mmio_base));
-       }
-       spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+       if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
+               I915_WRITE_IMR(engine,
+                              ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
+       else
+               I915_WRITE_IMR(engine, ~0);
 }
 
 static int
@@ -2909,23 +2812,23 @@ static void intel_ring_init_irq(struct drm_i915_private *dev_priv,
                                struct intel_engine_cs *engine)
 {
        if (INTEL_GEN(dev_priv) >= 8) {
-               engine->irq_get = gen8_ring_get_irq;
-               engine->irq_put = gen8_ring_put_irq;
+               engine->irq_enable = gen8_irq_enable;
+               engine->irq_disable = gen8_irq_disable;
                engine->irq_seqno_barrier = gen6_seqno_barrier;
        } else if (INTEL_GEN(dev_priv) >= 6) {
-               engine->irq_get = gen6_ring_get_irq;
-               engine->irq_put = gen6_ring_put_irq;
+               engine->irq_enable = gen6_irq_enable;
+               engine->irq_disable = gen6_irq_disable;
                engine->irq_seqno_barrier = gen6_seqno_barrier;
        } else if (INTEL_GEN(dev_priv) >= 5) {
-               engine->irq_get = gen5_ring_get_irq;
-               engine->irq_put = gen5_ring_put_irq;
+               engine->irq_enable = gen5_irq_enable;
+               engine->irq_disable = gen5_irq_disable;
                engine->irq_seqno_barrier = gen5_seqno_barrier;
        } else if (INTEL_GEN(dev_priv) >= 3) {
-               engine->irq_get = i9xx_ring_get_irq;
-               engine->irq_put = i9xx_ring_put_irq;
+               engine->irq_enable = i9xx_irq_enable;
+               engine->irq_disable = i9xx_irq_disable;
        } else {
-               engine->irq_get = i8xx_ring_get_irq;
-               engine->irq_put = i8xx_ring_put_irq;
+               engine->irq_enable = i8xx_irq_enable;
+               engine->irq_disable = i8xx_irq_disable;
        }
 }
 
@@ -3115,8 +3018,8 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
                        GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT;
        } else {
                engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
-               engine->irq_get = hsw_vebox_get_irq;
-               engine->irq_put = hsw_vebox_put_irq;
+               engine->irq_enable = hsw_vebox_irq_enable;
+               engine->irq_disable = hsw_vebox_irq_disable;
        }
 
        return intel_init_ring_buffer(dev, engine);

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index cac838384588725b2435cf8f290bd4b1483ac338..3fdf2a06e131910ed3a5d7e875eb0924903a4a68 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -189,11 +189,10 @@ struct intel_engine_cs {
        struct intel_hw_status_page status_page;
        struct i915_ctx_workarounds wa_ctx;
 
-       unsigned irq_refcount; /* protected by dev_priv->irq_lock */
        bool            irq_posted;
        u32             irq_enable_mask;        /* bitmask to enable ring interrupt */
-       bool __must_check (*irq_get)(struct intel_engine_cs *ring);
-       void            (*irq_put)(struct intel_engine_cs *ring);
+       void            (*irq_enable)(struct intel_engine_cs *ring);
+       void            (*irq_disable)(struct intel_engine_cs *ring);
 
        int             (*init_hw)(struct intel_engine_cs *ring);