[Intel-gfx] [RFC 0/2] Add mmio register rw tracing

[Intel-gfx] [RFC 0/2] Add mmio register rw tracing

[Radhakrishna Sripada]

This patchset adds xe_reg_rw mmio tracing event for the xe driver. This
tracing event should be helpful for developers especially to debug Display
code.

Radhakrishna Sripada (2):
  drm/xe: Move mmio read/write functions to xe_mmio.c
  drm/xe: Add reg read/write trace

 .../drm/xe/compat-i915-headers/intel_uncore.h |   8 +-
 drivers/gpu/drm/xe/xe_mmio.c                  | 187 ++++++++++++++++++
 drivers/gpu/drm/xe/xe_mmio.h                  | 136 +++----------
 drivers/gpu/drm/xe/xe_trace.h                 |  26 +++
 4 files changed, 239 insertions(+), 118 deletions(-)

-- 
2.34.1

[Intel-gfx] [RFC 1/2] drm/xe: Move mmio read/write functions to xe_mmio.c

[Radhakrishna Sripada]

Move the register read/write apis to xe_mmio.c to prepare for
adding tracing infrastructure for the same. Adding tracing support
in xe_mmio.h messes with the compilation of the display code.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/xe/xe_mmio.c | 113 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_mmio.h | 129 ++++-------------------------------
 2 files changed, 128 insertions(+), 114 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index 475b14fe4356..70ad1b6a17a0 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -435,6 +435,119 @@ static const struct xe_reg mmio_read_whitelist[] = {
 	RING_TIMESTAMP(RENDER_RING_BASE),
 };
 
+inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	return readb(tile->mmio.regs + reg.addr);
+}
+
+inline void xe_mmio_write32(struct xe_gt *gt,
+			    struct xe_reg reg, u32 val)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	writel(val, tile->mmio.regs + reg.addr);
+}
+
+inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	return readl(tile->mmio.regs + reg.addr);
+}
+
+inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
+			 u32 set)
+{
+	u32 old, reg_val;
+
+	old = xe_mmio_read32(gt, reg);
+	reg_val = (old & ~clr) | set;
+	xe_mmio_write32(gt, reg, reg_val);
+
+	return old;
+}
+
+inline void xe_mmio_write64(struct xe_gt *gt,
+			    struct xe_reg reg, u64 val)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	writeq(val, tile->mmio.regs + reg.addr);
+}
+
+inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	return readq(tile->mmio.regs + reg.addr);
+}
+
+inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
+				      struct xe_reg reg, u32 val,
+				      u32 mask, u32 eval)
+{
+	u32 reg_val;
+
+	xe_mmio_write32(gt, reg, val);
+	reg_val = xe_mmio_read32(gt, reg);
+
+	return (reg_val & mask) != eval ? -EINVAL : 0;
+}
+
+inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
+			  u32 mask, u32 timeout_us, u32 *out_val,
+			  bool atomic)
+{
+	ktime_t cur = ktime_get_raw();
+	const ktime_t end = ktime_add_us(cur, timeout_us);
+	int ret = -ETIMEDOUT;
+	s64 wait = 10;
+	u32 read;
+
+	for (;;) {
+		read = xe_mmio_read32(gt, reg);
+		if ((read & mask) == val) {
+			ret = 0;
+			break;
+		}
+
+		cur = ktime_get_raw();
+		if (!ktime_before(cur, end))
+			break;
+
+		if (ktime_after(ktime_add_us(cur, wait), end))
+			wait = ktime_us_delta(end, cur);
+
+		if (atomic)
+			udelay(wait);
+		else
+			usleep_range(wait, wait << 1);
+		wait <<= 1;
+	}
+
+	if (out_val)
+		*out_val = read;
+
+	return ret;
+}
+
 int xe_mmio_ioctl(struct drm_device *dev, void *data,
 		  struct drm_file *file)
 {
diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
index 3c547d78afba..2aa2c01e60dd 100644
--- a/drivers/gpu/drm/xe/xe_mmio.h
+++ b/drivers/gpu/drm/xe/xe_mmio.h
@@ -20,120 +20,21 @@ struct xe_device;
 #define GEN12_LMEM_BAR		2
 
 int xe_mmio_init(struct xe_device *xe);
-
-static inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
-{
-	struct xe_tile *tile = gt_to_tile(gt);
-
-	if (reg.addr < gt->mmio.adj_limit)
-		reg.addr += gt->mmio.adj_offset;
-
-	return readb(tile->mmio.regs + reg.addr);
-}
-
-static inline void xe_mmio_write32(struct xe_gt *gt,
-				   struct xe_reg reg, u32 val)
-{
-	struct xe_tile *tile = gt_to_tile(gt);
-
-	if (reg.addr < gt->mmio.adj_limit)
-		reg.addr += gt->mmio.adj_offset;
-
-	writel(val, tile->mmio.regs + reg.addr);
-}
-
-static inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
-{
-	struct xe_tile *tile = gt_to_tile(gt);
-
-	if (reg.addr < gt->mmio.adj_limit)
-		reg.addr += gt->mmio.adj_offset;
-
-	return readl(tile->mmio.regs + reg.addr);
-}
-
-static inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
-				u32 set)
-{
-	u32 old, reg_val;
-
-	old = xe_mmio_read32(gt, reg);
-	reg_val = (old & ~clr) | set;
-	xe_mmio_write32(gt, reg, reg_val);
-
-	return old;
-}
-
-static inline void xe_mmio_write64(struct xe_gt *gt,
-				   struct xe_reg reg, u64 val)
-{
-	struct xe_tile *tile = gt_to_tile(gt);
-
-	if (reg.addr < gt->mmio.adj_limit)
-		reg.addr += gt->mmio.adj_offset;
-
-	writeq(val, tile->mmio.regs + reg.addr);
-}
-
-static inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
-{
-	struct xe_tile *tile = gt_to_tile(gt);
-
-	if (reg.addr < gt->mmio.adj_limit)
-		reg.addr += gt->mmio.adj_offset;
-
-	return readq(tile->mmio.regs + reg.addr);
-}
-
-static inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
-					     struct xe_reg reg, u32 val,
-					     u32 mask, u32 eval)
-{
-	u32 reg_val;
-
-	xe_mmio_write32(gt, reg, val);
-	reg_val = xe_mmio_read32(gt, reg);
-
-	return (reg_val & mask) != eval ? -EINVAL : 0;
-}
-
-static inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
-				 u32 mask, u32 timeout_us, u32 *out_val,
-				 bool atomic)
-{
-	ktime_t cur = ktime_get_raw();
-	const ktime_t end = ktime_add_us(cur, timeout_us);
-	int ret = -ETIMEDOUT;
-	s64 wait = 10;
-	u32 read;
-
-	for (;;) {
-		read = xe_mmio_read32(gt, reg);
-		if ((read & mask) == val) {
-			ret = 0;
-			break;
-		}
-
-		cur = ktime_get_raw();
-		if (!ktime_before(cur, end))
-			break;
-
-		if (ktime_after(ktime_add_us(cur, wait), end))
-			wait = ktime_us_delta(end, cur);
-
-		if (atomic)
-			udelay(wait);
-		else
-			usleep_range(wait, wait << 1);
-		wait <<= 1;
-	}
-
-	if (out_val)
-		*out_val = read;
-
-	return ret;
-}
-
+inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg);
+inline void xe_mmio_write32(struct xe_gt *gt,
+			    struct xe_reg reg, u32 val);
+inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg);
+inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
+			 u32 set);
+inline void xe_mmio_write64(struct xe_gt *gt,
+			    struct xe_reg reg, u64 val);
+inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg);
+inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
+				      struct xe_reg reg, u32 val,
+				      u32 mask, u32 eval);
+inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
+			  u32 mask, u32 timeout_us, u32 *out_val,
+			  bool atomic);
 int xe_mmio_ioctl(struct drm_device *dev, void *data,
 		  struct drm_file *file);
 
-- 
2.34.1

[Intel-gfx] [RFC 2/2] drm/xe: Add reg read/write trace

[Radhakrishna Sripada]

This will help debug register read/writes and provide
a way to trace all the mmio transactions.

In order to avoid spam from xe_mmio_wait32,
xe_mmio_read32_notrace is introduced and used.

Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 .../drm/xe/compat-i915-headers/intel_uncore.h |  8 +-
 drivers/gpu/drm/xe/xe_mmio.c                  | 82 ++++++++++++++++++-
 drivers/gpu/drm/xe/xe_mmio.h                  |  7 ++
 drivers/gpu/drm/xe/xe_trace.h                 | 26 ++++++
 4 files changed, 115 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h b/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
index fae6213d26f1..036f3b963a84 100644
--- a/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
+++ b/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
@@ -113,7 +113,7 @@ static inline u32 intel_uncore_read_fw(struct fake_uncore *uncore,
 {
 	struct xe_reg reg = XE_REG(i915_mmio_reg_offset(i915_reg));
 
-	return xe_mmio_read32(__fake_uncore_to_gt(uncore), reg);
+	return xe_mmio_read32_notrace(__fake_uncore_to_gt(uncore), reg);
 }
 
 static inline void intel_uncore_write_fw(struct fake_uncore *uncore,
@@ -121,7 +121,7 @@ static inline void intel_uncore_write_fw(struct fake_uncore *uncore,
 {
 	struct xe_reg reg = XE_REG(i915_mmio_reg_offset(i915_reg));
 
-	xe_mmio_write32(__fake_uncore_to_gt(uncore), reg, val);
+	xe_mmio_write32_notrace(__fake_uncore_to_gt(uncore), reg, val);
 }
 
 static inline u32 intel_uncore_read_notrace(struct fake_uncore *uncore,
@@ -129,7 +129,7 @@ static inline u32 intel_uncore_read_notrace(struct fake_uncore *uncore,
 {
 	struct xe_reg reg = XE_REG(i915_mmio_reg_offset(i915_reg));
 
-	return xe_mmio_read32(__fake_uncore_to_gt(uncore), reg);
+	return xe_mmio_read32_notrace(__fake_uncore_to_gt(uncore), reg);
 }
 
 static inline void intel_uncore_write_notrace(struct fake_uncore *uncore,
@@ -137,7 +137,7 @@ static inline void intel_uncore_write_notrace(struct fake_uncore *uncore,
 {
 	struct xe_reg reg = XE_REG(i915_mmio_reg_offset(i915_reg));
 
-	xe_mmio_write32(__fake_uncore_to_gt(uncore), reg, val);
+	xe_mmio_write32_notrace(__fake_uncore_to_gt(uncore), reg, val);
 }
 
 #endif /* __INTEL_UNCORE_H__ */
diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index 70ad1b6a17a0..fc1af35e445e 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -19,6 +19,7 @@
 #include "xe_gt_mcr.h"
 #include "xe_macros.h"
 #include "xe_module.h"
+#include "xe_trace.h"
 
 #define XEHP_MTCFG_ADDR		XE_REG(0x101800)
 #define TILE_COUNT		REG_GENMASK(15, 8)
@@ -435,7 +436,7 @@ static const struct xe_reg mmio_read_whitelist[] = {
 	RING_TIMESTAMP(RENDER_RING_BASE),
 };
 
-inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
+inline u8 xe_mmio_read8_notrace(struct xe_gt *gt, struct xe_reg reg)
 {
 	struct xe_tile *tile = gt_to_tile(gt);
 
@@ -445,6 +446,32 @@ inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
 	return readb(tile->mmio.regs + reg.addr);
 }
 
+inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+	u8 val;
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	val = readb(tile->mmio.regs + reg.addr);
+
+	trace_xe_reg_rw(false, &reg, val, sizeof(val));
+
+	return val;
+}
+
+inline void xe_mmio_write32_notrace(struct xe_gt *gt,
+				    struct xe_reg reg, u32 val)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	writel(val, tile->mmio.regs + reg.addr);
+}
+
 inline void xe_mmio_write32(struct xe_gt *gt,
 			    struct xe_reg reg, u32 val)
 {
@@ -453,10 +480,12 @@ inline void xe_mmio_write32(struct xe_gt *gt,
 	if (reg.addr < gt->mmio.adj_limit)
 		reg.addr += gt->mmio.adj_offset;
 
+	trace_xe_reg_rw(true, &reg, val, sizeof(val));
+
 	writel(val, tile->mmio.regs + reg.addr);
 }
 
-inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
+inline u32 xe_mmio_read32_notrace(struct xe_gt *gt, struct xe_reg reg)
 {
 	struct xe_tile *tile = gt_to_tile(gt);
 
@@ -466,6 +495,21 @@ inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
 	return readl(tile->mmio.regs + reg.addr);
 }
 
+inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+	u32 val;
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	val = readl(tile->mmio.regs + reg.addr);
+
+	trace_xe_reg_rw(false, &reg, val, sizeof(val));
+
+	return val;
+}
+
 inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
 			 u32 set)
 {
@@ -478,6 +522,17 @@ inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
 	return old;
 }
 
+inline void xe_mmio_write64_notrace(struct xe_gt *gt,
+				    struct xe_reg reg, u64 val)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	writeq(val, tile->mmio.regs + reg.addr);
+}
+
 inline void xe_mmio_write64(struct xe_gt *gt,
 			    struct xe_reg reg, u64 val)
 {
@@ -486,10 +541,12 @@ inline void xe_mmio_write64(struct xe_gt *gt,
 	if (reg.addr < gt->mmio.adj_limit)
 		reg.addr += gt->mmio.adj_offset;
 
+	trace_xe_reg_rw(true, &reg, val, sizeof(val));
+
 	writeq(val, tile->mmio.regs + reg.addr);
 }
 
-inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
+inline u64 xe_mmio_read64_notrace(struct xe_gt *gt, struct xe_reg reg)
 {
 	struct xe_tile *tile = gt_to_tile(gt);
 
@@ -499,6 +556,21 @@ inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
 	return readq(tile->mmio.regs + reg.addr);
 }
 
+inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
+{
+	struct xe_tile *tile = gt_to_tile(gt);
+	u64 val;
+
+	if (reg.addr < gt->mmio.adj_limit)
+		reg.addr += gt->mmio.adj_offset;
+
+	val = readq(tile->mmio.regs + reg.addr);
+
+	trace_xe_reg_rw(false, &reg, val, sizeof(val));
+
+	return val;
+}
+
 inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
 				      struct xe_reg reg, u32 val,
 				      u32 mask, u32 eval)
@@ -522,7 +594,7 @@ inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
 	u32 read;
 
 	for (;;) {
-		read = xe_mmio_read32(gt, reg);
+		read = xe_mmio_read32_notrace(gt, reg);
 		if ((read & mask) == val) {
 			ret = 0;
 			break;
@@ -542,6 +614,8 @@ inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
 		wait <<= 1;
 	}
 
+	trace_xe_reg_rw(false, &reg, read, sizeof(read));
+
 	if (out_val)
 		*out_val = read;
 
diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
index 2aa2c01e60dd..25c7fa41f494 100644
--- a/drivers/gpu/drm/xe/xe_mmio.h
+++ b/drivers/gpu/drm/xe/xe_mmio.h
@@ -20,14 +20,21 @@ struct xe_device;
 #define GEN12_LMEM_BAR		2
 
 int xe_mmio_init(struct xe_device *xe);
+inline u8 xe_mmio_read8_notrace(struct xe_gt *gt, struct xe_reg reg);
 inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg);
+inline void xe_mmio_write32_notrace(struct xe_gt *gt,
+				    struct xe_reg reg, u32 val);
 inline void xe_mmio_write32(struct xe_gt *gt,
 			    struct xe_reg reg, u32 val);
+inline u32 xe_mmio_read32_notrace(struct xe_gt *gt, struct xe_reg reg);
 inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg);
 inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
 			 u32 set);
+inline void xe_mmio_write64_notrace(struct xe_gt *gt,
+				    struct xe_reg reg, u64 val);
 inline void xe_mmio_write64(struct xe_gt *gt,
 			    struct xe_reg reg, u64 val);
+inline u64 xe_mmio_read64_notrace(struct xe_gt *gt, struct xe_reg reg);
 inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg);
 inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
 				      struct xe_reg reg, u32 val,
