Re: [PATCH v4 10/12] drm/xe: Add exec queue active vfunc

["Summers, Stuart" <stuart.summers@intel.com>]

On Mon, 2026-01-12 at 18:52 -0800, Matthew Brost wrote:
> If an exec queue is inactive (e.g., not registered or scheduling is
> disabled), TLB invalidations are not issued for that queue. Add a
> virtual function to determine the active state, which TLB
> invalidation
> logic can hook into.
> 
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_exec_queue_types.h | 2 ++
>  drivers/gpu/drm/xe/xe_execlist.c         | 7 +++++++
>  drivers/gpu/drm/xe/xe_guc_submit.c       | 6 ++++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h
> b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> index d3e2789cf5bc..9cca558c5809 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue_types.h
> +++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h
> @@ -298,6 +298,8 @@ struct xe_exec_queue_ops {
>         void (*resume)(struct xe_exec_queue *q);
>         /** @reset_status: check exec queue reset status */
>         bool (*reset_status)(struct xe_exec_queue *q);
> +       /** @active: check exec queue is active */
> +       bool (*active)(struct xe_exec_queue *q);
>  };
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_execlist.c
> b/drivers/gpu/drm/xe/xe_execlist.c
> index 46c17a18a3f4..e4a2674f48c7 100644
> --- a/drivers/gpu/drm/xe/xe_execlist.c
> +++ b/drivers/gpu/drm/xe/xe_execlist.c
> @@ -469,6 +469,12 @@ static bool
> execlist_exec_queue_reset_status(struct xe_exec_queue *q)
>         return false;
>  }
>  
> +static bool execlist_exec_queue_active(struct xe_exec_queue *q)
> +{
> +       /* NIY */
> +       return false;
> +}
> +
>  static const struct xe_exec_queue_ops execlist_exec_queue_ops = {
>         .init = execlist_exec_queue_init,
>         .kill = execlist_exec_queue_kill,
> @@ -481,6 +487,7 @@ static const struct xe_exec_queue_ops
> execlist_exec_queue_ops = {
>         .suspend_wait = execlist_exec_queue_suspend_wait,
>         .resume = execlist_exec_queue_resume,
>         .reset_status = execlist_exec_queue_reset_status,
> +       .active = execlist_exec_queue_active,
>  };
>  
>  int xe_execlist_init(struct xe_gt *gt)
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
> b/drivers/gpu/drm/xe/xe_guc_submit.c
> index be8fa76baf1d..19282e5ca897 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -2203,6 +2203,11 @@ static bool guc_exec_queue_reset_status(struct
> xe_exec_queue *q)
>         return exec_queue_reset(q) ||
> exec_queue_killed_or_banned_or_wedged(q);
>  }
>  
> +static bool guc_exec_queue_active(struct xe_exec_queue *q)
> +{
> +       return exec_queue_enabled(q) &&
> !exec_queue_pending_disable(q);

So I think the requirement here is !exec_queue_pending_disable &&
!exec_queue_destroyed rather than exec_queue_enabled... But if the
context isn't enabled (set runnable to true in GuC), it means hardware
shouldn't be touching any of the associated memory anyway and we
shouldn't be issuing any invalidations, so this still looks ok to me.
It means we'll be doing full invalidations for those I think though
which could add some performance impact in the time between register
and enable. For now though as long as this is working (still doing some
testing on my end and I had some questions in a later patch), we can go
with this.

Thanks,
Stuart

> +}
> +
>  /*
>   * All of these functions are an abstraction layer which other parts
> of Xe can
>   * use to trap into the GuC backend. All of these functions, aside
> from init,
> @@ -2222,6 +2227,7 @@ static const struct xe_exec_queue_ops
> guc_exec_queue_ops = {
>         .suspend_wait = guc_exec_queue_suspend_wait,
>         .resume = guc_exec_queue_resume,
>         .reset_status = guc_exec_queue_reset_status,
> +       .active = guc_exec_queue_active,
>  };
>  
>  static void guc_exec_queue_stop(struct xe_guc *guc, struct
> xe_exec_queue *q)