[PATCH] lib/xe/xe_spin: Add switch point for preemptible spinner
Cavitt, Jonathan
jonathan.cavitt at intel.com
Thu Aug 14 17:42:04 UTC 2025
-----Original Message-----
From: Brost, Matthew <matthew.brost at intel.com>
Sent: Thursday, August 14, 2025 10:24 AM
To: Kempczynski, Zbigniew <zbigniew.kempczynski at intel.com>
Cc: Cavitt, Jonathan <jonathan.cavitt at intel.com>; igt-dev at lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta at intel.com>; Zuo, Alex <alex.zuo at intel.com>; Vishwanathapura, Niranjana <niranjana.vishwanathapura at intel.com>
Subject: Re: [PATCH] lib/xe/xe_spin: Add switch point for preemptible spinner
>
> 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?
Try Bspec 60244
-Jonathan Cavitt
>
> > 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
> > >
>
More information about the igt-dev
mailing list