diff --git a/drivers/gpu/drm/xe/xe_trace.h b/drivers/gpu/drm/xe/xe_trace.h
index 2f8eb7ebe9a7..a5d514c190a2 100644
--- a/drivers/gpu/drm/xe/xe_trace.h
+++ b/drivers/gpu/drm/xe/xe_trace.h
@@ -12,6 +12,7 @@
 #include <linux/tracepoint.h>
 #include <linux/types.h>
 
+#include "regs/xe_reg_defs.h"
 #include "xe_bo_types.h"
 #include "xe_engine_types.h"
 #include "xe_gt_tlb_invalidation_types.h"
@@ -507,6 +508,31 @@ DEFINE_EVENT(xe_vm, xe_vm_rebind_worker_exit,
 	     TP_ARGS(vm)
 );
 
+TRACE_EVENT(xe_reg_rw,
+	    TP_PROTO(bool write, struct xe_reg *reg, u64 val, int len),
+	    TP_ARGS(write, reg, val, len),
+
+	    TP_STRUCT__entry(
+		    __field(u64, val)
+		    __field(struct xe_reg *, reg)
+		    __field(u16, write)
+		    __field(u16, len)
+		    ),
+
+	    TP_fast_assign(
+		   __entry->val = (u64)val;
+		   __entry->reg = reg;
+		   __entry->write = write;
+		   __entry->len = len;
+		),
+
+	    TP_printk("%s reg=0x%x, len=%d, val=(0x%x, 0x%x)",
+		      __entry->write ? "write" : "read",
+		      __entry->reg->addr, __entry->len,
+		      (u32)(__entry->val & 0xffffffff),
+		      (u32)(__entry->val >> 32))
+);
+
 TRACE_EVENT(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),
-- 
2.34.1

[Intel-gfx] ✗ Fi.CI.BUILD: failure for Add mmio register rw tracing

[Patchwork]

== Series Details ==

