Re: [PATCH 3/3] drm/i915/irq: remove GEN8_IRQ_RESET_NDX() and GEN8_IRQ_INIT_NDX() macros

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Sep 26, 2024 at 07:57:48PM +0300, Jani Nikula wrote:
> Define register offset triplets for all registers used with
> GEN8_IRQ_RESET_NDX() and GEN8_IRQ_INIT_NDX() macros, and call the
> underlying gen3_irq_reset() and gen3_irq_init() functions
> directly. Remove the macros, along with the macro name concatenation
> hackery.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  .../gpu/drm/i915/display/intel_display_irq.c   | 18 +++++++++---------
>  drivers/gpu/drm/i915/gt/intel_gt_irq.c         | 16 ++++++++--------
>  drivers/gpu/drm/i915/i915_irq.h                | 17 -----------------
>  drivers/gpu/drm/i915/i915_reg.h                |  8 ++++++++
>  4 files changed, 25 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index 5c6b9918ed3a..ed243283ba6b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -1546,7 +1546,7 @@ void gen8_display_irq_reset(struct drm_i915_private *dev_priv)
>  	for_each_pipe(dev_priv, pipe)
>  		if (intel_display_power_is_enabled(dev_priv,
>  						   POWER_DOMAIN_PIPE(pipe)))
> -			GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
> +			gen3_irq_reset(uncore, GEN8_DE_PIPE_IRQ_REGS(pipe));
>  
>  	gen3_irq_reset(uncore, GEN8_DE_PORT_IRQ_REGS);
>  	gen3_irq_reset(uncore, GEN8_DE_MISC_IRQ_REGS);
> @@ -1589,7 +1589,7 @@ void gen11_display_irq_reset(struct drm_i915_private *dev_priv)
>  	for_each_pipe(dev_priv, pipe)
>  		if (intel_display_power_is_enabled(dev_priv,
>  						   POWER_DOMAIN_PIPE(pipe)))
> -			GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
> +			gen3_irq_reset(uncore, GEN8_DE_PIPE_IRQ_REGS(pipe));
>  
>  	gen3_irq_reset(uncore, GEN8_DE_PORT_IRQ_REGS);
>  	gen3_irq_reset(uncore, GEN8_DE_MISC_IRQ_REGS);
> @@ -1620,9 +1620,9 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
>  	}
>  
>  	for_each_pipe_masked(dev_priv, pipe, pipe_mask)
> -		GEN8_IRQ_INIT_NDX(uncore, DE_PIPE, pipe,
> -				  dev_priv->display.irq.de_irq_mask[pipe],
> -				  ~dev_priv->display.irq.de_irq_mask[pipe] | extra_ier);
> +		gen3_irq_init(uncore, GEN8_DE_PIPE_IRQ_REGS(pipe),
> +			      dev_priv->display.irq.de_irq_mask[pipe],
> +			      ~dev_priv->display.irq.de_irq_mask[pipe] | extra_ier);
>  
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  }
> @@ -1641,7 +1641,7 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
>  	}
>  
>  	for_each_pipe_masked(dev_priv, pipe, pipe_mask)
> -		GEN8_IRQ_RESET_NDX(uncore, DE_PIPE, pipe);
> +		gen3_irq_reset(uncore, GEN8_DE_PIPE_IRQ_REGS(pipe));
>  
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
> @@ -1829,9 +1829,9 @@ void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  
>  		if (intel_display_power_is_enabled(dev_priv,
>  						   POWER_DOMAIN_PIPE(pipe)))
> -			GEN8_IRQ_INIT_NDX(uncore, DE_PIPE, pipe,
> -					  dev_priv->display.irq.de_irq_mask[pipe],
> -					  de_pipe_enables);
> +			gen3_irq_init(uncore, GEN8_DE_PIPE_IRQ_REGS(pipe),
> +				      dev_priv->display.irq.de_irq_mask[pipe],
> +				      de_pipe_enables);
>  	}
>  
>  	gen3_irq_init(uncore, GEN8_DE_PORT_IRQ_REGS, ~de_port_masked, de_port_enables);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> index fbb3117e324a..0c1e405240af 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> @@ -452,10 +452,10 @@ void gen8_gt_irq_reset(struct intel_gt *gt)
>  {
>  	struct intel_uncore *uncore = gt->uncore;
>  
> -	GEN8_IRQ_RESET_NDX(uncore, GT, 0);
> -	GEN8_IRQ_RESET_NDX(uncore, GT, 1);
> -	GEN8_IRQ_RESET_NDX(uncore, GT, 2);
> -	GEN8_IRQ_RESET_NDX(uncore, GT, 3);
> +	gen3_irq_reset(uncore, GEN8_GT_IRQ_REGS(0));
> +	gen3_irq_reset(uncore, GEN8_GT_IRQ_REGS(1));
> +	gen3_irq_reset(uncore, GEN8_GT_IRQ_REGS(2));
> +	gen3_irq_reset(uncore, GEN8_GT_IRQ_REGS(3));
>  }
>  
>  void gen8_gt_irq_postinstall(struct intel_gt *gt)
> @@ -476,14 +476,14 @@ void gen8_gt_irq_postinstall(struct intel_gt *gt)
>  
>  	gt->pm_ier = 0x0;
>  	gt->pm_imr = ~gt->pm_ier;
> -	GEN8_IRQ_INIT_NDX(uncore, GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
> -	GEN8_IRQ_INIT_NDX(uncore, GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
> +	gen3_irq_init(uncore, GEN8_GT_IRQ_REGS(0), ~gt_interrupts[0], gt_interrupts[0]);
> +	gen3_irq_init(uncore, GEN8_GT_IRQ_REGS(1), ~gt_interrupts[1], gt_interrupts[1]);
>  	/*
>  	 * RPS interrupts will get enabled/disabled on demand when RPS itself
>  	 * is enabled/disabled. Same wil be the case for GuC interrupts.
>  	 */
> -	GEN8_IRQ_INIT_NDX(uncore, GT, 2, gt->pm_imr, gt->pm_ier);
> -	GEN8_IRQ_INIT_NDX(uncore, GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
> +	gen3_irq_init(uncore, GEN8_GT_IRQ_REGS(2), gt->pm_imr, gt->pm_ier);
> +	gen3_irq_init(uncore, GEN8_GT_IRQ_REGS(3), ~gt_interrupts[3], gt_interrupts[3]);
>  }
>  
>  static void gen5_gt_update_irq(struct intel_gt *gt,
> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
> index 06a38671b32b..da3d97143511 100644
> --- a/drivers/gpu/drm/i915/i915_irq.h
> +++ b/drivers/gpu/drm/i915/i915_irq.h
> @@ -47,21 +47,4 @@ void gen3_irq_reset(struct intel_uncore *uncore, struct i915_irq_regs regs);
>  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), I915_IRQ_REGS(GEN8_##type##_IMR(which_), \
> -					       GEN8_##type##_IER(which_), \
> -					       GEN8_##type##_IIR(which_))); \
> -})
> -
> -#define GEN8_IRQ_INIT_NDX(uncore, type, which, imr_val, ier_val) \
> -({ \
> -	unsigned int which_ = which; \
> -	gen3_irq_init((uncore), I915_IRQ_REGS(GEN8_##type##_IMR(which_), \
> -					      GEN8_##type##_IER(which_), \
> -					      GEN8_##type##_IIR(which_)), \
> -		      imr_val, ier_val); \
> -})
> -
>  #endif /* __I915_IRQ_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 818fb71f7efc..818142f5a10c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2491,6 +2491,10 @@
>  #define GEN8_GT_IIR(which) _MMIO(0x44308 + (0x10 * (which)))
>  #define GEN8_GT_IER(which) _MMIO(0x4430c + (0x10 * (which)))
>  
> +#define GEN8_GT_IRQ_REGS(which)		I915_IRQ_REGS(GEN8_GT_IMR(which), \
> +						      GEN8_GT_IER(which), \
> +						      GEN8_GT_IIR(which))
> +
>  #define GEN8_RCS_IRQ_SHIFT 0
>  #define GEN8_BCS_IRQ_SHIFT 16
>  #define GEN8_VCS0_IRQ_SHIFT 0  /* NB: VCS1 in bspec! */
> @@ -2542,6 +2546,10 @@
>  #define  GEN8_PIPE_VSYNC		REG_BIT(1)
>  #define  GEN8_PIPE_VBLANK		REG_BIT(0)
>  
> +#define GEN8_DE_PIPE_IRQ_REGS(pipe)	I915_IRQ_REGS(GEN8_DE_PIPE_IMR(pipe), \
> +						      GEN8_DE_PIPE_IER(pipe), \
> +						      GEN8_DE_PIPE_IIR(pipe))
> +
>  #define _HPD_PIN_DDI(hpd_pin)	((hpd_pin) - HPD_PORT_A)
>  #define _HPD_PIN_TC(hpd_pin)	((hpd_pin) - HPD_PORT_TC1)
>  
> -- 
> 2.39.2
>