| Message ID | 20181002140552.1051-4-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/gen8: Disable master intr before reading | expand |
Quoting Mika Kuoppala (2018-10-02 15:05:52) > Don't keep master disabled while we handle the current > interrupts. This should help a little on latency of > generating the next interrupt. > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 5d1f53723388..9d8d5fb68c84 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3163,9 +3163,6 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) > return IRQ_NONE; > } > > - /* Find, clear, then process each source of interrupt. */ > - gen11_gt_irq_handler(i915, master_ctl); > - > /* IRQs are synced during runtime_suspend, we don't require a wakeref */ > if (master_ctl & GEN11_DISPLAY_IRQ) { > const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL); > @@ -3183,6 +3180,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) > > gen11_master_intr_enable(regs); > > + gen11_gt_irq_handler(i915, master_ctl); Are we not still acking GT_INTR_DW at this point? -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2018-10-02 15:05:52) >> Don't keep master disabled while we handle the current >> interrupts. This should help a little on latency of >> generating the next interrupt. >> >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c >> index 5d1f53723388..9d8d5fb68c84 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -3163,9 +3163,6 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) >> return IRQ_NONE; >> } >> >> - /* Find, clear, then process each source of interrupt. */ >> - gen11_gt_irq_handler(i915, master_ctl); >> - >> /* IRQs are synced during runtime_suspend, we don't require a wakeref */ >> if (master_ctl & GEN11_DISPLAY_IRQ) { >> const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL); >> @@ -3183,6 +3180,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) >> >> gen11_master_intr_enable(regs); >> >> + gen11_gt_irq_handler(i915, master_ctl); > > Are we not still acking GT_INTR_DW at this point? We are. -Mika
Quoting Mika Kuoppala (2018-10-11 15:19:24) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Quoting Mika Kuoppala (2018-10-02 15:05:52) > >> Don't keep master disabled while we handle the current > >> interrupts. This should help a little on latency of > >> generating the next interrupt. > >> > >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_irq.c | 4 +--- > >> 1 file changed, 1 insertion(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > >> index 5d1f53723388..9d8d5fb68c84 100644 > >> --- a/drivers/gpu/drm/i915/i915_irq.c > >> +++ b/drivers/gpu/drm/i915/i915_irq.c > >> @@ -3163,9 +3163,6 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) > >> return IRQ_NONE; > >> } > >> > >> - /* Find, clear, then process each source of interrupt. */ > >> - gen11_gt_irq_handler(i915, master_ctl); > >> - > >> /* IRQs are synced during runtime_suspend, we don't require a wakeref */ > >> if (master_ctl & GEN11_DISPLAY_IRQ) { > >> const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL); > >> @@ -3183,6 +3180,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) > >> > >> gen11_master_intr_enable(regs); > >> > >> + gen11_gt_irq_handler(i915, master_ctl); > > > > Are we not still acking GT_INTR_DW at this point? > > We are. Then we re-enable MASTER before we ack the in the individual GT_IIR, and by the logic before, that means MASTER still has the GT bits asserted (as we said they are not cleared until we ack the individual GT_IIR). Doesn't that mean that the interrupt will be raised again immediately? -Chris
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 5d1f53723388..9d8d5fb68c84 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3163,9 +3163,6 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) return IRQ_NONE; } - /* Find, clear, then process each source of interrupt. */ - gen11_gt_irq_handler(i915, master_ctl); - /* IRQs are synced during runtime_suspend, we don't require a wakeref */ if (master_ctl & GEN11_DISPLAY_IRQ) { const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL); @@ -3183,6 +3180,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg) gen11_master_intr_enable(regs); + gen11_gt_irq_handler(i915, master_ctl); gen11_gu_misc_irq_handler(i915, gu_misc_iir); return IRQ_HANDLED;
Don't keep master disabled while we handle the current interrupts. This should help a little on latency of generating the next interrupt. Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)