Series: Add mmio register rw tracing
URL   : https://patchwork.freedesktop.org/series/119311/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/119311/revisions/1/mbox/ not applied
Applying: drm/xe: Move mmio read/write functions to xe_mmio.c
error: sha1 information is lacking or useless (drivers/gpu/drm/xe/xe_mmio.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/xe: Move mmio read/write functions to xe_mmio.c
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced

Re: [Intel-gfx] [RFC 1/2] drm/xe: Move mmio read/write functions to xe_mmio.c

[Matt Roper]

On Tue, Jun 13, 2023 at 05:13:14PM -0700, Radhakrishna Sripada wrote:
> Move the register read/write apis to xe_mmio.c to prepare for
> adding tracing infrastructure for the same. Adding tracing support
> in xe_mmio.h messes with the compilation of the display code.
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_mmio.c | 113 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_mmio.h | 129 ++++-------------------------------
>  2 files changed, 128 insertions(+), 114 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index 475b14fe4356..70ad1b6a17a0 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -435,6 +435,119 @@ static const struct xe_reg mmio_read_whitelist[] = {
>  	RING_TIMESTAMP(RENDER_RING_BASE),
>  };
>  
> +inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)

We shouldn't have 'inline' on non-static functions in a .c file (and
generally we don't really want it on static functions either since the
compiler can generally do a better job of figuring out whether or not
inlining would be beneficial).


Matt

