[PATCH v3] drm/i915:move and rename reg_in_range

Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v3] drm/i915:move and rename reg_in_range_table
@ 2025-10-09 21:52 Matt Atwood
  2025-10-10  0:07 ` ✗ i915.CI.BAT: failure for drm/i915:move and rename reg_in_range_table (rev2) Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Matt Atwood @ 2025-10-09 21:52 UTC (permalink / raw)
  To: intel-gfx, matthew.d.roper
  Cc: rodrigo.vivi, jani.nikula, andi.shyti, Matt Atwood

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 16296 bytes --]

reg_in_range_table is a useful function that is used in multiple places,
and will be needed for WA_BB implementation later.

Let's move this function and i915_range struct to its own file, as we are
trying to move away from i915_utils files.

v2: move functions to their own file
v3: use correct naming convention

Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |  1 +
 drivers/gpu/drm/i915/gt/intel_workarounds.c   |  9 +--
 drivers/gpu/drm/i915/i915_mmio_range.c        | 18 +++++
 drivers/gpu/drm/i915/i915_mmio_range.h        | 19 ++++++
 drivers/gpu/drm/i915/i915_perf.c              | 67 ++++++++-----------
 drivers/gpu/drm/i915/intel_uncore.c           | 15 +++--
 drivers/gpu/drm/i915/intel_uncore.h           |  8 +--
 drivers/gpu/drm/i915/selftests/intel_uncore.c |  4 +-
 8 files changed, 82 insertions(+), 59 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_mmio_range.c
 create mode 100644 drivers/gpu/drm/i915/i915_mmio_range.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 78a45a6681df..e5819c4320bf 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -26,6 +26,7 @@ i915-y += \
 	i915_ioctl.o \
 	i915_irq.o \
 	i915_mitigations.o \
+	i915_mmio_range.o \
 	i915_module.o \
 	i915_params.o \
 	i915_pci.o \
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 7d486dfa2fc1..ece88c612e27 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -5,6 +5,7 @@
 
 #include "i915_drv.h"
 #include "i915_reg.h"
+#include "i915_mmio_range.h"
 #include "intel_context.h"
 #include "intel_engine_pm.h"
 #include "intel_engine_regs.h"
@@ -2923,7 +2924,7 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine)
 	wa_list_apply(&engine->wa_list);
 }
 
-static const struct i915_range mcr_ranges_gen8[] = {
+static const struct i915_mmio_range mcr_ranges_gen8[] = {
 	{ .start = 0x5500, .end = 0x55ff },
 	{ .start = 0x7000, .end = 0x7fff },
 	{ .start = 0x9400, .end = 0x97ff },
@@ -2932,7 +2933,7 @@ static const struct i915_range mcr_ranges_gen8[] = {
 	{},
 };
 
-static const struct i915_range mcr_ranges_gen12[] = {
+static const struct i915_mmio_range mcr_ranges_gen12[] = {
 	{ .start =  0x8150, .end =  0x815f },
 	{ .start =  0x9520, .end =  0x955f },
 	{ .start =  0xb100, .end =  0xb3ff },
@@ -2941,7 +2942,7 @@ static const struct i915_range mcr_ranges_gen12[] = {
 	{},
 };
 
-static const struct i915_range mcr_ranges_xehp[] = {
+static const struct i915_mmio_range mcr_ranges_xehp[] = {
 	{ .start =  0x4000, .end =  0x4aff },
 	{ .start =  0x5200, .end =  0x52ff },
 	{ .start =  0x5400, .end =  0x7fff },
@@ -2960,7 +2961,7 @@ static const struct i915_range mcr_ranges_xehp[] = {
 
 static bool mcr_range(struct drm_i915_private *i915, u32 offset)
 {
-	const struct i915_range *mcr_ranges;
+	const struct i915_mmio_range *mcr_ranges;
 	int i;
 
 	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55))
diff --git a/drivers/gpu/drm/i915/i915_mmio_range.c b/drivers/gpu/drm/i915/i915_mmio_range.c
new file mode 100644
index 000000000000..724041e81aa7
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_mmio_range.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#include "i915_mmio_range.h"
+
+bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table)
+{
+	while (table->start || table->end) {
+		if (addr >= table->start && addr <= table->end)
+			return true;
+
+		table++;
+	}
+
+	return false;
+}
diff --git a/drivers/gpu/drm/i915/i915_mmio_range.h b/drivers/gpu/drm/i915/i915_mmio_range.h
new file mode 100644
index 000000000000..f1c7086d3e3c
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_mmio_range.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#ifndef __I915_MMIO_RANGE_H__
+#define __I915_MMIO_RANGE_H__
+
+#include <linux/types.h>
+
+/* Other register ranges (e.g., shadow tables, MCR tables, etc.) */
+struct i915_mmio_range {
+	u32 start;
+	u32 end;
+};
+
+bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table);
+
+#endif /* __I915_MMIO_RANGE_H__ */
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 1658f1246c6f..0b9d9f3f7813 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -219,6 +219,7 @@
 #include "i915_perf.h"
 #include "i915_perf_oa_regs.h"
 #include "i915_reg.h"
+#include "i915_mmio_range.h"
 
 /* HW requires this to be a power of two, between 128k and 16M, though driver
  * is currently generally designed assuming the largest 16M size is used such
@@ -4320,29 +4321,17 @@ static bool gen8_is_valid_flex_addr(struct i915_perf *perf, u32 addr)
 	return false;
 }
 
-static bool reg_in_range_table(u32 addr, const struct i915_range *table)
-{
-	while (table->start || table->end) {
-		if (addr >= table->start && addr <= table->end)
-			return true;
-
-		table++;
-	}
-
-	return false;
-}
-
 #define REG_EQUAL(addr, mmio) \
 	((addr) == i915_mmio_reg_offset(mmio))
 
-static const struct i915_range gen7_oa_b_counters[] = {
+static const struct i915_mmio_range gen7_oa_b_counters[] = {
 	{ .start = 0x2710, .end = 0x272c },	/* OASTARTTRIG[1-8] */
 	{ .start = 0x2740, .end = 0x275c },	/* OAREPORTTRIG[1-8] */
 	{ .start = 0x2770, .end = 0x27ac },	/* OACEC[0-7][0-1] */
 	{}
 };
 
-static const struct i915_range gen12_oa_b_counters[] = {
+static const struct i915_mmio_range gen12_oa_b_counters[] = {
 	{ .start = 0x2b2c, .end = 0x2b2c },	/* GEN12_OAG_OA_PESS */
 	{ .start = 0xd900, .end = 0xd91c },	/* GEN12_OAG_OASTARTTRIG[1-8] */
 	{ .start = 0xd920, .end = 0xd93c },	/* GEN12_OAG_OAREPORTTRIG1[1-8] */
@@ -4353,7 +4342,7 @@ static const struct i915_range gen12_oa_b_counters[] = {
 	{}
 };
 
-static const struct i915_range mtl_oam_b_counters[] = {
+static const struct i915_mmio_range mtl_oam_b_counters[] = {
 	{ .start = 0x393000, .end = 0x39301c },	/* GEN12_OAM_STARTTRIG1[1-8] */
 	{ .start = 0x393020, .end = 0x39303c },	/* GEN12_OAM_REPORTTRIG1[1-8] */
 	{ .start = 0x393040, .end = 0x39307c },	/* GEN12_OAM_CEC[0-7][0-1] */
@@ -4361,43 +4350,43 @@ static const struct i915_range mtl_oam_b_counters[] = {
 	{}
 };
 
-static const struct i915_range xehp_oa_b_counters[] = {
+static const struct i915_mmio_range xehp_oa_b_counters[] = {
 	{ .start = 0xdc48, .end = 0xdc48 },	/* OAA_ENABLE_REG */
 	{ .start = 0xdd00, .end = 0xdd48 },	/* OAG_LCE0_0 - OAA_LENABLE_REG */
 	{}
 };
 
-static const struct i915_range gen7_oa_mux_regs[] = {
+static const struct i915_mmio_range gen7_oa_mux_regs[] = {
 	{ .start = 0x91b8, .end = 0x91cc },	/* OA_PERFCNT[1-2], OA_PERFMATRIX */
 	{ .start = 0x9800, .end = 0x9888 },	/* MICRO_BP0_0 - NOA_WRITE */
 	{ .start = 0xe180, .end = 0xe180 },	/* HALF_SLICE_CHICKEN2 */
 	{}
 };
 
-static const struct i915_range hsw_oa_mux_regs[] = {
+static const struct i915_mmio_range hsw_oa_mux_regs[] = {
 	{ .start = 0x09e80, .end = 0x09ea4 }, /* HSW_MBVID2_NOA[0-9] */
 	{ .start = 0x09ec0, .end = 0x09ec0 }, /* HSW_MBVID2_MISR0 */
 	{ .start = 0x25100, .end = 0x2ff90 },
 	{}
 };
 
-static const struct i915_range chv_oa_mux_regs[] = {
+static const struct i915_mmio_range chv_oa_mux_regs[] = {
 	{ .start = 0x182300, .end = 0x1823a4 },
 	{}
 };
 
-static const struct i915_range gen8_oa_mux_regs[] = {
+static const struct i915_mmio_range gen8_oa_mux_regs[] = {
 	{ .start = 0x0d00, .end = 0x0d2c },	/* RPM_CONFIG[0-1], NOA_CONFIG[0-8] */
 	{ .start = 0x20cc, .end = 0x20cc },	/* WAIT_FOR_RC6_EXIT */
 	{}
 };
 
-static const struct i915_range gen11_oa_mux_regs[] = {
+static const struct i915_mmio_range gen11_oa_mux_regs[] = {
 	{ .start = 0x91c8, .end = 0x91dc },	/* OA_PERFCNT[3-4] */
 	{}
 };
 
-static const struct i915_range gen12_oa_mux_regs[] = {
+static const struct i915_mmio_range gen12_oa_mux_regs[] = {
 	{ .start = 0x0d00, .end = 0x0d04 },     /* RPM_CONFIG[0-1] */
 	{ .start = 0x0d0c, .end = 0x0d2c },     /* NOA_CONFIG[0-8] */
 	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
@@ -4410,7 +4399,7 @@ static const struct i915_range gen12_oa_mux_regs[] = {
  * Ref: 14010536224:
  * 0x20cc is repurposed on MTL, so use a separate array for MTL.
  */
-static const struct i915_range mtl_oa_mux_regs[] = {
+static const struct i915_mmio_range mtl_oa_mux_regs[] = {
 	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
 	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
 	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
@@ -4421,61 +4410,61 @@ static const struct i915_range mtl_oa_mux_regs[] = {
 
 static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
 {
-	return reg_in_range_table(addr, gen7_oa_b_counters);
+	return i915_mmio_range_table_contains(addr, gen7_oa_b_counters);
 }
 
 static bool gen8_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
 {
-	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
-		reg_in_range_table(addr, gen8_oa_mux_regs);
+	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
+		i915_mmio_range_table_contains(addr, gen8_oa_mux_regs);
 }
 
 static bool gen11_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
 {
-	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
-		reg_in_range_table(addr, gen8_oa_mux_regs) ||
-		reg_in_range_table(addr, gen11_oa_mux_regs);
+	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
+		i915_mmio_range_table_contains(addr, gen8_oa_mux_regs) ||
+		i915_mmio_range_table_contains(addr, gen11_oa_mux_regs);
 }
 
 static bool hsw_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
 {
-	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
-		reg_in_range_table(addr, hsw_oa_mux_regs);
+	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
+		i915_mmio_range_table_contains(addr, hsw_oa_mux_regs);
 }
 
 static bool chv_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
 {
-	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
-		reg_in_range_table(addr, chv_oa_mux_regs);
+	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
+		i915_mmio_range_table_contains(addr, chv_oa_mux_regs);
 }
 
 static bool gen12_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
 {
-	return reg_in_range_table(addr, gen12_oa_b_counters);
+	return i915_mmio_range_table_contains(addr, gen12_oa_b_counters);
 }
 
 static bool mtl_is_valid_oam_b_counter_addr(struct i915_perf *perf, u32 addr)
 {
 	if (HAS_OAM(perf->i915) &&
 	    GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 70))
-		return reg_in_range_table(addr, mtl_oam_b_counters);
+		return i915_mmio_range_table_contains(addr, mtl_oam_b_counters);
 
 	return false;
 }
 
 static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
 {
-	return reg_in_range_table(addr, xehp_oa_b_counters) ||
-		reg_in_range_table(addr, gen12_oa_b_counters) ||
+	return i915_mmio_range_table_contains(addr, xehp_oa_b_counters) ||
+		i915_mmio_range_table_contains(addr, gen12_oa_b_counters) ||
 		mtl_is_valid_oam_b_counter_addr(perf, addr);
 }
 
 static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
 {
 	if (GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 70))
-		return reg_in_range_table(addr, mtl_oa_mux_regs);
+		return i915_mmio_range_table_contains(addr, mtl_oa_mux_regs);
 	else
-		return reg_in_range_table(addr, gen12_oa_mux_regs);
+		return i915_mmio_range_table_contains(addr, gen12_oa_mux_regs);
 }
 
 static u32 mask_reg_value(u32 reg, u32 val)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 8cb59f8d1f4c..aba90b80854f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -35,6 +35,7 @@
 #include "i915_reg.h"
 #include "i915_vgpu.h"
 #include "i915_wait_util.h"
+#include "i915_mmio_range.h"
 #include "intel_uncore_trace.h"
 
 #define FORCEWAKE_ACK_TIMEOUT_MS 50
@@ -999,7 +1000,7 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
  * scanned for obvious mistakes or typos by the selftests.
  */
 
-static const struct i915_range gen8_shadowed_regs[] = {
+static const struct i915_mmio_range gen8_shadowed_regs[] = {
 	{ .start =  0x2030, .end =  0x2030 },
 	{ .start =  0xA008, .end =  0xA00C },
 	{ .start = 0x12030, .end = 0x12030 },
@@ -1007,7 +1008,7 @@ static const struct i915_range gen8_shadowed_regs[] = {
 	{ .start = 0x22030, .end = 0x22030 },
 };
 
-static const struct i915_range gen11_shadowed_regs[] = {
+static const struct i915_mmio_range gen11_shadowed_regs[] = {
 	{ .start =   0x2030, .end =   0x2030 },
 	{ .start =   0x2550, .end =   0x2550 },
 	{ .start =   0xA008, .end =   0xA00C },
@@ -1034,7 +1035,7 @@ static const struct i915_range gen11_shadowed_regs[] = {
 	{ .start = 0x1D8510, .end = 0x1D8550 },
 };
 
-static const struct i915_range gen12_shadowed_regs[] = {
+static const struct i915_mmio_range gen12_shadowed_regs[] = {
 	{ .start =   0x2030, .end =   0x2030 },
 	{ .start =   0x2510, .end =   0x2550 },
 	{ .start =   0xA008, .end =   0xA00C },
@@ -1078,7 +1079,7 @@ static const struct i915_range gen12_shadowed_regs[] = {
 	{ .start = 0x1F8510, .end = 0x1F8550 },
 };
 
-static const struct i915_range dg2_shadowed_regs[] = {
+static const struct i915_mmio_range dg2_shadowed_regs[] = {
 	{ .start =   0x2030, .end =   0x2030 },
 	{ .start =   0x2510, .end =   0x2550 },
 	{ .start =   0xA008, .end =   0xA00C },
@@ -1117,7 +1118,7 @@ static const struct i915_range dg2_shadowed_regs[] = {
 	{ .start = 0x1F8510, .end = 0x1F8550 },
 };
 
-static const struct i915_range mtl_shadowed_regs[] = {
+static const struct i915_mmio_range mtl_shadowed_regs[] = {
 	{ .start =   0x2030, .end =   0x2030 },
 	{ .start =   0x2510, .end =   0x2550 },
 	{ .start =   0xA008, .end =   0xA00C },
@@ -1135,7 +1136,7 @@ static const struct i915_range mtl_shadowed_regs[] = {
 	{ .start =  0x22510, .end =  0x22550 },
 };
 
-static const struct i915_range xelpmp_shadowed_regs[] = {
+static const struct i915_mmio_range xelpmp_shadowed_regs[] = {
 	{ .start = 0x1C0030, .end = 0x1C0030 },
 	{ .start = 0x1C0510, .end = 0x1C0550 },
 	{ .start = 0x1C8030, .end = 0x1C8030 },
@@ -1156,7 +1157,7 @@ static const struct i915_range xelpmp_shadowed_regs[] = {
 	{ .start = 0x38CFD4, .end = 0x38CFDC },
 };
 
-static int mmio_range_cmp(u32 key, const struct i915_range *range)
+static int mmio_range_cmp(u32 key, const struct i915_mmio_range *range)
 {
 	if (key < range->start)
 		return -1;
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 6048b99b96cb..fafc2ca9a237 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -123,12 +123,6 @@ struct intel_forcewake_range {
 	enum forcewake_domains domains;
 };
 
-/* Other register ranges (e.g., shadow tables, MCR tables, etc.) */
-struct i915_range {
-	u32 start;
-	u32 end;
-};
-
 struct intel_uncore {
 	void __iomem *regs;
 
@@ -162,7 +156,7 @@ struct intel_uncore {
 	 * Shadowed registers are special cases where we can safely write
 	 * to the register *without* grabbing forcewake.
 	 */
-	const struct i915_range *shadowed_reg_table;
+	const struct i915_mmio_range *shadowed_reg_table;
 	unsigned int shadowed_reg_table_entries;
 
 	struct notifier_block pmic_bus_access_nb;
diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c
index 58bcbdcef563..507bf42a1aaf 100644
--- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
+++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
@@ -64,7 +64,7 @@ static int intel_fw_table_check(const struct intel_forcewake_range *ranges,
 static int intel_shadow_table_check(void)
 {
 	struct {
-		const struct i915_range *regs;
+		const struct i915_mmio_range *regs;
 		unsigned int size;
 	} range_lists[] = {
 		{ gen8_shadowed_regs, ARRAY_SIZE(gen8_shadowed_regs) },
@@ -74,7 +74,7 @@ static int intel_shadow_table_check(void)
 		{ mtl_shadowed_regs, ARRAY_SIZE(mtl_shadowed_regs) },
 		{ xelpmp_shadowed_regs, ARRAY_SIZE(xelpmp_shadowed_regs) },
 	};
-	const struct i915_range *range;
+	const struct i915_mmio_range *range;
 	unsigned int i, j;
 	s32 prev;
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* ✗ i915.CI.BAT: failure for drm/i915:move and rename reg_in_range_table (rev2)
  2025-10-09 21:52 [PATCH v3] drm/i915:move and rename reg_in_range_table Matt Atwood
@ 2025-10-10  0:07 ` Patchwork
  2025-10-10  9:55 ` [PATCH v3] drm/i915:move and rename reg_in_range_table Jani Nikula
  2025-10-10 13:07 ` Ville Syrjälä
  2 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2025-10-10  0:07 UTC (permalink / raw)
  To: Matt Atwood; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 4255 bytes --]

