Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Ever noticed that our interrupt handlers are where we spend most of our
> time on a busy system? In part this is unavoidable as each interrupt
> requires to poll and reset several registers, but we can try and do so as
> efficiently as possible.
>
> Function old new delta
> ilk_irq_handler 2317 2156 -161
>
> v2: Restore the irqreturn_t ret
>
> Function old new delta
> ilk_irq_handler.cold 63 72 +9
> ilk_irq_handler 2221 2080 -141
>
> A slight improvement in the baseline overnight as well!
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 59 +++++++++++++++++----------------
> 1 file changed, 30 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 63579ab71cf6..01d4e3cad69d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2097,67 +2097,68 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
> */
> static irqreturn_t ilk_irq_handler(int irq, void *arg)
> {
> - struct drm_i915_private *dev_priv = arg;
> + struct drm_i915_private *i915 = arg;
> + void __iomem * const regs = i915->uncore.regs;
> u32 de_iir, gt_iir, de_ier, sde_ier = 0;
> irqreturn_t ret = IRQ_NONE;
>
> - if (!intel_irqs_enabled(dev_priv))
> + if (unlikely(!intel_irqs_enabled(i915)))
Doesn't hurt anymore.
And dont have to worry about ret so only thing different is
void of forcewake dance.
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> return IRQ_NONE;
>
> /* IRQs are synced during runtime_suspend, we don't require a wakeref */
> - disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
> + disable_rpm_wakeref_asserts(&i915->runtime_pm);
>
> /* disable master interrupt before clearing iir */
> - de_ier = I915_READ(DEIER);
> - I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
> + de_ier = raw_reg_read(regs, DEIER);
> + raw_reg_write(regs, DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
>
> /* Disable south interrupts. We'll only write to SDEIIR once, so further
> * interrupts will will be stored on its back queue, and then we'll be
> * able to process them after we restore SDEIER (as soon as we restore
> * it, we'll get an interrupt if SDEIIR still has something to process
> * due to its back queue). */
> - if (!HAS_PCH_NOP(dev_priv)) {
> - sde_ier = I915_READ(SDEIER);
> - I915_WRITE(SDEIER, 0);
> + if (!HAS_PCH_NOP(i915)) {
> + sde_ier = raw_reg_read(regs, SDEIER);
> + raw_reg_write(regs, SDEIER, 0);
> }
>
> /* Find, clear, then process each source of interrupt */
>
> - gt_iir = I915_READ(GTIIR);
> + gt_iir = raw_reg_read(regs, GTIIR);
> if (gt_iir) {
> - I915_WRITE(GTIIR, gt_iir);
> - ret = IRQ_HANDLED;
> - if (INTEL_GEN(dev_priv) >= 6)
> - gen6_gt_irq_handler(&dev_priv->gt, gt_iir);
> + raw_reg_write(regs, GTIIR, gt_iir);
> + if (INTEL_GEN(i915) >= 6)
> + gen6_gt_irq_handler(&i915->gt, gt_iir);
> else
> - gen5_gt_irq_handler(&dev_priv->gt, gt_iir);
> + gen5_gt_irq_handler(&i915->gt, gt_iir);
> + ret = IRQ_HANDLED;
> }
>
> - de_iir = I915_READ(DEIIR);
> + de_iir = raw_reg_read(regs, DEIIR);
> if (de_iir) {
> - I915_WRITE(DEIIR, de_iir);
> - ret = IRQ_HANDLED;
> - if (INTEL_GEN(dev_priv) >= 7)
> - ivb_display_irq_handler(dev_priv, de_iir);
> + raw_reg_write(regs, DEIIR, de_iir);
> + if (INTEL_GEN(i915) >= 7)
> + ivb_display_irq_handler(i915, de_iir);
> else
> - ilk_display_irq_handler(dev_priv, de_iir);
> + ilk_display_irq_handler(i915, de_iir);
> + ret = IRQ_HANDLED;
> }
>
> - if (INTEL_GEN(dev_priv) >= 6) {
> - u32 pm_iir = I915_READ(GEN6_PMIIR);
> + if (INTEL_GEN(i915) >= 6) {
> + u32 pm_iir = raw_reg_read(regs, GEN6_PMIIR);
> if (pm_iir) {
> - I915_WRITE(GEN6_PMIIR, pm_iir);
> - ret = IRQ_HANDLED;
> - gen6_rps_irq_handler(&dev_priv->gt.rps, pm_iir);
> + raw_reg_write(regs, GEN6_PMIIR, pm_iir);
> + gen6_rps_irq_handler(&i915->gt.rps, pm_iir);
> }
> + ret = IRQ_HANDLED;
> }
>
> - I915_WRITE(DEIER, de_ier);
> - if (!HAS_PCH_NOP(dev_priv))
> - I915_WRITE(SDEIER, sde_ier);
> + raw_reg_write(regs, DEIER, de_ier);
> + if (sde_ier)
> + raw_reg_write(regs, SDEIER, sde_ier);
>
> /* IRQs are synced during runtime_suspend, we don't require a wakeref */
> - enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
> + enable_rpm_wakeref_asserts(&i915->runtime_pm);
>
> return ret;
> }
> --
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
