Re: [Intel-gfx] [PATCH v5] drm/i915: Make IRQ reset and postinstall multi-gt aware

[Andrzej Hajda <andrzej.hajda@xxxxxxxxx>]

On 18.04.2023 01:53, Andi Shyti wrote:
> In multi-gt systems IRQs need to be reset and enabled per GT.
>
> This might add some redundancy when handling interrupts for
> engines that might not exist in every tile, but helps to keep the
> code cleaner and more understandable.
>
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> Hi,
>
> thanks Matt for the review.
>
> Andi
>
> Changelog
> =========
> v4 -> v5
>   - Another little cosmetic on the variable declaration. Go back
>     to v3 but using "&dev_priv->uncore" instead of
>     "to_gt(dev_priv)->uncore", which is much cleaner.
>   - Add Matt's r-b.
> v3 -> v4
>   - do not change the initial gt and uncore initialization in
>     order to gain a better understanding at a glance of the
>     purpose of all the local variables.
> v2 -> v3
>   - keep GUnit irq initialization out of the for_each_gt() loop as
>     the media GT doesn't have a GUnit.
> v1 -> v2
>   - improve description in the commit log.
>
>   drivers/gpu/drm/i915/i915_irq.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
>
>   drivers/gpu/drm/i915/i915_irq.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)

Harmless duplication, I guess.

Reviewed-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>

Regards
Andrzej

> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index dea1a117f3fa1..eead067f18c7a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2856,12 +2856,15 @@ static void gen11_irq_reset(struct drm_i915_private *dev_priv)
>   
>   static void dg1_irq_reset(struct drm_i915_private *dev_priv)
>   {
> -	struct intel_gt *gt = to_gt(dev_priv);
> -	struct intel_uncore *uncore = gt->uncore;
> +	struct intel_uncore *uncore = &dev_priv->uncore;
> +	struct intel_gt *gt;
> +	unsigned int i;
>   
>   	dg1_master_intr_disable(dev_priv->uncore.regs);
>   
> -	gen11_gt_irq_reset(gt);
> +	for_each_gt(gt, dev_priv, i)
> +		gen11_gt_irq_reset(gt);
> +
>   	gen11_display_irq_reset(dev_priv);
>   
>   	GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_);
> @@ -3643,11 +3646,13 @@ static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
>   
>   static void dg1_irq_postinstall(struct drm_i915_private *dev_priv)
>   {
> -	struct intel_gt *gt = to_gt(dev_priv);
> -	struct intel_uncore *uncore = gt->uncore;
> +	struct intel_uncore *uncore = &dev_priv->uncore;
>   	u32 gu_misc_masked = GEN11_GU_MISC_GSE;
> +	struct intel_gt *gt;
> +	unsigned int i;
>   
> -	gen11_gt_irq_postinstall(gt);
> +	for_each_gt(gt, dev_priv, i)
> +		gen11_gt_irq_postinstall(gt);
>   
>   	GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
>