== Series Details ==

Series: drm/i915:move and rename reg_in_range_table (rev2)
URL   : https://patchwork.freedesktop.org/series/155556/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_17335 -> Patchwork_155556v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_155556v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_155556v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/index.html

Participating hosts (44 -> 43)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_155556v2:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_fence@basic-await@bcs0:
    - bat-twl-1:          [PASS][1] -> [FAIL][2] +1 other test fail
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17335/bat-twl-1/igt@gem_exec_fence@basic-await@bcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/bat-twl-1/igt@gem_exec_fence@basic-await@bcs0.html

  
Known issues
------------

  Here are the changes found in Patchwork_155556v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [PASS][3] -> [DMESG-FAIL][4] ([i915#12061]) +1 other test dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17335/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/bat-arls-5/igt@i915_selftest@live@workarounds.html
    - bat-mtlp-6:         [PASS][5] -> [DMESG-FAIL][6] ([i915#12061]) +1 other test dmesg-fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17335/bat-mtlp-6/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/bat-mtlp-6/igt@i915_selftest@live@workarounds.html
    - bat-dg2-9:          [PASS][7] -> [DMESG-FAIL][8] ([i915#12061]) +1 other test dmesg-fail
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17335/bat-dg2-9/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/bat-dg2-9/igt@i915_selftest@live@workarounds.html
    - bat-arls-6:         [PASS][9] -> [DMESG-FAIL][10] ([i915#12061]) +1 other test dmesg-fail
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17335/bat-arls-6/igt@i915_selftest@live@workarounds.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/bat-arls-6/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@dmabuf@all-tests@dma_fence_chain:
    - fi-bsw-n3050:       [ABORT][11] ([i915#12904]) -> [PASS][12] +1 other test pass
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17335/fi-bsw-n3050/igt@dmabuf@all-tests@dma_fence_chain.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/fi-bsw-n3050/igt@dmabuf@all-tests@dma_fence_chain.html

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-9:         [DMESG-FAIL][13] ([i915#12061]) -> [PASS][14] +1 other test pass
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17335/bat-mtlp-9/igt@i915_selftest@live@workarounds.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/bat-mtlp-9/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12904]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12904


Build changes
-------------

  * Linux: CI_DRM_17335 -> Patchwork_155556v2

  CI-20190529: 20190529
  CI_DRM_17335: ac65b2261663d50cc761c94a40b6093ee714e62f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8581: 8581
  Patchwork_155556v2: ac65b2261663d50cc761c94a40b6093ee714e62f @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155556v2/index.html

[-- Attachment #2: Type: text/html, Size: 5257 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-09 21:52 [PATCH v3] drm/i915:move and rename reg_in_range_table Matt Atwood
  2025-10-10  0:07 ` ✗ i915.CI.BAT: failure for drm/i915:move and rename reg_in_range_table (rev2) Patchwork
@ 2025-10-10  9:55 ` Jani Nikula
  2025-10-10 13:19   ` Rodrigo Vivi
  2025-10-10 13:07 ` Ville Syrjälä
  2 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2025-10-10  9:55 UTC (permalink / raw)
  To: Matt Atwood, intel-gfx, matthew.d.roper
  Cc: rodrigo.vivi, andi.shyti, Matt Atwood

On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> reg_in_range_table is a useful function that is used in multiple places,
> and will be needed for WA_BB implementation later.
>
> Let's move this function and i915_range struct to its own file, as we are
> trying to move away from i915_utils files.
>
> v2: move functions to their own file
> v3: use correct naming convention

Okay, Message from the Department of Bikeshedding and Nitpicking.

There's really nothing mmio specific about the functionality being
abstracted. You have a range represented by two u32's in a struct, and a
function to check if another u32 is within that range.

The struct could just as well remain i915_range, the files could be
i915_range.[ch], and the function could be, say,
i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.

> +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table)

Usually, the "context" parameter goes first. I get that this wasn't the
case before, but I'd use the opportunity to swap these around.

BR,
Jani.

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-10  9:55 ` [PATCH v3] drm/i915:move and rename reg_in_range_table Jani Nikula
@ 2025-10-10 13:19   ` Rodrigo Vivi
  2025-10-10 21:26     ` Matt Atwood
  2025-10-13 23:09     ` Andi Shyti
  0 siblings, 2 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2025-10-10 13:19 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Matt Atwood, intel-gfx, matthew.d.roper, andi.shyti

On Fri, Oct 10, 2025 at 12:55:02PM +0300, Jani Nikula wrote:
> On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > reg_in_range_table is a useful function that is used in multiple places,
> > and will be needed for WA_BB implementation later.
> >
> > Let's move this function and i915_range struct to its own file, as we are
> > trying to move away from i915_utils files.
> >
> > v2: move functions to their own file
> > v3: use correct naming convention
> 
> Okay, Message from the Department of Bikeshedding and Nitpicking.
> 
> There's really nothing mmio specific about the functionality being
> abstracted. You have a range represented by two u32's in a struct, and a
> function to check if another u32 is within that range.
> 
> The struct could just as well remain i915_range, the files could be
> i915_range.[ch], and the function could be, say,
> i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.

hmm, I'm really sorry about that... That is my bad. I'm so bad with naming.

I suggested mmio in the name because i915_range is way to generic.
The other extreme side.

Perhaps i915_addr_range ?

But I would be okay if the consensus is simply i915_range.

> 
> > +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table)
> 
> Usually, the "context" parameter goes first. I get that this wasn't the
> case before, but I'd use the opportunity to swap these around.

I just had the same feeling while reading this patch again.
Specially because it phrases like table contain ... table first contain last...

Sorry for not noticing it before as well.

But I was on the fence on this one because it was already like that addr,range
and the other range infra that we consider also uses the style addr,range.

> 
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-10 13:19   ` Rodrigo Vivi
@ 2025-10-10 21:26     ` Matt Atwood
  2025-10-13 23:09     ` Andi Shyti
  1 sibling, 0 replies; 13+ messages in thread
From: Matt Atwood @ 2025-10-10 21:26 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Jani Nikula, intel-gfx, matthew.d.roper, andi.shyti

On Fri, Oct 10, 2025 at 09:19:24AM -0400, Rodrigo Vivi wrote:
> On Fri, Oct 10, 2025 at 12:55:02PM +0300, Jani Nikula wrote:
> > On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > > reg_in_range_table is a useful function that is used in multiple places,
> > > and will be needed for WA_BB implementation later.
> > >
> > > Let's move this function and i915_range struct to its own file, as we are
> > > trying to move away from i915_utils files.
> > >
> > > v2: move functions to their own file
> > > v3: use correct naming convention
> > 
> > Okay, Message from the Department of Bikeshedding and Nitpicking.
> > 
> > There's really nothing mmio specific about the functionality being
> > abstracted. You have a range represented by two u32's in a struct, and a
> > function to check if another u32 is within that range.
> > 
> > The struct could just as well remain i915_range, the files could be
> > i915_range.[ch], and the function could be, say,
> > i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.
I think this would best fwiw.
> 
> hmm, I'm really sorry about that... That is my bad. I'm so bad with naming.
> 
> I suggested mmio in the name because i915_range is way to generic.
> The other extreme side.
> 
> Perhaps i915_addr_range ?
> 
> But I would be okay if the consensus is simply i915_range. 
> 
> > 
> > > +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table)
> > 
> > Usually, the "context" parameter goes first. I get that this wasn't the
> > case before, but I'd use the opportunity to swap these around.
> 
> I just had the same feeling while reading this patch again.
> Specially because it phrases like table contain ... table first contain last...
Ack.
> 
> Sorry for not noticing it before as well.
> 
> But I was on the fence on this one because it was already like that addr,range
> and the other range infra that we consider also uses the style addr,range.
> 
> > 
> > 
> > BR,
> > Jani.
> > 
> > 
> > -- 
> > Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-10 13:19   ` Rodrigo Vivi
  2025-10-10 21:26     ` Matt Atwood
@ 2025-10-13 23:09     ` Andi Shyti
  2025-10-16 12:19       ` Rodrigo Vivi
  1 sibling, 1 reply; 13+ messages in thread
From: Andi Shyti @ 2025-10-13 23:09 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Jani Nikula, Matt Atwood, intel-gfx, matthew.d.roper

Hi,

On Fri, Oct 10, 2025 at 09:19:24AM -0400, Rodrigo Vivi wrote:
> On Fri, Oct 10, 2025 at 12:55:02PM +0300, Jani Nikula wrote:
> > On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > > reg_in_range_table is a useful function that is used in multiple places,
> > > and will be needed for WA_BB implementation later.
> > >
> > > Let's move this function and i915_range struct to its own file, as we are
> > > trying to move away from i915_utils files.
> > >
> > > v2: move functions to their own file
> > > v3: use correct naming convention
> > 
> > Okay, Message from the Department of Bikeshedding and Nitpicking.
> > 
> > There's really nothing mmio specific about the functionality being
> > abstracted. You have a range represented by two u32's in a struct, and a
> > function to check if another u32 is within that range.
> > 
> > The struct could just as well remain i915_range, the files could be
> > i915_range.[ch], and the function could be, say,
> > i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.
> 
> hmm, I'm really sorry about that... That is my bad. I'm so bad with naming.
> 
> I suggested mmio in the name because i915_range is way to generic.
> The other extreme side.
> 
> Perhaps i915_addr_range ?

If we use it only for mmio, why should we make it generic? If we
want to keep things generic we could well use things from in
range.h, as Jani has suggested in one of his reviews and add our
function directly there.

Andi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-13 23:09     ` Andi Shyti
@ 2025-10-16 12:19       ` Rodrigo Vivi
  2025-10-16 15:20         ` Jani Nikula
  0 siblings, 1 reply; 13+ messages in thread
From: Rodrigo Vivi @ 2025-10-16 12:19 UTC (permalink / raw)
  To: Andi Shyti, Jani Nikula
  Cc: Jani Nikula, Matt Atwood, intel-gfx, matthew.d.roper

On Tue, Oct 14, 2025 at 01:09:46AM +0200, Andi Shyti wrote:
> Hi,
> 
> On Fri, Oct 10, 2025 at 09:19:24AM -0400, Rodrigo Vivi wrote:
> > On Fri, Oct 10, 2025 at 12:55:02PM +0300, Jani Nikula wrote:
> > > On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > > > reg_in_range_table is a useful function that is used in multiple places,
> > > > and will be needed for WA_BB implementation later.
> > > >
> > > > Let's move this function and i915_range struct to its own file, as we are
> > > > trying to move away from i915_utils files.
> > > >
> > > > v2: move functions to their own file
> > > > v3: use correct naming convention
> > > 
> > > Okay, Message from the Department of Bikeshedding and Nitpicking.
> > > 
> > > There's really nothing mmio specific about the functionality being
> > > abstracted. You have a range represented by two u32's in a struct, and a
> > > function to check if another u32 is within that range.
> > > 
> > > The struct could just as well remain i915_range, the files could be
> > > i915_range.[ch], and the function could be, say,
> > > i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.
> > 
> > hmm, I'm really sorry about that... That is my bad. I'm so bad with naming.
> > 
> > I suggested mmio in the name because i915_range is way to generic.
> > The other extreme side.
> > 
> > Perhaps i915_addr_range ?
> 
> If we use it only for mmio, why should we make it generic? If we
> want to keep things generic we could well use things from in
> range.h, as Jani has suggested in one of his reviews and add our
> function directly there.

Well, I don't have strong feelings here.

Perhaps i915_addr_range is more generic and middle ground.

Jani?

> 
> Andi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-16 12:19       ` Rodrigo Vivi
@ 2025-10-16 15:20         ` Jani Nikula
  2025-10-16 15:26           ` Raag Jadav
  0 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2025-10-16 15:20 UTC (permalink / raw)
  To: Rodrigo Vivi, Andi Shyti; +Cc: Matt Atwood, intel-gfx, matthew.d.roper

On Thu, 16 Oct 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Tue, Oct 14, 2025 at 01:09:46AM +0200, Andi Shyti wrote:
>> Hi,
>> 
>> On Fri, Oct 10, 2025 at 09:19:24AM -0400, Rodrigo Vivi wrote:
>> > On Fri, Oct 10, 2025 at 12:55:02PM +0300, Jani Nikula wrote:
>> > > On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
>> > > > reg_in_range_table is a useful function that is used in multiple places,
>> > > > and will be needed for WA_BB implementation later.
>> > > >
>> > > > Let's move this function and i915_range struct to its own file, as we are
>> > > > trying to move away from i915_utils files.
>> > > >
>> > > > v2: move functions to their own file
>> > > > v3: use correct naming convention
>> > > 
>> > > Okay, Message from the Department of Bikeshedding and Nitpicking.
>> > > 
>> > > There's really nothing mmio specific about the functionality being
>> > > abstracted. You have a range represented by two u32's in a struct, and a
>> > > function to check if another u32 is within that range.
>> > > 
>> > > The struct could just as well remain i915_range, the files could be
>> > > i915_range.[ch], and the function could be, say,
>> > > i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.
>> > 
>> > hmm, I'm really sorry about that... That is my bad. I'm so bad with naming.
>> > 
>> > I suggested mmio in the name because i915_range is way to generic.
>> > The other extreme side.
>> > 
>> > Perhaps i915_addr_range ?
>> 
>> If we use it only for mmio, why should we make it generic? If we
>> want to keep things generic we could well use things from in
>> range.h, as Jani has suggested in one of his reviews and add our
>> function directly there.
>
> Well, I don't have strong feelings here.
>
> Perhaps i915_addr_range is more generic and middle ground.
>
> Jani?

Lots of bikeshedding here, but in the end just get it merged with
whatever naming and move on?

BR,
Jani.


-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-16 15:20         ` Jani Nikula
@ 2025-10-16 15:26           ` Raag Jadav
  2025-10-16 19:25             ` Rodrigo Vivi
  0 siblings, 1 reply; 13+ messages in thread
From: Raag Jadav @ 2025-10-16 15:26 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Rodrigo Vivi, Andi Shyti, Matt Atwood, intel-gfx, matthew.d.roper

On Thu, Oct 16, 2025 at 06:20:01PM +0300, Jani Nikula wrote:
> On Thu, 16 Oct 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Tue, Oct 14, 2025 at 01:09:46AM +0200, Andi Shyti wrote:
> > > Hi,
> > > 
> > > On Fri, Oct 10, 2025 at 09:19:24AM -0400, Rodrigo Vivi wrote:
> > > > On Fri, Oct 10, 2025 at 12:55:02PM +0300, Jani Nikula wrote:
> > > > > On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > > > > > reg_in_range_table is a useful function that is used in multiple places,
> > > > > > and will be needed for WA_BB implementation later.
> > > > > >
> > > > > > Let's move this function and i915_range struct to its own file, as we are
> > > > > > trying to move away from i915_utils files.
> > > > > >
> > > > > > v2: move functions to their own file
> > > > > > v3: use correct naming convention
> > > > > 
> > > > > Okay, Message from the Department of Bikeshedding and Nitpicking.
> > > > > 
> > > > > There's really nothing mmio specific about the functionality being
> > > > > abstracted. You have a range represented by two u32's in a struct, and a
> > > > > function to check if another u32 is within that range.
> > > > > 
> > > > > The struct could just as well remain i915_range, the files could be
> > > > > i915_range.[ch], and the function could be, say,
> > > > > i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.
> > > > 
> > > > hmm, I'm really sorry about that... That is my bad. I'm so bad with naming.
> > > > 
> > > > I suggested mmio in the name because i915_range is way to generic.
> > > > The other extreme side.
> > > > 
> > > > Perhaps i915_addr_range ?
> > > 
> > > If we use it only for mmio, why should we make it generic? If we
> > > want to keep things generic we could well use things from in
> > > range.h, as Jani has suggested in one of his reviews and add our
> > > function directly there.
> >
> > Well, I don't have strong feelings here.
> >
> > Perhaps i915_addr_range is more generic and middle ground.
> >
> > Jani?
> 
> Lots of bikeshedding here, but in the end just get it merged with
> whatever naming and move on?

We've all been here at some point, this is arguably the hardest part :D

Raag

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-16 15:26           ` Raag Jadav
@ 2025-10-16 19:25             ` Rodrigo Vivi
  0 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2025-10-16 19:25 UTC (permalink / raw)
  To: Raag Jadav
  Cc: Jani Nikula, Andi Shyti, Matt Atwood, intel-gfx, matthew.d.roper

On Thu, Oct 16, 2025 at 05:26:17PM +0200, Raag Jadav wrote:
> On Thu, Oct 16, 2025 at 06:20:01PM +0300, Jani Nikula wrote:
> > On Thu, 16 Oct 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > On Tue, Oct 14, 2025 at 01:09:46AM +0200, Andi Shyti wrote:
> > > > Hi,
> > > > 
> > > > On Fri, Oct 10, 2025 at 09:19:24AM -0400, Rodrigo Vivi wrote:
> > > > > On Fri, Oct 10, 2025 at 12:55:02PM +0300, Jani Nikula wrote:
> > > > > > On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > > > > > > reg_in_range_table is a useful function that is used in multiple places,
> > > > > > > and will be needed for WA_BB implementation later.
> > > > > > >
> > > > > > > Let's move this function and i915_range struct to its own file, as we are
> > > > > > > trying to move away from i915_utils files.
> > > > > > >
> > > > > > > v2: move functions to their own file
> > > > > > > v3: use correct naming convention
> > > > > > 
> > > > > > Okay, Message from the Department of Bikeshedding and Nitpicking.
> > > > > > 
> > > > > > There's really nothing mmio specific about the functionality being
> > > > > > abstracted. You have a range represented by two u32's in a struct, and a
> > > > > > function to check if another u32 is within that range.
> > > > > > 
> > > > > > The struct could just as well remain i915_range, the files could be
> > > > > > i915_range.[ch], and the function could be, say,
> > > > > > i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.
> > > > > 
> > > > > hmm, I'm really sorry about that... That is my bad. I'm so bad with naming.
> > > > > 
> > > > > I suggested mmio in the name because i915_range is way to generic.
> > > > > The other extreme side.
> > > > > 
> > > > > Perhaps i915_addr_range ?
> > > > 
> > > > If we use it only for mmio, why should we make it generic? If we
> > > > want to keep things generic we could well use things from in
> > > > range.h, as Jani has suggested in one of his reviews and add our
> > > > function directly there.
> > >
> > > Well, I don't have strong feelings here.
> > >
> > > Perhaps i915_addr_range is more generic and middle ground.
> > >
> > > Jani?
> > 
> > Lots of bikeshedding here, but in the end just get it merged with
> > whatever naming and move on?

done. rv-b. added an extra space in the subject. and pushed to drm-intel-next.

> 
> We've all been here at some point, this is arguably the hardest part :D
> 
> Raag

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-09 21:52 [PATCH v3] drm/i915:move and rename reg_in_range_table Matt Atwood
  2025-10-10  0:07 ` ✗ i915.CI.BAT: failure for drm/i915:move and rename reg_in_range_table (rev2) Patchwork
  2025-10-10  9:55 ` [PATCH v3] drm/i915:move and rename reg_in_range_table Jani Nikula
@ 2025-10-10 13:07 ` Ville Syrjälä
  2025-10-10 21:23   ` Matt Atwood
  2 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2025-10-10 13:07 UTC (permalink / raw)
  To: Matt Atwood
  Cc: intel-gfx, matthew.d.roper, rodrigo.vivi, jani.nikula, andi.shyti

On Thu, Oct 09, 2025 at 02:52:08PM -0700, Matt Atwood wrote:
> reg_in_range_table is a useful function that is used in multiple places,
> and will be needed for WA_BB implementation later.
> 
> Let's move this function and i915_range struct to its own file, as we are
> trying to move away from i915_utils files.
> 
> v2: move functions to their own file
> v3: use correct naming convention
> 
> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |  1 +
>  drivers/gpu/drm/i915/gt/intel_workarounds.c   |  9 +--
>  drivers/gpu/drm/i915/i915_mmio_range.c        | 18 +++++
>  drivers/gpu/drm/i915/i915_mmio_range.h        | 19 ++++++
>  drivers/gpu/drm/i915/i915_perf.c              | 67 ++++++++-----------
>  drivers/gpu/drm/i915/intel_uncore.c           | 15 +++--
>  drivers/gpu/drm/i915/intel_uncore.h           |  8 +--
>  drivers/gpu/drm/i915/selftests/intel_uncore.c |  4 +-
>  8 files changed, 82 insertions(+), 59 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_mmio_range.c
>  create mode 100644 drivers/gpu/drm/i915/i915_mmio_range.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 78a45a6681df..e5819c4320bf 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -26,6 +26,7 @@ i915-y += \
>  	i915_ioctl.o \
>  	i915_irq.o \
>  	i915_mitigations.o \
> +	i915_mmio_range.o \
>  	i915_module.o \
>  	i915_params.o \
>  	i915_pci.o \
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 7d486dfa2fc1..ece88c612e27 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -5,6 +5,7 @@
>  
>  #include "i915_drv.h"
>  #include "i915_reg.h"
> +#include "i915_mmio_range.h"
>  #include "intel_context.h"
>  #include "intel_engine_pm.h"
>  #include "intel_engine_regs.h"
> @@ -2923,7 +2924,7 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine)
>  	wa_list_apply(&engine->wa_list);
>  }
>  
> -static const struct i915_range mcr_ranges_gen8[] = {
> +static const struct i915_mmio_range mcr_ranges_gen8[] = {
>  	{ .start = 0x5500, .end = 0x55ff },
>  	{ .start = 0x7000, .end = 0x7fff },
>  	{ .start = 0x9400, .end = 0x97ff },
> @@ -2932,7 +2933,7 @@ static const struct i915_range mcr_ranges_gen8[] = {
>  	{},
>  };
>  
> -static const struct i915_range mcr_ranges_gen12[] = {
> +static const struct i915_mmio_range mcr_ranges_gen12[] = {
>  	{ .start =  0x8150, .end =  0x815f },
>  	{ .start =  0x9520, .end =  0x955f },
>  	{ .start =  0xb100, .end =  0xb3ff },
> @@ -2941,7 +2942,7 @@ static const struct i915_range mcr_ranges_gen12[] = {
>  	{},
>  };
>  
> -static const struct i915_range mcr_ranges_xehp[] = {
> +static const struct i915_mmio_range mcr_ranges_xehp[] = {
>  	{ .start =  0x4000, .end =  0x4aff },
>  	{ .start =  0x5200, .end =  0x52ff },
>  	{ .start =  0x5400, .end =  0x7fff },
> @@ -2960,7 +2961,7 @@ static const struct i915_range mcr_ranges_xehp[] = {
>  
>  static bool mcr_range(struct drm_i915_private *i915, u32 offset)
>  {
> -	const struct i915_range *mcr_ranges;
> +	const struct i915_mmio_range *mcr_ranges;
>  	int i;
>  
>  	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55))
> diff --git a/drivers/gpu/drm/i915/i915_mmio_range.c b/drivers/gpu/drm/i915/i915_mmio_range.c
> new file mode 100644
> index 000000000000..724041e81aa7
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_mmio_range.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#include "i915_mmio_range.h"
> +
> +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table)
> +{
> +	while (table->start || table->end) {
> +		if (addr >= table->start && addr <= table->end)
> +			return true;
> +
> +		table++;
> +	}
> +
> +	return false;
> +}

Should perhaps follow up with:
- Convert intel_uncore BSEARCH() into __inline_bsearch() and double
  check things still get inlined
- Add a function to validate the table is sorted and call that for all
  the tables in some common init functions (ideally should be compile
  time checked but alas we have no constexpr functions in C)
- Use __inline_bsearch() here as well. Not sure if this itself would
  need to be inline to allow intel_uncore.c to use it for the shadow range
  checks (I suspect the concern there was about inlining the comparisons
  rather than avoiding a single bsearch() function call...)

> diff --git a/drivers/gpu/drm/i915/i915_mmio_range.h b/drivers/gpu/drm/i915/i915_mmio_range.h
> new file mode 100644
> index 000000000000..f1c7086d3e3c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_mmio_range.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef __I915_MMIO_RANGE_H__
> +#define __I915_MMIO_RANGE_H__
> +
> +#include <linux/types.h>
> +
> +/* Other register ranges (e.g., shadow tables, MCR tables, etc.) */

"other" than what?

> +struct i915_mmio_range {
> +	u32 start;
> +	u32 end;

Need to document that the end is inclusive.

And I'm wondering if we should perhaps have more consistency
in how the end is specified. Curently it looks like oa and
shadow specify it as 4 byte aligned, but forcewake and mcr
use 1 byte alignment.

I think making the end exclusive would actually make the range
definitions a bit easier to parse for mere humans. But dunno
what other people think. And f some of these ranges are listed
in the sepc with end being inclusive then I guess keeping it
inclusive might be nice for quick visual comparisons against
the spec.

> +};
> +
> +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table);
> +
> +#endif /* __I915_MMIO_RANGE_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 1658f1246c6f..0b9d9f3f7813 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -219,6 +219,7 @@
>  #include "i915_perf.h"
>  #include "i915_perf_oa_regs.h"
>  #include "i915_reg.h"
> +#include "i915_mmio_range.h"
>  
>  /* HW requires this to be a power of two, between 128k and 16M, though driver
>   * is currently generally designed assuming the largest 16M size is used such
> @@ -4320,29 +4321,17 @@ static bool gen8_is_valid_flex_addr(struct i915_perf *perf, u32 addr)
>  	return false;
>  }
>  
> -static bool reg_in_range_table(u32 addr, const struct i915_range *table)
> -{
> -	while (table->start || table->end) {
> -		if (addr >= table->start && addr <= table->end)
> -			return true;
> -
> -		table++;
> -	}
> -
> -	return false;
> -}
> -
>  #define REG_EQUAL(addr, mmio) \
>  	((addr) == i915_mmio_reg_offset(mmio))
>  
> -static const struct i915_range gen7_oa_b_counters[] = {
> +static const struct i915_mmio_range gen7_oa_b_counters[] = {
>  	{ .start = 0x2710, .end = 0x272c },	/* OASTARTTRIG[1-8] */
>  	{ .start = 0x2740, .end = 0x275c },	/* OAREPORTTRIG[1-8] */
>  	{ .start = 0x2770, .end = 0x27ac },	/* OACEC[0-7][0-1] */
>  	{}
>  };
>  
> -static const struct i915_range gen12_oa_b_counters[] = {
> +static const struct i915_mmio_range gen12_oa_b_counters[] = {
>  	{ .start = 0x2b2c, .end = 0x2b2c },	/* GEN12_OAG_OA_PESS */
>  	{ .start = 0xd900, .end = 0xd91c },	/* GEN12_OAG_OASTARTTRIG[1-8] */
>  	{ .start = 0xd920, .end = 0xd93c },	/* GEN12_OAG_OAREPORTTRIG1[1-8] */
> @@ -4353,7 +4342,7 @@ static const struct i915_range gen12_oa_b_counters[] = {
>  	{}
>  };
>  
> -static const struct i915_range mtl_oam_b_counters[] = {
> +static const struct i915_mmio_range mtl_oam_b_counters[] = {
>  	{ .start = 0x393000, .end = 0x39301c },	/* GEN12_OAM_STARTTRIG1[1-8] */
>  	{ .start = 0x393020, .end = 0x39303c },	/* GEN12_OAM_REPORTTRIG1[1-8] */
>  	{ .start = 0x393040, .end = 0x39307c },	/* GEN12_OAM_CEC[0-7][0-1] */
> @@ -4361,43 +4350,43 @@ static const struct i915_range mtl_oam_b_counters[] = {
>  	{}
>  };
>  
> -static const struct i915_range xehp_oa_b_counters[] = {
> +static const struct i915_mmio_range xehp_oa_b_counters[] = {
>  	{ .start = 0xdc48, .end = 0xdc48 },	/* OAA_ENABLE_REG */
>  	{ .start = 0xdd00, .end = 0xdd48 },	/* OAG_LCE0_0 - OAA_LENABLE_REG */
>  	{}
>  };
>  
> -static const struct i915_range gen7_oa_mux_regs[] = {
> +static const struct i915_mmio_range gen7_oa_mux_regs[] = {
>  	{ .start = 0x91b8, .end = 0x91cc },	/* OA_PERFCNT[1-2], OA_PERFMATRIX */
>  	{ .start = 0x9800, .end = 0x9888 },	/* MICRO_BP0_0 - NOA_WRITE */
>  	{ .start = 0xe180, .end = 0xe180 },	/* HALF_SLICE_CHICKEN2 */
>  	{}
>  };
>  
> -static const struct i915_range hsw_oa_mux_regs[] = {
> +static const struct i915_mmio_range hsw_oa_mux_regs[] = {
>  	{ .start = 0x09e80, .end = 0x09ea4 }, /* HSW_MBVID2_NOA[0-9] */
>  	{ .start = 0x09ec0, .end = 0x09ec0 }, /* HSW_MBVID2_MISR0 */
>  	{ .start = 0x25100, .end = 0x2ff90 },
>  	{}
>  };
>  
> -static const struct i915_range chv_oa_mux_regs[] = {
> +static const struct i915_mmio_range chv_oa_mux_regs[] = {
>  	{ .start = 0x182300, .end = 0x1823a4 },
>  	{}
>  };
>  
> -static const struct i915_range gen8_oa_mux_regs[] = {
> +static const struct i915_mmio_range gen8_oa_mux_regs[] = {
>  	{ .start = 0x0d00, .end = 0x0d2c },	/* RPM_CONFIG[0-1], NOA_CONFIG[0-8] */
>  	{ .start = 0x20cc, .end = 0x20cc },	/* WAIT_FOR_RC6_EXIT */
>  	{}
>  };
>  
> -static const struct i915_range gen11_oa_mux_regs[] = {
> +static const struct i915_mmio_range gen11_oa_mux_regs[] = {
>  	{ .start = 0x91c8, .end = 0x91dc },	/* OA_PERFCNT[3-4] */
>  	{}
>  };
>  
> -static const struct i915_range gen12_oa_mux_regs[] = {
> +static const struct i915_mmio_range gen12_oa_mux_regs[] = {
>  	{ .start = 0x0d00, .end = 0x0d04 },     /* RPM_CONFIG[0-1] */
>  	{ .start = 0x0d0c, .end = 0x0d2c },     /* NOA_CONFIG[0-8] */
>  	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
> @@ -4410,7 +4399,7 @@ static const struct i915_range gen12_oa_mux_regs[] = {
>   * Ref: 14010536224:
>   * 0x20cc is repurposed on MTL, so use a separate array for MTL.
>   */
> -static const struct i915_range mtl_oa_mux_regs[] = {
> +static const struct i915_mmio_range mtl_oa_mux_regs[] = {
>  	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
>  	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
>  	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
> @@ -4421,61 +4410,61 @@ static const struct i915_range mtl_oa_mux_regs[] = {
>  
>  static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>  {
> -	return reg_in_range_table(addr, gen7_oa_b_counters);
> +	return i915_mmio_range_table_contains(addr, gen7_oa_b_counters);
>  }
>  
>  static bool gen8_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>  {
> -	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
> -		reg_in_range_table(addr, gen8_oa_mux_regs);
> +	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
> +		i915_mmio_range_table_contains(addr, gen8_oa_mux_regs);
>  }
>  
>  static bool gen11_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>  {
> -	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
> -		reg_in_range_table(addr, gen8_oa_mux_regs) ||
> -		reg_in_range_table(addr, gen11_oa_mux_regs);
> +	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
> +		i915_mmio_range_table_contains(addr, gen8_oa_mux_regs) ||
> +		i915_mmio_range_table_contains(addr, gen11_oa_mux_regs);
>  }
>  
>  static bool hsw_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>  {
> -	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
> -		reg_in_range_table(addr, hsw_oa_mux_regs);
> +	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
> +		i915_mmio_range_table_contains(addr, hsw_oa_mux_regs);
>  }
>  
>  static bool chv_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>  {
> -	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
> -		reg_in_range_table(addr, chv_oa_mux_regs);
> +	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
> +		i915_mmio_range_table_contains(addr, chv_oa_mux_regs);
>  }
>  
>  static bool gen12_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>  {
> -	return reg_in_range_table(addr, gen12_oa_b_counters);
> +	return i915_mmio_range_table_contains(addr, gen12_oa_b_counters);
>  }
>  
>  static bool mtl_is_valid_oam_b_counter_addr(struct i915_perf *perf, u32 addr)
>  {
>  	if (HAS_OAM(perf->i915) &&
>  	    GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 70))
> -		return reg_in_range_table(addr, mtl_oam_b_counters);
> +		return i915_mmio_range_table_contains(addr, mtl_oam_b_counters);
>  
>  	return false;
>  }
>  
>  static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>  {
> -	return reg_in_range_table(addr, xehp_oa_b_counters) ||
> -		reg_in_range_table(addr, gen12_oa_b_counters) ||
> +	return i915_mmio_range_table_contains(addr, xehp_oa_b_counters) ||
> +		i915_mmio_range_table_contains(addr, gen12_oa_b_counters) ||
>  		mtl_is_valid_oam_b_counter_addr(perf, addr);
>  }
>  
>  static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>  {
>  	if (GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 70))
> -		return reg_in_range_table(addr, mtl_oa_mux_regs);
> +		return i915_mmio_range_table_contains(addr, mtl_oa_mux_regs);
>  	else
> -		return reg_in_range_table(addr, gen12_oa_mux_regs);
> +		return i915_mmio_range_table_contains(addr, gen12_oa_mux_regs);
>  }
>  
>  static u32 mask_reg_value(u32 reg, u32 val)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 8cb59f8d1f4c..aba90b80854f 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -35,6 +35,7 @@
>  #include "i915_reg.h"
>  #include "i915_vgpu.h"
>  #include "i915_wait_util.h"
> +#include "i915_mmio_range.h"
>  #include "intel_uncore_trace.h"
>  
>  #define FORCEWAKE_ACK_TIMEOUT_MS 50
> @@ -999,7 +1000,7 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
>   * scanned for obvious mistakes or typos by the selftests.
>   */
>  
> -static const struct i915_range gen8_shadowed_regs[] = {
> +static const struct i915_mmio_range gen8_shadowed_regs[] = {
>  	{ .start =  0x2030, .end =  0x2030 },
>  	{ .start =  0xA008, .end =  0xA00C },
>  	{ .start = 0x12030, .end = 0x12030 },
> @@ -1007,7 +1008,7 @@ static const struct i915_range gen8_shadowed_regs[] = {
>  	{ .start = 0x22030, .end = 0x22030 },
>  };
>  
> -static const struct i915_range gen11_shadowed_regs[] = {
> +static const struct i915_mmio_range gen11_shadowed_regs[] = {
>  	{ .start =   0x2030, .end =   0x2030 },
>  	{ .start =   0x2550, .end =   0x2550 },
>  	{ .start =   0xA008, .end =   0xA00C },
> @@ -1034,7 +1035,7 @@ static const struct i915_range gen11_shadowed_regs[] = {
>  	{ .start = 0x1D8510, .end = 0x1D8550 },
>  };
>  
> -static const struct i915_range gen12_shadowed_regs[] = {
> +static const struct i915_mmio_range gen12_shadowed_regs[] = {
>  	{ .start =   0x2030, .end =   0x2030 },
>  	{ .start =   0x2510, .end =   0x2550 },
>  	{ .start =   0xA008, .end =   0xA00C },
> @@ -1078,7 +1079,7 @@ static const struct i915_range gen12_shadowed_regs[] = {
>  	{ .start = 0x1F8510, .end = 0x1F8550 },
>  };
>  
> -static const struct i915_range dg2_shadowed_regs[] = {
> +static const struct i915_mmio_range dg2_shadowed_regs[] = {
>  	{ .start =   0x2030, .end =   0x2030 },
>  	{ .start =   0x2510, .end =   0x2550 },
>  	{ .start =   0xA008, .end =   0xA00C },
> @@ -1117,7 +1118,7 @@ static const struct i915_range dg2_shadowed_regs[] = {
>  	{ .start = 0x1F8510, .end = 0x1F8550 },
>  };
>  
> -static const struct i915_range mtl_shadowed_regs[] = {
> +static const struct i915_mmio_range mtl_shadowed_regs[] = {
>  	{ .start =   0x2030, .end =   0x2030 },
>  	{ .start =   0x2510, .end =   0x2550 },
>  	{ .start =   0xA008, .end =   0xA00C },
> @@ -1135,7 +1136,7 @@ static const struct i915_range mtl_shadowed_regs[] = {
>  	{ .start =  0x22510, .end =  0x22550 },
>  };
>  
> -static const struct i915_range xelpmp_shadowed_regs[] = {
> +static const struct i915_mmio_range xelpmp_shadowed_regs[] = {
>  	{ .start = 0x1C0030, .end = 0x1C0030 },
>  	{ .start = 0x1C0510, .end = 0x1C0550 },
>  	{ .start = 0x1C8030, .end = 0x1C8030 },
> @@ -1156,7 +1157,7 @@ static const struct i915_range xelpmp_shadowed_regs[] = {
>  	{ .start = 0x38CFD4, .end = 0x38CFDC },
>  };
>  
> -static int mmio_range_cmp(u32 key, const struct i915_range *range)
> +static int mmio_range_cmp(u32 key, const struct i915_mmio_range *range)
>  {
>  	if (key < range->start)
>  		return -1;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index 6048b99b96cb..fafc2ca9a237 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -123,12 +123,6 @@ struct intel_forcewake_range {
>  	enum forcewake_domains domains;
>  };
>  
> -/* Other register ranges (e.g., shadow tables, MCR tables, etc.) */
> -struct i915_range {
> -	u32 start;
> -	u32 end;
> -};
> -
>  struct intel_uncore {
>  	void __iomem *regs;
>  
> @@ -162,7 +156,7 @@ struct intel_uncore {
>  	 * Shadowed registers are special cases where we can safely write
>  	 * to the register *without* grabbing forcewake.
>  	 */
> -	const struct i915_range *shadowed_reg_table;
> +	const struct i915_mmio_range *shadowed_reg_table;
>  	unsigned int shadowed_reg_table_entries;
>  
>  	struct notifier_block pmic_bus_access_nb;
> diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> index 58bcbdcef563..507bf42a1aaf 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> @@ -64,7 +64,7 @@ static int intel_fw_table_check(const struct intel_forcewake_range *ranges,
>  static int intel_shadow_table_check(void)
>  {
>  	struct {
> -		const struct i915_range *regs;
> +		const struct i915_mmio_range *regs;
>  		unsigned int size;
>  	} range_lists[] = {
>  		{ gen8_shadowed_regs, ARRAY_SIZE(gen8_shadowed_regs) },
> @@ -74,7 +74,7 @@ static int intel_shadow_table_check(void)
>  		{ mtl_shadowed_regs, ARRAY_SIZE(mtl_shadowed_regs) },
>  		{ xelpmp_shadowed_regs, ARRAY_SIZE(xelpmp_shadowed_regs) },
>  	};
> -	const struct i915_range *range;
> +	const struct i915_mmio_range *range;
>  	unsigned int i, j;
>  	s32 prev;
>  
> -- 
> 2.51.0

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-10 13:07 ` Ville Syrjälä
@ 2025-10-10 21:23   ` Matt Atwood
  2025-10-10 22:56     ` Ville Syrjälä
  0 siblings, 1 reply; 13+ messages in thread
From: Matt Atwood @ 2025-10-10 21:23 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, matthew.d.roper, rodrigo.vivi, jani.nikula, andi.shyti

On Fri, Oct 10, 2025 at 04:07:55PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 09, 2025 at 02:52:08PM -0700, Matt Atwood wrote:
> > reg_in_range_table is a useful function that is used in multiple places,
> > and will be needed for WA_BB implementation later.
> > 
> > Let's move this function and i915_range struct to its own file, as we are
> > trying to move away from i915_utils files.
> > 
> > v2: move functions to their own file
> > v3: use correct naming convention
> > 
> > Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile                 |  1 +
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c   |  9 +--
> >  drivers/gpu/drm/i915/i915_mmio_range.c        | 18 +++++
> >  drivers/gpu/drm/i915/i915_mmio_range.h        | 19 ++++++
> >  drivers/gpu/drm/i915/i915_perf.c              | 67 ++++++++-----------
> >  drivers/gpu/drm/i915/intel_uncore.c           | 15 +++--
> >  drivers/gpu/drm/i915/intel_uncore.h           |  8 +--
> >  drivers/gpu/drm/i915/selftests/intel_uncore.c |  4 +-
> >  8 files changed, 82 insertions(+), 59 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/i915_mmio_range.c
> >  create mode 100644 drivers/gpu/drm/i915/i915_mmio_range.h
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 78a45a6681df..e5819c4320bf 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -26,6 +26,7 @@ i915-y += \
> >  	i915_ioctl.o \
> >  	i915_irq.o \
> >  	i915_mitigations.o \
> > +	i915_mmio_range.o \
> >  	i915_module.o \
> >  	i915_params.o \
> >  	i915_pci.o \
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index 7d486dfa2fc1..ece88c612e27 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -5,6 +5,7 @@
> >  
> >  #include "i915_drv.h"
> >  #include "i915_reg.h"
> > +#include "i915_mmio_range.h"
> >  #include "intel_context.h"
> >  #include "intel_engine_pm.h"
> >  #include "intel_engine_regs.h"
> > @@ -2923,7 +2924,7 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine)
> >  	wa_list_apply(&engine->wa_list);
> >  }
> >  
> > -static const struct i915_range mcr_ranges_gen8[] = {
> > +static const struct i915_mmio_range mcr_ranges_gen8[] = {
> >  	{ .start = 0x5500, .end = 0x55ff },
> >  	{ .start = 0x7000, .end = 0x7fff },
> >  	{ .start = 0x9400, .end = 0x97ff },
> > @@ -2932,7 +2933,7 @@ static const struct i915_range mcr_ranges_gen8[] = {
> >  	{},
> >  };
> >  
> > -static const struct i915_range mcr_ranges_gen12[] = {
> > +static const struct i915_mmio_range mcr_ranges_gen12[] = {
> >  	{ .start =  0x8150, .end =  0x815f },
> >  	{ .start =  0x9520, .end =  0x955f },
> >  	{ .start =  0xb100, .end =  0xb3ff },
> > @@ -2941,7 +2942,7 @@ static const struct i915_range mcr_ranges_gen12[] = {
> >  	{},
> >  };
> >  
> > -static const struct i915_range mcr_ranges_xehp[] = {
> > +static const struct i915_mmio_range mcr_ranges_xehp[] = {
> >  	{ .start =  0x4000, .end =  0x4aff },
> >  	{ .start =  0x5200, .end =  0x52ff },
> >  	{ .start =  0x5400, .end =  0x7fff },
> > @@ -2960,7 +2961,7 @@ static const struct i915_range mcr_ranges_xehp[] = {
> >  
> >  static bool mcr_range(struct drm_i915_private *i915, u32 offset)
> >  {
> > -	const struct i915_range *mcr_ranges;
> > +	const struct i915_mmio_range *mcr_ranges;
> >  	int i;
> >  
> >  	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55))
> > diff --git a/drivers/gpu/drm/i915/i915_mmio_range.c b/drivers/gpu/drm/i915/i915_mmio_range.c
> > new file mode 100644
> > index 000000000000..724041e81aa7
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/i915_mmio_range.c
> > @@ -0,0 +1,18 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2025 Intel Corporation
> > + */
> > +
> > +#include "i915_mmio_range.h"
> > +
> > +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table)
> > +{
> > +	while (table->start || table->end) {
> > +		if (addr >= table->start && addr <= table->end)
> > +			return true;
> > +
> > +		table++;
> > +	}
> > +
> > +	return false;
> > +}
> 
> Should perhaps follow up with:
> - Convert intel_uncore BSEARCH() into __inline_bsearch() and double
>   check things still get inlined
> - Add a function to validate the table is sorted and call that for all
>   the tables in some common init functions (ideally should be compile
>   time checked but alas we have no constexpr functions in C)
> - Use __inline_bsearch() here as well. Not sure if this itself would
>   need to be inline to allow intel_uncore.c to use it for the shadow range
>   checks (I suspect the concern there was about inlining the comparisons
>   rather than avoiding a single bsearch() function call...)
to be clear, you want this work in a follow up patch, not in the next
revision?
> 
> > diff --git a/drivers/gpu/drm/i915/i915_mmio_range.h b/drivers/gpu/drm/i915/i915_mmio_range.h
> > new file mode 100644
> > index 000000000000..f1c7086d3e3c
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/i915_mmio_range.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2025 Intel Corporation
> > + */
> > +
> > +#ifndef __I915_MMIO_RANGE_H__
> > +#define __I915_MMIO_RANGE_H__
> > +
> > +#include <linux/types.h>
> > +
> > +/* Other register ranges (e.g., shadow tables, MCR tables, etc.) */
> 
> "other" than what?
Ack
> 
> > +struct i915_mmio_range {
> > +	u32 start;
> > +	u32 end;
> 
> Need to document that the end is inclusive.
Ack
> 
> And I'm wondering if we should perhaps have more consistency
> in how the end is specified. Curently it looks like oa and
> shadow specify it as 4 byte aligned, but forcewake and mcr
> use 1 byte alignment.
> 
> I think making the end exclusive would actually make the range
> definitions a bit easier to parse for mere humans. But dunno
> what other people think. And f some of these ranges are listed
> in the sepc with end being inclusive then I guess keeping it
> inclusive might be nice for quick visual comparisons against
> the spec.
I would prefer to keep it the way it is.

MattA
> 
> > +};
> > +
> > +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table);
> > +
> > +#endif /* __I915_MMIO_RANGE_H__ */
> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> > index 1658f1246c6f..0b9d9f3f7813 100644
> > --- a/drivers/gpu/drm/i915/i915_perf.c
> > +++ b/drivers/gpu/drm/i915/i915_perf.c
> > @@ -219,6 +219,7 @@
> >  #include "i915_perf.h"
> >  #include "i915_perf_oa_regs.h"
> >  #include "i915_reg.h"
> > +#include "i915_mmio_range.h"
> >  
> >  /* HW requires this to be a power of two, between 128k and 16M, though driver
> >   * is currently generally designed assuming the largest 16M size is used such
> > @@ -4320,29 +4321,17 @@ static bool gen8_is_valid_flex_addr(struct i915_perf *perf, u32 addr)
> >  	return false;
> >  }
> >  
> > -static bool reg_in_range_table(u32 addr, const struct i915_range *table)
> > -{
> > -	while (table->start || table->end) {
> > -		if (addr >= table->start && addr <= table->end)
> > -			return true;
> > -
> > -		table++;
> > -	}
> > -
> > -	return false;
> > -}
> > -
> >  #define REG_EQUAL(addr, mmio) \
> >  	((addr) == i915_mmio_reg_offset(mmio))
> >  
> > -static const struct i915_range gen7_oa_b_counters[] = {
> > +static const struct i915_mmio_range gen7_oa_b_counters[] = {
> >  	{ .start = 0x2710, .end = 0x272c },	/* OASTARTTRIG[1-8] */
> >  	{ .start = 0x2740, .end = 0x275c },	/* OAREPORTTRIG[1-8] */
> >  	{ .start = 0x2770, .end = 0x27ac },	/* OACEC[0-7][0-1] */
> >  	{}
> >  };
> >  
> > -static const struct i915_range gen12_oa_b_counters[] = {
> > +static const struct i915_mmio_range gen12_oa_b_counters[] = {
> >  	{ .start = 0x2b2c, .end = 0x2b2c },	/* GEN12_OAG_OA_PESS */
> >  	{ .start = 0xd900, .end = 0xd91c },	/* GEN12_OAG_OASTARTTRIG[1-8] */
> >  	{ .start = 0xd920, .end = 0xd93c },	/* GEN12_OAG_OAREPORTTRIG1[1-8] */
> > @@ -4353,7 +4342,7 @@ static const struct i915_range gen12_oa_b_counters[] = {
> >  	{}
> >  };
> >  
> > -static const struct i915_range mtl_oam_b_counters[] = {
> > +static const struct i915_mmio_range mtl_oam_b_counters[] = {
> >  	{ .start = 0x393000, .end = 0x39301c },	/* GEN12_OAM_STARTTRIG1[1-8] */
> >  	{ .start = 0x393020, .end = 0x39303c },	/* GEN12_OAM_REPORTTRIG1[1-8] */
> >  	{ .start = 0x393040, .end = 0x39307c },	/* GEN12_OAM_CEC[0-7][0-1] */
> > @@ -4361,43 +4350,43 @@ static const struct i915_range mtl_oam_b_counters[] = {
> >  	{}
> >  };
> >  
> > -static const struct i915_range xehp_oa_b_counters[] = {
> > +static const struct i915_mmio_range xehp_oa_b_counters[] = {
> >  	{ .start = 0xdc48, .end = 0xdc48 },	/* OAA_ENABLE_REG */
> >  	{ .start = 0xdd00, .end = 0xdd48 },	/* OAG_LCE0_0 - OAA_LENABLE_REG */
> >  	{}
> >  };
> >  
> > -static const struct i915_range gen7_oa_mux_regs[] = {
> > +static const struct i915_mmio_range gen7_oa_mux_regs[] = {
> >  	{ .start = 0x91b8, .end = 0x91cc },	/* OA_PERFCNT[1-2], OA_PERFMATRIX */
> >  	{ .start = 0x9800, .end = 0x9888 },	/* MICRO_BP0_0 - NOA_WRITE */
> >  	{ .start = 0xe180, .end = 0xe180 },	/* HALF_SLICE_CHICKEN2 */
> >  	{}
> >  };
> >  
> > -static const struct i915_range hsw_oa_mux_regs[] = {
> > +static const struct i915_mmio_range hsw_oa_mux_regs[] = {
> >  	{ .start = 0x09e80, .end = 0x09ea4 }, /* HSW_MBVID2_NOA[0-9] */
> >  	{ .start = 0x09ec0, .end = 0x09ec0 }, /* HSW_MBVID2_MISR0 */
> >  	{ .start = 0x25100, .end = 0x2ff90 },
> >  	{}
> >  };
> >  
> > -static const struct i915_range chv_oa_mux_regs[] = {
> > +static const struct i915_mmio_range chv_oa_mux_regs[] = {
> >  	{ .start = 0x182300, .end = 0x1823a4 },
> >  	{}
> >  };
> >  
> > -static const struct i915_range gen8_oa_mux_regs[] = {
> > +static const struct i915_mmio_range gen8_oa_mux_regs[] = {
> >  	{ .start = 0x0d00, .end = 0x0d2c },	/* RPM_CONFIG[0-1], NOA_CONFIG[0-8] */
> >  	{ .start = 0x20cc, .end = 0x20cc },	/* WAIT_FOR_RC6_EXIT */
> >  	{}
> >  };
> >  
> > -static const struct i915_range gen11_oa_mux_regs[] = {
> > +static const struct i915_mmio_range gen11_oa_mux_regs[] = {
> >  	{ .start = 0x91c8, .end = 0x91dc },	/* OA_PERFCNT[3-4] */
> >  	{}
> >  };
> >  
> > -static const struct i915_range gen12_oa_mux_regs[] = {
> > +static const struct i915_mmio_range gen12_oa_mux_regs[] = {
> >  	{ .start = 0x0d00, .end = 0x0d04 },     /* RPM_CONFIG[0-1] */
> >  	{ .start = 0x0d0c, .end = 0x0d2c },     /* NOA_CONFIG[0-8] */
> >  	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
> > @@ -4410,7 +4399,7 @@ static const struct i915_range gen12_oa_mux_regs[] = {
> >   * Ref: 14010536224:
> >   * 0x20cc is repurposed on MTL, so use a separate array for MTL.
> >   */
> > -static const struct i915_range mtl_oa_mux_regs[] = {
> > +static const struct i915_mmio_range mtl_oa_mux_regs[] = {
> >  	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
> >  	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
> >  	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
> > @@ -4421,61 +4410,61 @@ static const struct i915_range mtl_oa_mux_regs[] = {
> >  
> >  static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
> >  {
> > -	return reg_in_range_table(addr, gen7_oa_b_counters);
> > +	return i915_mmio_range_table_contains(addr, gen7_oa_b_counters);
> >  }
> >  
> >  static bool gen8_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
> >  {
> > -	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
> > -		reg_in_range_table(addr, gen8_oa_mux_regs);
> > +	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
> > +		i915_mmio_range_table_contains(addr, gen8_oa_mux_regs);
> >  }
> >  
> >  static bool gen11_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
> >  {
> > -	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
> > -		reg_in_range_table(addr, gen8_oa_mux_regs) ||
> > -		reg_in_range_table(addr, gen11_oa_mux_regs);
> > +	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
> > +		i915_mmio_range_table_contains(addr, gen8_oa_mux_regs) ||
> > +		i915_mmio_range_table_contains(addr, gen11_oa_mux_regs);
> >  }
> >  
> >  static bool hsw_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
> >  {
> > -	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
> > -		reg_in_range_table(addr, hsw_oa_mux_regs);
> > +	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
> > +		i915_mmio_range_table_contains(addr, hsw_oa_mux_regs);
> >  }
> >  
> >  static bool chv_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
> >  {
> > -	return reg_in_range_table(addr, gen7_oa_mux_regs) ||
> > -		reg_in_range_table(addr, chv_oa_mux_regs);
> > +	return i915_mmio_range_table_contains(addr, gen7_oa_mux_regs) ||
> > +		i915_mmio_range_table_contains(addr, chv_oa_mux_regs);
> >  }
> >  
> >  static bool gen12_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
> >  {
> > -	return reg_in_range_table(addr, gen12_oa_b_counters);
> > +	return i915_mmio_range_table_contains(addr, gen12_oa_b_counters);
> >  }
> >  
> >  static bool mtl_is_valid_oam_b_counter_addr(struct i915_perf *perf, u32 addr)
> >  {
> >  	if (HAS_OAM(perf->i915) &&
> >  	    GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 70))
> > -		return reg_in_range_table(addr, mtl_oam_b_counters);
> > +		return i915_mmio_range_table_contains(addr, mtl_oam_b_counters);
> >  
> >  	return false;
> >  }
> >  
> >  static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
> >  {
> > -	return reg_in_range_table(addr, xehp_oa_b_counters) ||
> > -		reg_in_range_table(addr, gen12_oa_b_counters) ||
> > +	return i915_mmio_range_table_contains(addr, xehp_oa_b_counters) ||
> > +		i915_mmio_range_table_contains(addr, gen12_oa_b_counters) ||
> >  		mtl_is_valid_oam_b_counter_addr(perf, addr);
> >  }
> >  
> >  static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
> >  {
> >  	if (GRAPHICS_VER_FULL(perf->i915) >= IP_VER(12, 70))
> > -		return reg_in_range_table(addr, mtl_oa_mux_regs);
> > +		return i915_mmio_range_table_contains(addr, mtl_oa_mux_regs);
> >  	else
> > -		return reg_in_range_table(addr, gen12_oa_mux_regs);
> > +		return i915_mmio_range_table_contains(addr, gen12_oa_mux_regs);
> >  }
> >  
> >  static u32 mask_reg_value(u32 reg, u32 val)
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 8cb59f8d1f4c..aba90b80854f 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -35,6 +35,7 @@
> >  #include "i915_reg.h"
> >  #include "i915_vgpu.h"
> >  #include "i915_wait_util.h"
> > +#include "i915_mmio_range.h"
> >  #include "intel_uncore_trace.h"
> >  
> >  #define FORCEWAKE_ACK_TIMEOUT_MS 50
> > @@ -999,7 +1000,7 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
> >   * scanned for obvious mistakes or typos by the selftests.
> >   */
> >  
> > -static const struct i915_range gen8_shadowed_regs[] = {
> > +static const struct i915_mmio_range gen8_shadowed_regs[] = {
> >  	{ .start =  0x2030, .end =  0x2030 },
> >  	{ .start =  0xA008, .end =  0xA00C },
> >  	{ .start = 0x12030, .end = 0x12030 },
> > @@ -1007,7 +1008,7 @@ static const struct i915_range gen8_shadowed_regs[] = {
> >  	{ .start = 0x22030, .end = 0x22030 },
> >  };
> >  
> > -static const struct i915_range gen11_shadowed_regs[] = {
> > +static const struct i915_mmio_range gen11_shadowed_regs[] = {
> >  	{ .start =   0x2030, .end =   0x2030 },
> >  	{ .start =   0x2550, .end =   0x2550 },
> >  	{ .start =   0xA008, .end =   0xA00C },
> > @@ -1034,7 +1035,7 @@ static const struct i915_range gen11_shadowed_regs[] = {
> >  	{ .start = 0x1D8510, .end = 0x1D8550 },
> >  };
> >  
> > -static const struct i915_range gen12_shadowed_regs[] = {
> > +static const struct i915_mmio_range gen12_shadowed_regs[] = {
> >  	{ .start =   0x2030, .end =   0x2030 },
> >  	{ .start =   0x2510, .end =   0x2550 },
> >  	{ .start =   0xA008, .end =   0xA00C },
> > @@ -1078,7 +1079,7 @@ static const struct i915_range gen12_shadowed_regs[] = {
> >  	{ .start = 0x1F8510, .end = 0x1F8550 },
> >  };
> >  
> > -static const struct i915_range dg2_shadowed_regs[] = {
> > +static const struct i915_mmio_range dg2_shadowed_regs[] = {
> >  	{ .start =   0x2030, .end =   0x2030 },
> >  	{ .start =   0x2510, .end =   0x2550 },
> >  	{ .start =   0xA008, .end =   0xA00C },
> > @@ -1117,7 +1118,7 @@ static const struct i915_range dg2_shadowed_regs[] = {
> >  	{ .start = 0x1F8510, .end = 0x1F8550 },
> >  };
> >  
> > -static const struct i915_range mtl_shadowed_regs[] = {
> > +static const struct i915_mmio_range mtl_shadowed_regs[] = {
> >  	{ .start =   0x2030, .end =   0x2030 },
> >  	{ .start =   0x2510, .end =   0x2550 },
> >  	{ .start =   0xA008, .end =   0xA00C },
> > @@ -1135,7 +1136,7 @@ static const struct i915_range mtl_shadowed_regs[] = {
> >  	{ .start =  0x22510, .end =  0x22550 },
> >  };
> >  
> > -static const struct i915_range xelpmp_shadowed_regs[] = {
> > +static const struct i915_mmio_range xelpmp_shadowed_regs[] = {
> >  	{ .start = 0x1C0030, .end = 0x1C0030 },
> >  	{ .start = 0x1C0510, .end = 0x1C0550 },
> >  	{ .start = 0x1C8030, .end = 0x1C8030 },
> > @@ -1156,7 +1157,7 @@ static const struct i915_range xelpmp_shadowed_regs[] = {
> >  	{ .start = 0x38CFD4, .end = 0x38CFDC },
> >  };
> >  
> > -static int mmio_range_cmp(u32 key, const struct i915_range *range)
> > +static int mmio_range_cmp(u32 key, const struct i915_mmio_range *range)
> >  {
> >  	if (key < range->start)
> >  		return -1;
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> > index 6048b99b96cb..fafc2ca9a237 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.h
> > +++ b/drivers/gpu/drm/i915/intel_uncore.h
> > @@ -123,12 +123,6 @@ struct intel_forcewake_range {
> >  	enum forcewake_domains domains;
> >  };
> >  
> > -/* Other register ranges (e.g., shadow tables, MCR tables, etc.) */
> > -struct i915_range {
> > -	u32 start;
> > -	u32 end;
> > -};
> > -
> >  struct intel_uncore {
> >  	void __iomem *regs;
> >  
> > @@ -162,7 +156,7 @@ struct intel_uncore {
> >  	 * Shadowed registers are special cases where we can safely write
> >  	 * to the register *without* grabbing forcewake.
> >  	 */
> > -	const struct i915_range *shadowed_reg_table;
> > +	const struct i915_mmio_range *shadowed_reg_table;
> >  	unsigned int shadowed_reg_table_entries;
> >  
> >  	struct notifier_block pmic_bus_access_nb;
> > diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> > index 58bcbdcef563..507bf42a1aaf 100644
> > --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> > @@ -64,7 +64,7 @@ static int intel_fw_table_check(const struct intel_forcewake_range *ranges,
> >  static int intel_shadow_table_check(void)
> >  {
> >  	struct {
> > -		const struct i915_range *regs;
> > +		const struct i915_mmio_range *regs;
> >  		unsigned int size;
> >  	} range_lists[] = {
> >  		{ gen8_shadowed_regs, ARRAY_SIZE(gen8_shadowed_regs) },
> > @@ -74,7 +74,7 @@ static int intel_shadow_table_check(void)
> >  		{ mtl_shadowed_regs, ARRAY_SIZE(mtl_shadowed_regs) },
> >  		{ xelpmp_shadowed_regs, ARRAY_SIZE(xelpmp_shadowed_regs) },
> >  	};
> > -	const struct i915_range *range;
> > +	const struct i915_mmio_range *range;
> >  	unsigned int i, j;
> >  	s32 prev;
> >  
> > -- 
> > 2.51.0
> 
> -- 
> Ville Syrjälä
> Intel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] drm/i915:move and rename reg_in_range_table
  2025-10-10 21:23   ` Matt Atwood
@ 2025-10-10 22:56     ` Ville Syrjälä
  0 siblings, 0 replies; 13+ messages in thread
From: Ville Syrjälä @ 2025-10-10 22:56 UTC (permalink / raw)
  To: Matt Atwood
  Cc: intel-gfx, matthew.d.roper, rodrigo.vivi, jani.nikula, andi.shyti

On Fri, Oct 10, 2025 at 02:23:48PM -0700, Matt Atwood wrote:
> On Fri, Oct 10, 2025 at 04:07:55PM +0300, Ville Syrjälä wrote:
> > On Thu, Oct 09, 2025 at 02:52:08PM -0700, Matt Atwood wrote:
> > > reg_in_range_table is a useful function that is used in multiple places,
> > > and will be needed for WA_BB implementation later.
> > > 
> > > Let's move this function and i915_range struct to its own file, as we are
> > > trying to move away from i915_utils files.
> > > 
> > > v2: move functions to their own file
> > > v3: use correct naming convention
> > > 
> > > Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/Makefile                 |  1 +
> > >  drivers/gpu/drm/i915/gt/intel_workarounds.c   |  9 +--
> > >  drivers/gpu/drm/i915/i915_mmio_range.c        | 18 +++++
> > >  drivers/gpu/drm/i915/i915_mmio_range.h        | 19 ++++++
> > >  drivers/gpu/drm/i915/i915_perf.c              | 67 ++++++++-----------
> > >  drivers/gpu/drm/i915/intel_uncore.c           | 15 +++--
> > >  drivers/gpu/drm/i915/intel_uncore.h           |  8 +--
> > >  drivers/gpu/drm/i915/selftests/intel_uncore.c |  4 +-
> > >  8 files changed, 82 insertions(+), 59 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/i915/i915_mmio_range.c
> > >  create mode 100644 drivers/gpu/drm/i915/i915_mmio_range.h
> > > 
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index 78a45a6681df..e5819c4320bf 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -26,6 +26,7 @@ i915-y += \
> > >  	i915_ioctl.o \
> > >  	i915_irq.o \
> > >  	i915_mitigations.o \
> > > +	i915_mmio_range.o \
> > >  	i915_module.o \
> > >  	i915_params.o \
> > >  	i915_pci.o \
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > index 7d486dfa2fc1..ece88c612e27 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > @@ -5,6 +5,7 @@
> > >  
> > >  #include "i915_drv.h"
> > >  #include "i915_reg.h"
> > > +#include "i915_mmio_range.h"
> > >  #include "intel_context.h"
> > >  #include "intel_engine_pm.h"
> > >  #include "intel_engine_regs.h"
> > > @@ -2923,7 +2924,7 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine)
> > >  	wa_list_apply(&engine->wa_list);
> > >  }
> > >  
> > > -static const struct i915_range mcr_ranges_gen8[] = {
> > > +static const struct i915_mmio_range mcr_ranges_gen8[] = {
> > >  	{ .start = 0x5500, .end = 0x55ff },
> > >  	{ .start = 0x7000, .end = 0x7fff },
> > >  	{ .start = 0x9400, .end = 0x97ff },
> > > @@ -2932,7 +2933,7 @@ static const struct i915_range mcr_ranges_gen8[] = {
> > >  	{},
> > >  };
> > >  
> > > -static const struct i915_range mcr_ranges_gen12[] = {
> > > +static const struct i915_mmio_range mcr_ranges_gen12[] = {
> > >  	{ .start =  0x8150, .end =  0x815f },
> > >  	{ .start =  0x9520, .end =  0x955f },
> > >  	{ .start =  0xb100, .end =  0xb3ff },
> > > @@ -2941,7 +2942,7 @@ static const struct i915_range mcr_ranges_gen12[] = {
> > >  	{},
> > >  };
> > >  
> > > -static const struct i915_range mcr_ranges_xehp[] = {
> > > +static const struct i915_mmio_range mcr_ranges_xehp[] = {
> > >  	{ .start =  0x4000, .end =  0x4aff },
> > >  	{ .start =  0x5200, .end =  0x52ff },
> > >  	{ .start =  0x5400, .end =  0x7fff },
> > > @@ -2960,7 +2961,7 @@ static const struct i915_range mcr_ranges_xehp[] = {
> > >  
> > >  static bool mcr_range(struct drm_i915_private *i915, u32 offset)
> > >  {
> > > -	const struct i915_range *mcr_ranges;
> > > +	const struct i915_mmio_range *mcr_ranges;
> > >  	int i;
> > >  
> > >  	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55))
> > > diff --git a/drivers/gpu/drm/i915/i915_mmio_range.c b/drivers/gpu/drm/i915/i915_mmio_range.c
> > > new file mode 100644
> > > index 000000000000..724041e81aa7
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/i915_mmio_range.c
> > > @@ -0,0 +1,18 @@
> > > +// SPDX-License-Identifier: MIT
> > > +/*
> > > + * Copyright © 2025 Intel Corporation
> > > + */
> > > +
> > > +#include "i915_mmio_range.h"
> > > +
> > > +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table)
> > > +{
> > > +	while (table->start || table->end) {
> > > +		if (addr >= table->start && addr <= table->end)
> > > +			return true;
> > > +
> > > +		table++;
> > > +	}
> > > +
> > > +	return false;
> > > +}
> > 
> > Should perhaps follow up with:
> > - Convert intel_uncore BSEARCH() into __inline_bsearch() and double
> >   check things still get inlined
> > - Add a function to validate the table is sorted and call that for all
> >   the tables in some common init functions (ideally should be compile
> >   time checked but alas we have no constexpr functions in C)
> > - Use __inline_bsearch() here as well. Not sure if this itself would
> >   need to be inline to allow intel_uncore.c to use it for the shadow range
> >   checks (I suspect the concern there was about inlining the comparisons
> >   rather than avoiding a single bsearch() function call...)
> to be clear, you want this work in a follow up patch, not in the next
> revision?

Yeah, shouldn't mix all that in pne patch. Just some things that
stood out to me when I had a quick look at all the range checks.

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-10-16 19:25 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-09 21:52 [PATCH v3] drm/i915:move and rename reg_in_range_table Matt Atwood
2025-10-10  0:07 ` ✗ i915.CI.BAT: failure for drm/i915:move and rename reg_in_range_table (rev2) Patchwork
2025-10-10  9:55 ` [PATCH v3] drm/i915:move and rename reg_in_range_table Jani Nikula
2025-10-10 13:19   ` Rodrigo Vivi
2025-10-10 21:26     ` Matt Atwood
2025-10-13 23:09     ` Andi Shyti
2025-10-16 12:19       ` Rodrigo Vivi
2025-10-16 15:20         ` Jani Nikula
2025-10-16 15:26           ` Raag Jadav
2025-10-16 19:25             ` Rodrigo Vivi
2025-10-10 13:07 ` Ville Syrjälä
2025-10-10 21:23   ` Matt Atwood
2025-10-10 22:56     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox