Re: [PATCH v3 07/10] drm/xe/trace: Add trace for engine activity

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Mon, Jan 06, 2025 at 01:25:56PM +0530, Riana Tauro wrote:
>Add engine activity related information to trace events for
>better debuggability
>
>v2: add trace for engine activity (Umesh)
>
>Signed-off-by: Riana Tauro <riana.tauro@intel.com>

LGTM,

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

>---
> drivers/gpu/drm/xe/xe_guc_engine_activity.c |  5 +++
> drivers/gpu/drm/xe/xe_trace_guc.h           | 49 +++++++++++++++++++++
> 2 files changed, 54 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_guc_engine_activity.c b/drivers/gpu/drm/xe/xe_guc_engine_activity.c
>index 687d90873e97..14950b42d1b1 100644
>--- a/drivers/gpu/drm/xe/xe_guc_engine_activity.c
>+++ b/drivers/gpu/drm/xe/xe_guc_engine_activity.c
>@@ -15,6 +15,7 @@
> #include "xe_hw_engine.h"
> #include "xe_map.h"
> #include "xe_mmio.h"
>+#include "xe_trace_guc.h"
>
> #define TOTAL_QUANTA 0x8000
>
>@@ -165,6 +166,8 @@ static u64 get_engine_active_ticks(struct xe_guc *guc, struct xe_hw_engine *hwe)
> 		}
> 	}
>
>+	trace_xe_guc_engine_activity(xe, ea, hwe->name, hwe->instance);
>+
> 	return ea->total + ea->active;
> }
>
>@@ -198,6 +201,8 @@ static u64 get_engine_total_ticks(struct xe_guc *guc, struct xe_hw_engine *hwe)
> 	ea->quanta_remainder_ns = numerator % TOTAL_QUANTA;
> 	ea->quanta = cpu_ns_to_guc_tsc_tick(ea->quanta_ns, cached_metadata->guc_tsc_frequency_hz);
>
>+	trace_xe_guc_engine_activity(xe, ea, hwe->name, hwe->instance);
>+
> 	return ea->quanta;
> }
>
>diff --git a/drivers/gpu/drm/xe/xe_trace_guc.h b/drivers/gpu/drm/xe/xe_trace_guc.h
>index 23abdd55dc62..156d429d7e79 100644
>--- a/drivers/gpu/drm/xe/xe_trace_guc.h
>+++ b/drivers/gpu/drm/xe/xe_trace_guc.h
>@@ -14,6 +14,7 @@
>
> #include "xe_device_types.h"
> #include "xe_guc_exec_queue_types.h"
>+#include "xe_guc_engine_activity_types.h"
>
> #define __dev_name_xe(xe)	dev_name((xe)->drm.dev)
>
>@@ -100,6 +101,54 @@ DEFINE_EVENT_PRINT(xe_guc_ctb, xe_guc_ctb_g2h,
>
> );
>
>+TRACE_EVENT(xe_guc_engine_activity,
>+	    TP_PROTO(struct xe_device *xe, struct engine_activity *ea, const char *name,
>+		     u16 instance),
>+	    TP_ARGS(xe, ea, name, instance),
>+
>+	    TP_STRUCT__entry(
>+			__string(dev, __dev_name_xe(xe))
>+			__string(name, name)
>+			__field(u32, global_change_num)
>+			__field(u32, guc_tsc_frequency_hz)
>+			__field(u32, lag_latency_usec)
>+			__field(u16, instance)
>+			__field(u16, change_num)
>+			__field(u16, quanta_ratio)
>+			__field(u32, last_update_tick)
>+			__field(u64, active_ticks)
>+			__field(u64, active)
>+			__field(u64, total)
>+			__field(u64, quanta)
>+			__field(u64, last_cpu_ts)
>+	    ),
>+
>+	    TP_fast_assign(
>+			__assign_str(dev);
>+			__assign_str(name);
>+			__entry->global_change_num = ea->metadata.global_change_num;
>+			__entry->guc_tsc_frequency_hz = ea->metadata.guc_tsc_frequency_hz;
>+			__entry->lag_latency_usec = ea->metadata.lag_latency_usec;
>+			__entry->instance = instance;
>+			__entry->change_num = ea->activity.change_num;
>+			__entry->quanta_ratio = ea->activity.quanta_ratio;
>+			__entry->last_update_tick = ea->activity.last_update_tick;
>+			__entry->active_ticks = ea->activity.active_ticks;
>+			__entry->active = ea->active;
>+			__entry->total = ea->total;
>+			__entry->quanta = ea->quanta;
>+			__entry->last_cpu_ts = ea->last_cpu_ts;
>+	    ),
>+
>+	    TP_printk("dev=%s engine %s:%d Active=%llu, quanta=%llu, last_cpu_ts=%llu\n"
>+		      "Activity metadata: global_change_num=%u, guc_tsc_frequency_hz=%u lag_latency_usec=%u\n"
>+		      "Activity data: change_num=%u, quanta_ratio=%u, last_update_tick=%u, active_ticks=%llu\n",
>+		      __get_str(dev), __get_str(name), __entry->instance,
>+		      (__entry->active +  __entry->total), __entry->quanta, __entry->last_cpu_ts,
>+		      __entry->global_change_num, __entry->guc_tsc_frequency_hz,
>+		      __entry->lag_latency_usec, __entry->change_num, __entry->quanta_ratio,
>+		      __entry->last_update_tick, __entry->active_ticks)
>+);
> #endif
>
> /* This part must be outside protection */
>-- 
>2.47.1
>