[PATCH] lib/xe/xe_spin: Add additional MI_ARB_CHECK to preemptable spin init

[Cavitt, Jonathan]

-----Original Message-----
From: Brost, Matthew <matthew.brost at intel.com> 
Sent: Thursday, August 14, 2025 12:05 PM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
Cc: igt-dev at lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta at intel.com>; Zuo, Alex <alex.zuo at intel.com>; Kempczynski, Zbigniew <zbigniew.kempczynski at intel.com>; Vishwanathapura, Niranjana <niranjana.vishwanathapura at intel.com>
Subject: Re: [PATCH] lib/xe/xe_spin: Add additional MI_ARB_CHECK to preemptable spin init
> 
> On Thu, Aug 14, 2025 at 06:50:10PM +0000, Jonathan Cavitt wrote:
> > Add an additional MI_ARB_CHECK to the spin initialization function,
> > just before the conditional batch buffer end, when preemption is
> > enabled.  The MI_ARB_CHECK forces a preemption in this case, which can
> > allow additionally scheduled exec queues to make independent progress
> > earlier than if we relied on the automatic timeout of the exec queue.
> > 
> > Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/5783
> > Suggested-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> > Cc: Matthew Brost <matthew.brost at intel.com>
> > Cc: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
> > ---
> >  lib/xe/xe_spin.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> > index 25f7555e27..b3a937fe80 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);
> > @@ -167,6 +166,9 @@ void xe_spin_init(struct xe_spin *spin, struct xe_spin_opts *opts)
> >  				   opts->mem_copy->dst->mocs_index;
> >  	}
> >  
> > +	if (opts->preempt)
> > +		spin->batch[b++] = MI_ARB_CHECK;
> 
> But we already have one these at the top of the loop:
> 
>  72         loop_addr = opts->addr + b * sizeof(uint32_t);
>  73
>  74         spin->batch[b++] = MI_STORE_DWORD_IMM_GEN4;
>  75         spin->batch[b++] = start_addr;
>  76         spin->batch[b++] = start_addr >> 32;
>  77         spin->batch[b++] = 0xc0ffee;
>  78
>  79         if (opts->preempt)
>  80                 spin->batch[b++] = MI_ARB_CHECK;
> 
> Line 74 is here the ring loop restarts.

I tried removing the top MI_ARB_CHECK, but that caused the test to start
failing again, so it looks like both MI_ARB_CHECK calls are necessary.
-Jonathan Cavitt

> 
> Matt
> 
> > +
> >  	spin->batch[b++] = MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE | 2;
> >  	spin->batch[b++] = 0;
> >  	spin->batch[b++] = end_addr;
> > -- 
> > 2.43.0
> > 
>