> +{
> +	struct xe_tile *tile = gt_to_tile(gt);
> +
> +	if (reg.addr < gt->mmio.adj_limit)
> +		reg.addr += gt->mmio.adj_offset;
> +
> +	return readb(tile->mmio.regs + reg.addr);
> +}
> +
> +inline void xe_mmio_write32(struct xe_gt *gt,
> +			    struct xe_reg reg, u32 val)
> +{
> +	struct xe_tile *tile = gt_to_tile(gt);
> +
> +	if (reg.addr < gt->mmio.adj_limit)
> +		reg.addr += gt->mmio.adj_offset;
> +
> +	writel(val, tile->mmio.regs + reg.addr);
> +}
> +
> +inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
> +{
> +	struct xe_tile *tile = gt_to_tile(gt);
> +
> +	if (reg.addr < gt->mmio.adj_limit)
> +		reg.addr += gt->mmio.adj_offset;
> +
> +	return readl(tile->mmio.regs + reg.addr);
> +}
> +
> +inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
> +			 u32 set)
> +{
> +	u32 old, reg_val;
> +
> +	old = xe_mmio_read32(gt, reg);
> +	reg_val = (old & ~clr) | set;
> +	xe_mmio_write32(gt, reg, reg_val);
> +
> +	return old;
> +}
> +
> +inline void xe_mmio_write64(struct xe_gt *gt,
> +			    struct xe_reg reg, u64 val)
> +{
> +	struct xe_tile *tile = gt_to_tile(gt);
> +
> +	if (reg.addr < gt->mmio.adj_limit)
> +		reg.addr += gt->mmio.adj_offset;
> +
> +	writeq(val, tile->mmio.regs + reg.addr);
> +}
> +
> +inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
> +{
> +	struct xe_tile *tile = gt_to_tile(gt);
> +
> +	if (reg.addr < gt->mmio.adj_limit)
> +		reg.addr += gt->mmio.adj_offset;
> +
> +	return readq(tile->mmio.regs + reg.addr);
> +}
> +
> +inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
> +				      struct xe_reg reg, u32 val,
> +				      u32 mask, u32 eval)
> +{
> +	u32 reg_val;
> +
> +	xe_mmio_write32(gt, reg, val);
> +	reg_val = xe_mmio_read32(gt, reg);
> +
> +	return (reg_val & mask) != eval ? -EINVAL : 0;
> +}
> +
> +inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
> +			  u32 mask, u32 timeout_us, u32 *out_val,
> +			  bool atomic)
> +{
> +	ktime_t cur = ktime_get_raw();
> +	const ktime_t end = ktime_add_us(cur, timeout_us);
> +	int ret = -ETIMEDOUT;
> +	s64 wait = 10;
> +	u32 read;
> +
> +	for (;;) {
> +		read = xe_mmio_read32(gt, reg);
> +		if ((read & mask) == val) {
> +			ret = 0;
> +			break;
> +		}
> +
> +		cur = ktime_get_raw();
> +		if (!ktime_before(cur, end))
> +			break;
> +
> +		if (ktime_after(ktime_add_us(cur, wait), end))
> +			wait = ktime_us_delta(end, cur);
> +
> +		if (atomic)
> +			udelay(wait);
> +		else
> +			usleep_range(wait, wait << 1);
> +		wait <<= 1;
> +	}
> +
> +	if (out_val)
> +		*out_val = read;
> +
> +	return ret;
> +}
> +
>  int xe_mmio_ioctl(struct drm_device *dev, void *data,
>  		  struct drm_file *file)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
> index 3c547d78afba..2aa2c01e60dd 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.h
> +++ b/drivers/gpu/drm/xe/xe_mmio.h
> @@ -20,120 +20,21 @@ struct xe_device;
>  #define GEN12_LMEM_BAR		2
>  
>  int xe_mmio_init(struct xe_device *xe);
> -
> -static inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
> -{
> -	struct xe_tile *tile = gt_to_tile(gt);
> -
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	return readb(tile->mmio.regs + reg.addr);
> -}
> -
> -static inline void xe_mmio_write32(struct xe_gt *gt,
> -				   struct xe_reg reg, u32 val)
> -{
> -	struct xe_tile *tile = gt_to_tile(gt);
> -
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	writel(val, tile->mmio.regs + reg.addr);
> -}
> -
> -static inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
> -{
> -	struct xe_tile *tile = gt_to_tile(gt);
> -
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	return readl(tile->mmio.regs + reg.addr);
> -}
> -
> -static inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
> -				u32 set)
> -{
> -	u32 old, reg_val;
> -
> -	old = xe_mmio_read32(gt, reg);
> -	reg_val = (old & ~clr) | set;
> -	xe_mmio_write32(gt, reg, reg_val);
> -
> -	return old;
> -}
> -
> -static inline void xe_mmio_write64(struct xe_gt *gt,
> -				   struct xe_reg reg, u64 val)
> -{
> -	struct xe_tile *tile = gt_to_tile(gt);
> -
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	writeq(val, tile->mmio.regs + reg.addr);
> -}
> -
> -static inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
> -{
> -	struct xe_tile *tile = gt_to_tile(gt);
> -
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	return readq(tile->mmio.regs + reg.addr);
> -}
> -
> -static inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
> -					     struct xe_reg reg, u32 val,
> -					     u32 mask, u32 eval)
> -{
> -	u32 reg_val;
> -
> -	xe_mmio_write32(gt, reg, val);
> -	reg_val = xe_mmio_read32(gt, reg);
> -
> -	return (reg_val & mask) != eval ? -EINVAL : 0;
> -}
> -
> -static inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
> -				 u32 mask, u32 timeout_us, u32 *out_val,
> -				 bool atomic)
> -{
> -	ktime_t cur = ktime_get_raw();
> -	const ktime_t end = ktime_add_us(cur, timeout_us);
> -	int ret = -ETIMEDOUT;
> -	s64 wait = 10;
> -	u32 read;
> -
> -	for (;;) {
> -		read = xe_mmio_read32(gt, reg);
> -		if ((read & mask) == val) {
> -			ret = 0;
> -			break;
> -		}
> -
> -		cur = ktime_get_raw();
> -		if (!ktime_before(cur, end))
> -			break;
> -
> -		if (ktime_after(ktime_add_us(cur, wait), end))
> -			wait = ktime_us_delta(end, cur);
> -
> -		if (atomic)
> -			udelay(wait);
> -		else
> -			usleep_range(wait, wait << 1);
> -		wait <<= 1;
> -	}
> -
> -	if (out_val)
> -		*out_val = read;
> -
> -	return ret;
> -}
> -
> +inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg);
> +inline void xe_mmio_write32(struct xe_gt *gt,
> +			    struct xe_reg reg, u32 val);
> +inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg);
> +inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
> +			 u32 set);
> +inline void xe_mmio_write64(struct xe_gt *gt,
> +			    struct xe_reg reg, u64 val);
> +inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg);
> +inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
> +				      struct xe_reg reg, u32 val,
> +				      u32 mask, u32 eval);
> +inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
> +			  u32 mask, u32 timeout_us, u32 *out_val,
> +			  bool atomic);
>  int xe_mmio_ioctl(struct drm_device *dev, void *data,
>  		  struct drm_file *file);
>  
> -- 
> 2.34.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [Intel-gfx] [RFC 1/2] drm/xe: Move mmio read/write functions to xe_mmio.c

[Sripada, Radhakrishna]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper@intel.com>
> Sent: Tuesday, June 13, 2023 7:03 PM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; De Marchi,
> Lucas <lucas.demarchi@intel.com>
> Subject: Re: [RFC 1/2] drm/xe: Move mmio read/write functions to xe_mmio.c
> 
> On Tue, Jun 13, 2023 at 05:13:14PM -0700, Radhakrishna Sripada wrote:
> > Move the register read/write apis to xe_mmio.c to prepare for
> > adding tracing infrastructure for the same. Adding tracing support
> > in xe_mmio.h messes with the compilation of the display code.
> >
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_mmio.c | 113 ++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/xe/xe_mmio.h | 129 ++++-------------------------------
> >  2 files changed, 128 insertions(+), 114 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> > index 475b14fe4356..70ad1b6a17a0 100644
> > --- a/drivers/gpu/drm/xe/xe_mmio.c
> > +++ b/drivers/gpu/drm/xe/xe_mmio.c
> > @@ -435,6 +435,119 @@ static const struct xe_reg mmio_read_whitelist[] = {
> >  	RING_TIMESTAMP(RENDER_RING_BASE),
> >  };
> >
> > +inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
> 
> We shouldn't have 'inline' on non-static functions in a .c file (and
> generally we don't really want it on static functions either since the
> compiler can generally do a better job of figuring out whether or not
> inlining would be beneficial).
> 
Sure Matt. Will do that in next rev.

