From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Jani Nikula <jani.nikula@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: Thu, 26 Sep 2024 15:12:23 -0400 [thread overview]
Message-ID: <ZvWyF0RORVLoXMVP@intel.com> (raw)
In-Reply-To: <a51dac3cb572e58654fa705b011d357671c17413.1727369787.git.jani.nikula@intel.com>
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,
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> })
>
> #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
>
next prev parent reply other threads:[~2024-09-26 19:12 UTC|newest]
Thread overview: 29+ 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 [this message]
2024-09-27 8:04 ` Jani Nikula
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-09-26 19:53 ` ✓ CI.Patch_applied: success for drm/i915/irq: clean up irq reset/init macro hacks Patchwork
2024-09-26 19:53 ` ✗ CI.checkpatch: warning " Patchwork
2024-09-26 19:55 ` ✓ CI.KUnit: success " Patchwork
2024-09-26 20:06 ` ✓ CI.Build: " Patchwork
2024-09-26 20:09 ` ✓ CI.Hooks: " Patchwork
2024-09-26 20:10 ` ✗ CI.checksparse: warning " Patchwork
2024-09-27 10:23 ` Jani Nikula
2024-09-26 20:36 ` ✓ CI.BAT: success " Patchwork
2024-09-27 21:36 ` ✗ CI.FULL: failure " Patchwork
2024-09-30 18:20 ` ✓ CI.Patch_applied: success for drm/i915/irq: clean up irq reset/init macro hacks (rev2) Patchwork
2024-09-30 18:21 ` ✗ CI.checkpatch: warning " Patchwork
2024-09-30 18:22 ` ✓ CI.KUnit: success " Patchwork
2024-09-30 18:33 ` ✓ CI.Build: " Patchwork
2024-09-30 18:36 ` ✓ CI.Hooks: " Patchwork
2024-09-30 18:37 ` ✗ CI.checksparse: warning " Patchwork
2024-09-30 19:02 ` ✗ CI.BAT: failure " Patchwork
2024-10-01 0:34 ` ✗ CI.FULL: " Patchwork
2024-10-01 2:46 ` ✗ Fi.CI.CHECKPATCH: warning " 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=ZvWyF0RORVLoXMVP@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=jani.nikula@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.