On 12/03/18 08:23, Chris Wilson wrote:
Quoting Daniele Ceraolo Spurio (2018-03-12 15:09:31)

On 12/03/18 07:52, Chris Wilson wrote:
Quoting Mika Kuoppala (2018-03-12 14:41:31)
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>
   static void
@@ -2825,12 +2787,28 @@ gen11_gt_irq_handler(struct drm_i915_private * const 
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))

Now if (!ident) or actually just use u32 iir as we can pass it straight
through to cs_irq_handler.

Can't use (!ident) here because bit 31 (iir valid) or the class/instance
bits might be set when the iir is empty (because we had a buffered irq
that we actually already handled).

If there's a valid bit, surely that's the one we want to be testing?

We do already test the valid bit in gen11_gt_engine_intr and return zero from that function if the bit doesn't go to 1 in a reasonable time.

If the low iir bits are 0, that's fine. The question is whether it's
common enough to worry about; and I note it's marked as unlikely() so
it seems like we can just let it fallthrough and do nothing.

Difficult to say how common it is going to be yet. If my reading of the specs is correct, double buffering is only active between reading GEN11_GT_INTR_DW and clearing it, so the iir might be empty if the second interrupt lands between reading GEN11_GT_INTR_DW and reading the iir (in which case both events will be handled in the first round). I'm ok with just falling through for now and we can revisit once we have better data.

Intel-gfx mailing list

Reply via email to