[Intel-gfx] [PATCH 4/4] drm/i915: split gen11_irq_handler to make it shareable

[Lucas De Marchi]

On Thu, Oct 24, 2019 at 12:51:22PM -0700, Lucas De Marchi wrote:
>Split gen11_irq_handler() to receive as parameter the function
>pointers. This allows to share the interrupt handler even if the enable/disable
>functions are different.
>
>Make sure it's always inlined to avoid the extra indirect call on the
>hot path. Checking with gcc 9 this produce the exact same code as of
>now:
>
>$ size drivers/gpu/drm/i915/i915_irq*.o
>   text	   data	    bss	    dec	    hex	filename
>  47511	    560	      0	  48071	   bbc7	drivers/gpu/drm/i915/i915_irq.o
>  47511	    560	      0	  48071	   bbc7	drivers/gpu/drm/i915/i915_irq_new.o
>
>$ gdb -batch -ex 'file drivers/gpu/drm/i915/i915_irq.o' -ex 'disassemble gen11_irq_handler' > /tmp/old.s
>$ gdb -batch -ex 'file drivers/gpu/drm/i915/i915_irq_new.o' -ex 'disassemble gen11_irq_handler' > /tmp/new.s
>$ git diff --no-index /tmp/{old,new}.s
>$
>
>So, no change in behavior, just a simple refactor.
>
>Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>---
> drivers/gpu/drm/i915/i915_irq.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>index 572a5c37cc61..8eb7d02b4a55 100644
>--- a/drivers/gpu/drm/i915/i915_irq.c
>+++ b/drivers/gpu/drm/i915/i915_irq.c
>@@ -2859,9 +2859,11 @@ static inline void gen11_master_intr_enable(void __iomem * const regs)
> 	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ);
> }
>
>-static irqreturn_t gen11_irq_handler(int irq, void *arg)
>+static __always_inline irqreturn_t
>+__gen11_irq_handler(struct drm_i915_private * const i915,
>+		    u32 (*intr_disable)(void __iomem * const regs),
>+		    void (*intr_enable)(void __iomem * const regs))

offline review from Chris:

Hmm. __always_inline indeed should avoid the compiler using vfuncs here,
no matter how may times we use the same base handler.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>


Lucas De Marchi

> {
>-	struct drm_i915_private * const i915 = arg;
> 	void __iomem * const regs = i915->uncore.regs;
> 	struct intel_gt *gt = &i915->gt;
> 	u32 master_ctl;
>@@ -2870,9 +2872,9 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
> 	if (!intel_irqs_enabled(i915))
> 		return IRQ_NONE;
>
>-	master_ctl = gen11_master_intr_disable(regs);
>+	master_ctl = intr_disable(regs);
> 	if (!master_ctl) {
>-		gen11_master_intr_enable(regs);
>+		intr_enable(regs);
> 		return IRQ_NONE;
> 	}
>
>@@ -2894,13 +2896,20 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
>
> 	gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
>
>-	gen11_master_intr_enable(regs);
>+	intr_enable(regs);
>
> 	gen11_gu_misc_irq_handler(gt, gu_misc_iir);
>
> 	return IRQ_HANDLED;
> }
>
>+static irqreturn_t gen11_irq_handler(int irq, void *arg)
>+{
>+	return __gen11_irq_handler(arg,
>+				   gen11_master_intr_disable,
>+				   gen11_master_intr_enable);
>+}
>+
> /* Called from drm generic code, passed 'crtc' which
>  * we use as a pipe index
>  */
>-- 
>2.23.0
>