Quoting Mika Kuoppala (2020-01-28 12:20:46) > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > > > We always use a deferred bottom-half (either tasklet or irq_work) for > > processing the response to an interrupt which means we can recombine the > > GT irq ack+handler into one. This simplicity is important in later > > patches as we will need to handle and then ack multiple interrupt levels > > before acking the GT and master interrupts. > > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/gt/intel_gt_irq.c | 70 +++++++++++--------------- > > drivers/gpu/drm/i915/gt/intel_gt_irq.h | 3 +- > > drivers/gpu/drm/i915/i915_irq.c | 10 +--- > > 3 files changed, 33 insertions(+), 50 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c > > index f796bdf1ed30..71873a4cafc0 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c > > @@ -286,59 +286,49 @@ void gen6_gt_irq_handler(struct intel_gt *gt, u32 gt_iir) > > gen7_parity_error_irq_handler(gt, gt_iir); > > } > > > > -void gen8_gt_irq_ack(struct intel_gt *gt, u32 master_ctl, u32 gt_iir[4]) > > +void gen8_gt_irq_handler(struct intel_gt *gt, u32 master_ctl) > > { > > void __iomem * const regs = gt->uncore->regs; > > + u32 iir; > > > > if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > > - gt_iir[0] = raw_reg_read(regs, GEN8_GT_IIR(0)); > > - if (likely(gt_iir[0])) > > - raw_reg_write(regs, GEN8_GT_IIR(0), gt_iir[0]); > > - } > > - > > - if (master_ctl & (GEN8_GT_VCS0_IRQ | GEN8_GT_VCS1_IRQ)) { > > - gt_iir[1] = raw_reg_read(regs, GEN8_GT_IIR(1)); > > - if (likely(gt_iir[1])) > > - raw_reg_write(regs, GEN8_GT_IIR(1), gt_iir[1]); > > - } > > - > > - if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) { > > - gt_iir[2] = raw_reg_read(regs, GEN8_GT_IIR(2)); > > - if (likely(gt_iir[2])) > > - raw_reg_write(regs, GEN8_GT_IIR(2), gt_iir[2]); > > - } > > - > > - if (master_ctl & GEN8_GT_VECS_IRQ) { > > - gt_iir[3] = raw_reg_read(regs, GEN8_GT_IIR(3)); > > - if (likely(gt_iir[3])) > > - raw_reg_write(regs, GEN8_GT_IIR(3), gt_iir[3]); > > - } > > -} > > - > > -void gen8_gt_irq_handler(struct intel_gt *gt, u32 master_ctl, u32 gt_iir[4]) > > -{ > > - if (master_ctl & (GEN8_GT_RCS_IRQ | GEN8_GT_BCS_IRQ)) { > > - cs_irq_handler(gt->engine_class[RENDER_CLASS][0], > > - gt_iir[0] >> GEN8_RCS_IRQ_SHIFT); > > - cs_irq_handler(gt->engine_class[COPY_ENGINE_CLASS][0], > > - gt_iir[0] >> GEN8_BCS_IRQ_SHIFT); > > + iir = raw_reg_read(regs, GEN8_GT_IIR(0)); > > + if (likely(iir)) { > > + cs_irq_handler(gt->engine_class[RENDER_CLASS][0], > > + iir >> GEN8_RCS_IRQ_SHIFT); > > + cs_irq_handler(gt->engine_class[COPY_ENGINE_CLASS][0], > > + iir >> GEN8_BCS_IRQ_SHIFT); > > + raw_reg_write(regs, GEN8_GT_IIR(0), iir); > > + } > > } > > > > if (master_ctl & (GEN8_GT_VCS0_IRQ | GEN8_GT_VCS1_IRQ)) { > > - cs_irq_handler(gt->engine_class[VIDEO_DECODE_CLASS][0], > > - gt_iir[1] >> GEN8_VCS0_IRQ_SHIFT); > > - cs_irq_handler(gt->engine_class[VIDEO_DECODE_CLASS][1], > > - gt_iir[1] >> GEN8_VCS1_IRQ_SHIFT); > > + iir = raw_reg_read(regs, GEN8_GT_IIR(1)); > > + if (likely(iir)) { > > + cs_irq_handler(gt->engine_class[VIDEO_DECODE_CLASS][0], > > + iir >> GEN8_VCS0_IRQ_SHIFT); > > + cs_irq_handler(gt->engine_class[VIDEO_DECODE_CLASS][1], > > + iir >> GEN8_VCS1_IRQ_SHIFT); > > + raw_reg_write(regs, GEN8_GT_IIR(1), iir); > > + } > > } > > > > if (master_ctl & GEN8_GT_VECS_IRQ) { > > - cs_irq_handler(gt->engine_class[VIDEO_ENHANCEMENT_CLASS][0], > > - gt_iir[3] >> GEN8_VECS_IRQ_SHIFT); > > + iir = raw_reg_read(regs, GEN8_GT_IIR(3)); > > + if (likely(iir)) { > > + cs_irq_handler(gt->engine_class[VIDEO_ENHANCEMENT_CLASS][0], > > + iir >> GEN8_VECS_IRQ_SHIFT); > > + raw_reg_write(regs, GEN8_GT_IIR(3), iir); > > + } > > } > > > > if (master_ctl & (GEN8_GT_PM_IRQ | GEN8_GT_GUC_IRQ)) { > > - gen6_rps_irq_handler(>->rps, gt_iir[2]); > > - guc_irq_handler(>->uc.guc, gt_iir[2] >> 16); > > + iir = raw_reg_read(regs, GEN8_GT_IIR(2)); > > + if (likely(iir)) { > > + gen6_rps_irq_handler(>->rps, iir); > > + guc_irq_handler(>->uc.guc, iir >> 16); > > + raw_reg_write(regs, GEN8_GT_IIR(2), iir); > > + } > > } > > } > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.h b/drivers/gpu/drm/i915/gt/intel_gt_irq.h > > index 8f37593712c9..886c5cf408a2 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.h > > @@ -36,9 +36,8 @@ void gen5_gt_enable_irq(struct intel_gt *gt, u32 mask); > > > > void gen6_gt_irq_handler(struct intel_gt *gt, u32 gt_iir); > > > > -void gen8_gt_irq_ack(struct intel_gt *gt, u32 master_ctl, u32 gt_iir[4]); > > +void gen8_gt_irq_handler(struct intel_gt *gt, u32 master_ctl); > > void gen8_gt_irq_reset(struct intel_gt *gt); > > -void gen8_gt_irq_handler(struct intel_gt *gt, u32 master_ctl, u32 gt_iir[4]); > > void gen8_gt_irq_postinstall(struct intel_gt *gt); > > > > #endif /* INTEL_GT_IRQ_H */ > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index 87a6662abc1b..e40dd226fde8 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -1614,7 +1614,6 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg) > > u32 master_ctl, iir; > > u32 pipe_stats[I915_MAX_PIPES] = {}; > > u32 hotplug_status = 0; > > - u32 gt_iir[4]; > > u32 ier = 0; > > > > master_ctl = I915_READ(GEN8_MASTER_IRQ) & ~GEN8_MASTER_IRQ_CONTROL; > > @@ -1642,7 +1641,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg) > > ier = I915_READ(VLV_IER); > > I915_WRITE(VLV_IER, 0); > > > > - gen8_gt_irq_ack(&dev_priv->gt, master_ctl, gt_iir); > > + gen8_gt_irq_handler(&dev_priv->gt, master_ctl); > > > > if (iir & I915_DISPLAY_PORT_INTERRUPT) > > hotplug_status = i9xx_hpd_irq_ack(dev_priv); > > @@ -1666,8 +1665,6 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg) > > I915_WRITE(VLV_IER, ier); > > I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); > > > > - gen8_gt_irq_handler(&dev_priv->gt, master_ctl, gt_iir); > > - > > if (hotplug_status) > > i9xx_hpd_irq_handler(dev_priv, hotplug_status); > > > > @@ -2396,7 +2393,6 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg) > > struct drm_i915_private *dev_priv = arg; > > void __iomem * const regs = dev_priv->uncore.regs; > > u32 master_ctl; > > - u32 gt_iir[4]; > > > > if (!intel_irqs_enabled(dev_priv)) > > return IRQ_NONE; > > @@ -2408,7 +2404,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg) > > } > > > > /* Find, clear, then process each source of interrupt */ > > /* Find, process and then clear each source of interrupt */ The order is still ack / process, and that is quite important for allowing interrupt delivery as we process. It's just that we use bottom-halves rather than a local deferral. /* Find, clear, then process (via bottom-halves) each source of interrupt */ -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx