On Thu, Oct 28, 2021 at 06:30:09PM +0200, Andi Shyti wrote:
Hi Paulo and Matt,
[...]
@@ -3190,14 +3190,19 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv)
mmmhhh... bad naming :/
Even though dg1 wasn't a multi-tile platform, it was the platform that introduced the singleton "master tile interrupt" register that is responsible for telling us which tile(s) had interrupts; we then proceed to read the per-tile master register to find out what those interrupts are. So I think the name is accurate since the hardware introduced the extra level of indirection, and we do need to use this handler on DG1 (we'll just never have more than a single GT to loop over in that case).
[...]
- dg1_master_intr_enable(uncore->regs);
- intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR);
- dg1_master_intr_enable(dev_priv->gt.uncore->regs);
- intel_uncore_posting_read(dev_priv->gt.uncore, DG1_MSTR_TILE_INTR);
I guess this should also go under a for_each_gt()
DG1_MSTR_TILE_INTR (0x190008) is the top-level, one-per-PCI device interrupt register; we always access it via tile0's MMIO . So in this case we do want to do this outside the loop since it's not a per-tile operation.
We could probably simplify the dev_priv->gt.uncore parameter to just dev_priv->uncore to make this more obvious.
Matt
Andi