[PATCH v3 4/6] drm/xe/trace: Print device_id in xe_trace_guc events
Sripada, Radhakrishna
radhakrishna.sripada at intel.com
Mon Jun 3 15:03:46 UTC 2024
Hi Gustavo,
> -----Original Message-----
> From: Sousa, Gustavo <gustavo.sousa at intel.com>
> Sent: Monday, June 3, 2024 7:17 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada at intel.com>; intel-
> xe at lists.freedesktop.org
> Cc: De Marchi, Lucas <lucas.demarchi at intel.com>; Sripada, Radhakrishna
> <radhakrishna.sripada at intel.com>; Ville Syrjälä <ville.syrjala at linux.intel.com>
> Subject: Re: [PATCH v3 4/6] drm/xe/trace: Print device_id in xe_trace_guc events
>
> Quoting Radhakrishna Sripada (2024-05-30 12:13:11-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.
> >
> >Suggested-by: Ville Syrjälä <ville.syrjala at linux.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 | 13 +++++----
> > drivers/gpu/drm/xe/xe_trace_guc.h | 44 +++++++++++++++++--------------
> > 2 files changed, 32 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..5b08cceb994a 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(dev_name(xe->drm.dev), 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,8 @@ 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(dev_name(xe->drm.dev),
> >+ h2g->info.head, h2g->info.tail,
> > h2g->info.size,
> > h2g->info.space,
> > len + GUC_CTB_HDR_LEN);
> >@@ -680,7 +682,8 @@ 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(dev_name(xe->drm.dev),
> >+ g2h->info.head,
> > desc_read(xe, g2h, tail),
> > g2h->info.size,
> > g2h->info.space,
> >@@ -1170,8 +1173,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(dev_name(xe->drm.dev), 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 fb4976499812..8a7953803696 100644
> >--- a/drivers/gpu/drm/xe/xe_trace_guc.h
> >+++ b/drivers/gpu/drm/xe/xe_trace_guc.h
> >@@ -15,10 +15,11 @@
> > #include "xe_guc_exec_queue_types.h"
> >
> > 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(const char *devid, u32 _head, u32 _tail, u32 size, u32
> space, u32 len),
> >+ TP_ARGS(devid, _head, _tail, size, space, len),
> >
> > TP_STRUCT__entry(
> >+ __array(char, devid, 12)
> > __field(u32, _head)
> > __field(u32, _tail)
> > __field(u32, size)
> >@@ -27,6 +28,7 @@ DECLARE_EVENT_CLASS(xe_guc_ct_flow_control,
> > ),
> >
> > TP_fast_assign(
> >+ memcpy(__entry->devid, devid, 12);
>
> The same question from the previous patch applies here regarding using
> variable length copy vs using a fixed sized buffer.
I will use the style that is already there for xe_bo_move in xe_trace_bo.h
I can use a common dev length macro for each file.
>
> Also, any reason for the choice of memcpy() here against strscpy(),
> which was used in the previous patch?
No particular reason. But can use strscpy for all.
>
> > __entry->_head = _head;
> > __entry->_tail = _tail;
> > __entry->size = size;
> >@@ -34,30 +36,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",
>
> Similarly to the previous patch, maybe use "h2g flow control: dev=%s, "?
Sure will make the changes to all the events.
Regards,
Radhakrishna Sripada
>
> --
> Gustavo Sousa
>
> >+ __entry->devid, __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(const char *devid, u32 _head, u32 _tail, u32 size, u32 space,
> u32 len),
> >+ TP_ARGS(devid, _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(const char *devid, u32 _head, u32 _tail, u32 size, u32
> space, u32 len),
> >+ TP_ARGS(devid, _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",
> >+ __entry->devid, __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(const char *devid, u8 gt_id, u32 action, u32 len, u32
> _head, u32 tail),
> >+ TP_ARGS(devid, gt_id, action, len, _head, tail),
> >
> > TP_STRUCT__entry(
> >+ __array(char, devid, 12)
> > __field(u8, gt_id)
> > __field(u32, action)
> > __field(u32, len)
> >@@ -66,6 +69,7 @@ DECLARE_EVENT_CLASS(xe_guc_ctb,
> > ),
> >
> > TP_fast_assign(
> >+ memcpy(__entry->devid, devid, 12);
> > __entry->gt_id = gt_id;
> > __entry->action = action;
> > __entry->len = len;
> >@@ -73,22 +77,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",
> >+ __entry->devid, __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(const char *devid, u8 gt_id, u32 action, u32 len, u32 _head,
> u32 tail),
> >+ TP_ARGS(devid, 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(const char *devid, u8 gt_id, u32 action, u32 len, u32
> _head, u32 tail),
> >+ TP_ARGS(devid, 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",
> >+ __entry->devid, __entry->gt_id, __entry->action, __entry->len,
> > __entry->tail, __entry->_head)
> >
> > );
> >--
> >2.34.1
> >
More information about the Intel-xe
mailing list