From: Jani Nikula <jani.nikula@intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/i915/irq: add struct i915_irq_regs triplet
Date: Fri, 27 Sep 2024 11:04:18 +0300 [thread overview]
Message-ID: <87y13dzfnh.fsf@intel.com> (raw)
In-Reply-To: <ZvWyF0RORVLoXMVP@intel.com>
On Thu, 26 Sep 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> 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@intel.com>
>> ---
>> 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@intel.com>
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
next prev parent reply other threads:[~2024-09-27 8:04 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-26 16:57 [PATCH 0/3] drm/i915/irq: clean up irq reset/init macro hacks Jani Nikula
2024-09-26 16:57 ` [PATCH 1/3] drm/i915/irq: add struct i915_irq_regs triplet Jani Nikula
2024-09-26 19:12 ` Rodrigo Vivi
2024-09-27 8:04 ` Jani Nikula [this message]
2024-09-26 16:57 ` [PATCH 2/3] drm/i915/irq: remove GEN3_IRQ_RESET() and GEN3_IRQ_INIT() macros Jani Nikula
2024-09-26 19:13 ` Rodrigo Vivi
2024-09-26 16:57 ` [PATCH 3/3] drm/i915/irq: remove GEN8_IRQ_RESET_NDX() and GEN8_IRQ_INIT_NDX() macros Jani Nikula
2024-09-26 19:14 ` Rodrigo Vivi
2024-10-01 2:46 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/irq: clean up irq reset/init macro hacks (rev2) Patchwork
2024-10-01 2:46 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-10-01 2:55 ` ✓ Fi.CI.BAT: success " Patchwork
2024-10-01 16:25 ` ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y13dzfnh.fsf@intel.com \
--to=jani.nikula@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=rodrigo.vivi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox