[Intel-xe] [PATCH 0/8] Interrupt cleanup and future-proofing

[Intel-xe] [PATCH 0/8] Interrupt cleanup and future-proofing

[Matt Roper]

A lot of the interrupt code in the Xe driver was copied directly from
i915.  Since Xe only supports Xe_LP and later platforms, there's some
cleanup and simplification that can be done.

Also fix one mistake that would have become a bug in the future if we
ever wind up with a multi-tile platform that also supports display.

Matt Roper (8):
  drm/xe/irq: Drop gen3_ prefixes
  drm/xe/irq: Add helpers to find ISR/IIR/IMR/IER registers
  drm/xe/irq: Drop IRQ_INIT and IRQ_RESET macros
  drm/xe/irq: Drop unnecessary GEN11_ and GEN12_ register prefixes
  drm/xe/irq: Rename and clarify top-level interrupt handling routines
  drm/xe/irq: Drop remaining "gen11_" prefix from IRQ functions
  drm/xe/irq: Drop commented-out code for non-existent media engines
  drm/xe/irq: Don't clobber display interrupts on multi-tile platforms

 drivers/gpu/drm/xe/regs/xe_gt_regs.h  |  42 ++--
 drivers/gpu/drm/xe/regs/xe_reg_defs.h |   8 +
 drivers/gpu/drm/xe/regs/xe_regs.h     |  25 +--
 drivers/gpu/drm/xe/xe_guc.c           |   6 +-
 drivers/gpu/drm/xe/xe_irq.c           | 280 +++++++++++++-------------
 5 files changed, 178 insertions(+), 183 deletions(-)

-- 
2.39.2

[Intel-xe] [PATCH 1/8] drm/xe/irq: Drop gen3_ prefixes

[Matt Roper]

"Gen" terminology should be avoided in the Xe driver and "gen3" refers
to platforms that are 9 (!!) graphics generations earlier than the
oldest supported by the Xe driver, so this prefix really doesn't make
sense.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/xe/xe_irq.c | 42 ++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index 529b42d9c9af..d8fde8caff1e 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -19,7 +19,7 @@
 #include "xe_hw_engine.h"
 #include "xe_mmio.h"
 
-static void gen3_assert_iir_is_zero(struct xe_gt *gt, i915_reg_t reg)
+static void assert_iir_is_zero(struct xe_gt *gt, i915_reg_t reg)
 {
 	u32 val = xe_mmio_read32(gt, reg.reg);
 
@@ -35,24 +35,24 @@ static void gen3_assert_iir_is_zero(struct xe_gt *gt, i915_reg_t reg)
 	xe_mmio_read32(gt, reg.reg);
 }
 
-static void gen3_irq_init(struct xe_gt *gt,
-			  i915_reg_t imr, u32 imr_val,
-			  i915_reg_t ier, u32 ier_val,
-			  i915_reg_t iir)
+static void irq_init(struct xe_gt *gt,
+		     i915_reg_t imr, u32 imr_val,
+		     i915_reg_t ier, u32 ier_val,
+		     i915_reg_t iir)
 {
-	gen3_assert_iir_is_zero(gt, iir);
+	assert_iir_is_zero(gt, iir);
 
 	xe_mmio_write32(gt, ier.reg, ier_val);
 	xe_mmio_write32(gt, imr.reg, imr_val);
 	xe_mmio_read32(gt, imr.reg);
 }
