| Message ID | 20180920143350.29249-7-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ICL interrupt handling improvements | expand |
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@chris-wilson.co.uk> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@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
On 20/09/18 07:33, Mika Kuoppala wrote: > The returning of iir through function parameter is eyesore. > Make guc irq acking inline and return the iir directly, being pedantic, this is not the guc irq but gu_misc irq (in the commit title as well). Sounds similar but it's a different interrupt :P GuC is under INTR_DW0, bit 25. Daniele
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; - - if (unlikely(!iir)) { - DRM_ERROR("GU_MISC iir blank!\n"); + if (!iir) return; - } if (iir & GEN11_GU_MISC_GSE) - intel_opregion_asle_intr(dev_priv); - else - DRM_ERROR("Unexpected GU_MISC interrupt 0x%x\n", iir); + return intel_opregion_asle_intr(dev_priv); + + DRM_ERROR("Unexpected GU_MISC interrupt 0x%x\n", iir); } static inline void gen11_master_irq_enable(void __iomem * const regs) @@ -3160,11 +3158,11 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) enable_rpm_wakeref_asserts(i915); } - gen11_gu_misc_irq_ack(i915, master_ctl, &gu_misc_iir); + gu_misc_iir = gen11_gu_misc_irq_ack(regs, master_ctl); gen11_master_irq_enable(regs); - gen11_gu_misc_irq_handler(i915, master_ctl, gu_misc_iir); + gen11_gu_misc_irq_handler(i915, gu_misc_iir); return master_ctl ? IRQ_HANDLED : IRQ_NONE; }
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@chris-wilson.co.uk> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 38 ++++++++++++++++----------------- 1 file changed, 18 insertions(+), 20 deletions(-)