<snip>

+
+static void
+gen11_gt_irq_handler(struct drm_i915_private * const i915,
+                    const u32 master_ctl)
+{
+       void __iomem * const regs = i915->regs;
+       unsigned int bank;
+
+       for (bank = 0; bank < 2; bank++) {
+               unsigned long intr_dw;
+               unsigned int bit;
+
+               if (!(master_ctl & GEN11_GT_DW_IRQ(bank)))
+                       continue;
+
+               intr_dw = readl(regs +
+                               i915_mmio_reg_offset(GEN11_GT_INTR_DW(bank)));
+


I think we should keep some kind of polling here, because there is going to be some delay between writing the selector and getting the correct value in the identity register and the CPU might also be running at a higher frequency than the GPU. Spec does not specify a time but implies that we have to wait for the valid bit to be set.

+               if (unlikely(!intr_dw))
+                       DRM_ERROR("GT_INTR_DW%u blank!\n", bank);
+
+               for_each_set_bit(bit, &intr_dw, 32) {
+                       const u16 iir = gen11_gt_engine_intr(i915, bank, bit);
+
+                       if (unlikely(!iir))
+                               continue;
+
+                       gen11_gt_engine_irq_handler(i915, bank, bit, iir);

The identity register contains the class (bits 16-18) and instance (bits 20-25), so we can potentially decode them and call directly:

        gen11_cs_irq_handler(i915->engine_class[class][instance], iir);

instead of having a big switch. The only problem with that would be that GuC and PM interrupts come out as class 4 instances 0 and 1, so once we add support for those we'd have to do something like

        if (class != OTHER_CLASS)
                gen11_cs_irq_handler(...);
        else {
                switch (instance) {
                case 0:
                        gen11_guc_irq_handler(...);
                        break;
                case 1:
                        gen11_rps_irq_handler(...);
                        break;
                default:
                        MISSING_CASE();
                }
        }

not sure if decoding class and instance and having a small switch wins in the end against the big switch, so I won't complain whichever you pick ;)

Daniele

+               }
+
+               /* Clear must be after shared has been served for engine */
+               writel(intr_dw,
+                      regs + i915_mmio_reg_offset(GEN11_GT_INTR_DW(bank)));
+       }
+}
+
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to