Re: [PATCH 20/23] drm/xe/xe3p_xpc: Add support for compute walker for non-MSIx

[Matt Roper <matthew.d.roper@intel.com>]

On Wed, Oct 15, 2025 at 12:07:47PM -0500, Lucas De Marchi wrote:
> On Tue, Oct 14, 2025 at 11:07:05AM -0700, Matt Roper wrote:
> > On Mon, Oct 13, 2025 at 08:24:52PM -0700, Lucas De Marchi wrote:
> > > From: S A Muqthyar Ahmed <syed.abdul.muqthyar.ahmed@intel.com>
> > > 
> > > Current implementation of compute walker has dependency on GPU/SW Stack
> > > which requires SW/UMD to wait for event from KMD to indicate
> > > PIPE_CONTROL interrup was done. This created latency on SW stack.
> > > 
> > > This feature adds support to generate completion interrupt from GPGPU
> > > walker which does not support MSIx and avoid software using Pipe control
> > > drain/idle latency.
> > > 
> > > Suggested-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > > Signed-off-by: S A Muqthyar Ahmed <syed.abdul.muqthyar.ahmed@intel.com>
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/regs/xe_irq_regs.h | 1 +
> > >  drivers/gpu/drm/xe/xe_irq.c           | 4 ++++
> > >  2 files changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/regs/xe_irq_regs.h b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> > > index 7c2a3a1401424..9c46b5fb81412 100644
> > > --- a/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> > > +++ b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> > > @@ -82,6 +82,7 @@
> > >  #define   GSC_ER_COMPLETE			REG_BIT(5)
> > >  #define   GT_RENDER_PIPECTL_NOTIFY_INTERRUPT	REG_BIT(4)
> > >  #define   GT_CS_MASTER_ERROR_INTERRUPT		REG_BIT(3)
> > > +#define   GT_COMPUTE_WALKER_INTERRUPT		REG_BIT(2)
> > >  #define   GT_RENDER_USER_INTERRUPT		REG_BIT(0)
> > > 
> > >  /* irqs for OTHER_KCR_INSTANCE */
> > > diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> > > index af519414a4297..e01b158895342 100644
> > > --- a/drivers/gpu/drm/xe/xe_irq.c
> > > +++ b/drivers/gpu/drm/xe/xe_irq.c
> > > @@ -150,6 +150,10 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
> > >  	if (xe_device_uc_enabled(xe)) {
> > >  		irqs = GT_RENDER_USER_INTERRUPT |
> > >  			GT_RENDER_PIPECTL_NOTIFY_INTERRUPT;
> > > +
> > > +		/* Enable Compute Walker Interrupt for non-MSIX platforms */
> > > +		if (GRAPHICS_VERx100(xe) >= 3511 && !xe_device_has_msix(xe))
> > > +			irqs |= GT_COMPUTE_WALKER_INTERRUPT;
> > 
> > This bit only exists in the compute engine interrupt vector (bspec
> > 62346) and render engine interrupt vector (bspec 62353).  It does not
> > exist in the blitter (62345), vcs (62354), vecs (62355), or gsc (63341),
> > which makes sense because none of those engines can run compute walkers.
> > Should we be making sure we only try to unmask and enable this interrupt
> > on supported engine types?
> 
> right... but then we also have similar issues about being lazy with
> other bits as well. Checking the other interrupt vector structures, bit
> 2 is not defined for them.
> 
> I'm leaning towards we are being lazy in the programming here and this
> is not currently a problem. Then we can refactor this on top to stop
> being lazy and make the intention of the code clearer (e.g. I don't
> really like that use of smask/dmask because engines are bundled together
> in a single register).

My main worry here is that since different engines do explicitly have
different interrupt vectors, there's more potential for a future
platform to use this "unused" bit for a completely different type of
interrupt on blitter, media, etc.  That would be easy for us to overlook
during early platform bringup.

It's less likely that the enable/mask bits for an engine (for example,
BCS6) would get repurposed on a platform that doesn't have that engine,
because the engine still has reserved register ranges and such in the
architecture, even for individual platforms that don't use it.


Matt

> 
> Lucas De Marchi
> 
> > 
> > 
> > Matt
> > 
> > >  	} else {
> > >  		irqs = GT_RENDER_USER_INTERRUPT |
> > >  		       GT_CS_MASTER_ERROR_INTERRUPT |
> > > 
> > > --
> > > 2.51.0
> > > 
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation

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