-#define GEN3_IRQ_INIT(gt, type, imr_val, ier_val) \
-	gen3_irq_init((gt), \
-		      type##IMR, imr_val, \
-		      type##IER, ier_val, \
-		      type##IIR)
+#define IRQ_INIT(gt, type, imr_val, ier_val) \
+	irq_init((gt), \
+		 type##IMR, imr_val, \
+		 type##IER, ier_val, \
+		 type##IIR)
 
-static void gen3_irq_reset(struct xe_gt *gt, i915_reg_t imr, i915_reg_t iir,
+static void irq_reset(struct xe_gt *gt, i915_reg_t imr, i915_reg_t iir,
 			   i915_reg_t ier)
 {
 	xe_mmio_write32(gt, imr.reg, 0xffffffff);
@@ -66,8 +66,8 @@ static void gen3_irq_reset(struct xe_gt *gt, i915_reg_t imr, i915_reg_t iir,
 	xe_mmio_write32(gt, iir.reg, 0xffffffff);
 	xe_mmio_read32(gt, iir.reg);
 }
-#define GEN3_IRQ_RESET(gt, type) \
-	gen3_irq_reset((gt), type##IMR, type##IIR, type##IER)
+#define IRQ_RESET(gt, type) \
+	irq_reset((gt), type##IMR, type##IIR, type##IER)
 
 static u32 gen11_intr_disable(struct xe_gt *gt)
 {
@@ -173,8 +173,7 @@ static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 
 	gen11_gt_irq_postinstall(xe, gt);
 
-	GEN3_IRQ_INIT(gt, GEN11_GU_MISC_, ~GEN11_GU_MISC_GSE,
-		      GEN11_GU_MISC_GSE);
+	IRQ_INIT(gt, GEN11_GU_MISC_, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
 
 	gen11_intr_enable(gt, true);
 }
@@ -337,8 +336,7 @@ static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 {
 	gen11_gt_irq_postinstall(xe, gt);
 
-	GEN3_IRQ_INIT(gt, GEN11_GU_MISC_, ~GEN11_GU_MISC_GSE,
-		      GEN11_GU_MISC_GSE);
+	IRQ_INIT(gt, GEN11_GU_MISC_, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
 
 	if (gt->info.id == XE_GT0)
 		dg1_intr_enable(xe, true);
@@ -443,8 +441,8 @@ static void gen11_irq_reset(struct xe_gt *gt)
 
 	gen11_gt_irq_reset(gt);
 
-	GEN3_IRQ_RESET(gt, GEN11_GU_MISC_);
-	GEN3_IRQ_RESET(gt, GEN8_PCU_);
+	IRQ_RESET(gt, GEN11_GU_MISC_);
+	IRQ_RESET(gt, GEN8_PCU_);
 }
 
 static void dg1_irq_reset(struct xe_gt *gt)
@@ -454,8 +452,8 @@ static void dg1_irq_reset(struct xe_gt *gt)
 
 	gen11_gt_irq_reset(gt);
 
-	GEN3_IRQ_RESET(gt, GEN11_GU_MISC_);
-	GEN3_IRQ_RESET(gt, GEN8_PCU_);
+	IRQ_RESET(gt, GEN11_GU_MISC_);
+	IRQ_RESET(gt, GEN8_PCU_);
 }
 
 static void xe_irq_reset(struct xe_device *xe)
-- 
2.39.2

Re: [Intel-xe] [PATCH 1/8] drm/xe/irq: Drop gen3_ prefixes

[Lucas De Marchi]

On Thu, Mar 30, 2023 at 11:23:58AM -0700, Matt Roper wrote:
>"Gen" terminology should be avoided in the Xe driver and "gen3" refers
>to platforms that are 9 (!!) graphics generations earlier than the
>oldest supported by the Xe driver, so this prefix really doesn't make
>sense.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Not sure what we are going to do with
drivers/gpu/drm/xe/display/ext/i915_irq.c that is tangentially
related.... I think eventually it will be dropped, so let's ignore.

thanks
Lucas De Marchi

[Intel-xe] [PATCH 2/8] drm/xe/irq: Add helpers to find ISR/IIR/IMR/IER registers

[Matt Roper]

For cases where IRQ_INIT and IRQ_RESET are used, the relevant interrupt
registers are always consecutive and ordered ISR, IMR, IIR, IER.  Adding
helpers to look these up from a base offset will let us eliminate some
of the CPP pasting and simplify other upcoming patches.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/xe/regs/xe_reg_defs.h |  8 ++++++++
 drivers/gpu/drm/xe/regs/xe_regs.h     | 11 ++---------
 drivers/gpu/drm/xe/xe_irq.c           | 24 ++++++++++++------------
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/xe/regs/xe_reg_defs.h b/drivers/gpu/drm/xe/regs/xe_reg_defs.h
index b5c25e31b889..7ff3aa9322af 100644
--- a/drivers/gpu/drm/xe/regs/xe_reg_defs.h
+++ b/drivers/gpu/drm/xe/regs/xe_reg_defs.h
@@ -8,4 +8,12 @@
 
 #include "compat-i915-headers/i915_reg_defs.h"
 
+/*
+ * Interrupt registers for a unit are always consecutive and ordered
+ * ISR, IMR, IIR, IER.
+ */
+#define IMR(offset)				_MMIO(offset + 0x4)
+#define IIR(offset)				_MMIO(offset + 0x8)
+#define IER(offset)				_MMIO(offset + 0xc)
+
 #endif
diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
index c1c829c23df1..ffe5d726e196 100644
--- a/drivers/gpu/drm/xe/regs/xe_regs.h
+++ b/drivers/gpu/drm/xe/regs/xe_regs.h
@@ -76,15 +76,8 @@
 
 #define SOFTWARE_FLAGS_SPR33			_MMIO(0x4f084)
 
-#define GEN8_PCU_ISR				_MMIO(0x444e0)
-#define GEN8_PCU_IMR				_MMIO(0x444e4)
-#define GEN8_PCU_IIR				_MMIO(0x444e8)
-#define GEN8_PCU_IER				_MMIO(0x444ec)
-
-#define GEN11_GU_MISC_ISR			_MMIO(0x444f0)
-#define GEN11_GU_MISC_IMR			_MMIO(0x444f4)
-#define GEN11_GU_MISC_IIR			_MMIO(0x444f8)
-#define GEN11_GU_MISC_IER			_MMIO(0x444fc)
+#define PCU_IRQ_REGS				0x444e0
+#define GU_MISC_IRQ_REGS			0x444f0
 #define   GEN11_GU_MISC_GSE			(1 << 27)
 
 #define GEN11_GFX_MSTR_IRQ			_MMIO(0x190010)
diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index d8fde8caff1e..74d7d999a383 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -48,9 +48,9 @@ static void irq_init(struct xe_gt *gt,
 }
 #define IRQ_INIT(gt, type, imr_val, ier_val) \
 	irq_init((gt), \
-		 type##IMR, imr_val, \
-		 type##IER, ier_val, \
-		 type##IIR)
+		 IMR(type), imr_val, \
+		 IER(type), ier_val, \
+		 IIR(type))
 
 static void irq_reset(struct xe_gt *gt, i915_reg_t imr, i915_reg_t iir,
 			   i915_reg_t ier)
@@ -67,7 +67,7 @@ static void irq_reset(struct xe_gt *gt, i915_reg_t imr, i915_reg_t iir,
 	xe_mmio_read32(gt, iir.reg);
 }
 #define IRQ_RESET(gt, type) \
-	irq_reset((gt), type##IMR, type##IIR, type##IER)
+	irq_reset((gt), IMR(type), IIR(type), IER(type))
 
 static u32 gen11_intr_disable(struct xe_gt *gt)
 {
@@ -90,9 +90,9 @@ gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
 	if (!(master_ctl & GEN11_GU_MISC_IRQ))
 		return 0;
 
-	iir = xe_mmio_read32(gt, GEN11_GU_MISC_IIR.reg);
+	iir = xe_mmio_read32(gt, IIR(GU_MISC_IRQ_REGS).reg);
 	if (likely(iir))
-		xe_mmio_write32(gt, GEN11_GU_MISC_IIR.reg, iir);
+		xe_mmio_write32(gt, IIR(GU_MISC_IRQ_REGS).reg, iir);
 
 	return iir;
 }
@@ -173,7 +173,7 @@ static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 
 	gen11_gt_irq_postinstall(xe, gt);
 
-	IRQ_INIT(gt, GEN11_GU_MISC_, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
+	IRQ_INIT(gt, GU_MISC_IRQ_REGS, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
 
 	gen11_intr_enable(gt, true);
 }
@@ -336,7 +336,7 @@ static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 {
 	gen11_gt_irq_postinstall(xe, gt);
 
-	IRQ_INIT(gt, GEN11_GU_MISC_, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
+	IRQ_INIT(gt, GU_MISC_IRQ_REGS, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
 
 	if (gt->info.id == XE_GT0)
 		dg1_intr_enable(xe, true);
@@ -441,8 +441,8 @@ static void gen11_irq_reset(struct xe_gt *gt)
 
 	gen11_gt_irq_reset(gt);
 
-	IRQ_RESET(gt, GEN11_GU_MISC_);
-	IRQ_RESET(gt, GEN8_PCU_);
+	IRQ_RESET(gt, GU_MISC_IRQ_REGS);
+	IRQ_RESET(gt, PCU_IRQ_REGS);
 }
 
 static void dg1_irq_reset(struct xe_gt *gt)
@@ -452,8 +452,8 @@ static void dg1_irq_reset(struct xe_gt *gt)
 
 	gen11_gt_irq_reset(gt);
 
-	IRQ_RESET(gt, GEN11_GU_MISC_);
-	IRQ_RESET(gt, GEN8_PCU_);
+	IRQ_RESET(gt, GU_MISC_IRQ_REGS);
+	IRQ_RESET(gt, PCU_IRQ_REGS);
 }
 
 static void xe_irq_reset(struct xe_device *xe)
-- 
2.39.2

Re: [Intel-xe] [PATCH 2/8] drm/xe/irq: Add helpers to find ISR/IIR/IMR/IER registers

[Lucas De Marchi]

On Thu, Mar 30, 2023 at 11:23:59AM -0700, Matt Roper wrote:
>For cases where IRQ_INIT and IRQ_RESET are used, the relevant interrupt
>registers are always consecutive and ordered ISR, IMR, IIR, IER.  Adding
>helpers to look these up from a base offset will let us eliminate some
>of the CPP pasting and simplify other upcoming patches.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
> drivers/gpu/drm/xe/regs/xe_reg_defs.h |  8 ++++++++
> drivers/gpu/drm/xe/regs/xe_regs.h     | 11 ++---------
> drivers/gpu/drm/xe/xe_irq.c           | 24 ++++++++++++------------
> 3 files changed, 22 insertions(+), 21 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/regs/xe_reg_defs.h b/drivers/gpu/drm/xe/regs/xe_reg_defs.h
>index b5c25e31b889..7ff3aa9322af 100644
>--- a/drivers/gpu/drm/xe/regs/xe_reg_defs.h
>+++ b/drivers/gpu/drm/xe/regs/xe_reg_defs.h
>@@ -8,4 +8,12 @@
>
> #include "compat-i915-headers/i915_reg_defs.h"
>
>+/*
>+ * Interrupt registers for a unit are always consecutive and ordered
>+ * ISR, IMR, IIR, IER.
>+ */
>+#define IMR(offset)				_MMIO(offset + 0x4)
>+#define IIR(offset)				_MMIO(offset + 0x8)
>+#define IER(offset)				_MMIO(offset + 0xc)

since this is only to ever be used while handling the irqs in xe_irq.c,
wouldn't it be better o make it local to them local to that file?

My worry is that xe_reg_defs.h is the one file included by the world,
directly or indirectly and these IMR/IIR/IER short macros, although nice
would be poluting the "namespace".


>+
> #endif
>diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
>index c1c829c23df1..ffe5d726e196 100644
>--- a/drivers/gpu/drm/xe/regs/xe_regs.h
>+++ b/drivers/gpu/drm/xe/regs/xe_regs.h
>@@ -76,15 +76,8 @@
>
> #define SOFTWARE_FLAGS_SPR33			_MMIO(0x4f084)
>
>-#define GEN8_PCU_ISR				_MMIO(0x444e0)
>-#define GEN8_PCU_IMR				_MMIO(0x444e4)
>-#define GEN8_PCU_IIR				_MMIO(0x444e8)
>-#define GEN8_PCU_IER				_MMIO(0x444ec)
>-
>-#define GEN11_GU_MISC_ISR			_MMIO(0x444f0)
>-#define GEN11_GU_MISC_IMR			_MMIO(0x444f4)
>-#define GEN11_GU_MISC_IIR			_MMIO(0x444f8)
>-#define GEN11_GU_MISC_IER			_MMIO(0x444fc)
>+#define PCU_IRQ_REGS				0x444e0
>+#define GU_MISC_IRQ_REGS			0x444f0

we now have:

	*_BASE	
	*_OFFSET
	*_REGS

to refer to the plain u32 value. Should we consolidate in just one?
Maybe _OFFSET  would be one that could be applied both when it's a
"base", in the sense of HW unit, and when it's "the start of similar
registers".

otherwise the simplication and avoiding the macro pasting looks pretty
good and a little bit more grep-able.

thanks
Lucas De Marchi

> #define   GEN11_GU_MISC_GSE			(1 << 27)
>
> #define GEN11_GFX_MSTR_IRQ			_MMIO(0x190010)
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index d8fde8caff1e..74d7d999a383 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -48,9 +48,9 @@ static void irq_init(struct xe_gt *gt,
> }
> #define IRQ_INIT(gt, type, imr_val, ier_val) \
> 	irq_init((gt), \
>-		 type##IMR, imr_val, \
>-		 type##IER, ier_val, \
>-		 type##IIR)
>+		 IMR(type), imr_val, \
>+		 IER(type), ier_val, \
>+		 IIR(type))
>
> static void irq_reset(struct xe_gt *gt, i915_reg_t imr, i915_reg_t iir,
> 			   i915_reg_t ier)
>@@ -67,7 +67,7 @@ static void irq_reset(struct xe_gt *gt, i915_reg_t imr, i915_reg_t iir,
> 	xe_mmio_read32(gt, iir.reg);
> }
> #define IRQ_RESET(gt, type) \
>-	irq_reset((gt), type##IMR, type##IIR, type##IER)
>+	irq_reset((gt), IMR(type), IIR(type), IER(type))
>
> static u32 gen11_intr_disable(struct xe_gt *gt)
> {
>@@ -90,9 +90,9 @@ gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
> 	if (!(master_ctl & GEN11_GU_MISC_IRQ))
> 		return 0;
>
>-	iir = xe_mmio_read32(gt, GEN11_GU_MISC_IIR.reg);
>+	iir = xe_mmio_read32(gt, IIR(GU_MISC_IRQ_REGS).reg);
> 	if (likely(iir))
>-		xe_mmio_write32(gt, GEN11_GU_MISC_IIR.reg, iir);
>+		xe_mmio_write32(gt, IIR(GU_MISC_IRQ_REGS).reg, iir);
>
> 	return iir;
> }
>@@ -173,7 +173,7 @@ static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
>
> 	gen11_gt_irq_postinstall(xe, gt);
>
>-	IRQ_INIT(gt, GEN11_GU_MISC_, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
>+	IRQ_INIT(gt, GU_MISC_IRQ_REGS, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
>
> 	gen11_intr_enable(gt, true);
> }
>@@ -336,7 +336,7 @@ static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> {
> 	gen11_gt_irq_postinstall(xe, gt);
>
>-	IRQ_INIT(gt, GEN11_GU_MISC_, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
>+	IRQ_INIT(gt, GU_MISC_IRQ_REGS, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
>
> 	if (gt->info.id == XE_GT0)
> 		dg1_intr_enable(xe, true);
>@@ -441,8 +441,8 @@ static void gen11_irq_reset(struct xe_gt *gt)
>
> 	gen11_gt_irq_reset(gt);
>
>-	IRQ_RESET(gt, GEN11_GU_MISC_);
>-	IRQ_RESET(gt, GEN8_PCU_);
>+	IRQ_RESET(gt, GU_MISC_IRQ_REGS);
>+	IRQ_RESET(gt, PCU_IRQ_REGS);
> }
>
> static void dg1_irq_reset(struct xe_gt *gt)
>@@ -452,8 +452,8 @@ static void dg1_irq_reset(struct xe_gt *gt)
>
> 	gen11_gt_irq_reset(gt);
>
>-	IRQ_RESET(gt, GEN11_GU_MISC_);
>-	IRQ_RESET(gt, GEN8_PCU_);
>+	IRQ_RESET(gt, GU_MISC_IRQ_REGS);
>+	IRQ_RESET(gt, PCU_IRQ_REGS);
> }
>
> static void xe_irq_reset(struct xe_device *xe)
>-- 
>2.39.2
>

[Intel-xe] [PATCH 3/8] drm/xe/irq: Drop IRQ_INIT and IRQ_RESET macros

[Matt Roper]

It's no longer necessary to wrap these operations in macros; a simple
function will suffice.  Also switch to function names that more clearly
describe what operation is being performed:  unmask_and_enable() and
mask_and_disable().

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/xe/xe_irq.c | 63 +++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index 74d7d999a383..bf097ba6a10b 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -35,39 +35,40 @@ static void assert_iir_is_zero(struct xe_gt *gt, i915_reg_t reg)
 	xe_mmio_read32(gt, reg.reg);
 }
 
-static void irq_init(struct xe_gt *gt,
-		     i915_reg_t imr, u32 imr_val,
-		     i915_reg_t ier, u32 ier_val,
-		     i915_reg_t iir)
+/*
+ * Unmask and enable the specified interrupts.  Does not check current state,
+ * so any bits not specified here will become masked and disabled.
+ */
+static void unmask_and_enable(struct xe_gt *gt, u32 irqregs, u32 bits)
 {
-	assert_iir_is_zero(gt, iir);
+	/*
+	 * If we're just enabling an interrupt now, it shouldn't already
+	 * be raised in the IIR.
+	 */
+	assert_iir_is_zero(gt, IIR(irqregs));
+
+	xe_mmio_write32(gt, IER(irqregs).reg, bits);
+	xe_mmio_write32(gt, IMR(irqregs).reg, ~bits);
 
-	xe_mmio_write32(gt, ier.reg, ier_val);
-	xe_mmio_write32(gt, imr.reg, imr_val);
-	xe_mmio_read32(gt, imr.reg);
+	/* Posting read */
+	xe_mmio_read32(gt, IMR(irqregs).reg);
 }
-#define IRQ_INIT(gt, type, imr_val, ier_val) \
-	irq_init((gt), \
-		 IMR(type), imr_val, \
-		 IER(type), ier_val, \
-		 IIR(type))
-
-static void irq_reset(struct xe_gt *gt, i915_reg_t imr, i915_reg_t iir,
-			   i915_reg_t ier)
+
+/* Mask and disable all interrupts. */
+static void mask_and_disable(struct xe_gt *gt, u32 irqregs)
 {
-	xe_mmio_write32(gt, imr.reg, 0xffffffff);
-	xe_mmio_read32(gt, imr.reg);
+	xe_mmio_write32(gt, IMR(irqregs).reg, ~0);
+	/* Posting read */
+	xe_mmio_read32(gt, IMR(irqregs).reg);
 
-	xe_mmio_write32(gt, ier.reg, 0);
+	xe_mmio_write32(gt, IER(irqregs).reg, 0);
 
 	/* IIR can theoretically queue up two events. Be paranoid. */
-	xe_mmio_write32(gt, iir.reg, 0xffffffff);
-	xe_mmio_read32(gt, iir.reg);
-	xe_mmio_write32(gt, iir.reg, 0xffffffff);
-	xe_mmio_read32(gt, iir.reg);
+	xe_mmio_write32(gt, IIR(irqregs).reg, ~0);
+	xe_mmio_read32(gt, IIR(irqregs).reg);
+	xe_mmio_write32(gt, IIR(irqregs).reg, ~0);
+	xe_mmio_read32(gt, IIR(irqregs).reg);
 }
-#define IRQ_RESET(gt, type) \
-	irq_reset((gt), IMR(type), IIR(type), IER(type))
 
 static u32 gen11_intr_disable(struct xe_gt *gt)
 {
@@ -173,7 +174,7 @@ static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 
 	gen11_gt_irq_postinstall(xe, gt);
 
-	IRQ_INIT(gt, GU_MISC_IRQ_REGS, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
+	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GEN11_GU_MISC_GSE);
 
 	gen11_intr_enable(gt, true);
 }
@@ -336,7 +337,7 @@ static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 {
 	gen11_gt_irq_postinstall(xe, gt);
 
-	IRQ_INIT(gt, GU_MISC_IRQ_REGS, ~GEN11_GU_MISC_GSE, GEN11_GU_MISC_GSE);
+	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GEN11_GU_MISC_GSE);
 
 	if (gt->info.id == XE_GT0)
 		dg1_intr_enable(xe, true);
@@ -441,8 +442,8 @@ static void gen11_irq_reset(struct xe_gt *gt)
 
 	gen11_gt_irq_reset(gt);
 
-	IRQ_RESET(gt, GU_MISC_IRQ_REGS);
-	IRQ_RESET(gt, PCU_IRQ_REGS);
+	mask_and_disable(gt, GU_MISC_IRQ_REGS);
+	mask_and_disable(gt, PCU_IRQ_REGS);
 }
 
 static void dg1_irq_reset(struct xe_gt *gt)
@@ -452,8 +453,8 @@ static void dg1_irq_reset(struct xe_gt *gt)
 
 	gen11_gt_irq_reset(gt);
 
-	IRQ_RESET(gt, GU_MISC_IRQ_REGS);
-	IRQ_RESET(gt, PCU_IRQ_REGS);
+	mask_and_disable(gt, GU_MISC_IRQ_REGS);
+	mask_and_disable(gt, PCU_IRQ_REGS);
 }
 
 static void xe_irq_reset(struct xe_device *xe)
-- 
2.39.2

[Intel-xe] [PATCH 4/8] drm/xe/irq: Drop unnecessary GEN11_ and GEN12_ register prefixes

[Matt Roper]

Any interrupt registers that were introduced by platforms i915
considered to be "gen11" or "gen12" are present on all platforms that
the Xe driver supports; drop the unnecessary prefixes.

While working in the area, also convert a few open-coded bit
manipulations over to REG_BIT and REG_FIELD_GET notation.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/xe/regs/xe_gt_regs.h |  42 +++++------
 drivers/gpu/drm/xe/regs/xe_regs.h    |  12 +--
 drivers/gpu/drm/xe/xe_guc.c          |   6 +-
 drivers/gpu/drm/xe/xe_irq.c          | 109 +++++++++++++--------------
 4 files changed, 84 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
index f45251df5715..a8a37e6a45a3 100644
--- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
+++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
@@ -348,34 +348,34 @@
 #define GFX_FLSH_CNTL_GEN6			_MMIO(0x101008)
 #define   GFX_FLSH_CNTL_EN			(1 << 0)
 
-#define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
+#define GT_INTR_DW(x)				_MMIO(0x190018 + ((x) * 4))
 
-#define GEN11_GUC_SG_INTR_ENABLE		_MMIO(0x190038)
+#define GUC_SG_INTR_ENABLE			_MMIO(0x190038)
 #define   ENGINE1_MASK				REG_GENMASK(31, 16)
 #define   ENGINE0_MASK				REG_GENMASK(15, 0)
 
-#define GEN11_GPM_WGBOXPERF_INTR_ENABLE		_MMIO(0x19003c)
+#define GPM_WGBOXPERF_INTR_ENABLE		_MMIO(0x19003c)
 
-#define GEN11_INTR_IDENTITY_REG(x)		_MMIO(0x190060 + ((x) * 4))
-#define   GEN11_INTR_DATA_VALID			(1 << 31)
-#define   GEN11_INTR_ENGINE_INSTANCE(x)		(((x) & GENMASK(25, 20)) >> 20)
-#define   GEN11_INTR_ENGINE_CLASS(x)		(((x) & GENMASK(18, 16)) >> 16)
-#define   GEN11_INTR_ENGINE_INTR(x)		((x) & 0xffff)
+#define INTR_IDENTITY_REG(x)			_MMIO(0x190060 + ((x) * 4))
+#define   INTR_DATA_VALID			REG_BIT(31)
+#define   INTR_ENGINE_INSTANCE(x)		REG_FIELD_GET(GENMASK(25, 20), x)
+#define   INTR_ENGINE_CLASS(x)			REG_FIELD_GET(GENMASK(18, 16), x)
+#define   INTR_ENGINE_INTR(x)			REG_FIELD_GET(GENMASK(15, 0), x)
 #define   OTHER_GUC_INSTANCE			0
 
-#define GEN11_RENDER_COPY_INTR_ENABLE		_MMIO(0x190030)
-#define GEN11_VCS_VECS_INTR_ENABLE		_MMIO(0x190034)
-#define GEN12_CCS_RSVD_INTR_ENABLE		_MMIO(0x190048)
-#define GEN11_IIR_REG_SELECTOR(x)		_MMIO(0x190070 + ((x) * 4))
-#define GEN11_RCS0_RSVD_INTR_MASK		_MMIO(0x190090)
-#define GEN11_BCS_RSVD_INTR_MASK		_MMIO(0x1900a0)
-#define GEN11_VCS0_VCS1_INTR_MASK		_MMIO(0x1900a8)
-#define GEN11_VCS2_VCS3_INTR_MASK		_MMIO(0x1900ac)
-#define GEN11_VECS0_VECS1_INTR_MASK		_MMIO(0x1900d0)
-#define GEN11_GUC_SG_INTR_MASK			_MMIO(0x1900e8)
-#define GEN11_GPM_WGBOXPERF_INTR_MASK		_MMIO(0x1900ec)
-#define GEN12_CCS0_CCS1_INTR_MASK		_MMIO(0x190100)
-#define GEN12_CCS2_CCS3_INTR_MASK		_MMIO(0x190104)
+#define RENDER_COPY_INTR_ENABLE			_MMIO(0x190030)
+#define VCS_VECS_INTR_ENABLE			_MMIO(0x190034)
+#define CCS_RSVD_INTR_ENABLE			_MMIO(0x190048)
+#define IIR_REG_SELECTOR(x)			_MMIO(0x190070 + ((x) * 4))
+#define RCS0_RSVD_INTR_MASK			_MMIO(0x190090)
+#define BCS_RSVD_INTR_MASK			_MMIO(0x1900a0)
+#define VCS0_VCS1_INTR_MASK			_MMIO(0x1900a8)
+#define VCS2_VCS3_INTR_MASK			_MMIO(0x1900ac)
+#define VECS0_VECS1_INTR_MASK			_MMIO(0x1900d0)
+#define GUC_SG_INTR_MASK			_MMIO(0x1900e8)
+#define GPM_WGBOXPERF_INTR_MASK			_MMIO(0x1900ec)
+#define CCS0_CCS1_INTR_MASK			_MMIO(0x190100)
+#define CCS2_CCS3_INTR_MASK			_MMIO(0x190104)
 #define XEHPC_BCS1_BCS2_INTR_MASK		_MMIO(0x190110)
 #define XEHPC_BCS3_BCS4_INTR_MASK		_MMIO(0x190114)
 #define XEHPC_BCS5_BCS6_INTR_MASK		_MMIO(0x190118)
diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
index ffe5d726e196..34f12eacd432 100644
--- a/drivers/gpu/drm/xe/regs/xe_regs.h
+++ b/drivers/gpu/drm/xe/regs/xe_regs.h
@@ -78,13 +78,13 @@
 
 #define PCU_IRQ_REGS				0x444e0
 #define GU_MISC_IRQ_REGS			0x444f0
-#define   GEN11_GU_MISC_GSE			(1 << 27)
+#define   GU_MISC_GSE				REG_BIT(27)
 
-#define GEN11_GFX_MSTR_IRQ			_MMIO(0x190010)
-#define   GEN11_MASTER_IRQ			(1 << 31)
-#define   GEN11_GU_MISC_IRQ			(1 << 29)
-#define   GEN11_DISPLAY_IRQ			(1 << 16)
-#define   GEN11_GT_DW_IRQ(x)			(1 << (x))
+#define GFX_MSTR_IRQ				_MMIO(0x190010)
+#define   MASTER_IRQ				REG_BIT(31)
+#define   GU_MISC_IRQ				REG_BIT(29)
+#define   DISPLAY_IRQ				REG_BIT(16)
+#define   GT_DW_IRQ(x)				REG_BIT(x)
 
 #define DG1_MSTR_TILE_INTR			_MMIO(0x190008)
 #define   DG1_MSTR_IRQ				REG_BIT(31)
diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
index 58b9841616e4..ee71b969bcbf 100644
--- a/drivers/gpu/drm/xe/xe_guc.c
+++ b/drivers/gpu/drm/xe/xe_guc.c
@@ -561,12 +561,12 @@ static void guc_enable_irq(struct xe_guc *guc)
 		REG_FIELD_PREP(ENGINE0_MASK, GUC_INTR_GUC2HOST)  :
 		REG_FIELD_PREP(ENGINE1_MASK, GUC_INTR_GUC2HOST);
 
-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_ENABLE.reg,
+	xe_mmio_write32(gt, GUC_SG_INTR_ENABLE.reg,
 			REG_FIELD_PREP(ENGINE1_MASK, GUC_INTR_GUC2HOST));
 	if (xe_gt_is_media_type(gt))
-		xe_mmio_rmw32(gt, GEN11_GUC_SG_INTR_MASK.reg, events, 0);
+		xe_mmio_rmw32(gt, GUC_SG_INTR_MASK.reg, events, 0);
 	else
-		xe_mmio_write32(gt, GEN11_GUC_SG_INTR_MASK.reg, ~events);
+		xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg, ~events);
 }
 
 int xe_guc_enable_communication(struct xe_guc *guc)
diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index bf097ba6a10b..b1b94ba43b3f 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -72,7 +72,7 @@ static void mask_and_disable(struct xe_gt *gt, u32 irqregs)
 
 static u32 gen11_intr_disable(struct xe_gt *gt)
 {
-	xe_mmio_write32(gt, GEN11_GFX_MSTR_IRQ.reg, 0);
+	xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, 0);
 
 	/*
 	 * Now with master disabled, get a sample of level indications
@@ -80,7 +80,7 @@ static u32 gen11_intr_disable(struct xe_gt *gt)
 	 * New indications can and will light up during processing,
 	 * and will generate new interrupt after enabling master.
 	 */
-	return xe_mmio_read32(gt, GEN11_GFX_MSTR_IRQ.reg);
+	return xe_mmio_read32(gt, GFX_MSTR_IRQ.reg);
 }
 
 static u32
@@ -88,7 +88,7 @@ gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
 {
 	u32 iir;
 
-	if (!(master_ctl & GEN11_GU_MISC_IRQ))
+	if (!(master_ctl & GU_MISC_IRQ))
 		return 0;
 
 	iir = xe_mmio_read32(gt, IIR(GU_MISC_IRQ_REGS).reg);
@@ -100,9 +100,9 @@ gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
 
 static inline void gen11_intr_enable(struct xe_gt *gt, bool stall)
 {
-	xe_mmio_write32(gt, GEN11_GFX_MSTR_IRQ.reg, GEN11_MASTER_IRQ);
+	xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, MASTER_IRQ);
 	if (stall)
-		xe_mmio_read32(gt, GEN11_GFX_MSTR_IRQ.reg);
+		xe_mmio_read32(gt, GFX_MSTR_IRQ.reg);
 }
 
 static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
@@ -125,14 +125,14 @@ static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 	smask = irqs << 16;
 
 	/* Enable RCS, BCS, VCS and VECS class interrupts. */
-	xe_mmio_write32(gt, GEN11_RENDER_COPY_INTR_ENABLE.reg, dmask);
-	xe_mmio_write32(gt, GEN11_VCS_VECS_INTR_ENABLE.reg, dmask);
+	xe_mmio_write32(gt, RENDER_COPY_INTR_ENABLE.reg, dmask);
+	xe_mmio_write32(gt, VCS_VECS_INTR_ENABLE.reg, dmask);
 	if (ccs_mask)
-		xe_mmio_write32(gt, GEN12_CCS_RSVD_INTR_ENABLE.reg, smask);
+		xe_mmio_write32(gt, CCS_RSVD_INTR_ENABLE.reg, smask);
 
 	/* Unmask irqs on RCS, BCS, VCS and VECS engines. */
-	xe_mmio_write32(gt, GEN11_RCS0_RSVD_INTR_MASK.reg, ~smask);
-	xe_mmio_write32(gt, GEN11_BCS_RSVD_INTR_MASK.reg, ~smask);
+	xe_mmio_write32(gt, RCS0_RSVD_INTR_MASK.reg, ~smask);
+	xe_mmio_write32(gt, BCS_RSVD_INTR_MASK.reg, ~smask);
 	if (bcs_mask & (BIT(1)|BIT(2)))
 		xe_mmio_write32(gt, XEHPC_BCS1_BCS2_INTR_MASK.reg, ~dmask);
 	if (bcs_mask & (BIT(3)|BIT(4)))
@@ -141,31 +141,31 @@ static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 		xe_mmio_write32(gt, XEHPC_BCS5_BCS6_INTR_MASK.reg, ~dmask);
 	if (bcs_mask & (BIT(7)|BIT(8)))
 		xe_mmio_write32(gt, XEHPC_BCS7_BCS8_INTR_MASK.reg, ~dmask);
-	xe_mmio_write32(gt, GEN11_VCS0_VCS1_INTR_MASK.reg, ~dmask);
-	xe_mmio_write32(gt, GEN11_VCS2_VCS3_INTR_MASK.reg, ~dmask);
+	xe_mmio_write32(gt, VCS0_VCS1_INTR_MASK.reg, ~dmask);
+	xe_mmio_write32(gt, VCS2_VCS3_INTR_MASK.reg, ~dmask);
 	//if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
-	//	intel_uncore_write(uncore, GEN12_VCS4_VCS5_INTR_MASK, ~dmask);
+	//	intel_uncore_write(uncore, VCS4_VCS5_INTR_MASK, ~dmask);
 	//if (HAS_ENGINE(gt, VCS6) || HAS_ENGINE(gt, VCS7))
-	//	intel_uncore_write(uncore, GEN12_VCS6_VCS7_INTR_MASK, ~dmask);
-	xe_mmio_write32(gt, GEN11_VECS0_VECS1_INTR_MASK.reg, ~dmask);
+	//	intel_uncore_write(uncore, VCS6_VCS7_INTR_MASK, ~dmask);
+	xe_mmio_write32(gt, VECS0_VECS1_INTR_MASK.reg, ~dmask);
 	//if (HAS_ENGINE(gt, VECS2) || HAS_ENGINE(gt, VECS3))
-	//	intel_uncore_write(uncore, GEN12_VECS2_VECS3_INTR_MASK, ~dmask);
+	//	intel_uncore_write(uncore, VECS2_VECS3_INTR_MASK, ~dmask);
 	if (ccs_mask & (BIT(0)|BIT(1)))
-		xe_mmio_write32(gt, GEN12_CCS0_CCS1_INTR_MASK.reg, ~dmask);
+		xe_mmio_write32(gt, CCS0_CCS1_INTR_MASK.reg, ~dmask);
 	if (ccs_mask & (BIT(2)|BIT(3)))
-		xe_mmio_write32(gt,  GEN12_CCS2_CCS3_INTR_MASK.reg, ~dmask);
+		xe_mmio_write32(gt,  CCS2_CCS3_INTR_MASK.reg, ~dmask);
 
 	/*
 	 * RPS interrupts will get enabled/disabled on demand when RPS itself
 	 * is enabled/disabled.
 	 */
 	/* TODO: gt->pm_ier, gt->pm_imr */
-	xe_mmio_write32(gt, GEN11_GPM_WGBOXPERF_INTR_ENABLE.reg, 0);
-	xe_mmio_write32(gt, GEN11_GPM_WGBOXPERF_INTR_MASK.reg,  ~0);
+	xe_mmio_write32(gt, GPM_WGBOXPERF_INTR_ENABLE.reg, 0);
+	xe_mmio_write32(gt, GPM_WGBOXPERF_INTR_MASK.reg,  ~0);
 
 	/* Same thing for GuC interrupts */
-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_ENABLE.reg, 0);
-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_MASK.reg,  ~0);
+	xe_mmio_write32(gt, GUC_SG_INTR_ENABLE.reg, 0);
+	xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg,  ~0);
 }
 
 static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
@@ -174,7 +174,7 @@ static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 
 	gen11_gt_irq_postinstall(xe, gt);
 
-	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GEN11_GU_MISC_GSE);
+	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
 
 	gen11_intr_enable(gt, true);
 }
@@ -190,7 +190,7 @@ gen11_gt_engine_identity(struct xe_device *xe,
 
 	lockdep_assert_held(&xe->irq.lock);
 
-	xe_mmio_write32(gt, GEN11_IIR_REG_SELECTOR(bank).reg, BIT(bit));
+	xe_mmio_write32(gt, IIR_REG_SELECTOR(bank).reg, BIT(bit));
 
 	/*
 	 * NB: Specs do not specify how long to spin wait,
@@ -198,18 +198,17 @@ gen11_gt_engine_identity(struct xe_device *xe,
 	 */
 	timeout_ts = (local_clock() >> 10) + 100;
 	do {
-		ident = xe_mmio_read32(gt, GEN11_INTR_IDENTITY_REG(bank).reg);
-	} while (!(ident & GEN11_INTR_DATA_VALID) &&
+		ident = xe_mmio_read32(gt, INTR_IDENTITY_REG(bank).reg);
+	} while (!(ident & INTR_DATA_VALID) &&
 		 !time_after32(local_clock() >> 10, timeout_ts));
 
-	if (unlikely(!(ident & GEN11_INTR_DATA_VALID))) {
+	if (unlikely(!(ident & INTR_DATA_VALID))) {
 		drm_err(&xe->drm, "INTR_IDENTITY_REG%u:%u 0x%08x not valid!\n",
 			bank, bit, ident);
 		return 0;
 	}
 
-	xe_mmio_write32(gt, GEN11_INTR_IDENTITY_REG(bank).reg,
-			GEN11_INTR_DATA_VALID);
+	xe_mmio_write32(gt, INTR_IDENTITY_REG(bank).reg, INTR_DATA_VALID);
 
 	return ident;
 }
@@ -243,24 +242,24 @@ static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
 	spin_lock(&xe->irq.lock);
 
 	for (bank = 0; bank < 2; bank++) {
-		if (!(master_ctl & GEN11_GT_DW_IRQ(bank)))
+		if (!(master_ctl & GT_DW_IRQ(bank)))
 			continue;
 
 		if (!xe_gt_is_media_type(gt)) {
 			intr_dw[bank] =
-				xe_mmio_read32(gt, GEN11_GT_INTR_DW(bank).reg);
+				xe_mmio_read32(gt, GT_INTR_DW(bank).reg);
 			for_each_set_bit(bit, intr_dw + bank, 32)
 				identity[bit] = gen11_gt_engine_identity(xe, gt,
 									 bank,
 									 bit);
-			xe_mmio_write32(gt, GEN11_GT_INTR_DW(bank).reg,
+			xe_mmio_write32(gt, GT_INTR_DW(bank).reg,
 					intr_dw[bank]);
 		}
 
 		for_each_set_bit(bit, intr_dw + bank, 32) {
-			class = GEN11_INTR_ENGINE_CLASS(identity[bit]);
-			instance = GEN11_INTR_ENGINE_INSTANCE(identity[bit]);
-			intr_vec = GEN11_INTR_ENGINE_INTR(identity[bit]);
+			class = INTR_ENGINE_CLASS(identity[bit]);
+			instance = INTR_ENGINE_INSTANCE(identity[bit]);
+			intr_vec = INTR_ENGINE_INTR(identity[bit]);
 
 			if (class == XE_ENGINE_CLASS_OTHER) {
 				gen11_gt_other_irq_handler(gt, instance,
@@ -337,7 +336,7 @@ static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 {
 	gen11_gt_irq_postinstall(xe, gt);
 
-	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GEN11_GU_MISC_GSE);
+	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
 
 	if (gt->info.id == XE_GT0)
 		dg1_intr_enable(xe, true);
@@ -365,7 +364,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
 			continue;
 
 		if (!xe_gt_is_media_type(gt))
-			master_ctl = xe_mmio_read32(gt, GEN11_GFX_MSTR_IRQ.reg);
+			master_ctl = xe_mmio_read32(gt, GFX_MSTR_IRQ.reg);
 
 		/*
 		 * We might be in irq handler just when PCIe DPC is initiated
@@ -379,7 +378,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
 		}
 
 		if (!xe_gt_is_media_type(gt))
-			xe_mmio_write32(gt, GEN11_GFX_MSTR_IRQ.reg, master_ctl);
+			xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, master_ctl);
 		gen11_gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
 	}
 
@@ -400,14 +399,14 @@ static void gen11_gt_irq_reset(struct xe_gt *gt)
 	u32 bcs_mask = xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_COPY);
 
 	/* Disable RCS, BCS, VCS and VECS class engines. */
-	xe_mmio_write32(gt, GEN11_RENDER_COPY_INTR_ENABLE.reg,	 0);
-	xe_mmio_write32(gt, GEN11_VCS_VECS_INTR_ENABLE.reg,	 0);
+	xe_mmio_write32(gt, RENDER_COPY_INTR_ENABLE.reg,	 0);
+	xe_mmio_write32(gt, VCS_VECS_INTR_ENABLE.reg,	 0);
 	if (ccs_mask)
-		xe_mmio_write32(gt, GEN12_CCS_RSVD_INTR_ENABLE.reg, 0);
+		xe_mmio_write32(gt, CCS_RSVD_INTR_ENABLE.reg, 0);
 
 	/* Restore masks irqs on RCS, BCS, VCS and VECS engines. */
-	xe_mmio_write32(gt, GEN11_RCS0_RSVD_INTR_MASK.reg,	~0);
-	xe_mmio_write32(gt, GEN11_BCS_RSVD_INTR_MASK.reg,	~0);
+	xe_mmio_write32(gt, RCS0_RSVD_INTR_MASK.reg,	~0);
+	xe_mmio_write32(gt, BCS_RSVD_INTR_MASK.reg,	~0);
 	if (bcs_mask & (BIT(1)|BIT(2)))
 		xe_mmio_write32(gt, XEHPC_BCS1_BCS2_INTR_MASK.reg, ~0);
 	if (bcs_mask & (BIT(3)|BIT(4)))
@@ -416,24 +415,24 @@ static void gen11_gt_irq_reset(struct xe_gt *gt)
 		xe_mmio_write32(gt, XEHPC_BCS5_BCS6_INTR_MASK.reg, ~0);
 	if (bcs_mask & (BIT(7)|BIT(8)))
 		xe_mmio_write32(gt, XEHPC_BCS7_BCS8_INTR_MASK.reg, ~0);
-	xe_mmio_write32(gt, GEN11_VCS0_VCS1_INTR_MASK.reg,	~0);
-	xe_mmio_write32(gt, GEN11_VCS2_VCS3_INTR_MASK.reg,	~0);
+	xe_mmio_write32(gt, VCS0_VCS1_INTR_MASK.reg,	~0);
+	xe_mmio_write32(gt, VCS2_VCS3_INTR_MASK.reg,	~0);
 //	if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
-//		xe_mmio_write32(xe, GEN12_VCS4_VCS5_INTR_MASK.reg,   ~0);
+//		xe_mmio_write32(xe, VCS4_VCS5_INTR_MASK.reg,   ~0);
 //	if (HAS_ENGINE(gt, VCS6) || HAS_ENGINE(gt, VCS7))
-//		xe_mmio_write32(xe, GEN12_VCS6_VCS7_INTR_MASK.reg,   ~0);
-	xe_mmio_write32(gt, GEN11_VECS0_VECS1_INTR_MASK.reg,	~0);
+//		xe_mmio_write32(xe, VCS6_VCS7_INTR_MASK.reg,   ~0);
+	xe_mmio_write32(gt, VECS0_VECS1_INTR_MASK.reg,	~0);
 //	if (HAS_ENGINE(gt, VECS2) || HAS_ENGINE(gt, VECS3))
-//		xe_mmio_write32(xe, GEN12_VECS2_VECS3_INTR_MASK.reg, ~0);
+//		xe_mmio_write32(xe, VECS2_VECS3_INTR_MASK.reg, ~0);
 	if (ccs_mask & (BIT(0)|BIT(1)))
-		xe_mmio_write32(gt, GEN12_CCS0_CCS1_INTR_MASK.reg, ~0);
+		xe_mmio_write32(gt, CCS0_CCS1_INTR_MASK.reg, ~0);
 	if (ccs_mask & (BIT(2)|BIT(3)))
-		xe_mmio_write32(gt,  GEN12_CCS2_CCS3_INTR_MASK.reg, ~0);
+		xe_mmio_write32(gt,  CCS2_CCS3_INTR_MASK.reg, ~0);
 
-	xe_mmio_write32(gt, GEN11_GPM_WGBOXPERF_INTR_ENABLE.reg, 0);
-	xe_mmio_write32(gt, GEN11_GPM_WGBOXPERF_INTR_MASK.reg,  ~0);
-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_ENABLE.reg,	 0);
-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_MASK.reg,		~0);
+	xe_mmio_write32(gt, GPM_WGBOXPERF_INTR_ENABLE.reg, 0);
+	xe_mmio_write32(gt, GPM_WGBOXPERF_INTR_MASK.reg,  ~0);
+	xe_mmio_write32(gt, GUC_SG_INTR_ENABLE.reg,	 0);
+	xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg,		~0);
 }
 
 static void gen11_irq_reset(struct xe_gt *gt)
-- 
2.39.2

Re: [Intel-xe] [PATCH 4/8] drm/xe/irq: Drop unnecessary GEN11_ and GEN12_ register prefixes

[Lucas De Marchi]

On Thu, Mar 30, 2023 at 11:24:01AM -0700, Matt Roper wrote:
>Any interrupt registers that were introduced by platforms i915
>considered to be "gen11" or "gen12" are present on all platforms that
>the Xe driver supports; drop the unnecessary prefixes.
>
>While working in the area, also convert a few open-coded bit
>manipulations over to REG_BIT and REG_FIELD_GET notation.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
> drivers/gpu/drm/xe/regs/xe_gt_regs.h |  42 +++++------
> drivers/gpu/drm/xe/regs/xe_regs.h    |  12 +--

I was thinking more on an approach of cleaning up the entire driver:
get all the defines from regs/* that have the GEN*_ or _GEN* suffix and
drop the suffix.

I wrote it some time ago, but was having some issues with display/
failing to compile because of that (some of the ifdef's inside i915 use
our register defines).

Since this is done for irq, with more contained changes, I won't oppose
though. It's going in the right direction.

Checked with --color-words and it does what it says


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>


thanks
Lucas De Marchi

> drivers/gpu/drm/xe/xe_guc.c          |   6 +-
> drivers/gpu/drm/xe/xe_irq.c          | 109 +++++++++++++--------------
> 4 files changed, 84 insertions(+), 85 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>index f45251df5715..a8a37e6a45a3 100644
>--- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>+++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>@@ -348,34 +348,34 @@
> #define GFX_FLSH_CNTL_GEN6			_MMIO(0x101008)
> #define   GFX_FLSH_CNTL_EN			(1 << 0)
>
>-#define GEN11_GT_INTR_DW(x)			_MMIO(0x190018 + ((x) * 4))
>+#define GT_INTR_DW(x)				_MMIO(0x190018 + ((x) * 4))
>
>-#define GEN11_GUC_SG_INTR_ENABLE		_MMIO(0x190038)
>+#define GUC_SG_INTR_ENABLE			_MMIO(0x190038)
> #define   ENGINE1_MASK				REG_GENMASK(31, 16)
> #define   ENGINE0_MASK				REG_GENMASK(15, 0)
>
>-#define GEN11_GPM_WGBOXPERF_INTR_ENABLE		_MMIO(0x19003c)
>+#define GPM_WGBOXPERF_INTR_ENABLE		_MMIO(0x19003c)
>
>-#define GEN11_INTR_IDENTITY_REG(x)		_MMIO(0x190060 + ((x) * 4))
>-#define   GEN11_INTR_DATA_VALID			(1 << 31)
>-#define   GEN11_INTR_ENGINE_INSTANCE(x)		(((x) & GENMASK(25, 20)) >> 20)
>-#define   GEN11_INTR_ENGINE_CLASS(x)		(((x) & GENMASK(18, 16)) >> 16)
>-#define   GEN11_INTR_ENGINE_INTR(x)		((x) & 0xffff)
>+#define INTR_IDENTITY_REG(x)			_MMIO(0x190060 + ((x) * 4))
>+#define   INTR_DATA_VALID			REG_BIT(31)
>+#define   INTR_ENGINE_INSTANCE(x)		REG_FIELD_GET(GENMASK(25, 20), x)
>+#define   INTR_ENGINE_CLASS(x)			REG_FIELD_GET(GENMASK(18, 16), x)
>+#define   INTR_ENGINE_INTR(x)			REG_FIELD_GET(GENMASK(15, 0), x)
> #define   OTHER_GUC_INSTANCE			0
>
>-#define GEN11_RENDER_COPY_INTR_ENABLE		_MMIO(0x190030)
>-#define GEN11_VCS_VECS_INTR_ENABLE		_MMIO(0x190034)
>-#define GEN12_CCS_RSVD_INTR_ENABLE		_MMIO(0x190048)
>-#define GEN11_IIR_REG_SELECTOR(x)		_MMIO(0x190070 + ((x) * 4))
>-#define GEN11_RCS0_RSVD_INTR_MASK		_MMIO(0x190090)
>-#define GEN11_BCS_RSVD_INTR_MASK		_MMIO(0x1900a0)
>-#define GEN11_VCS0_VCS1_INTR_MASK		_MMIO(0x1900a8)
>-#define GEN11_VCS2_VCS3_INTR_MASK		_MMIO(0x1900ac)
>-#define GEN11_VECS0_VECS1_INTR_MASK		_MMIO(0x1900d0)
>-#define GEN11_GUC_SG_INTR_MASK			_MMIO(0x1900e8)
>-#define GEN11_GPM_WGBOXPERF_INTR_MASK		_MMIO(0x1900ec)
>-#define GEN12_CCS0_CCS1_INTR_MASK		_MMIO(0x190100)
>-#define GEN12_CCS2_CCS3_INTR_MASK		_MMIO(0x190104)
>+#define RENDER_COPY_INTR_ENABLE			_MMIO(0x190030)
>+#define VCS_VECS_INTR_ENABLE			_MMIO(0x190034)
>+#define CCS_RSVD_INTR_ENABLE			_MMIO(0x190048)
>+#define IIR_REG_SELECTOR(x)			_MMIO(0x190070 + ((x) * 4))
>+#define RCS0_RSVD_INTR_MASK			_MMIO(0x190090)
>+#define BCS_RSVD_INTR_MASK			_MMIO(0x1900a0)
>+#define VCS0_VCS1_INTR_MASK			_MMIO(0x1900a8)
>+#define VCS2_VCS3_INTR_MASK			_MMIO(0x1900ac)
>+#define VECS0_VECS1_INTR_MASK			_MMIO(0x1900d0)
>+#define GUC_SG_INTR_MASK			_MMIO(0x1900e8)
>+#define GPM_WGBOXPERF_INTR_MASK			_MMIO(0x1900ec)
>+#define CCS0_CCS1_INTR_MASK			_MMIO(0x190100)
>+#define CCS2_CCS3_INTR_MASK			_MMIO(0x190104)
> #define XEHPC_BCS1_BCS2_INTR_MASK		_MMIO(0x190110)
> #define XEHPC_BCS3_BCS4_INTR_MASK		_MMIO(0x190114)
> #define XEHPC_BCS5_BCS6_INTR_MASK		_MMIO(0x190118)
>diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
>index ffe5d726e196..34f12eacd432 100644
>--- a/drivers/gpu/drm/xe/regs/xe_regs.h
>+++ b/drivers/gpu/drm/xe/regs/xe_regs.h
>@@ -78,13 +78,13 @@
>
> #define PCU_IRQ_REGS				0x444e0
> #define GU_MISC_IRQ_REGS			0x444f0
>-#define   GEN11_GU_MISC_GSE			(1 << 27)
>+#define   GU_MISC_GSE				REG_BIT(27)
>
>-#define GEN11_GFX_MSTR_IRQ			_MMIO(0x190010)
>-#define   GEN11_MASTER_IRQ			(1 << 31)
>-#define   GEN11_GU_MISC_IRQ			(1 << 29)
>-#define   GEN11_DISPLAY_IRQ			(1 << 16)
>-#define   GEN11_GT_DW_IRQ(x)			(1 << (x))
>+#define GFX_MSTR_IRQ				_MMIO(0x190010)
>+#define   MASTER_IRQ				REG_BIT(31)
>+#define   GU_MISC_IRQ				REG_BIT(29)
>+#define   DISPLAY_IRQ				REG_BIT(16)
>+#define   GT_DW_IRQ(x)				REG_BIT(x)
>
> #define DG1_MSTR_TILE_INTR			_MMIO(0x190008)
> #define   DG1_MSTR_IRQ				REG_BIT(31)
>diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>index 58b9841616e4..ee71b969bcbf 100644
>--- a/drivers/gpu/drm/xe/xe_guc.c
>+++ b/drivers/gpu/drm/xe/xe_guc.c
>@@ -561,12 +561,12 @@ static void guc_enable_irq(struct xe_guc *guc)
> 		REG_FIELD_PREP(ENGINE0_MASK, GUC_INTR_GUC2HOST)  :
> 		REG_FIELD_PREP(ENGINE1_MASK, GUC_INTR_GUC2HOST);
>
>-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_ENABLE.reg,
>+	xe_mmio_write32(gt, GUC_SG_INTR_ENABLE.reg,
> 			REG_FIELD_PREP(ENGINE1_MASK, GUC_INTR_GUC2HOST));
> 	if (xe_gt_is_media_type(gt))
>-		xe_mmio_rmw32(gt, GEN11_GUC_SG_INTR_MASK.reg, events, 0);
>+		xe_mmio_rmw32(gt, GUC_SG_INTR_MASK.reg, events, 0);
> 	else
>-		xe_mmio_write32(gt, GEN11_GUC_SG_INTR_MASK.reg, ~events);
>+		xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg, ~events);
> }
>
> int xe_guc_enable_communication(struct xe_guc *guc)
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index bf097ba6a10b..b1b94ba43b3f 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -72,7 +72,7 @@ static void mask_and_disable(struct xe_gt *gt, u32 irqregs)
>
> static u32 gen11_intr_disable(struct xe_gt *gt)
> {
>-	xe_mmio_write32(gt, GEN11_GFX_MSTR_IRQ.reg, 0);
>+	xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, 0);
>
> 	/*
> 	 * Now with master disabled, get a sample of level indications
>@@ -80,7 +80,7 @@ static u32 gen11_intr_disable(struct xe_gt *gt)
> 	 * New indications can and will light up during processing,
> 	 * and will generate new interrupt after enabling master.
> 	 */
>-	return xe_mmio_read32(gt, GEN11_GFX_MSTR_IRQ.reg);
>+	return xe_mmio_read32(gt, GFX_MSTR_IRQ.reg);
> }
>
> static u32
>@@ -88,7 +88,7 @@ gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
> {
> 	u32 iir;
>
>-	if (!(master_ctl & GEN11_GU_MISC_IRQ))
>+	if (!(master_ctl & GU_MISC_IRQ))
> 		return 0;
>
> 	iir = xe_mmio_read32(gt, IIR(GU_MISC_IRQ_REGS).reg);
>@@ -100,9 +100,9 @@ gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
>
> static inline void gen11_intr_enable(struct xe_gt *gt, bool stall)
> {
>-	xe_mmio_write32(gt, GEN11_GFX_MSTR_IRQ.reg, GEN11_MASTER_IRQ);
>+	xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, MASTER_IRQ);
> 	if (stall)
>-		xe_mmio_read32(gt, GEN11_GFX_MSTR_IRQ.reg);
>+		xe_mmio_read32(gt, GFX_MSTR_IRQ.reg);
> }
>
> static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
>@@ -125,14 +125,14 @@ static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> 	smask = irqs << 16;
>
> 	/* Enable RCS, BCS, VCS and VECS class interrupts. */
>-	xe_mmio_write32(gt, GEN11_RENDER_COPY_INTR_ENABLE.reg, dmask);
>-	xe_mmio_write32(gt, GEN11_VCS_VECS_INTR_ENABLE.reg, dmask);
>+	xe_mmio_write32(gt, RENDER_COPY_INTR_ENABLE.reg, dmask);
>+	xe_mmio_write32(gt, VCS_VECS_INTR_ENABLE.reg, dmask);
> 	if (ccs_mask)
>-		xe_mmio_write32(gt, GEN12_CCS_RSVD_INTR_ENABLE.reg, smask);
>+		xe_mmio_write32(gt, CCS_RSVD_INTR_ENABLE.reg, smask);
>
> 	/* Unmask irqs on RCS, BCS, VCS and VECS engines. */
>-	xe_mmio_write32(gt, GEN11_RCS0_RSVD_INTR_MASK.reg, ~smask);
>-	xe_mmio_write32(gt, GEN11_BCS_RSVD_INTR_MASK.reg, ~smask);
>+	xe_mmio_write32(gt, RCS0_RSVD_INTR_MASK.reg, ~smask);
>+	xe_mmio_write32(gt, BCS_RSVD_INTR_MASK.reg, ~smask);
> 	if (bcs_mask & (BIT(1)|BIT(2)))
> 		xe_mmio_write32(gt, XEHPC_BCS1_BCS2_INTR_MASK.reg, ~dmask);
> 	if (bcs_mask & (BIT(3)|BIT(4)))
>@@ -141,31 +141,31 @@ static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> 		xe_mmio_write32(gt, XEHPC_BCS5_BCS6_INTR_MASK.reg, ~dmask);
> 	if (bcs_mask & (BIT(7)|BIT(8)))
> 		xe_mmio_write32(gt, XEHPC_BCS7_BCS8_INTR_MASK.reg, ~dmask);
>-	xe_mmio_write32(gt, GEN11_VCS0_VCS1_INTR_MASK.reg, ~dmask);
>-	xe_mmio_write32(gt, GEN11_VCS2_VCS3_INTR_MASK.reg, ~dmask);
>+	xe_mmio_write32(gt, VCS0_VCS1_INTR_MASK.reg, ~dmask);
>+	xe_mmio_write32(gt, VCS2_VCS3_INTR_MASK.reg, ~dmask);
> 	//if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
>-	//	intel_uncore_write(uncore, GEN12_VCS4_VCS5_INTR_MASK, ~dmask);
>+	//	intel_uncore_write(uncore, VCS4_VCS5_INTR_MASK, ~dmask);
> 	//if (HAS_ENGINE(gt, VCS6) || HAS_ENGINE(gt, VCS7))
>-	//	intel_uncore_write(uncore, GEN12_VCS6_VCS7_INTR_MASK, ~dmask);
>-	xe_mmio_write32(gt, GEN11_VECS0_VECS1_INTR_MASK.reg, ~dmask);
>+	//	intel_uncore_write(uncore, VCS6_VCS7_INTR_MASK, ~dmask);
>+	xe_mmio_write32(gt, VECS0_VECS1_INTR_MASK.reg, ~dmask);
> 	//if (HAS_ENGINE(gt, VECS2) || HAS_ENGINE(gt, VECS3))
>-	//	intel_uncore_write(uncore, GEN12_VECS2_VECS3_INTR_MASK, ~dmask);
>+	//	intel_uncore_write(uncore, VECS2_VECS3_INTR_MASK, ~dmask);
> 	if (ccs_mask & (BIT(0)|BIT(1)))
>-		xe_mmio_write32(gt, GEN12_CCS0_CCS1_INTR_MASK.reg, ~dmask);
>+		xe_mmio_write32(gt, CCS0_CCS1_INTR_MASK.reg, ~dmask);
> 	if (ccs_mask & (BIT(2)|BIT(3)))
>-		xe_mmio_write32(gt,  GEN12_CCS2_CCS3_INTR_MASK.reg, ~dmask);
>+		xe_mmio_write32(gt,  CCS2_CCS3_INTR_MASK.reg, ~dmask);
>
> 	/*
> 	 * RPS interrupts will get enabled/disabled on demand when RPS itself
> 	 * is enabled/disabled.
> 	 */
> 	/* TODO: gt->pm_ier, gt->pm_imr */
>-	xe_mmio_write32(gt, GEN11_GPM_WGBOXPERF_INTR_ENABLE.reg, 0);
>-	xe_mmio_write32(gt, GEN11_GPM_WGBOXPERF_INTR_MASK.reg,  ~0);
>+	xe_mmio_write32(gt, GPM_WGBOXPERF_INTR_ENABLE.reg, 0);
>+	xe_mmio_write32(gt, GPM_WGBOXPERF_INTR_MASK.reg,  ~0);
>
> 	/* Same thing for GuC interrupts */
>-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_ENABLE.reg, 0);
>-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_MASK.reg,  ~0);
>+	xe_mmio_write32(gt, GUC_SG_INTR_ENABLE.reg, 0);
>+	xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg,  ~0);
> }
>
> static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
>@@ -174,7 +174,7 @@ static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
>
> 	gen11_gt_irq_postinstall(xe, gt);
>
>-	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GEN11_GU_MISC_GSE);
>+	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
>
> 	gen11_intr_enable(gt, true);
> }
>@@ -190,7 +190,7 @@ gen11_gt_engine_identity(struct xe_device *xe,
>
> 	lockdep_assert_held(&xe->irq.lock);
>
>-	xe_mmio_write32(gt, GEN11_IIR_REG_SELECTOR(bank).reg, BIT(bit));
>+	xe_mmio_write32(gt, IIR_REG_SELECTOR(bank).reg, BIT(bit));
>
> 	/*
> 	 * NB: Specs do not specify how long to spin wait,
>@@ -198,18 +198,17 @@ gen11_gt_engine_identity(struct xe_device *xe,
> 	 */
> 	timeout_ts = (local_clock() >> 10) + 100;
> 	do {
>-		ident = xe_mmio_read32(gt, GEN11_INTR_IDENTITY_REG(bank).reg);
>-	} while (!(ident & GEN11_INTR_DATA_VALID) &&
>+		ident = xe_mmio_read32(gt, INTR_IDENTITY_REG(bank).reg);
>+	} while (!(ident & INTR_DATA_VALID) &&
> 		 !time_after32(local_clock() >> 10, timeout_ts));
>
>-	if (unlikely(!(ident & GEN11_INTR_DATA_VALID))) {
>+	if (unlikely(!(ident & INTR_DATA_VALID))) {
> 		drm_err(&xe->drm, "INTR_IDENTITY_REG%u:%u 0x%08x not valid!\n",
> 			bank, bit, ident);
> 		return 0;
> 	}
>
>-	xe_mmio_write32(gt, GEN11_INTR_IDENTITY_REG(bank).reg,
>-			GEN11_INTR_DATA_VALID);
>+	xe_mmio_write32(gt, INTR_IDENTITY_REG(bank).reg, INTR_DATA_VALID);
>
> 	return ident;
> }
>@@ -243,24 +242,24 @@ static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
> 	spin_lock(&xe->irq.lock);
>
> 	for (bank = 0; bank < 2; bank++) {
>-		if (!(master_ctl & GEN11_GT_DW_IRQ(bank)))
>+		if (!(master_ctl & GT_DW_IRQ(bank)))
> 			continue;
>
> 		if (!xe_gt_is_media_type(gt)) {
> 			intr_dw[bank] =
>-				xe_mmio_read32(gt, GEN11_GT_INTR_DW(bank).reg);
>+				xe_mmio_read32(gt, GT_INTR_DW(bank).reg);
> 			for_each_set_bit(bit, intr_dw + bank, 32)
> 				identity[bit] = gen11_gt_engine_identity(xe, gt,
> 									 bank,
> 									 bit);
>-			xe_mmio_write32(gt, GEN11_GT_INTR_DW(bank).reg,
>+			xe_mmio_write32(gt, GT_INTR_DW(bank).reg,
> 					intr_dw[bank]);
> 		}
>
> 		for_each_set_bit(bit, intr_dw + bank, 32) {
>-			class = GEN11_INTR_ENGINE_CLASS(identity[bit]);
>-			instance = GEN11_INTR_ENGINE_INSTANCE(identity[bit]);
>-			intr_vec = GEN11_INTR_ENGINE_INTR(identity[bit]);
>+			class = INTR_ENGINE_CLASS(identity[bit]);
>+			instance = INTR_ENGINE_INSTANCE(identity[bit]);
>+			intr_vec = INTR_ENGINE_INTR(identity[bit]);
>
> 			if (class == XE_ENGINE_CLASS_OTHER) {
> 				gen11_gt_other_irq_handler(gt, instance,
>@@ -337,7 +336,7 @@ static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> {
> 	gen11_gt_irq_postinstall(xe, gt);
>
>-	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GEN11_GU_MISC_GSE);
>+	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
>
> 	if (gt->info.id == XE_GT0)
> 		dg1_intr_enable(xe, true);
>@@ -365,7 +364,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
> 			continue;
>
> 		if (!xe_gt_is_media_type(gt))
>-			master_ctl = xe_mmio_read32(gt, GEN11_GFX_MSTR_IRQ.reg);
>+			master_ctl = xe_mmio_read32(gt, GFX_MSTR_IRQ.reg);
>
> 		/*
> 		 * We might be in irq handler just when PCIe DPC is initiated
>@@ -379,7 +378,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
> 		}
>
> 		if (!xe_gt_is_media_type(gt))
>-			xe_mmio_write32(gt, GEN11_GFX_MSTR_IRQ.reg, master_ctl);
>+			xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, master_ctl);
> 		gen11_gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
> 	}
>
>@@ -400,14 +399,14 @@ static void gen11_gt_irq_reset(struct xe_gt *gt)
> 	u32 bcs_mask = xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_COPY);
>
> 	/* Disable RCS, BCS, VCS and VECS class engines. */
>-	xe_mmio_write32(gt, GEN11_RENDER_COPY_INTR_ENABLE.reg,	 0);
>-	xe_mmio_write32(gt, GEN11_VCS_VECS_INTR_ENABLE.reg,	 0);
>+	xe_mmio_write32(gt, RENDER_COPY_INTR_ENABLE.reg,	 0);
>+	xe_mmio_write32(gt, VCS_VECS_INTR_ENABLE.reg,	 0);
> 	if (ccs_mask)
>-		xe_mmio_write32(gt, GEN12_CCS_RSVD_INTR_ENABLE.reg, 0);
>+		xe_mmio_write32(gt, CCS_RSVD_INTR_ENABLE.reg, 0);
>
> 	/* Restore masks irqs on RCS, BCS, VCS and VECS engines. */
>-	xe_mmio_write32(gt, GEN11_RCS0_RSVD_INTR_MASK.reg,	~0);
>-	xe_mmio_write32(gt, GEN11_BCS_RSVD_INTR_MASK.reg,	~0);
>+	xe_mmio_write32(gt, RCS0_RSVD_INTR_MASK.reg,	~0);
>+	xe_mmio_write32(gt, BCS_RSVD_INTR_MASK.reg,	~0);
> 	if (bcs_mask & (BIT(1)|BIT(2)))
> 		xe_mmio_write32(gt, XEHPC_BCS1_BCS2_INTR_MASK.reg, ~0);
> 	if (bcs_mask & (BIT(3)|BIT(4)))
>@@ -416,24 +415,24 @@ static void gen11_gt_irq_reset(struct xe_gt *gt)
> 		xe_mmio_write32(gt, XEHPC_BCS5_BCS6_INTR_MASK.reg, ~0);
> 	if (bcs_mask & (BIT(7)|BIT(8)))
> 		xe_mmio_write32(gt, XEHPC_BCS7_BCS8_INTR_MASK.reg, ~0);
>-	xe_mmio_write32(gt, GEN11_VCS0_VCS1_INTR_MASK.reg,	~0);
>-	xe_mmio_write32(gt, GEN11_VCS2_VCS3_INTR_MASK.reg,	~0);
>+	xe_mmio_write32(gt, VCS0_VCS1_INTR_MASK.reg,	~0);
>+	xe_mmio_write32(gt, VCS2_VCS3_INTR_MASK.reg,	~0);
> //	if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
>-//		xe_mmio_write32(xe, GEN12_VCS4_VCS5_INTR_MASK.reg,   ~0);
>+//		xe_mmio_write32(xe, VCS4_VCS5_INTR_MASK.reg,   ~0);
> //	if (HAS_ENGINE(gt, VCS6) || HAS_ENGINE(gt, VCS7))
>-//		xe_mmio_write32(xe, GEN12_VCS6_VCS7_INTR_MASK.reg,   ~0);
>-	xe_mmio_write32(gt, GEN11_VECS0_VECS1_INTR_MASK.reg,	~0);
>+//		xe_mmio_write32(xe, VCS6_VCS7_INTR_MASK.reg,   ~0);
>+	xe_mmio_write32(gt, VECS0_VECS1_INTR_MASK.reg,	~0);
> //	if (HAS_ENGINE(gt, VECS2) || HAS_ENGINE(gt, VECS3))
>-//		xe_mmio_write32(xe, GEN12_VECS2_VECS3_INTR_MASK.reg, ~0);
>+//		xe_mmio_write32(xe, VECS2_VECS3_INTR_MASK.reg, ~0);
> 	if (ccs_mask & (BIT(0)|BIT(1)))
>-		xe_mmio_write32(gt, GEN12_CCS0_CCS1_INTR_MASK.reg, ~0);
>+		xe_mmio_write32(gt, CCS0_CCS1_INTR_MASK.reg, ~0);
> 	if (ccs_mask & (BIT(2)|BIT(3)))
>-		xe_mmio_write32(gt,  GEN12_CCS2_CCS3_INTR_MASK.reg, ~0);
>+		xe_mmio_write32(gt,  CCS2_CCS3_INTR_MASK.reg, ~0);
>
>-	xe_mmio_write32(gt, GEN11_GPM_WGBOXPERF_INTR_ENABLE.reg, 0);
>-	xe_mmio_write32(gt, GEN11_GPM_WGBOXPERF_INTR_MASK.reg,  ~0);
>-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_ENABLE.reg,	 0);
>-	xe_mmio_write32(gt, GEN11_GUC_SG_INTR_MASK.reg,		~0);
>+	xe_mmio_write32(gt, GPM_WGBOXPERF_INTR_ENABLE.reg, 0);
>+	xe_mmio_write32(gt, GPM_WGBOXPERF_INTR_MASK.reg,  ~0);
>+	xe_mmio_write32(gt, GUC_SG_INTR_ENABLE.reg,	 0);
>+	xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg,		~0);
> }
>
> static void gen11_irq_reset(struct xe_gt *gt)
>-- 
>2.39.2
>

Re: [Intel-xe] [PATCH 4/8] drm/xe/irq: Drop unnecessary GEN11_ and GEN12_ register prefixes

[Lucas De Marchi]

On Fri, Mar 31, 2023 at 03:20:12PM -0700, Lucas De Marchi wrote:
>On Thu, Mar 30, 2023 at 11:24:01AM -0700, Matt Roper wrote:
>>Any interrupt registers that were introduced by platforms i915
>>considered to be "gen11" or "gen12" are present on all platforms that
>>the Xe driver supports; drop the unnecessary prefixes.
>>
>>While working in the area, also convert a few open-coded bit
>>manipulations over to REG_BIT and REG_FIELD_GET notation.
>>
>>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>---
>>drivers/gpu/drm/xe/regs/xe_gt_regs.h |  42 +++++------
>>drivers/gpu/drm/xe/regs/xe_regs.h    |  12 +--
>
>I was thinking more on an approach of cleaning up the entire driver:
>get all the defines from regs/* that have the GEN*_ or _GEN* suffix and
>drop the suffix.
>
>I wrote it some time ago, but was having some issues with display/
>failing to compile because of that (some of the ifdef's inside i915 use
>our register defines).
>
>Since this is done for irq, with more contained changes, I won't oppose
>though. It's going in the right direction.
>
>Checked with --color-words and it does what it says

but fails to build:

../drivers/gpu/drm/xe/xe_display.c: In function ‘xe_display_irq_handler’:
../drivers/gpu/drm/xe/xe_display.c:278:26: error: ‘GEN11_DISPLAY_IRQ’ undeclared (first use in this function); did you mean ‘DISPLAY_IRQ’?
   278 |         if (master_ctl & GEN11_DISPLAY_IRQ)
       |                          ^~~~~~~~~~~~~~~~~
       |                          DISPLAY_IRQ


Lucas De Marchi

[Intel-xe] [PATCH 5/8] drm/xe/irq: Rename and clarify top-level interrupt handling routines

[Matt Roper]

Platforms supported by the Xe driver handle top-level interrupts in one
of two ways:
 - Xe_LP platforms only have a "graphics master" register and lack a
   "master tile" register, so top-level interrupt detection and
   enable/disable happens in the graphics master.
 - Xe_LP+ (aka DG1) and beyond have a "master tile" interrupt register
   that controls the enable/disable of top-level interrupts and must
   also be consulted to determine which tiles have received interrupts
   before the driver moves on the process the graphics master register.

For functions that are only relevant to the first set of platforms,
rename the function prefix to Xe_LP since "gen11" doesn't make sense in
the Xe driver.  Also add some comments briefly describing the two
top-level handlers.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/xe/xe_irq.c | 46 +++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index b1b94ba43b3f..de0c27c0a09c 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -70,7 +70,7 @@ static void mask_and_disable(struct xe_gt *gt, u32 irqregs)
 	xe_mmio_read32(gt, IIR(irqregs).reg);
 }
 
-static u32 gen11_intr_disable(struct xe_gt *gt)
+static u32 xelp_intr_disable(struct xe_gt *gt)
 {
 	xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, 0);
 
@@ -98,7 +98,7 @@ gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
 	return iir;
 }
 
-static inline void gen11_intr_enable(struct xe_gt *gt, bool stall)
+static inline void xelp_intr_enable(struct xe_gt *gt, bool stall)
 {
 	xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, MASTER_IRQ);
 	if (stall)
@@ -168,7 +168,7 @@ static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 	xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg,  ~0);
 }
 
-static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
+static void xelp_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 {
 	/* TODO: PCH */
 
@@ -176,7 +176,7 @@ static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 
 	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
 
-	gen11_intr_enable(gt, true);
+	xelp_intr_enable(gt, true);
 }
 
 static u32
@@ -278,7 +278,11 @@ static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
 	spin_unlock(&xe->irq.lock);
 }
 
-static irqreturn_t gen11_irq_handler(int irq, void *arg)
+/*
+ * Top-level interrupt handler for Xe_LP platforms (which did not have
+ * a "master tile" interrupt register.
+ */
+static irqreturn_t xelp_irq_handler(int irq, void *arg)
 {
 	struct xe_device *xe = arg;
 	struct xe_gt *gt = xe_device_get_gt(xe, 0);	/* Only 1 GT here */
@@ -286,9 +290,9 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 	long unsigned int intr_dw[2];
 	u32 identity[32];
 
-	master_ctl = gen11_intr_disable(gt);
+	master_ctl = xelp_intr_disable(gt);
 	if (!master_ctl) {
-		gen11_intr_enable(gt, false);
+		xelp_intr_enable(gt, false);
 		return IRQ_NONE;
 	}
 
@@ -298,7 +302,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 
 	gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
 
-	gen11_intr_enable(gt, false);
+	xelp_intr_enable(gt, false);
 
 	xe_display_irq_enable(xe, gu_misc_iir);
 
@@ -342,6 +346,11 @@ static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 		dg1_intr_enable(xe, true);
 }
 
+/*
+ * Top-level interrupt handler for Xe_LP+ and beyond.  These platforms have
+ * a "master tile" interrupt register which must be consulted before the
+ * "graphics master" interrupt register.
+ */
 static irqreturn_t dg1_irq_handler(int irq, void *arg)
 {
 	struct xe_device *xe = arg;
@@ -435,9 +444,9 @@ static void gen11_gt_irq_reset(struct xe_gt *gt)
 	xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg,		~0);
 }
 
