On Thu, Sep 26, 2024 at 07:57:46PM +0300, Jani Nikula wrote:
> Add struct i915_irq_regs to hold IMR/IER/IIR register offsets to pass to
> gen3_irq_reset() and gen3_irq_init(). This helps in grouping the
> registers and further cleanup.
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 31 ++++++++++-------------
> drivers/gpu/drm/i915/i915_irq.h | 30 ++++++++++------------
> drivers/gpu/drm/i915/i915_reg_defs.h | 10 ++++++++
> drivers/gpu/drm/xe/display/ext/i915_irq.c | 31 ++++++++++-------------
> 4 files changed, 51 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a784803f709a..7938a44b5681 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -77,19 +77,18 @@ static inline void pmu_irq_stats(struct drm_i915_private *i915,
> WRITE_ONCE(i915->pmu.irq_count, i915->pmu.irq_count + 1);
> }
>
> -void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
> - i915_reg_t iir, i915_reg_t ier)
> +void gen3_irq_reset(struct intel_uncore *uncore, struct i915_irq_regs regs)
> {
> - intel_uncore_write(uncore, imr, 0xffffffff);
> - intel_uncore_posting_read(uncore, imr);
> + intel_uncore_write(uncore, regs.imr, 0xffffffff);
> + intel_uncore_posting_read(uncore, regs.imr);
>
> - intel_uncore_write(uncore, ier, 0);
> + intel_uncore_write(uncore, regs.ier, 0);
>
> /* IIR can theoretically queue up two events. Be paranoid. */
> - intel_uncore_write(uncore, iir, 0xffffffff);
> - intel_uncore_posting_read(uncore, iir);
> - intel_uncore_write(uncore, iir, 0xffffffff);
> - intel_uncore_posting_read(uncore, iir);
> + intel_uncore_write(uncore, regs.iir, 0xffffffff);
> + intel_uncore_posting_read(uncore, regs.iir);
> + intel_uncore_write(uncore, regs.iir, 0xffffffff);
> + intel_uncore_posting_read(uncore, regs.iir);
> }
>
> static void gen2_irq_reset(struct intel_uncore *uncore)
> @@ -141,16 +140,14 @@ static void gen2_assert_iir_is_zero(struct intel_uncore *uncore)
> intel_uncore_posting_read16(uncore, GEN2_IIR);
> }
>
> -void gen3_irq_init(struct intel_uncore *uncore,
> - i915_reg_t imr, u32 imr_val,
> - i915_reg_t ier, u32 ier_val,
> - i915_reg_t iir)
> +void gen3_irq_init(struct intel_uncore *uncore, struct i915_irq_regs regs,
> + u32 imr_val, u32 ier_val)
> {
> - gen3_assert_iir_is_zero(uncore, iir);
> + gen3_assert_iir_is_zero(uncore, regs.iir);
>
> - intel_uncore_write(uncore, ier, ier_val);
> - intel_uncore_write(uncore, imr, imr_val);
> - intel_uncore_posting_read(uncore, imr);
> + intel_uncore_write(uncore, regs.ier, ier_val);
> + intel_uncore_write(uncore, regs.imr, imr_val);
> + intel_uncore_posting_read(uncore, regs.imr);
> }
>
> static void gen2_irq_init(struct intel_uncore *uncore,
> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
> index cde4cac5eca2..361ba46eed76 100644
> --- a/drivers/gpu/drm/i915/i915_irq.h
> +++ b/drivers/gpu/drm/i915/i915_irq.h
> @@ -42,37 +42,33 @@ void intel_synchronize_hardirq(struct drm_i915_private *i915);
>
> void gen3_assert_iir_is_zero(struct intel_uncore *uncore, i915_reg_t reg);
>
> -void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
> - i915_reg_t iir, i915_reg_t ier);
> +void gen3_irq_reset(struct intel_uncore *uncore, struct i915_irq_regs regs);
>
> -void gen3_irq_init(struct intel_uncore *uncore,
> - i915_reg_t imr, u32 imr_val,
> - i915_reg_t ier, u32 ier_val,
> - i915_reg_t iir);
> +void gen3_irq_init(struct intel_uncore *uncore, struct i915_irq_regs regs,
> + u32 imr_val, u32 ier_val);
>
> #define GEN8_IRQ_RESET_NDX(uncore, type, which) \
> ({ \
> unsigned int which_ = which; \
> - gen3_irq_reset((uncore), GEN8_##type##_IMR(which_), \
> - GEN8_##type##_IIR(which_), GEN8_##type##_IER(which_)); \
> + gen3_irq_reset((uncore), I915_IRQ_REGS(GEN8_##type##_IMR(which_), \
> + GEN8_##type##_IER(which_), \
> + GEN8_##type##_IIR(which_))); \
after checking other patches and seeing that this is really going away,
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> })
>
> #define GEN3_IRQ_RESET(uncore, type) \
> - gen3_irq_reset((uncore), type##IMR, type##IIR, type##IER)
> + gen3_irq_reset((uncore), I915_IRQ_REGS(type##IMR, type##IER, type##IIR))
>
> #define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
> ({ \
> unsigned int which_ = which; \
> - gen3_irq_init((uncore), \
> - GEN8_##type##_IMR(which_), imr_val, \
> - GEN8_##type##_IER(which_), ier_val, \
> - GEN8_##type##_IIR(which_)); \
> + gen3_irq_init((uncore), I915_IRQ_REGS(GEN8_##type##_IMR(which_), \
> + GEN8_##type##_IER(which_), \
> + GEN8_##type##_IIR(which_)), \
> + imr_val, ier_val); \
> })
>
> #define GEN3_IRQ_INIT(uncore, type, imr_val, ier_val) \
> - gen3_irq_init((uncore), \
> - type##IMR, imr_val, \
> - type##IER, ier_val, \
> - type##IIR)
> + gen3_irq_init((uncore), I915_IRQ_REGS(type##IMR, type##IER, type##IIR), \
> + imr_val, ier_val)
>
> #endif /* __I915_IRQ_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
> index a685db1e815d..e251bcc0c89f 100644
> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
> @@ -284,4 +284,14 @@ typedef struct {
> #define i915_mmio_reg_equal(a, b) (i915_mmio_reg_offset(a) == i915_mmio_reg_offset(b))
> #define i915_mmio_reg_valid(r) (!i915_mmio_reg_equal(r, INVALID_MMIO_REG))
>
> +/* A triplet for IMR/IER/IIR registers. */
> +struct i915_irq_regs {
> + i915_reg_t imr;
> + i915_reg_t ier;
> + i915_reg_t iir;
> +};
> +
> +#define I915_IRQ_REGS(_imr, _ier, _iir) \
> + ((const struct i915_irq_regs){ .imr = (_imr), .ier = (_ier), .iir = (_iir) })
> +
> #endif /* __I915_REG_DEFS__ */
> diff --git a/drivers/gpu/drm/xe/display/ext/i915_irq.c b/drivers/gpu/drm/xe/display/ext/i915_irq.c
> index eb40f1cb44f6..977ef47ea1f9 100644
> --- a/drivers/gpu/drm/xe/display/ext/i915_irq.c
> +++ b/drivers/gpu/drm/xe/display/ext/i915_irq.c
> @@ -7,19 +7,18 @@
> #include "i915_reg.h"
> #include "intel_uncore.h"
>
> -void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
> - i915_reg_t iir, i915_reg_t ier)
> +void gen3_irq_reset(struct intel_uncore *uncore, struct i915_irq_regs regs)
> {
> - intel_uncore_write(uncore, imr, 0xffffffff);
> - intel_uncore_posting_read(uncore, imr);
> + intel_uncore_write(uncore, regs.imr, 0xffffffff);
> + intel_uncore_posting_read(uncore, regs.imr);
>
> - intel_uncore_write(uncore, ier, 0);
> + intel_uncore_write(uncore, regs.ier, 0);
>
> /* IIR can theoretically queue up two events. Be paranoid. */
> - intel_uncore_write(uncore, iir, 0xffffffff);
> - intel_uncore_posting_read(uncore, iir);
> - intel_uncore_write(uncore, iir, 0xffffffff);
> - intel_uncore_posting_read(uncore, iir);
> + intel_uncore_write(uncore, regs.iir, 0xffffffff);
> + intel_uncore_posting_read(uncore, regs.iir);
> + intel_uncore_write(uncore, regs.iir, 0xffffffff);
> + intel_uncore_posting_read(uncore, regs.iir);
> }
>
> /*
> @@ -42,16 +41,14 @@ void gen3_assert_iir_is_zero(struct intel_uncore *uncore, i915_reg_t reg)
> intel_uncore_posting_read(uncore, reg);
> }
>
> -void gen3_irq_init(struct intel_uncore *uncore,
> - i915_reg_t imr, u32 imr_val,
> - i915_reg_t ier, u32 ier_val,
> - i915_reg_t iir)
> +void gen3_irq_init(struct intel_uncore *uncore, struct i915_irq_regs regs,
> + u32 imr_val, u32 ier_val)
> {
> - gen3_assert_iir_is_zero(uncore, iir);
> + gen3_assert_iir_is_zero(uncore, regs.iir);
>
> - intel_uncore_write(uncore, ier, ier_val);
> - intel_uncore_write(uncore, imr, imr_val);
> - intel_uncore_posting_read(uncore, imr);
> + intel_uncore_write(uncore, regs.ier, ier_val);
> + intel_uncore_write(uncore, regs.imr, imr_val);
> + intel_uncore_posting_read(uncore, regs.imr);
> }
>
> bool intel_irqs_enabled(struct xe_device *xe)
> --
> 2.39.2
>
