Re: [PATCH v3 03/25] drm/xe: Decouple exec queue idle check from LRC

[Matthew Brost <matthew.brost@intel.com>]

On Mon, Mar 02, 2026 at 01:50:11PM -0700, Summers, Stuart wrote:
> On Fri, 2026-02-27 at 17:34 -0800, Matthew Brost wrote:
> > We already maintain a job count for each exec queue, so simplify the
> > idle
> > check to rely on the job count rather than the LRC state. This
> > decouples
> > exec queues from LRC-based backends and avoids unnecessarily coupling
> > idle
> > detection to backend-specific implementation details.
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_exec_queue.c | 15 +--------------
> >  1 file changed, 1 insertion(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c
> > b/drivers/gpu/drm/xe/xe_exec_queue.c
> > index 2d0e73a6a6ee..b3f700a9d425 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > @@ -1382,20 +1382,7 @@ bool xe_exec_queue_is_lr(struct xe_exec_queue
> > *q)
> >   */
> >  bool xe_exec_queue_is_idle(struct xe_exec_queue *q)
> >  {
> > -       if (xe_exec_queue_is_parallel(q)) {
> > -               int i;
> > -
> > -               for (i = 0; i < q->width; ++i) {
> > -                       if (xe_lrc_seqno(q->lrc[i]) !=
> > -                           q->lrc[i]->fence_ctx.next_seqno - 1)
> > -                               return false;
> > -               }
> > -
> > -               return true;
> > -       }
> > -
> > -       return xe_lrc_seqno(q->lrc[0]) ==
> > -               q->lrc[0]->fence_ctx.next_seqno - 1;
> > +       return !atomic_read(&q->job_cnt);
> 
> Still looking through the series, so might be handled elsewhere, but
> just looking at this patch alone, I'm a little worried this will cause
> unexpected issues in the exec queue cleanup. This function currently
> ensures that the job is idle from the hardware level. The change you

The current check is actually incorrect if, for example, a queue is
reset and the LRC head != tail. However, I believe the only places we
use xe_exec_queue_is_idle are cases where a queue hasn’t been reset, so
it happens to work in practice. It’s also just an advisory check, so
nothing bad happens if it incorrectly reports “not idle".

> make here moves that to a software level check. And this is getting
> decremented and checked before we tear down the exec queue. So
> presumably, GuC and the command streamer could still be doing something
> here and we're falsely telling other parts of the driver that rely on
> the engine to really be idle to trust us here.
> 

See above for part of the explanation, but the other part involves
reference counting and fence signaling. A job can only have its last
reference dropped when its fence is signaled.

A fence can only signal under the following conditions:

- Its seqno is incremented via ring instructions (which corresponds to
  the LRC head == tail if it’s the last job on the queue).
- We time out jobs on the queue and signal their fences in software. We
  only signal fences in software once the queue has been kicked off the
  hardware (i.e., scheduling-disable H2G triggers a G2H response).

> For reference, I'm looking at xe_sched_job_destroy() where we do the
> decrement and then the exec queue put.
> 
> So my question is, how are we guaranteeing that hardware is indeed idle
> after this change? Are we moving the sequence number check somewhere
> else?
> 

I think above explains this.

Matt

> Thanks,
> Stuart
> 
> >  }
> >  
> >  /**
>