Hi,

On 3/12/2018 7:41 AM, Mika Kuoppala wrote:
Interrupt identity register we already read from hardware
contains engine class and instance fields. Leverage
these fields to find correct engine to handle the interrupt.

add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-160 (-160)
Function                                     old     new   delta
gen11_irq_handler                            764     604    -160
Total: Before=1506953, After=1506793, chg -0.01%

v2: handle class/instance overflow correctly (Mika)

Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuopp...@linux.intel.com>
---
  drivers/gpu/drm/i915/i915_irq.c | 64 ++++++++++++++---------------------------
  drivers/gpu/drm/i915/i915_reg.h |  2 ++
  2 files changed, 23 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 828f3104488c..49816d0a380b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2733,47 +2733,9 @@ static void __fini_wedge(struct wedge_me *w)
             (W)->i915;                                                      \
             __fini_wedge((W)))
-static void
-gen11_gt_engine_irq_handler(struct drm_i915_private * const i915,
-                           const unsigned int bank,
-                           const unsigned int engine_n,
-                           const u16 iir)
-{
-       struct intel_engine_cs ** const engine = i915->engine;
-
-       switch (bank) {
-       case 0:
-               switch (engine_n) {
-
-               case GEN11_RCS0:
-                       return gen8_cs_irq_handler(engine[RCS], iir);
-
-               case GEN11_BCS:
-                       return gen8_cs_irq_handler(engine[BCS], iir);
-               }
-       case 1:
-               switch (engine_n) {
-
-               case GEN11_VCS(0):
-                       return gen8_cs_irq_handler(engine[_VCS(0)], iir);
-               case GEN11_VCS(1):
-                       return gen8_cs_irq_handler(engine[_VCS(1)], iir);
-               case GEN11_VCS(2):
-                       return gen8_cs_irq_handler(engine[_VCS(2)], iir);
-               case GEN11_VCS(3):
-                       return gen8_cs_irq_handler(engine[_VCS(3)], iir);
-
-               case GEN11_VECS(0):
-                       return gen8_cs_irq_handler(engine[_VECS(0)], iir);
-               case GEN11_VECS(1):
-                       return gen8_cs_irq_handler(engine[_VECS(1)], iir);
-               }
-       }
-}
-
  static u32
-gen11_gt_engine_intr(struct drm_i915_private * const i915,
-                    const unsigned int bank, const unsigned int bit)
+gen11_gt_engine_identity(struct drm_i915_private * const i915,
+                        const unsigned int bank, const unsigned int bit)
  {
        void __iomem * const regs = i915->regs;
        u32 timeout_ts;
@@ -2800,7 +2762,7 @@ gen11_gt_engine_intr(struct drm_i915_private * const i915,
        raw_reg_write(regs, GEN11_INTR_IDENTITY_REG(bank),
                      GEN11_INTR_DATA_VALID);
- return ident & GEN11_INTR_ENGINE_MASK;
+       return ident;
  }
static void
@@ -2825,12 +2787,28 @@ gen11_gt_irq_handler(struct drm_i915_private * const 
i915,
                }
for_each_set_bit(bit, &intr_dw, 32) {
-                       const u16 iir = gen11_gt_engine_intr(i915, bank, bit);
+                       const u32 ident = gen11_gt_engine_identity(i915,
+                                                                  bank, bit);
+                       const u16 iir = ident & GEN11_INTR_ENGINE_MASK;
+                       u8 class, instance;
+                       struct intel_engine_cs *engine;
if (unlikely(!iir))
                                continue;
- gen11_gt_engine_irq_handler(i915, bank, bit, iir);
+                       class = GEN11_INTR_ENGINE_CLASS(ident);
+                       if (unlikely(class >= MAX_ENGINE_CLASS))

MAX_ENGINE_CLASS points to the last CLASS, so this should be

        if (unlikely(class > MAX_ENGINE_CLASS))

as you originally had in the v1 of this patch.
(GuC interrupts will be reported with class = OTHER_CLASS).

+                               continue;
+
+                       instance = GEN11_INTR_ENGINE_INSTANCE(ident);
+                       if (unlikely(instance >= MAX_ENGINE_INSTANCE))
+                               continue;
+

Same here, MAX_ENGINE_INSTANCE is inclusive too.

That's why we have the GEM_WARN_ON's in intel_engine_setup().

+                       engine = i915->engine_class[class][instance];
+                       if (unlikely(!engine))
+                               continue;
+
+                       gen8_cs_irq_handler(engine, iir);
                }
/* Clear must be after shared has been served for engine */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e6a8c0ee7df1..065825290482 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7118,6 +7118,8 @@ enum {
  #define GEN11_INTR_IDENTITY_REG1      _MMIO(0x190064)
  #define  GEN11_INTR_DATA_VALID                (1 << 31)
  #define  GEN11_INTR_ENGINE_MASK               (0xffff)
+#define  GEN11_INTR_ENGINE_CLASS(x)    (((x) & GENMASK(18, 16)) >> 16)
+#define  GEN11_INTR_ENGINE_INSTANCE(x) (((x) & GENMASK(25, 20)) >> 20)
#define GEN11_INTR_IDENTITY_REG(x) _MMIO(0x190060 + (x * 4))
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to