-static void gen11_irq_reset(struct xe_gt *gt)
+static void xelp_irq_reset(struct xe_gt *gt)
 {
-	gen11_intr_disable(gt);
+	xelp_intr_disable(gt);
 
 	gen11_gt_irq_reset(gt);
 
@@ -462,13 +471,10 @@ static void xe_irq_reset(struct xe_device *xe)
 	u8 id;
 
 	for_each_gt(gt, xe, id) {
-		if (GRAPHICS_VERx100(xe) >= 1210) {
+		if (GRAPHICS_VERx100(xe) >= 1210)
 			dg1_irq_reset(gt);
-		} else if (GRAPHICS_VER(xe) >= 11) {
-			gen11_irq_reset(gt);
-		} else {
-			drm_err(&xe->drm, "No interrupt reset hook");
-		}
+		else
+			xelp_irq_reset(gt);
 	}
 
 	xe_display_irq_reset(xe);
@@ -480,10 +486,8 @@ void xe_gt_irq_postinstall(struct xe_gt *gt)
 
 	if (GRAPHICS_VERx100(xe) >= 1210)
 		dg1_irq_postinstall(xe, gt);
-	else if (GRAPHICS_VER(xe) >= 11)
-		gen11_irq_postinstall(xe, gt);
 	else
-		drm_err(&xe->drm, "No interrupt postinstall hook");
+		xelp_irq_postinstall(xe, gt);
 
 	xe_display_irq_postinstall(xe, gt);
 }
@@ -501,10 +505,8 @@ static irq_handler_t xe_irq_handler(struct xe_device *xe)
 {
 	if (GRAPHICS_VERx100(xe) >= 1210) {
 		return dg1_irq_handler;
-	} else if (GRAPHICS_VER(xe) >= 11) {
-		return gen11_irq_handler;
 	} else {
-		return NULL;
+		return xelp_irq_handler;
 	}
 }
 
-- 
2.39.2

Re: [Intel-xe] [PATCH 5/8] drm/xe/irq: Rename and clarify top-level interrupt handling routines

[Lucas De Marchi]

On Thu, Mar 30, 2023 at 11:24:02AM -0700, Matt Roper wrote:
>Platforms supported by the Xe driver handle top-level interrupts in one
>of two ways:
> - Xe_LP platforms only have a "graphics master" register and lack a
>   "master tile" register, so top-level interrupt detection and
>   enable/disable happens in the graphics master.
> - Xe_LP+ (aka DG1) and beyond have a "master tile" interrupt register
>   that controls the enable/disable of top-level interrupts and must
>   also be consulted to determine which tiles have received interrupts
>   before the driver moves on the process the graphics master register.
>
>For functions that are only relevant to the first set of platforms,
>rename the function prefix to Xe_LP since "gen11" doesn't make sense in
>the Xe driver.  Also add some comments briefly describing the two
>top-level handlers.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
> drivers/gpu/drm/xe/xe_irq.c | 46 +++++++++++++++++++------------------
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index b1b94ba43b3f..de0c27c0a09c 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -70,7 +70,7 @@ static void mask_and_disable(struct xe_gt *gt, u32 irqregs)
> 	xe_mmio_read32(gt, IIR(irqregs).reg);
> }
>
>-static u32 gen11_intr_disable(struct xe_gt *gt)
>+static u32 xelp_intr_disable(struct xe_gt *gt)
> {
> 	xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, 0);
>
>@@ -98,7 +98,7 @@ gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
> 	return iir;
> }
>
>-static inline void gen11_intr_enable(struct xe_gt *gt, bool stall)
>+static inline void xelp_intr_enable(struct xe_gt *gt, bool stall)
> {
> 	xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, MASTER_IRQ);
> 	if (stall)
>@@ -168,7 +168,7 @@ static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> 	xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg,  ~0);
> }
>
>-static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
>+static void xelp_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> {
> 	/* TODO: PCH */
>
>@@ -176,7 +176,7 @@ static void gen11_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
>
> 	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
>
>-	gen11_intr_enable(gt, true);
>+	xelp_intr_enable(gt, true);
> }
>
> static u32
>@@ -278,7 +278,11 @@ static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
> 	spin_unlock(&xe->irq.lock);
> }
>
>-static irqreturn_t gen11_irq_handler(int irq, void *arg)
>+/*
>+ * Top-level interrupt handler for Xe_LP platforms (which did not have
>+ * a "master tile" interrupt register.
>+ */
>+static irqreturn_t xelp_irq_handler(int irq, void *arg)
> {
> 	struct xe_device *xe = arg;
> 	struct xe_gt *gt = xe_device_get_gt(xe, 0);	/* Only 1 GT here */
>@@ -286,9 +290,9 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
> 	long unsigned int intr_dw[2];
> 	u32 identity[32];
>
>-	master_ctl = gen11_intr_disable(gt);
>+	master_ctl = xelp_intr_disable(gt);
> 	if (!master_ctl) {
>-		gen11_intr_enable(gt, false);
>+		xelp_intr_enable(gt, false);
> 		return IRQ_NONE;
> 	}
>
>@@ -298,7 +302,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
>
> 	gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
>
>-	gen11_intr_enable(gt, false);
>+	xelp_intr_enable(gt, false);
>
> 	xe_display_irq_enable(xe, gu_misc_iir);
>
>@@ -342,6 +346,11 @@ static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> 		dg1_intr_enable(xe, true);
> }
>
>+/*
>+ * Top-level interrupt handler for Xe_LP+ and beyond.  These platforms have
>+ * a "master tile" interrupt register which must be consulted before the
>+ * "graphics master" interrupt register.
>+ */
> static irqreturn_t dg1_irq_handler(int irq, void *arg)
> {
> 	struct xe_device *xe = arg;
>@@ -435,9 +444,9 @@ static void gen11_gt_irq_reset(struct xe_gt *gt)
> 	xe_mmio_write32(gt, GUC_SG_INTR_MASK.reg,		~0);
> }
>
>-static void gen11_irq_reset(struct xe_gt *gt)
>+static void xelp_irq_reset(struct xe_gt *gt)
> {
>-	gen11_intr_disable(gt);
>+	xelp_intr_disable(gt);
>
> 	gen11_gt_irq_reset(gt);
>
>@@ -462,13 +471,10 @@ static void xe_irq_reset(struct xe_device *xe)
> 	u8 id;
>
> 	for_each_gt(gt, xe, id) {
>-		if (GRAPHICS_VERx100(xe) >= 1210) {
>+		if (GRAPHICS_VERx100(xe) >= 1210)
> 			dg1_irq_reset(gt);
>-		} else if (GRAPHICS_VER(xe) >= 11) {
>-			gen11_irq_reset(gt);
>-		} else {
>-			drm_err(&xe->drm, "No interrupt reset hook");
>-		}
>+		else
>+			xelp_irq_reset(gt);
> 	}
>
> 	xe_display_irq_reset(xe);
>@@ -480,10 +486,8 @@ void xe_gt_irq_postinstall(struct xe_gt *gt)
>
> 	if (GRAPHICS_VERx100(xe) >= 1210)
> 		dg1_irq_postinstall(xe, gt);
>-	else if (GRAPHICS_VER(xe) >= 11)
>-		gen11_irq_postinstall(xe, gt);
> 	else
>-		drm_err(&xe->drm, "No interrupt postinstall hook");
>+		xelp_irq_postinstall(xe, gt);
>
> 	xe_display_irq_postinstall(xe, gt);
> }
>@@ -501,10 +505,8 @@ static irq_handler_t xe_irq_handler(struct xe_device *xe)
> {
> 	if (GRAPHICS_VERx100(xe) >= 1210) {
> 		return dg1_irq_handler;
>-	} else if (GRAPHICS_VER(xe) >= 11) {
>-		return gen11_irq_handler;
> 	} else {
>-		return NULL;
>+		return xelp_irq_handler;


also a great cleanup of dead code since these else branches
unreachable.


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks
Lucas De Marchi

[Intel-xe] [PATCH 6/8] drm/xe/irq: Drop remaining "gen11_" prefix from IRQ functions

[Matt Roper]

The remaining "gen11_*" IRQ functions are common to all platforms
supported by the Xe driver.  Drop the unnecessary prefix.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/xe/xe_irq.c | 46 ++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index de0c27c0a09c..9fac03b63e7e 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -84,7 +84,7 @@ static u32 xelp_intr_disable(struct xe_gt *gt)
 }
 
 static u32
-gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
+gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
 {
 	u32 iir;
 
@@ -105,7 +105,7 @@ static inline void xelp_intr_enable(struct xe_gt *gt, bool stall)
 		xe_mmio_read32(gt, GFX_MSTR_IRQ.reg);
 }
 
-static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
+static void gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 {
 	u32 irqs, dmask, smask;
 	u32 ccs_mask = xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_COMPUTE);
@@ -172,7 +172,7 @@ static void xelp_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 {
 	/* TODO: PCH */
 
-	gen11_gt_irq_postinstall(xe, gt);
+	gt_irq_postinstall(xe, gt);
 
 	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
 
@@ -180,10 +180,10 @@ static void xelp_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 }
 
 static u32
-gen11_gt_engine_identity(struct xe_device *xe,
-			 struct xe_gt *gt,
-			 const unsigned int bank,
-			 const unsigned int bit)
+gt_engine_identity(struct xe_device *xe,
+		   struct xe_gt *gt,
+		   const unsigned int bank,
+		   const unsigned int bit)
 {
 	u32 timeout_ts;
 	u32 ident;
@@ -216,7 +216,7 @@ gen11_gt_engine_identity(struct xe_device *xe,
 #define   OTHER_MEDIA_GUC_INSTANCE           16
 
 static void
-gen11_gt_other_irq_handler(struct xe_gt *gt, const u8 instance, const u16 iir)
+gt_other_irq_handler(struct xe_gt *gt, const u8 instance, const u16 iir)
 {
 	if (instance == OTHER_GUC_INSTANCE && !xe_gt_is_media_type(gt))
 		return xe_guc_irq_handler(&gt->uc.guc, iir);
@@ -230,9 +230,9 @@ gen11_gt_other_irq_handler(struct xe_gt *gt, const u8 instance, const u16 iir)
 	}
 }
 
-static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
-				 u32 master_ctl, long unsigned int *intr_dw,
-				 u32 *identity)
+static void gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
+			   u32 master_ctl, long unsigned int *intr_dw,
+			   u32 *identity)
 {
 	unsigned int bank, bit;
 	u16 instance, intr_vec;
@@ -249,9 +249,8 @@ static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
 			intr_dw[bank] =
 				xe_mmio_read32(gt, GT_INTR_DW(bank).reg);
 			for_each_set_bit(bit, intr_dw + bank, 32)
-				identity[bit] = gen11_gt_engine_identity(xe, gt,
-									 bank,
-									 bit);
+				identity[bit] = gt_engine_identity(xe, gt,
+								   bank, bit);
 			xe_mmio_write32(gt, GT_INTR_DW(bank).reg,
 					intr_dw[bank]);
 		}
@@ -262,8 +261,7 @@ static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
 			intr_vec = INTR_ENGINE_INTR(identity[bit]);
 
 			if (class == XE_ENGINE_CLASS_OTHER) {
-				gen11_gt_other_irq_handler(gt, instance,
-							   intr_vec);
+				gt_other_irq_handler(gt, instance, intr_vec);
 				continue;
 			}
 
@@ -296,11 +294,11 @@ static irqreturn_t xelp_irq_handler(int irq, void *arg)
 		return IRQ_NONE;
 	}
 
-	gen11_gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
+	gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
 
 	xe_display_irq_handler(xe, master_ctl);
 
-	gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
+	gu_misc_iir = gu_misc_irq_ack(gt, master_ctl);
 
 	xelp_intr_enable(gt, false);
 
@@ -338,7 +336,7 @@ static void dg1_intr_enable(struct xe_device *xe, bool stall)
 
 static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 {
-	gen11_gt_irq_postinstall(xe, gt);
+	gt_irq_postinstall(xe, gt);
 
 	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
 
@@ -388,12 +386,12 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
 
 		if (!xe_gt_is_media_type(gt))
 			xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, master_ctl);
-		gen11_gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
+		gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
 	}
 
 	xe_display_irq_handler(xe, master_ctl);
 
-	gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
+	gu_misc_iir = gu_misc_irq_ack(gt, master_ctl);
 
 	dg1_intr_enable(xe, false);
 
@@ -402,7 +400,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-static void gen11_gt_irq_reset(struct xe_gt *gt)
+static void gt_irq_reset(struct xe_gt *gt)
 {
 	u32 ccs_mask = xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_COMPUTE);
 	u32 bcs_mask = xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_COPY);
@@ -448,7 +446,7 @@ static void xelp_irq_reset(struct xe_gt *gt)
 {
 	xelp_intr_disable(gt);
 
-	gen11_gt_irq_reset(gt);
+	gt_irq_reset(gt);
 
 	mask_and_disable(gt, GU_MISC_IRQ_REGS);
 	mask_and_disable(gt, PCU_IRQ_REGS);
@@ -459,7 +457,7 @@ static void dg1_irq_reset(struct xe_gt *gt)
 	if (gt->info.id == 0)
 		dg1_intr_disable(gt_to_xe(gt));
 
-	gen11_gt_irq_reset(gt);
+	gt_irq_reset(gt);
 
 	mask_and_disable(gt, GU_MISC_IRQ_REGS);
 	mask_and_disable(gt, PCU_IRQ_REGS);
-- 
2.39.2

Re: [Intel-xe] [PATCH 6/8] drm/xe/irq: Drop remaining "gen11_" prefix from IRQ functions

[Lucas De Marchi]

On Thu, Mar 30, 2023 at 11:24:03AM -0700, Matt Roper wrote:
>The remaining "gen11_*" IRQ functions are common to all platforms
>supported by the Xe driver.  Drop the unnecessary prefix.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>---
> drivers/gpu/drm/xe/xe_irq.c | 46 ++++++++++++++++++-------------------
> 1 file changed, 22 insertions(+), 24 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index de0c27c0a09c..9fac03b63e7e 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -84,7 +84,7 @@ static u32 xelp_intr_disable(struct xe_gt *gt)
> }
>
> static u32
>-gen11_gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
>+gu_misc_irq_ack(struct xe_gt *gt, const u32 master_ctl)
> {
> 	u32 iir;
>
>@@ -105,7 +105,7 @@ static inline void xelp_intr_enable(struct xe_gt *gt, bool stall)
> 		xe_mmio_read32(gt, GFX_MSTR_IRQ.reg);
> }
>
>-static void gen11_gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
>+static void gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> {
> 	u32 irqs, dmask, smask;
> 	u32 ccs_mask = xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_COMPUTE);
>@@ -172,7 +172,7 @@ static void xelp_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> {
> 	/* TODO: PCH */
>
>-	gen11_gt_irq_postinstall(xe, gt);
>+	gt_irq_postinstall(xe, gt);
>
> 	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
>
>@@ -180,10 +180,10 @@ static void xelp_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> }
>
> static u32
>-gen11_gt_engine_identity(struct xe_device *xe,
>-			 struct xe_gt *gt,
>-			 const unsigned int bank,
>-			 const unsigned int bit)
>+gt_engine_identity(struct xe_device *xe,
>+		   struct xe_gt *gt,
>+		   const unsigned int bank,
>+		   const unsigned int bit)
> {
> 	u32 timeout_ts;
> 	u32 ident;
>@@ -216,7 +216,7 @@ gen11_gt_engine_identity(struct xe_device *xe,
> #define   OTHER_MEDIA_GUC_INSTANCE           16
>
> static void
>-gen11_gt_other_irq_handler(struct xe_gt *gt, const u8 instance, const u16 iir)
>+gt_other_irq_handler(struct xe_gt *gt, const u8 instance, const u16 iir)
> {
> 	if (instance == OTHER_GUC_INSTANCE && !xe_gt_is_media_type(gt))
> 		return xe_guc_irq_handler(&gt->uc.guc, iir);
>@@ -230,9 +230,9 @@ gen11_gt_other_irq_handler(struct xe_gt *gt, const u8 instance, const u16 iir)
> 	}
> }
>
>-static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
>-				 u32 master_ctl, long unsigned int *intr_dw,
>-				 u32 *identity)
>+static void gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
>+			   u32 master_ctl, long unsigned int *intr_dw,
>+			   u32 *identity)
> {
> 	unsigned int bank, bit;
> 	u16 instance, intr_vec;
>@@ -249,9 +249,8 @@ static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
> 			intr_dw[bank] =
> 				xe_mmio_read32(gt, GT_INTR_DW(bank).reg);
> 			for_each_set_bit(bit, intr_dw + bank, 32)
>-				identity[bit] = gen11_gt_engine_identity(xe, gt,
>-									 bank,
>-									 bit);
>+				identity[bit] = gt_engine_identity(xe, gt,
>+								   bank, bit);
> 			xe_mmio_write32(gt, GT_INTR_DW(bank).reg,
> 					intr_dw[bank]);
> 		}
>@@ -262,8 +261,7 @@ static void gen11_gt_irq_handler(struct xe_device *xe, struct xe_gt *gt,
> 			intr_vec = INTR_ENGINE_INTR(identity[bit]);
>
> 			if (class == XE_ENGINE_CLASS_OTHER) {
>-				gen11_gt_other_irq_handler(gt, instance,
>-							   intr_vec);
>+				gt_other_irq_handler(gt, instance, intr_vec);
> 				continue;
> 			}
>
>@@ -296,11 +294,11 @@ static irqreturn_t xelp_irq_handler(int irq, void *arg)
> 		return IRQ_NONE;
> 	}
>
>-	gen11_gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
>+	gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
>
> 	xe_display_irq_handler(xe, master_ctl);
>
>-	gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
>+	gu_misc_iir = gu_misc_irq_ack(gt, master_ctl);
>
> 	xelp_intr_enable(gt, false);
>
>@@ -338,7 +336,7 @@ static void dg1_intr_enable(struct xe_device *xe, bool stall)
>
> static void dg1_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> {
>-	gen11_gt_irq_postinstall(xe, gt);
>+	gt_irq_postinstall(xe, gt);
>
> 	unmask_and_enable(gt, GU_MISC_IRQ_REGS, GU_MISC_GSE);
>
>@@ -388,12 +386,12 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
>
> 		if (!xe_gt_is_media_type(gt))
> 			xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, master_ctl);
>-		gen11_gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
>+		gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
> 	}
>
> 	xe_display_irq_handler(xe, master_ctl);
>
>-	gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
>+	gu_misc_iir = gu_misc_irq_ack(gt, master_ctl);
>
> 	dg1_intr_enable(xe, false);
>
>@@ -402,7 +400,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
> 	return IRQ_HANDLED;
> }
>
>-static void gen11_gt_irq_reset(struct xe_gt *gt)
>+static void gt_irq_reset(struct xe_gt *gt)
> {
> 	u32 ccs_mask = xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_COMPUTE);
> 	u32 bcs_mask = xe_hw_engine_mask_per_class(gt, XE_ENGINE_CLASS_COPY);
>@@ -448,7 +446,7 @@ static void xelp_irq_reset(struct xe_gt *gt)
> {
> 	xelp_intr_disable(gt);
>
>-	gen11_gt_irq_reset(gt);
>+	gt_irq_reset(gt);
>
> 	mask_and_disable(gt, GU_MISC_IRQ_REGS);
> 	mask_and_disable(gt, PCU_IRQ_REGS);
>@@ -459,7 +457,7 @@ static void dg1_irq_reset(struct xe_gt *gt)
> 	if (gt->info.id == 0)
> 		dg1_intr_disable(gt_to_xe(gt));
>
>-	gen11_gt_irq_reset(gt);
>+	gt_irq_reset(gt);
>
> 	mask_and_disable(gt, GU_MISC_IRQ_REGS);
> 	mask_and_disable(gt, PCU_IRQ_REGS);
>-- 
>2.39.2
>

[Intel-xe] [PATCH 7/8] drm/xe/irq: Drop commented-out code for non-existent media engines

[Matt Roper]

Although the hardware team has set aside some register bits for extra
media engines, no platform supported by the Xe driver today has VCS4-7
or VECS2-3.  Drop the corresponding code (which was already commented
out); we can bring it back easily enough if such engines show up on a
future platform.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/xe/xe_irq.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index 9fac03b63e7e..02292e60e52c 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -143,13 +143,7 @@ static void gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
 		xe_mmio_write32(gt, XEHPC_BCS7_BCS8_INTR_MASK.reg, ~dmask);
 	xe_mmio_write32(gt, VCS0_VCS1_INTR_MASK.reg, ~dmask);
 	xe_mmio_write32(gt, VCS2_VCS3_INTR_MASK.reg, ~dmask);
-	//if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
-	//	intel_uncore_write(uncore, VCS4_VCS5_INTR_MASK, ~dmask);
-	//if (HAS_ENGINE(gt, VCS6) || HAS_ENGINE(gt, VCS7))
-	//	intel_uncore_write(uncore, VCS6_VCS7_INTR_MASK, ~dmask);
 	xe_mmio_write32(gt, VECS0_VECS1_INTR_MASK.reg, ~dmask);
-	//if (HAS_ENGINE(gt, VECS2) || HAS_ENGINE(gt, VECS3))
-	//	intel_uncore_write(uncore, VECS2_VECS3_INTR_MASK, ~dmask);
 	if (ccs_mask & (BIT(0)|BIT(1)))
 		xe_mmio_write32(gt, CCS0_CCS1_INTR_MASK.reg, ~dmask);
 	if (ccs_mask & (BIT(2)|BIT(3)))
@@ -424,13 +418,7 @@ static void gt_irq_reset(struct xe_gt *gt)
 		xe_mmio_write32(gt, XEHPC_BCS7_BCS8_INTR_MASK.reg, ~0);
 	xe_mmio_write32(gt, VCS0_VCS1_INTR_MASK.reg,	~0);
 	xe_mmio_write32(gt, VCS2_VCS3_INTR_MASK.reg,	~0);
-//	if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
-//		xe_mmio_write32(xe, VCS4_VCS5_INTR_MASK.reg,   ~0);
-//	if (HAS_ENGINE(gt, VCS6) || HAS_ENGINE(gt, VCS7))
-//		xe_mmio_write32(xe, VCS6_VCS7_INTR_MASK.reg,   ~0);
 	xe_mmio_write32(gt, VECS0_VECS1_INTR_MASK.reg,	~0);
-//	if (HAS_ENGINE(gt, VECS2) || HAS_ENGINE(gt, VECS3))
-//		xe_mmio_write32(xe, VECS2_VECS3_INTR_MASK.reg, ~0);
 	if (ccs_mask & (BIT(0)|BIT(1)))
 		xe_mmio_write32(gt, CCS0_CCS1_INTR_MASK.reg, ~0);
 	if (ccs_mask & (BIT(2)|BIT(3)))
-- 
2.39.2

Re: [Intel-xe] [PATCH 7/8] drm/xe/irq: Drop commented-out code for non-existent media engines

[Lucas De Marchi]

On Thu, Mar 30, 2023 at 11:24:04AM -0700, Matt Roper wrote:
>Although the hardware team has set aside some register bits for extra
>media engines, no platform supported by the Xe driver today has VCS4-7
>or VECS2-3.  Drop the corresponding code (which was already commented
>out); we can bring it back easily enough if such engines show up on a
>future platform.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>---
> drivers/gpu/drm/xe/xe_irq.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index 9fac03b63e7e..02292e60e52c 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -143,13 +143,7 @@ static void gt_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> 		xe_mmio_write32(gt, XEHPC_BCS7_BCS8_INTR_MASK.reg, ~dmask);
> 	xe_mmio_write32(gt, VCS0_VCS1_INTR_MASK.reg, ~dmask);
> 	xe_mmio_write32(gt, VCS2_VCS3_INTR_MASK.reg, ~dmask);
>-	//if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
>-	//	intel_uncore_write(uncore, VCS4_VCS5_INTR_MASK, ~dmask);
>-	//if (HAS_ENGINE(gt, VCS6) || HAS_ENGINE(gt, VCS7))
>-	//	intel_uncore_write(uncore, VCS6_VCS7_INTR_MASK, ~dmask);
> 	xe_mmio_write32(gt, VECS0_VECS1_INTR_MASK.reg, ~dmask);
>-	//if (HAS_ENGINE(gt, VECS2) || HAS_ENGINE(gt, VECS3))
>-	//	intel_uncore_write(uncore, VECS2_VECS3_INTR_MASK, ~dmask);
> 	if (ccs_mask & (BIT(0)|BIT(1)))
> 		xe_mmio_write32(gt, CCS0_CCS1_INTR_MASK.reg, ~dmask);
> 	if (ccs_mask & (BIT(2)|BIT(3)))
>@@ -424,13 +418,7 @@ static void gt_irq_reset(struct xe_gt *gt)
> 		xe_mmio_write32(gt, XEHPC_BCS7_BCS8_INTR_MASK.reg, ~0);
> 	xe_mmio_write32(gt, VCS0_VCS1_INTR_MASK.reg,	~0);
> 	xe_mmio_write32(gt, VCS2_VCS3_INTR_MASK.reg,	~0);
>-//	if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
>-//		xe_mmio_write32(xe, VCS4_VCS5_INTR_MASK.reg,   ~0);
>-//	if (HAS_ENGINE(gt, VCS6) || HAS_ENGINE(gt, VCS7))
>-//		xe_mmio_write32(xe, VCS6_VCS7_INTR_MASK.reg,   ~0);
> 	xe_mmio_write32(gt, VECS0_VECS1_INTR_MASK.reg,	~0);
>-//	if (HAS_ENGINE(gt, VECS2) || HAS_ENGINE(gt, VECS3))
>-//		xe_mmio_write32(xe, VECS2_VECS3_INTR_MASK.reg, ~0);
> 	if (ccs_mask & (BIT(0)|BIT(1)))
> 		xe_mmio_write32(gt, CCS0_CCS1_INTR_MASK.reg, ~0);
> 	if (ccs_mask & (BIT(2)|BIT(3)))
>-- 
>2.39.2
>

[Intel-xe] [PATCH 8/8] drm/xe/irq: Don't clobber display interrupts on multi-tile platforms

[Matt Roper]

Although our only multi-tile platform today (PVC) doesn't support
display, it's possible that some future multi-tile platform will.
If/when this happens, display interrupts (both traditional display and
ASLE backlight interrupts raised as a Gunit interrupt) should be
delivered to the primary tile.  Save away tile0's master_ctl value so
that it can still be used for display interrupt handling after the GT
loop.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/xe/xe_irq.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index 02292e60e52c..636653f4e7ad 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -347,7 +347,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
 {
 	struct xe_device *xe = arg;
 	struct xe_gt *gt;
-	u32 master_tile_ctl, master_ctl = 0, gu_misc_iir;
+	u32 master_tile_ctl, master_ctl = 0, tile0_master_ctl = 0, gu_misc_iir;
 	long unsigned int intr_dw[2];
 	u32 identity[32];
 	u8 id;
@@ -381,11 +381,19 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
 		if (!xe_gt_is_media_type(gt))
 			xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, master_ctl);
 		gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
+
+		/*
+		 * Save primary tile's master interrupt register for display
+		 * processing below.
+		 */
+		if (id == 0)
+			tile0_master_ctl = master_ctl;
 	}
 
