Re: [PATCH 2/3] lib/xe/xe_spin: Add option for QUEUE_TIMESTAMP

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Thu, May 07, 2026 at 12:41:37PM -0700, Umesh Nerlige Ramappa wrote:
>On Thu, May 07, 2026 at 07:23:50PM +0000, Summers, Stuart wrote:
>>On Wed, 2026-05-06 at 11:31 -0700, Niranjana Vishwanathapura wrote:
>>>From: gta <gta@DUT4637NVLP.fm.intel.com>
>>>
>>>In multi-queue case, CTX_TIMESTAMP register does not
>>>provide the running time of individual queues of a group.
>>>Provide the option for spinner to read timestamp from the
>>>QUEUE_TIMESTAMP registers instead which is useful in the
>>>multi-queue scenarios.
>>>
>>>Signed-off-by: Niranjana Vishwanathapura
>>><niranjana.vishwanathapura@intel.com>
>>>---
>>> lib/xe/xe_spin.c | 9 +++++----
>>> lib/xe/xe_spin.h | 2 ++
>>> 2 files changed, 7 insertions(+), 4 deletions(-)
>>>
>>>diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
>>>index 14952ca90e..8743107898 100644
>>>--- a/lib/xe/xe_spin.c
>>>+++ b/lib/xe/xe_spin.c
>>>@@ -25,7 +25,8 @@
>>> #define MI_LRI_CS_MMIO                         (1 << 19)
>>> #define MI_LRR_DST_CS_MMIO                     (1 << 19)
>>> #define MI_LRR_SRC_CS_MMIO                     (1 << 18)
>>>-#define CTX_TIMESTAMP 0x3a8
>>>+#define CTX_TIMESTAMP          0x3a8
>>>+#define QUEUE_TIMESTAMP                0x4c0
>>> #define CS_GPR(x) (0x600 + 8 * (x))
>>> 
>>> enum { START_TS, NOW_TS };
>>>@@ -67,7 +68,7 @@ void xe_spin_init(struct xe_spin *spin, struct
>>>xe_spin_opts *opts)
>>>                spin->batch[b++] = CS_GPR(START_TS) + 4;
>>>                spin->batch[b++] = 0;
>>>                spin->batch[b++] = MI_LOAD_REGISTER_REG |
>>>MI_LRR_DST_CS_MMIO | MI_LRR_SRC_CS_MMIO;
>>>-               spin->batch[b++] = CTX_TIMESTAMP;
>>>+               spin->batch[b++] = opts->use_queue_timestamp ?
>>>QUEUE_TIMESTAMP : CTX_TIMESTAMP;
>>
>>I see in [1] we are programming this register in the ring also. Will
>>this cause issues if we overwrite this here? I see we do the same thing
>>for the context timestamp, so probably no issue here. But it would be
>>nice to document somehow the expected interaction between the kernel
>>ring programming and the user batch programming if any.
>>
>>[1]:
>>https://patchwork.freedesktop.org/patch/723502/?series=164654&rev=5
>
>The MI_LOAD_REGISTER_REG source is TIMESTAMP register and destination 
>is the GPR reg, so we should be fine. If we were writing to the 
>TIMESTAMP registers, then that's going to mess up counts.
>

Yes. Note that we program the batch buffer to only to read from this
register here. KMD whitelists these registers with only read permission. 
We are not changing any of that. Only using different timestamp resgister.

Niranjana

>Regards,
>Umesh
>
>
>>
>>>                spin->batch[b++] = CS_GPR(START_TS);
>>>        }
>>> 
>>>@@ -83,7 +84,7 @@ void xe_spin_init(struct xe_spin *spin, struct
>>>xe_spin_opts *opts)
>>> 
>>>        if (opts->write_timestamp) {
>>>                spin->batch[b++] = MI_LOAD_REGISTER_REG |
>>>MI_LRR_DST_CS_MMIO | MI_LRR_SRC_CS_MMIO;
>>>-               spin->batch[b++] = CTX_TIMESTAMP;
>>>+               spin->batch[b++] = opts->use_queue_timestamp ?
>>>QUEUE_TIMESTAMP : CTX_TIMESTAMP;
>>>                spin->batch[b++] = CS_GPR(NOW_TS);
>>> 
>>>                spin->batch[b++] = MI_STORE_REGISTER_MEM_GEN8 |
>>>MI_SRM_CS_MMIO;
>>>@@ -97,7 +98,7 @@ void xe_spin_init(struct xe_spin *spin, struct
>>>xe_spin_opts *opts)
>>>                spin->batch[b++] = CS_GPR(NOW_TS) + 4;
>>>                spin->batch[b++] = 0;
>>>                spin->batch[b++] = MI_LOAD_REGISTER_REG |
>>>MI_LRR_DST_CS_MMIO | MI_LRR_SRC_CS_MMIO;
>>>-               spin->batch[b++] = CTX_TIMESTAMP;
>>>+               spin->batch[b++] = opts->use_queue_timestamp ?
>>>QUEUE_TIMESTAMP : CTX_TIMESTAMP;
>>>                spin->batch[b++] = CS_GPR(NOW_TS);
>>> 
>>>                /* delta = now - start; inverted to match COND_BBE */
>>>diff --git a/lib/xe/xe_spin.h b/lib/xe/xe_spin.h
>>>index db0febd8ab..75a6ada420 100644
>>>--- a/lib/xe/xe_spin.h
>>>+++ b/lib/xe/xe_spin.h
>>>@@ -51,6 +51,7 @@ struct xe_spin_mem_copy {
>>>  * @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
>>>+ * @use_queue_timestamp: Use QUEUE_TIMESTAMP register instead of
>>>CTX_TIMESTAMP
>>>  * @mem_copy: container of objects used for memory copy (optional)
>>>  *
>>>  * Used to initialize struct xe_spin spinner behavior.
>>>@@ -62,6 +63,7 @@ struct xe_spin_opts {
>>>        bool multi_queue_switch_on_wait;
>>>        uint32_t ctx_ticks;
>>>        bool write_timestamp;
>>>+       bool use_queue_timestamp;
>>>        struct xe_spin_mem_copy *mem_copy;
>>> };
>>> 
>>