[Intel-xe] [PATCH 15/26] drm/xe: Interrupts are delivered per-tile, not per-GT

[Matt Roper]

On Thu, May 11, 2023 at 05:44:18PM +0530, Iddamsetty, Aravind wrote:
> 
> 
> On 11-05-2023 09:17, Matt Roper wrote:
> > IRQ delivery and handling needs to be handled on a per-tile basis.  Note
> > that this is true even for the "GT interrupts" relating to engines and
> > GuCs --- the interrupts relating to both GTs get raised through a single
> > set of registers in the tile's sgunit range.
> > 
> > The (mis)use of struct xe_gt as a target for MMIO operations in the
> > driver makes the code somewhat confusing since we wind up needing a GT
> > pointer to handle programming that's unrelated to the GT.  To mitigate
> > this confusion, all of the xe_gt structures used solely as an MMIO
> > target in interrupt code are renamed to 'mmio.'  Reworking the driver's
> > MMIO handling to not be dependent on xe_gt is planned as a future
> > update.
> > 
> > Note that GT initialization code currently calls xe_gt_irq_postinstall()
> > in an attempt to enable the HWE interrupts for the GT being initialized.
> > Unfortunately xe_gt_irq_postinstall() doesn't really match its name and
> > does a bunch of other stuff unrelated to the GT interrupts (such as
> > enabling the top-level device interrupts).  That will be addressed in
> > future patches.
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_gt.c  |   2 +-
> >  drivers/gpu/drm/xe/xe_irq.c | 334 ++++++++++++++++++++----------------
> >  drivers/gpu/drm/xe/xe_irq.h |   4 +-
> >  3 files changed, 187 insertions(+), 153 deletions(-)
> > 
> 
> 
> <snip>
> >  static u32 dg1_intr_disable(struct xe_device *xe)
> >  {
> > -	struct xe_gt *gt = xe_primary_mmio_gt(xe);
> > +	struct xe_gt *mmio = xe_primary_mmio_gt(xe);
> >  	u32 val;
> >  
> >  	/* First disable interrupts */
> > -	xe_mmio_write32(gt, DG1_MSTR_TILE_INTR, 0);
> > +	xe_mmio_write32(mmio, DG1_MSTR_TILE_INTR, 0);
> 
> if sgunit registers are replicated per tile, but why DG1_MSR_TILE_INTR
> is handled on primary tile only, is this not a sgunit register?

Right, this is an sgunit register, so there's technically a copy at each
tile.  However this specific register is the one responsible for showing
the interrupts that were forwarded to the root tile from the remote
tiles, and for enabling/disabling reporting of interrupts to the OS, so
only the copy in the root tile is relevant.  I should probably add some
additional explanation of that in the commit message.


Matt

> 
> >  
> >  	/* Get the indication levels and ack the master unit */
> > -	val = xe_mmio_read32(gt, DG1_MSTR_TILE_INTR);
> > +	val = xe_mmio_read32(mmio, DG1_MSTR_TILE_INTR);
> >  	if (unlikely(!val))
> >  		return 0;
> >  
> > -	xe_mmio_write32(gt, DG1_MSTR_TILE_INTR, val);
> > +	xe_mmio_write32(mmio, DG1_MSTR_TILE_INTR, val);
> >  
> >  	return val;
> >  }
> >  
> >  static void dg1_intr_enable(struct xe_device *xe, bool stall)
> >  {
> > -	struct xe_gt *gt = xe_primary_mmio_gt(xe);
> > +	struct xe_gt *mmio = xe_primary_mmio_gt(xe);
> >  
> > -	xe_mmio_write32(gt, DG1_MSTR_TILE_INTR, DG1_MSTR_IRQ);
> > +	xe_mmio_write32(mmio, DG1_MSTR_TILE_INTR, DG1_MSTR_IRQ);
> >  	if (stall)
> > -		xe_mmio_read32(gt, DG1_MSTR_TILE_INTR);
> > +		xe_mmio_read32(mmio, DG1_MSTR_TILE_INTR);
> >  }
> >
> 
> Thanks,
> Aravind.
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation