[PATCH v4 1/4] drm/xe: Save CTX_TIMESTAMP mmio value instead of LRC value

[Lucas De Marchi]

On Tue, May 06, 2025 at 06:30:40PM -0700, Umesh Nerlige Ramappa wrote:
>For determining actual job execution time, save the current value of the
>CTX_TIMESTAMP register rather than the value saved in LRC since the
>current register value is the closest to the start time of the job.
>
>v2: Define MI_STORE_REGISTER_MEM to fix compile error
>
>Fixes: 65921374c48f ("drm/xe: Emit ctx timestamp copy in ring ops")
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>Reviewed-by: Matthew Brost <matthew.brost at intel.com>
>---
> drivers/gpu/drm/xe/instructions/xe_mi_commands.h | 4 ++++
> drivers/gpu/drm/xe/xe_lrc.c                      | 2 +-
> drivers/gpu/drm/xe/xe_ring_ops.c                 | 7 ++-----
> 3 files changed, 7 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/instructions/xe_mi_commands.h b/drivers/gpu/drm/xe/instructions/xe_mi_commands.h
>index eba582058d55..9153a7cd2ceb 100644
>--- a/drivers/gpu/drm/xe/instructions/xe_mi_commands.h
>+++ b/drivers/gpu/drm/xe/instructions/xe_mi_commands.h
>@@ -62,6 +62,10 @@
> #define MI_LOAD_REGISTER_MEM		(__MI_INSTR(0x29) | XE_INSTR_NUM_DW(4))
> #define   MI_LRM_USE_GGTT		REG_BIT(22)
>
>+#define MI_STORE_REGISTER_MEM		(__MI_INSTR(0x24) | XE_INSTR_NUM_DW(4))

it should be sorted by __MI_INSTR(), so this define should be between
MI_FLUSH_DW and MI_LOAD_REGISTER_IMM.

other than that,

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>


thanks
Lucas De Marchi

>+#define   MI_SRM_USE_GGTT		REG_BIT(22)
>+#define   MI_SRM_ADD_CS_OFFSET		REG_BIT(19)
>+
> #define MI_LOAD_REGISTER_REG		(__MI_INSTR(0x2a) | XE_INSTR_NUM_DW(3))
> #define   MI_LRR_DST_CS_MMIO		REG_BIT(19)
> #define   MI_LRR_SRC_CS_MMIO		REG_BIT(18)
>diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
>index 855c8acaf3f1..015b00a73c0b 100644
>--- a/drivers/gpu/drm/xe/xe_lrc.c
>+++ b/drivers/gpu/drm/xe/xe_lrc.c
>@@ -687,7 +687,7 @@ static inline u32 __xe_lrc_start_seqno_offset(struct xe_lrc *lrc)
>
> static u32 __xe_lrc_ctx_job_timestamp_offset(struct xe_lrc *lrc)
> {
>-	/* The start seqno is stored in the driver-defined portion of PPHWSP */
>+	/* This is stored in the driver-defined portion of PPHWSP */
> 	return xe_lrc_pphwsp_offset(lrc) + LRC_CTX_JOB_TIMESTAMP_OFFSET;
> }
>
>diff --git a/drivers/gpu/drm/xe/xe_ring_ops.c b/drivers/gpu/drm/xe/xe_ring_ops.c
>index a7582b097ae6..bc1689db4cd7 100644
>--- a/drivers/gpu/drm/xe/xe_ring_ops.c
>+++ b/drivers/gpu/drm/xe/xe_ring_ops.c
>@@ -234,13 +234,10 @@ static u32 get_ppgtt_flag(struct xe_sched_job *job)
>
> static int emit_copy_timestamp(struct xe_lrc *lrc, u32 *dw, int i)
> {
>-	dw[i++] = MI_COPY_MEM_MEM | MI_COPY_MEM_MEM_SRC_GGTT |
>-		MI_COPY_MEM_MEM_DST_GGTT;
>+	dw[i++] = MI_STORE_REGISTER_MEM | MI_SRM_USE_GGTT | MI_SRM_ADD_CS_OFFSET;
>+	dw[i++] = RING_CTX_TIMESTAMP(0).addr;
> 	dw[i++] = xe_lrc_ctx_job_timestamp_ggtt_addr(lrc);
> 	dw[i++] = 0;
>-	dw[i++] = xe_lrc_ctx_timestamp_ggtt_addr(lrc);
>-	dw[i++] = 0;
>-	dw[i++] = MI_NOOP;
>
> 	return i;
> }
>-- 
>2.43.0
>