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 -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795