-	xe_display_irq_handler(xe, master_ctl);
+	xe_display_irq_handler(xe, tile0_master_ctl);
 
-	gu_misc_iir = gu_misc_irq_ack(gt, master_ctl);
+	/* Gunit GSE interrupts can trigger display backlight operations */
+	gu_misc_iir = gu_misc_irq_ack(gt, tile0_master_ctl);
 
 	dg1_intr_enable(xe, false);
 
-- 
2.39.2

Re: [Intel-xe] [PATCH 8/8] drm/xe/irq: Don't clobber display interrupts on multi-tile platforms

[Lucas De Marchi]

On Thu, Mar 30, 2023 at 11:24:05AM -0700, Matt Roper wrote:
>Although our only multi-tile platform today (PVC) doesn't support
>display, it's possible that some future multi-tile platform will.
>If/when this happens, display interrupts (both traditional display and
>ASLE backlight interrupts raised as a Gunit interrupt) should be
>delivered to the primary tile.  Save away tile0's master_ctl value so
>that it can still be used for display interrupt handling after the GT
>loop.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>---
> drivers/gpu/drm/xe/xe_irq.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index 02292e60e52c..636653f4e7ad 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -347,7 +347,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
> {
> 	struct xe_device *xe = arg;
> 	struct xe_gt *gt;
>-	u32 master_tile_ctl, master_ctl = 0, gu_misc_iir;
>+	u32 master_tile_ctl, master_ctl = 0, tile0_master_ctl = 0, gu_misc_iir;
> 	long unsigned int intr_dw[2];
> 	u32 identity[32];
> 	u8 id;
>@@ -381,11 +381,19 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
> 		if (!xe_gt_is_media_type(gt))
> 			xe_mmio_write32(gt, GFX_MSTR_IRQ.reg, master_ctl);
> 		gt_irq_handler(xe, gt, master_ctl, intr_dw, identity);
>+
>+		/*
>+		 * Save primary tile's master interrupt register for display
>+		 * processing below.
>+		 */
>+		if (id == 0)
>+			tile0_master_ctl = master_ctl;
> 	}
>
>-	xe_display_irq_handler(xe, master_ctl);
>+	xe_display_irq_handler(xe, tile0_master_ctl);
>
>-	gu_misc_iir = gu_misc_irq_ack(gt, master_ctl);
>+	/* Gunit GSE interrupts can trigger display backlight operations */
>+	gu_misc_iir = gu_misc_irq_ack(gt, tile0_master_ctl);
>
> 	dg1_intr_enable(xe, false);
>
>-- 
>2.39.2
>

