[Intel-gfx] [PATCH 06/10] drm/i915/icl: Streamline guc irq handling

[Chris Wilson]

Quoting Mika Kuoppala (2018-09-20 15:33:46)
> The returning of iir through function parameter is eyesore.
> Make guc irq acking inline and return the iir directly, handling
> the empty iir exception early. We can then omit passing the
> master control to guc handler as the iir now contains everything
> we need.
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 38 ++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index c35576f9c3f5..e9034d6d87b0 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3088,36 +3088,34 @@ gen11_gt_irq_handler(struct drm_i915_private * const i915,
>         spin_unlock(&i915->irq_lock);
>  }
>  
> -static void
> -gen11_gu_misc_irq_ack(struct drm_i915_private *dev_priv, const u32 master_ctl,
> -                     u32 *iir)
> +static inline u32
> +gen11_gu_misc_irq_ack(void __iomem * const regs, const u32 master_ctl)
>  {
> -       void __iomem * const regs = dev_priv->regs;
> +       u32 iir;
>  
>         if (!(master_ctl & GEN11_GU_MISC_IRQ))
> -               return;
> +               return 0;
> +
> +       iir = raw_reg_read(regs, GEN11_GU_MISC_IIR);
> +       if (likely(iir))
> +               raw_reg_write(regs, GEN11_GU_MISC_IIR, iir);
> +       else
> +               DRM_ERROR("GU_MISC iir blank!\n");
>  
> -       *iir = raw_reg_read(regs, GEN11_GU_MISC_IIR);
> -       if (likely(*iir))
> -               raw_reg_write(regs, GEN11_GU_MISC_IIR, *iir);
> +       return iir;
>  }
>  
>  static void
>  gen11_gu_misc_irq_handler(struct drm_i915_private *dev_priv,
> -                         const u32 master_ctl, const u32 iir)
> +                         const u32 iir)
>  {
> -       if (!(master_ctl & GEN11_GU_MISC_IRQ))
> -               return;

However, the argument is that by using master_ctl as our guard for all
functions, should encourage the compiler to keep it around in a
register. That's the thinking at least.

Care to test that theory if it makes any significant difference to code
layout and register usage?
-Chris