[PATCH v5 4/6] drm/xe/trace: Print device_id in xe_trace_guc events
Gustavo Sousa
gustavo.sousa at intel.com
Fri Jun 7 13:50:32 UTC 2024
Quoting Radhakrishna Sripada (2024-06-06 19:38:17-03:00)
>In multi-gpu environments it is important to know the device
>guc txn belongs to. The tracing information includes the device_id
>to indicate the device the event is associated with.
>
>v2: Use variable sized variant to display dev name(Gustavo)
>v3: Pass single argument to __assign_str to fix kunit error
>
>Suggested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>Cc: Gustavo Sousa <gustavo.sousa at intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
>---
> drivers/gpu/drm/xe/xe_guc_ct.c | 11 +++----
> drivers/gpu/drm/xe/xe_trace_guc.h | 48 ++++++++++++++++++-------------
> 2 files changed, 34 insertions(+), 25 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
>index 78f79df638d5..ff3fd10cd417 100644
>--- a/drivers/gpu/drm/xe/xe_guc_ct.c
>+++ b/drivers/gpu/drm/xe/xe_guc_ct.c
>@@ -528,7 +528,7 @@ static int h2g_write(struct xe_guc_ct *ct, const u32 *action, u32 len,
> /* Update descriptor */
> desc_write(xe, h2g, tail, h2g->info.tail);
>
>- trace_xe_guc_ctb_h2g(gt->info.id, *(action - 1), full_len,
>+ trace_xe_guc_ctb_h2g(xe, gt->info.id, *(action - 1), full_len,
> desc_read(xe, h2g, head), h2g->info.tail);
>
> return 0;
>@@ -641,6 +641,7 @@ static int guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action, u32 len,
> u32 g2h_len, u32 num_g2h,
> struct g2h_fence *g2h_fence)
> {
>+ struct xe_device *xe = ct_to_xe(ct);
> struct xe_gt *gt = ct_to_gt(ct);
> struct drm_printer p = xe_gt_info_printer(gt);
> unsigned int sleep_period_ms = 1;
>@@ -668,7 +669,7 @@ static int guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action, u32 len,
> if (sleep_period_ms == 1024)
> goto broken;
>
>- trace_xe_guc_ct_h2g_flow_control(h2g->info.head, h2g->info.tail,
>+ trace_xe_guc_ct_h2g_flow_control(xe, h2g->info.head, h2g->info.tail,
> h2g->info.size,
> h2g->info.space,
> len + GUC_CTB_HDR_LEN);
>@@ -680,7 +681,7 @@ static int guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action, u32 len,
> struct xe_device *xe = ct_to_xe(ct);
> struct guc_ctb *g2h = &ct->ctbs.g2h;
>
>- trace_xe_guc_ct_g2h_flow_control(g2h->info.head,
>+ trace_xe_guc_ct_g2h_flow_control(xe, g2h->info.head,
> desc_read(xe, g2h, tail),
> g2h->info.size,
> g2h->info.space,
>@@ -1170,8 +1171,8 @@ static int g2h_read(struct xe_guc_ct *ct, u32 *msg, bool fast_path)
> g2h->info.head = (head + avail) % g2h->info.size;
> desc_write(xe, g2h, head, g2h->info.head);
>
>- trace_xe_guc_ctb_g2h(ct_to_gt(ct)->info.id, action, len,
>- g2h->info.head, tail);
>+ trace_xe_guc_ctb_g2h(xe, ct_to_gt(ct)->info.id,
>+ action, len, g2h->info.head, tail);
>
> return len;
> }
>diff --git a/drivers/gpu/drm/xe/xe_trace_guc.h b/drivers/gpu/drm/xe/xe_trace_guc.h
>index d6830ff21822..3791baad1866 100644
>--- a/drivers/gpu/drm/xe/xe_trace_guc.h
>+++ b/drivers/gpu/drm/xe/xe_trace_guc.h
>@@ -9,16 +9,21 @@
> #if !defined(_XE_TRACE_GUC_H_) || defined(TRACE_HEADER_MULTI_READ)
> #define _XE_TRACE_GUC_H_
>
>+#include <linux/string_helpers.h>
I believe string_helpers.h is not necessary here.
> #include <linux/tracepoint.h>
> #include <linux/types.h>
>
>+#include "xe_device_types.h"
> #include "xe_guc_exec_queue_types.h"
>
>+#define __dev_name_xe(xe) dev_name((xe)->drm.dev)
>+
> DECLARE_EVENT_CLASS(xe_guc_ct_flow_control,
>- TP_PROTO(u32 _head, u32 _tail, u32 size, u32 space, u32 len),
>- TP_ARGS(_head, _tail, size, space, len),
>+ TP_PROTO(struct xe_device *xe, u32 _head, u32 _tail, u32 size, u32 space, u32 len),
>+ TP_ARGS(xe, _head, _tail, size, space, len),
>
> TP_STRUCT__entry(
>+ __string(dev, __dev_name_xe(xe))
> __field(u32, _head)
> __field(u32, _tail)
> __field(u32, size)
>@@ -27,6 +32,7 @@ DECLARE_EVENT_CLASS(xe_guc_ct_flow_control,
> ),
>
> TP_fast_assign(
>+ __assign_str(dev);
> __entry->_head = _head;
> __entry->_tail = _tail;
> __entry->size = size;
>@@ -34,30 +40,31 @@ DECLARE_EVENT_CLASS(xe_guc_ct_flow_control,
> __entry->len = len;
> ),
>
>- TP_printk("h2g flow control: head=%u, tail=%u, size=%u, space=%u, len=%u",
>- __entry->_head, __entry->_tail, __entry->size,
>+ TP_printk("dev=%s, h2g flow control: head=%u, tail=%u, size=%u, space=%u, len=%u",
I think "h2g flow control:" serves as a prefix for this line in the same
way as "g2h flow control:" does. Maybe it makes more sense to place
" dev=%s," after them?
>+ __get_str(dev), __entry->_head, __entry->_tail, __entry->size,
> __entry->space, __entry->len)
> );
>
> DEFINE_EVENT(xe_guc_ct_flow_control, xe_guc_ct_h2g_flow_control,
>- TP_PROTO(u32 _head, u32 _tail, u32 size, u32 space, u32 len),
>- TP_ARGS(_head, _tail, size, space, len)
>+ TP_PROTO(struct xe_device *xe, u32 _head, u32 _tail, u32 size, u32 space, u32 len),
>+ TP_ARGS(xe, _head, _tail, size, space, len)
> );
>
> DEFINE_EVENT_PRINT(xe_guc_ct_flow_control, xe_guc_ct_g2h_flow_control,
>- TP_PROTO(u32 _head, u32 _tail, u32 size, u32 space, u32 len),
>- TP_ARGS(_head, _tail, size, space, len),
>+ TP_PROTO(struct xe_device *xe, u32 _head, u32 _tail, u32 size, u32 space, u32 len),
>+ TP_ARGS(xe, _head, _tail, size, space, len),
>
>- TP_printk("g2h flow control: head=%u, tail=%u, size=%u, space=%u, len=%u",
>- __entry->_head, __entry->_tail, __entry->size,
>+ TP_printk("dev=%s, g2h flow control: head=%u, tail=%u, size=%u, space=%u, len=%u",
>+ __get_str(dev), __entry->_head, __entry->_tail, __entry->size,
> __entry->space, __entry->len)
> );
>
> DECLARE_EVENT_CLASS(xe_guc_ctb,
>- TP_PROTO(u8 gt_id, u32 action, u32 len, u32 _head, u32 tail),
>- TP_ARGS(gt_id, action, len, _head, tail),
>+ TP_PROTO(struct xe_device *xe, u8 gt_id, u32 action, u32 len, u32 _head, u32 tail),
>+ TP_ARGS(xe, gt_id, action, len, _head, tail),
>
> TP_STRUCT__entry(
>+ __string(dev, __dev_name_xe(xe))
> __field(u8, gt_id)
> __field(u32, action)
> __field(u32, len)
>@@ -66,6 +73,7 @@ DECLARE_EVENT_CLASS(xe_guc_ctb,
> ),
>
> TP_fast_assign(
>+ __assign_str(dev);
> __entry->gt_id = gt_id;
> __entry->action = action;
> __entry->len = len;
>@@ -73,22 +81,22 @@ DECLARE_EVENT_CLASS(xe_guc_ctb,
> __entry->_head = _head;
> ),
>
>- TP_printk("gt%d: H2G CTB: action=0x%x, len=%d, tail=%d, head=%d\n",
>- __entry->gt_id, __entry->action, __entry->len,
>+ TP_printk("dev=%s, gt%d: H2G CTB: action=0x%x, len=%d, tail=%d, head=%d\n",
>+ __get_str(dev), __entry->gt_id, __entry->action, __entry->len,
> __entry->tail, __entry->_head)
> );
>
> DEFINE_EVENT(xe_guc_ctb, xe_guc_ctb_h2g,
>- TP_PROTO(u8 gt_id, u32 action, u32 len, u32 _head, u32 tail),
>- TP_ARGS(gt_id, action, len, _head, tail)
>+ TP_PROTO(struct xe_device *xe, u8 gt_id, u32 action, u32 len, u32 _head, u32 tail),
>+ TP_ARGS(xe, gt_id, action, len, _head, tail)
> );
>
> DEFINE_EVENT_PRINT(xe_guc_ctb, xe_guc_ctb_g2h,
>- TP_PROTO(u8 gt_id, u32 action, u32 len, u32 _head, u32 tail),
>- TP_ARGS(gt_id, action, len, _head, tail),
>+ TP_PROTO(struct xe_device *xe, u8 gt_id, u32 action, u32 len, u32 _head, u32 tail),
>+ TP_ARGS(xe, gt_id, action, len, _head, tail),
>
>- TP_printk("gt%d: G2H CTB: action=0x%x, len=%d, tail=%d, head=%d\n",
>- __entry->gt_id, __entry->action, __entry->len,
>+ TP_printk("dev=%s, gt%d: G2H CTB: action=0x%x, len=%d, tail=%d, head=%d\n",
Same reasoning here: Maybe it makes more sense to do
"G2H CTB: dev=%s, gt=%d ..."?
With those tweaks,
Reviewed-by: Gustavo Sousa <gustavo.sousa at intel.com>
>+ __get_str(dev), __entry->gt_id, __entry->action, __entry->len,
> __entry->tail, __entry->_head)
>
> );
>--
>2.34.1
>
More information about the Intel-xe
mailing list