On Thu, 26 Sep 2024, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
> 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,
Yeah, it's not a pretty intermediate step, but I kept that intermediate
step with hopes that it's easier to review this way.
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Thanks, will push once the CI results are in.
>
>> })
>>
>> #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
>>
--
Jani Nikula, Intel
