[PATCH] lib/xe/xe_spin: Add switch point for preemptible spinner

[Matthew Brost]

On Wed, Aug 13, 2025 at 07:51:31PM +0200, Zbigniew Kempczyński wrote:
> On Wed, Aug 13, 2025 at 03:59:36PM +0000, Jonathan Cavitt wrote:
> > From: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
> > 
> > When 'preempt' option is specified, insert a MI_SEMAPHORE_WAIT
> > command in the spinner batch buffer. This provides the switch
> > point for preemption to happen in multi queue mode.
> > 
> > Additionally, allow the user to control the preempt wait condition.
> > 
> > Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/5783
> > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
> > Cc: Matthew Brost <matthew.brost at intel.com>
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> > ---
> >  include/intel_gpu_commands.h |  1 +
> >  lib/xe/xe_spin.c             | 42 +++++++++++++++++++++++++++++++++++-
> >  lib/xe/xe_spin.h             |  3 +++
> >  3 files changed, 45 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/intel_gpu_commands.h b/include/intel_gpu_commands.h
> > index cd281ba89d..5158bb0ea0 100644
> > --- a/include/intel_gpu_commands.h
> > +++ b/include/intel_gpu_commands.h
> > @@ -123,6 +123,7 @@
> >  #define MI_SEMAPHORE_WAIT	MI_INSTR(0x1c, 2) /* GEN8+ */
> >  #define MI_SEMAPHORE_WAIT_TOKEN	MI_INSTR(0x1c, 3) /* GEN12+ */
> >  #define   MI_SEMAPHORE_POLL		(1 << 15)
> > +#define   MI_SEMAPHORE_QUEUE_SWITCH_MODE	(1 << 11)
> >  #define   MI_SEMAPHORE_SAD_GT_SDD	(0 << 12)
> >  #define   MI_SEMAPHORE_SAD_GTE_SDD	(1 << 12)
> >  #define   MI_SEMAPHORE_SAD_LT_SDD	(2 << 12)
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> > index 25f7555e27..d62952b539 100644
> > --- a/lib/xe/xe_spin.c
> > +++ b/lib/xe/xe_spin.c
> > @@ -30,7 +30,6 @@
> >  
> >  enum { START_TS, NOW_TS };
> >  
> > -
> >  uint32_t xe_spin_nsec_to_ticks(int fd, int gt_id, uint64_t nsec)
> >  {
> >  	uint32_t ticks = xe_nsec_to_ticks(fd, gt_id, nsec);
> > @@ -57,6 +56,7 @@ void xe_spin_init(struct xe_spin *spin, struct xe_spin_opts *opts)
> >  
> >  	spin->start = 0;
> >  	spin->end = 0xffffffff;
> > +	spin->wait_cond = 0;
> >  	spin->ticks_delta = 0;
> >  
> >  	if (opts->ctx_ticks) {
> > @@ -167,6 +167,24 @@ void xe_spin_init(struct xe_spin *spin, struct xe_spin_opts *opts)
> >  				   opts->mem_copy->dst->mocs_index;
> >  	}
> >  
> > +	/*
> > +	 * Insert a MI_SEMAPHORE_WAIT_CMD instruction with condition controlled
> > +	 * by the user. It also acts as a queue switch point in multi queue mode.
> > +	 */
> > +	if (opts->preempt) {
> > +		uint64_t wait_addr = opts->addr + offsetof(struct xe_spin, wait_cond);
> > +
> > +		spin->batch[b++] = MI_SEMAPHORE_WAIT_CMD |
> > +				MI_SEMAPHORE_POLL |
> > +				MI_SEMAPHORE_QUEUE_SWITCH_MODE |
> 
> This bit is not documented for previous platforms so is it safe to
> set it?
> 

Yes, I'm looking at bspec 45749 and not seeing bit 11
(MI_SEMAPHORE_QUEUE_SWITCH_MODE) defined. Also I though SEMAPHOREs were
always preemptable too. Got a bspec link to exactly what this new bits
does?

> BTW similar effect was adding MI_ARB_CHECK (in term test is also
> passing here).
> 

Yes, arb checks are also preempt points too.

Also in general I'm just confused by how xe_spin_preempt_wait / nowait
functions would be used in IGTs. Can you explain?

Matt

> If my nit is not applying here, code lgtm:
> 
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> 
> --
> Zbigniew
> 
> > +				MI_SEMAPHORE_SAD_EQ_SDD |
> > +				3;
> > +		spin->batch[b++] = 0;
> > +		spin->batch[b++] = wait_addr;
> > +		spin->batch[b++] = wait_addr >> 32;
> > +		spin->batch[b++] = 0;
> > +	}
> > +
> >  	spin->batch[b++] = MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE | 2;
> >  	spin->batch[b++] = 0;
> >  	spin->batch[b++] = end_addr;
> > @@ -207,6 +225,28 @@ void xe_spin_end(struct xe_spin *spin)
> >  	WRITE_ONCE(spin->end, 0);
> >  }
> >  
> > +/**
> > + * xe_spin_preempt_wait:
> > + * @spin: pointer to spinner mapped bo
> > + *
> > + * Make the spinner wait on the preemption semaphore.
> > + */
> > +void xe_spin_preempt_wait(struct xe_spin *spin)
> > +{
> > +	spin->wait_cond = 1;
> > +}
> > +
> > +/**
> > + * xe_spin_preempt_nowait:
> > + * @spin: pointer to spinner mapped bo
> > + *
> > + * Make the spinner do not wait on the preemption semaphore.
> > + */
> > +void xe_spin_preempt_nowait(struct xe_spin *spin)
> > +{
> > +	spin->wait_cond = 0;
> > +}
> > +
> >  /**
> >   * xe_spin_create:
> >   * @opt: controlling options such as allocator handle, exec_queue, vm etc
> > diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h
> > index c2199d0aa2..eaf2a409e0 100644
> > --- a/lib/xe/xe_spin.h
> > +++ b/lib/xe/xe_spin.h
> > @@ -64,6 +64,7 @@ struct xe_spin {
> >  	uint64_t pad;
> >  	uint32_t start;
> >  	uint32_t end;
> > +	uint32_t wait_cond;
> >  	uint32_t ticks_delta;
> >  	uint64_t exec_sync;
> >  	uint32_t timestamp;
> > @@ -76,6 +77,8 @@ void xe_spin_init(struct xe_spin *spin, struct xe_spin_opts *opts);
> >  bool xe_spin_started(struct xe_spin *spin);
> >  void xe_spin_wait_started(struct xe_spin *spin);
> >  void xe_spin_end(struct xe_spin *spin);
> > +void xe_spin_preempt_wait(struct xe_spin *spin);
> > +void xe_spin_preempt_nowait(struct xe_spin *spin);
> >  
> >  /*
> >   * xe_cork: higher level API that simplifies exec'ing an xe_spin by taking care
> > -- 
> > 2.43.0
> >