Re: [PATCH 1/3] lib/xe/xe_spin: Enhance multi-queue switch option

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

On Wed, 2026-05-06 at 11:31 -0700, Niranjana Vishwanathapura wrote:
> From: gta <gta@DUT4637NVLP.fm.intel.com>
> 
> Allow user to control whether the multi-queue switch should
> happen after parsing the MI_SEMAPHORE_WAIT instruction or
> only if the instruction is unsuccessful in getting the
> semaphore, thus having to wait.
> 
> Ensure that in xe_exec_multi_queue@priority test, the multi-queue
> switch happens only when the spinner has to wait for the semaphore.
> 
> Signed-off-by: Niranjana Vishwanathapura
> <niranjana.vishwanathapura@intel.com>
> ---
>  lib/xe/xe_spin.c                  | 13 +++++++------
>  lib/xe/xe_spin.h                  |  6 +++++-
>  tests/intel/xe_exec_multi_queue.c |  2 +-
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 4dc110c222..14952ca90e 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -179,14 +179,15 @@ void xe_spin_init(struct xe_spin *spin, struct
> xe_spin_opts *opts)
>          * Insert a MI_SEMAPHORE_WAIT_CMD instruction with condition
> controlled
>          * by the user which acts as a queue switch point in multi
> queue mode.
>          */
> -       if (opts->multi_queue_switch) {
> +       if (opts->multi_queue_switch || opts-
> >multi_queue_switch_on_wait) {

I don't have any problem with the way you're doing this. I can see we
might want to use this without multi queue though at some point, so it
might be nice to have the switch_on_wait as an option on top of
multi_queue_switch - basically rename multi_queue_switch to
semaphore_wait or something and then add the multi_queue_switch_on_wait
as an additional option. But that also means we have to keep track of a
separate parameter in xe_spin_init_opts() and since multi queue is the
only user right now, it might be overkill.

Otherwise LGTM. Please apply the author related changes Xin had
requested before merging:
Reviewed-by: Stuart Summers <stuart.summers@intel.com>

>                 uint64_t wait_addr = opts->addr + offsetof(struct
> xe_spin, wait_cond);
> +               uint32_t sema_cmd = MI_SEMAPHORE_WAIT_CMD |
> MI_SEMAPHORE_POLL |
> +                                   MI_SEMAPHORE_SAD_EQ_SDD | 3;
>  
> -               spin->batch[b++] = MI_SEMAPHORE_WAIT_CMD |
> -                               MI_SEMAPHORE_POLL |
> -                               MI_SEMAPHORE_QUEUE_SWITCH_MODE |
> -                               MI_SEMAPHORE_SAD_EQ_SDD |
> -                               3;
> +               if (opts->multi_queue_switch_on_wait)
> +                       sema_cmd |= MI_SEMAPHORE_QUEUE_SWITCH_MODE;
> +
> +               spin->batch[b++] = sema_cmd;
>                 spin->batch[b++] = 0;
>                 spin->batch[b++] = wait_addr;
>                 spin->batch[b++] = wait_addr >> 32;
> diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h
> index 31154997b9..db0febd8ab 100644
> --- a/lib/xe/xe_spin.h
> +++ b/lib/xe/xe_spin.h
> @@ -46,7 +46,10 @@ struct xe_spin_mem_copy {
>   * struct xe_spin_opts
>   * @addr: offset of spinner within vm
>   * @preempt: allow spinner to be preempted or not
> - * @multi_queue_switch: Add a multi-queue switch point
> + * @multi_queue_switch: Add a SEMAPHORE_WAIT multi-queue switch
> point
> + * and have the queue switch happen after command is parsed.
> + * @multi_queue_switch_on_wait: Add a SEMAPHORE_WAIT multi-queue
> switch point
> + * and have the queue switch only happen if waiting on the
> semaphore.
>   * @ctx_ticks: number of ticks after which spinner is stopped,
> applied if > 0
>   * @mem_copy: container of objects used for memory copy (optional)
>   *
> @@ -56,6 +59,7 @@ struct xe_spin_opts {
>         uint64_t addr;
>         bool preempt;
>         bool multi_queue_switch;
> +       bool multi_queue_switch_on_wait;
>         uint32_t ctx_ticks;
>         bool write_timestamp;
>         struct xe_spin_mem_copy *mem_copy;
> diff --git a/tests/intel/xe_exec_multi_queue.c
> b/tests/intel/xe_exec_multi_queue.c
> index 0479554bb6..6f90c3e4e8 100644
> --- a/tests/intel/xe_exec_multi_queue.c
> +++ b/tests/intel/xe_exec_multi_queue.c
> @@ -459,7 +459,7 @@ __test_priority(int fd, struct
> drm_xe_engine_class_instance *eci,
>         for (i = 0; i < num_queues; i++) {
>                 uint64_t spin_addr = addr + i * sizeof(struct
> xe_spin);
>  
> -               xe_spin_init_opts(spin[i], .addr = spin_addr,
> .multi_queue_switch = true,
> +               xe_spin_init_opts(spin[i], .addr = spin_addr,
> .multi_queue_switch_on_wait = true,
>                                   .write_timestamp = true);
>                 /*
>                  * Pre-set all spinners to preempt-wait so each
> queue, once