Re: [PATCH v3 09/11] drm/xe/multi_queue: Add trace event for the multi queue timestamp

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

On Tue, May 05, 2026 at 04:44:18PM -0700, Umesh Nerlige Ramappa wrote:
>Add a trace event for multi queue timestamp capture.
>
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>---
>v2:
>- Split traces from original patch (Stuart)
>- Print primary lrc in the trace (Niranjana)
>
>v3:
>- Include primary_lrc as well as lrc in the trace (Niranjana)
>---
> drivers/gpu/drm/xe/xe_lrc.c       |  2 ++
> drivers/gpu/drm/xe/xe_trace_lrc.h | 29 +++++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
>index 28060a3d4f11..ea65160a582a 100644
>--- a/drivers/gpu/drm/xe/xe_lrc.c
>+++ b/drivers/gpu/drm/xe/xe_lrc.c
>@@ -2700,6 +2700,8 @@ static u64 xe_lrc_update_multi_queue_timestamp(struct xe_lrc *lrc, u64 *old_ts)
> 	*old_ts = lrc->queue_timestamp;
> 	lrc->queue_timestamp = xe_lrc_multi_queue_timestamp(lrc);
>
>+	trace_xe_lrc_update_queue_timestamp(lrc, *old_ts);
>+
> 	return lrc->queue_timestamp;
> }
>
>diff --git a/drivers/gpu/drm/xe/xe_trace_lrc.h b/drivers/gpu/drm/xe/xe_trace_lrc.h
>index d525cbee1e34..4f3c1a0de573 100644
>--- a/drivers/gpu/drm/xe/xe_trace_lrc.h
>+++ b/drivers/gpu/drm/xe/xe_trace_lrc.h
>@@ -12,6 +12,7 @@
> #include <linux/tracepoint.h>
> #include <linux/types.h>
>
>+#include "xe_exec_queue_types.h"
> #include "xe_gt_types.h"
> #include "xe_lrc.h"
> #include "xe_lrc_types.h"
>@@ -42,6 +43,34 @@ TRACE_EVENT(xe_lrc_update_timestamp,
> 		      __get_str(device_id))
> );
>
>+TRACE_EVENT(xe_lrc_update_queue_timestamp,
>+	    TP_PROTO(struct xe_lrc *lrc, uint64_t old),
>+	    TP_ARGS(lrc, old),
>+	    TP_STRUCT__entry(
>+		     __field(struct xe_lrc *, lrc)
>+		     __field(struct xe_lrc *, primary_lrc)
>+		     __field(u8, pos)
>+		     __field(u64, old)
>+		     __field(u64, new)
>+		     __string(name, lrc->fence_ctx.name)
>+		     __string(device_id, __dev_name_lrc(lrc))
>+		     ),
>+
>+	    TP_fast_assign(
>+		   __entry->lrc = lrc;
>+		   __entry->primary_lrc = lrc->multi_queue.primary_lrc;
>+		   __entry->pos = lrc->multi_queue.pos;
>+		   __entry->old = old;
>+		   __entry->new = lrc->queue_timestamp;
>+		   __assign_str(name);
>+		   __assign_str(device_id);
>+		   ),
>+	    TP_printk("lrc=:%p primary_lrc:%p pos=%d lrc->name=%s old=%llu new=%llu device_id:%s",
>+		      __entry->lrc, __entry->primary_lrc, __entry->pos, __get_str(name),
>+		      __entry->old, __entry->new,
>+		      __get_str(device_id))

I still think printing 'pos' here does not add much value (we are not printing it in
exec queue trace event also).

BTW, why do we need ':'s before %p? I see it is there in xe_lrc_update_timestamp(),
but that seems to be a mistake (I don't see other trace messages having it while
printing pointer values).

Niranjana

>+);
>+
> #endif
>
> /* This part must be outside protection */
>-- 
>2.51.0
>