[PATCH v3 6/6] drm/xe: Add reg read/write trace

[Gustavo Sousa]

Quoting Radhakrishna Sripada (2024-05-30 12:13:13-03:00)
>This will help debug register read/writes and provides
>a way to trace all the mmio transactions.
>
>v2: Fix kunit error
>v3: Print devid to help in multi-gpu setup
>
>Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
>---
> drivers/gpu/drm/xe/xe_mmio.c  | 22 ++++++++++++++++++----
> drivers/gpu/drm/xe/xe_trace.h | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
>index 248e93ec6df7..a44baa349109 100644
>--- a/drivers/gpu/drm/xe/xe_mmio.c
>+++ b/drivers/gpu/drm/xe/xe_mmio.c
>@@ -27,6 +27,7 @@
> #include "xe_module.h"
> #include "xe_sriov.h"
> #include "xe_tile.h"
>+#include "xe_trace.h"
> 
> #define XEHP_MTCFG_ADDR                XE_REG(0x101800)
> #define TILE_COUNT                REG_GENMASK(15, 8)
>@@ -455,16 +456,24 @@ u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
> {
>         struct xe_tile *tile = gt_to_tile(gt);
>         u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>+        u8 val;
> 
>-        return readb((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>+        val = readb((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>+        trace_xe_reg_rw(dev_name(gt_to_xe(gt)->drm.dev), false, addr, val, sizeof(val), true);
>+
>+        return val;
> }
> 
> u16 xe_mmio_read16(struct xe_gt *gt, struct xe_reg reg)
> {
>         struct xe_tile *tile = gt_to_tile(gt);
>         u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>+        u16 val;
>+
>+        val = readw((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>+        trace_xe_reg_rw(dev_name(gt_to_xe(gt)->drm.dev), false, addr, val, sizeof(val), true);
> 
>-        return readw((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>+        return val;
> }
> 
> void xe_mmio_write32(struct xe_gt *gt, struct xe_reg reg, u32 val)
>@@ -472,6 +481,7 @@ void xe_mmio_write32(struct xe_gt *gt, struct xe_reg reg, u32 val)
>         struct xe_tile *tile = gt_to_tile(gt);
>         u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
> 
>+        trace_xe_reg_rw(dev_name(gt_to_xe(gt)->drm.dev), true, addr, val, sizeof(val), true);
>         writel(val, (reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
> }
> 
>@@ -479,11 +489,15 @@ u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
> {
>         struct xe_tile *tile = gt_to_tile(gt);
>         u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>+        u32 val;
> 
>         if (!reg.vf && IS_SRIOV_VF(gt_to_xe(gt)))
>-                return xe_gt_sriov_vf_read32(gt, reg);
>+                val = xe_gt_sriov_vf_read32(gt, reg);
>+
>+        val = readl((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);

This readl() call needs to be inside an "else" statement, right?

>+        trace_xe_reg_rw(dev_name(gt_to_xe(gt)->drm.dev), false, addr, val, sizeof(val), true);
> 
>-        return readl((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>+        return val;
> }
> 
> u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr, u32 set)
>diff --git a/drivers/gpu/drm/xe/xe_trace.h b/drivers/gpu/drm/xe/xe_trace.h
>index 41e0b3365876..d56e073560ee 100644
>--- a/drivers/gpu/drm/xe/xe_trace.h
>+++ b/drivers/gpu/drm/xe/xe_trace.h
>@@ -342,6 +342,36 @@ DEFINE_EVENT(xe_hw_fence, xe_hw_fence_free,
>              TP_ARGS(fence)
> );
> 
>+TRACE_EVENT_CONDITION(xe_reg_rw,
>+        TP_PROTO(const char *devid, bool write, u32 reg, u64 val, int len, bool trace),
>+
>+        TP_ARGS(devid, write, reg, val, len, trace),
>+
>+        TP_CONDITION(trace),

Looks like trace is always true. Are there plans to have a false value?

--
Gustavo Sousa

>+
>+        TP_STRUCT__entry(
>+                __array(char, devid, 12)
>+                __field(u64, val)
>+                __field(u32, reg)
>+                __field(u16, write)
>+                __field(u16, len)
>+                ),
>+
>+        TP_fast_assign(
>+                memcpy(__entry->devid, devid, 12);
>+                __entry->val = (u64)val;
>+                __entry->reg = reg;
>+                __entry->write = write;
>+                __entry->len = len;
>+                ),
>+
>+        TP_printk("dev: %s: %s reg=0x%x, len=%d, val=(0x%x, 0x%x)",
>+                  __entry->devid, __entry->write ? "write" : "read",
>+                  __entry->reg, __entry->len,
>+                  (u32)(__entry->val & 0xffffffff),
>+                  (u32)(__entry->val >> 32))
>+);
>+
> #endif
> 
> /* This part must be outside protection */
>-- 
>2.34.1
>