-Radhakrishna(RK) Sripada
> 
> Matt
> 
> > +{
> > +	struct xe_tile *tile = gt_to_tile(gt);
> > +
> > +	if (reg.addr < gt->mmio.adj_limit)
> > +		reg.addr += gt->mmio.adj_offset;
> > +
> > +	return readb(tile->mmio.regs + reg.addr);
> > +}
> > +
> > +inline void xe_mmio_write32(struct xe_gt *gt,
> > +			    struct xe_reg reg, u32 val)
> > +{
> > +	struct xe_tile *tile = gt_to_tile(gt);
> > +
> > +	if (reg.addr < gt->mmio.adj_limit)
> > +		reg.addr += gt->mmio.adj_offset;
> > +
> > +	writel(val, tile->mmio.regs + reg.addr);
> > +}
> > +
> > +inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
> > +{
> > +	struct xe_tile *tile = gt_to_tile(gt);
> > +
> > +	if (reg.addr < gt->mmio.adj_limit)
> > +		reg.addr += gt->mmio.adj_offset;
> > +
> > +	return readl(tile->mmio.regs + reg.addr);
> > +}
> > +
> > +inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
> > +			 u32 set)
> > +{
> > +	u32 old, reg_val;
> > +
> > +	old = xe_mmio_read32(gt, reg);
> > +	reg_val = (old & ~clr) | set;
> > +	xe_mmio_write32(gt, reg, reg_val);
> > +
> > +	return old;
> > +}
> > +
> > +inline void xe_mmio_write64(struct xe_gt *gt,
> > +			    struct xe_reg reg, u64 val)
> > +{
> > +	struct xe_tile *tile = gt_to_tile(gt);
> > +
> > +	if (reg.addr < gt->mmio.adj_limit)
> > +		reg.addr += gt->mmio.adj_offset;
> > +
> > +	writeq(val, tile->mmio.regs + reg.addr);
> > +}
> > +
> > +inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
> > +{
> > +	struct xe_tile *tile = gt_to_tile(gt);
> > +
> > +	if (reg.addr < gt->mmio.adj_limit)
> > +		reg.addr += gt->mmio.adj_offset;
> > +
> > +	return readq(tile->mmio.regs + reg.addr);
> > +}
> > +
> > +inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
> > +				      struct xe_reg reg, u32 val,
> > +				      u32 mask, u32 eval)
> > +{
> > +	u32 reg_val;
> > +
> > +	xe_mmio_write32(gt, reg, val);
> > +	reg_val = xe_mmio_read32(gt, reg);
> > +
> > +	return (reg_val & mask) != eval ? -EINVAL : 0;
> > +}
> > +
> > +inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
> > +			  u32 mask, u32 timeout_us, u32 *out_val,
> > +			  bool atomic)
> > +{
> > +	ktime_t cur = ktime_get_raw();
> > +	const ktime_t end = ktime_add_us(cur, timeout_us);
> > +	int ret = -ETIMEDOUT;
> > +	s64 wait = 10;
> > +	u32 read;
> > +
> > +	for (;;) {
> > +		read = xe_mmio_read32(gt, reg);
> > +		if ((read & mask) == val) {
> > +			ret = 0;
> > +			break;
> > +		}
> > +
> > +		cur = ktime_get_raw();
> > +		if (!ktime_before(cur, end))
> > +			break;
> > +
> > +		if (ktime_after(ktime_add_us(cur, wait), end))
> > +			wait = ktime_us_delta(end, cur);
> > +
> > +		if (atomic)
> > +			udelay(wait);
> > +		else
> > +			usleep_range(wait, wait << 1);
> > +		wait <<= 1;
> > +	}
> > +
> > +	if (out_val)
> > +		*out_val = read;
> > +
> > +	return ret;
> > +}
> > +
> >  int xe_mmio_ioctl(struct drm_device *dev, void *data,
> >  		  struct drm_file *file)
> >  {
> > diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
> > index 3c547d78afba..2aa2c01e60dd 100644
> > --- a/drivers/gpu/drm/xe/xe_mmio.h
> > +++ b/drivers/gpu/drm/xe/xe_mmio.h
> > @@ -20,120 +20,21 @@ struct xe_device;
> >  #define GEN12_LMEM_BAR		2
> >
> >  int xe_mmio_init(struct xe_device *xe);
> > -
> > -static inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
> > -{
> > -	struct xe_tile *tile = gt_to_tile(gt);
> > -
> > -	if (reg.addr < gt->mmio.adj_limit)
> > -		reg.addr += gt->mmio.adj_offset;
> > -
> > -	return readb(tile->mmio.regs + reg.addr);
> > -}
> > -
> > -static inline void xe_mmio_write32(struct xe_gt *gt,
> > -				   struct xe_reg reg, u32 val)
> > -{
> > -	struct xe_tile *tile = gt_to_tile(gt);
> > -
> > -	if (reg.addr < gt->mmio.adj_limit)
> > -		reg.addr += gt->mmio.adj_offset;
> > -
> > -	writel(val, tile->mmio.regs + reg.addr);
> > -}
> > -
> > -static inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
> > -{
> > -	struct xe_tile *tile = gt_to_tile(gt);
> > -
> > -	if (reg.addr < gt->mmio.adj_limit)
> > -		reg.addr += gt->mmio.adj_offset;
> > -
> > -	return readl(tile->mmio.regs + reg.addr);
> > -}
> > -
> > -static inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
> > -				u32 set)
> > -{
> > -	u32 old, reg_val;
> > -
> > -	old = xe_mmio_read32(gt, reg);
> > -	reg_val = (old & ~clr) | set;
> > -	xe_mmio_write32(gt, reg, reg_val);
> > -
> > -	return old;
> > -}
> > -
> > -static inline void xe_mmio_write64(struct xe_gt *gt,
> > -				   struct xe_reg reg, u64 val)
> > -{
> > -	struct xe_tile *tile = gt_to_tile(gt);
> > -
> > -	if (reg.addr < gt->mmio.adj_limit)
> > -		reg.addr += gt->mmio.adj_offset;
> > -
> > -	writeq(val, tile->mmio.regs + reg.addr);
> > -}
> > -
> > -static inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
> > -{
> > -	struct xe_tile *tile = gt_to_tile(gt);
> > -
> > -	if (reg.addr < gt->mmio.adj_limit)
> > -		reg.addr += gt->mmio.adj_offset;
> > -
> > -	return readq(tile->mmio.regs + reg.addr);
> > -}
> > -
> > -static inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
> > -					     struct xe_reg reg, u32 val,
> > -					     u32 mask, u32 eval)
> > -{
> > -	u32 reg_val;
> > -
> > -	xe_mmio_write32(gt, reg, val);
> > -	reg_val = xe_mmio_read32(gt, reg);
> > -
> > -	return (reg_val & mask) != eval ? -EINVAL : 0;
> > -}
> > -
> > -static inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
> > -				 u32 mask, u32 timeout_us, u32 *out_val,
> > -				 bool atomic)
> > -{
> > -	ktime_t cur = ktime_get_raw();
> > -	const ktime_t end = ktime_add_us(cur, timeout_us);
> > -	int ret = -ETIMEDOUT;
> > -	s64 wait = 10;
> > -	u32 read;
> > -
> > -	for (;;) {
> > -		read = xe_mmio_read32(gt, reg);
> > -		if ((read & mask) == val) {
> > -			ret = 0;
> > -			break;
> > -		}
> > -
> > -		cur = ktime_get_raw();
> > -		if (!ktime_before(cur, end))
> > -			break;
> > -
> > -		if (ktime_after(ktime_add_us(cur, wait), end))
> > -			wait = ktime_us_delta(end, cur);
> > -
> > -		if (atomic)
> > -			udelay(wait);
> > -		else
> > -			usleep_range(wait, wait << 1);
> > -		wait <<= 1;
> > -	}
> > -
> > -	if (out_val)
> > -		*out_val = read;
> > -
> > -	return ret;
> > -}
> > -
> > +inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg);
> > +inline void xe_mmio_write32(struct xe_gt *gt,
> > +			    struct xe_reg reg, u32 val);
> > +inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg);
> > +inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
> > +			 u32 set);
> > +inline void xe_mmio_write64(struct xe_gt *gt,
> > +			    struct xe_reg reg, u64 val);
> > +inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg);
> > +inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
> > +				      struct xe_reg reg, u32 val,
> > +				      u32 mask, u32 eval);
> > +inline int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 val,
> > +			  u32 mask, u32 timeout_us, u32 *out_val,
> > +			  bool atomic);
> >  int xe_mmio_ioctl(struct drm_device *dev, void *data,
> >  		  struct drm_file *file);
> >
> > --
> > 2.34.1
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation