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

Summers, Stuart
2026-03-02T20:50:20Z

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
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.

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?

Thanks,
Stuart

>  }
>
>  /**