[Intel-xe] ✓ CI.Patch_applied: success for Interrupt cleanup and future-proofing

[Patchwork]

== Series Details ==

Series: Interrupt cleanup and future-proofing
URL   : https://patchwork.freedesktop.org/series/115884/
State : success

== Summary ==

=== Applying kernel patches on branch 'drm-xe-next' with base: ===
commit 5de0375b04455c7c8e516f53715eedde50f5ae45
Author:     Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
AuthorDate: Thu Mar 23 15:05:18 2023 +0100
Commit:     Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
CommitDate: Wed Mar 29 11:24:42 2023 +0200

    drm/xe: Display api changes update
    
    Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
=== git am output follows ===
Applying: drm/xe/irq: Drop gen3_ prefixes
Applying: drm/xe/irq: Add helpers to find ISR/IIR/IMR/IER registers
Applying: drm/xe/irq: Drop IRQ_INIT and IRQ_RESET macros
Applying: drm/xe/irq: Drop unnecessary GEN11_ and GEN12_ register prefixes
Applying: drm/xe/irq: Rename and clarify top-level interrupt handling routines
Applying: drm/xe/irq: Drop remaining "gen11_" prefix from IRQ functions
Applying: drm/xe/irq: Drop commented-out code for non-existent media engines
Applying: drm/xe/irq: Don't clobber display interrupts on multi-tile platforms

[Intel-xe] ✗ CI.KUnit: failure for Interrupt cleanup and future-proofing

[Patchwork]

== Series Details ==

Series: Interrupt cleanup and future-proofing
URL   : https://patchwork.freedesktop.org/series/115884/
State : failure

== Summary ==

+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
ERROR:root:In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/drivers/chan_user.c:6:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/os-Linux/helper.c:6:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/os-Linux/execvp.c:24:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/drivers/fd.c:7:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/os-Linux/irq.c:8:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/os-Linux/mem.c:8:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/os-Linux/main.c:8:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/os-Linux/file.c:8:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/x86/um/os-Linux/registers.c:8:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
make[3]: *** [../scripts/Makefile.build:252: arch/um/os-Linux/execvp.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[3]: *** [../scripts/Makefile.build:252: arch/um/os-Linux/irq.o] Error 1
make[3]: *** [../scripts/Makefile.build:252: arch/um/drivers/fd.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[4]: *** [../scripts/Makefile.build:252: arch/x86/um/os-Linux/registers.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [../scripts/Makefile.build:252: arch/um/drivers/chan_user.o] Error 1
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/x86/um/os-Linux/task_size.c:3:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
make[3]: *** [../scripts/Makefile.build:252: arch/um/os-Linux/helper.o] Error 1
make[3]: *** [../scripts/Makefile.build:252: arch/um/os-Linux/main.o] Error 1
make[3]: *** [../scripts/Makefile.build:252: arch/um/os-Linux/mem.o] Error 1
make[4]: *** [../scripts/Makefile.build:252: arch/x86/um/os-Linux/task_size.o] Error 1
make[3]: *** [../scripts/Makefile.build:494: arch/x86/um/os-Linux] Error 2
make[3]: *** Waiting for unfinished jobs....
make[3]: *** [../scripts/Makefile.build:252: arch/um/os-Linux/file.o] Error 1
In file included from /usr/include/stdlib.h:1013,
                 from ../arch/um/os-Linux/skas/process.c:7:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
make[4]: *** [../scripts/Makefile.build:252: arch/um/os-Linux/skas/process.o] Error 1
make[3]: *** [../scripts/Makefile.build:494: arch/um/os-Linux/skas] Error 2
make[2]: *** [../scripts/Makefile.build:494: arch/um/os-Linux] Error 2
make[2]: *** Waiting for unfinished jobs....
make[2]: *** [../scripts/Makefile.build:494: arch/x86/um] Error 2
make[2]: *** [../scripts/Makefile.build:494: arch/um/drivers] Error 2
In file included from /usr/include/stdlib.h:1013,
                 from arch/um/kernel/config.c:7:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
   26 | {
      | ^
make[3]: *** [../scripts/Makefile.build:252: arch/um/kernel/config.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [../scripts/Makefile.build:494: arch/um/kernel] Error 2
../drivers/gpu/drm/xe/xe_display.c: In function ‘xe_display_irq_handler’:
../drivers/gpu/drm/xe/xe_display.c:278:19: error: ‘GEN11_DISPLAY_IRQ’ undeclared (first use in this function); did you mean ‘DISPLAY_IRQ’?
  278 |  if (master_ctl & GEN11_DISPLAY_IRQ)
      |                   ^~~~~~~~~~~~~~~~~
      |                   DISPLAY_IRQ
../drivers/gpu/drm/xe/xe_display.c:278:19: note: each undeclared identifier is reported only once for each function it appears in
../drivers/gpu/drm/xe/xe_display.c: In function ‘xe_display_irq_enable’:
../drivers/gpu/drm/xe/xe_display.c:287:20: error: ‘GEN11_GU_MISC_GSE’ undeclared (first use in this function); did you mean ‘GU_MISC_GSE’?
  287 |  if (gu_misc_iir & GEN11_GU_MISC_GSE)
      |                    ^~~~~~~~~~~~~~~~~
      |                    GU_MISC_GSE
make[6]: *** [../scripts/Makefile.build:252: drivers/gpu/drm/xe/xe_display.o] Error 1
make[6]: *** Waiting for unfinished jobs....
make[5]: *** [../scripts/Makefile.build:494: drivers/gpu/drm/xe] Error 2
make[4]: *** [../scripts/Makefile.build:494: drivers/gpu/drm] Error 2
make[3]: *** [../scripts/Makefile.build:494: drivers/gpu] Error 2
make[2]: *** [../scripts/Makefile.build:494: drivers] Error 2
make[1]: *** [/kernel/Makefile:2025: .] Error 2
make: *** [Makefile:226: __sub-make] Error 2

[18:26:31] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[18:26:35] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=48
+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel