Mika Kuoppala <[email protected]> writes: > [ text/plain ] > [email protected] writes: > >> [ text/plain ] >> From: Ville Syrjälä <[email protected]> >> >> On VLV/CHV the master interrupt enable bit only affects GT/PM >> interrupts. Display interrupts are not affected by the master >> irq control. >> >> Also it seems that the CPU interrupt will only be generated when >> the combined result of all GT/PM/display interrupts has a 0->1 >> edge. We already use the master interrupt enable bit to make sure >> GT/PM interrupt can generate such an edge if we don't end up clearing >> all IIR bits. We must do the same for display interrupts, and for >> that we can simply clear out VLV_IER, and restore after we've acked >> all the interrupts we are about to process. >> >> So with both master interrupt enable and VLV_IER cleared out, we will >> guarantee that there will be a 0->1 edge if any IIR bits are still set >> at the end, and thus another CPU interrupt will be generated. >> >> Cc: Chris Wilson <[email protected]> >> Cc: Tvrtko Ursulin <[email protected]> >> Fixes: 579de73b048a ("drm/i915: Exit cherryview_irq_handler() after >> one pass") > > I have bisected the bsw ci/bat unstability (frequent gpu hangs) to the > commit pointed by Fixes: And reverting seems to help. > > I am now running the same drv_module_reload loop on top of this > series. It looks good but its too early to draw conclusions. > I will leave it running for the night. >
Seems to work fine in my tests and as also reflected by CI/bat. -Mika > -Mika > >> Signed-off-by: Ville Syrjälä <[email protected]> >> --- >> drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++++++- >> 1 file changed, 35 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c >> b/drivers/gpu/drm/i915/i915_irq.c >> index 626775039919..46be03c616f4 100644 >> --- a/drivers/gpu/drm/i915/i915_irq.c >> +++ b/drivers/gpu/drm/i915/i915_irq.c >> @@ -1778,7 +1778,7 @@ static irqreturn_t valleyview_irq_handler(int irq, >> void *arg) >> disable_rpm_wakeref_asserts(dev_priv); >> >> while (true) { >> - /* Find, clear, then process each source of interrupt */ >> + u32 ier = 0; >> >> gt_iir = I915_READ(GTIIR); >> pm_iir = I915_READ(GEN6_PMIIR); >> @@ -1789,7 +1789,22 @@ static irqreturn_t valleyview_irq_handler(int irq, >> void *arg) >> >> ret = IRQ_HANDLED; >> >> + /* >> + * Theory on interrupt generation, based on empirical evidence: >> + * >> + * x = ((VLV_IIR & VLV_IER) || >> + * (((GT_IIR & GT_IER) || (GEN6_PMIIR & GEN6_PMIER)) && >> + * (VLV_MASTER_IER & MASTER_INTERRUPT_ENABLE))); >> + * >> + * A CPU interrupt will only be raised when 'x' has a 0->1 edge. >> + * Hence we clear MASTER_INTERRUPT_ENABLE and VLV_IER to >> + * guarantee the CPU interrupt will be raised again even if we >> + * don't end up clearing all the VLV_IIR, GT_IIR, GEN6_PMIIR >> + * bits this time around. >> + */ >> I915_WRITE(VLV_MASTER_IER, 0); >> + ier = I915_READ(VLV_IER); >> + I915_WRITE(VLV_IER, 0); >> >> if (gt_iir) >> I915_WRITE(GTIIR, gt_iir); >> @@ -1815,6 +1830,7 @@ static irqreturn_t valleyview_irq_handler(int irq, >> void *arg) >> if (iir) >> I915_WRITE(VLV_IIR, iir); >> >> + I915_WRITE(VLV_IER, ier); >> I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE); >> POSTING_READ(VLV_MASTER_IER); >> } >> @@ -1839,6 +1855,8 @@ static irqreturn_t cherryview_irq_handler(int irq, >> void *arg) >> disable_rpm_wakeref_asserts(dev_priv); >> >> do { >> + u32 ier = 0; >> + >> master_ctl = I915_READ(GEN8_MASTER_IRQ) & >> ~GEN8_MASTER_IRQ_CONTROL; >> iir = I915_READ(VLV_IIR); >> >> @@ -1847,7 +1865,22 @@ static irqreturn_t cherryview_irq_handler(int irq, >> void *arg) >> >> ret = IRQ_HANDLED; >> >> + /* >> + * Theory on interrupt generation, based on empirical evidence: >> + * >> + * x = ((VLV_IIR & VLV_IER) || >> + * ((GEN8_MASTER_IRQ & ~GEN8_MASTER_IRQ_CONTROL) && >> + * (GEN8_MASTER_IRQ & GEN8_MASTER_IRQ_CONTROL))); >> + * >> + * A CPU interrupt will only be raised when 'x' has a 0->1 edge. >> + * Hence we clear GEN8_MASTER_IRQ_CONTROL and VLV_IER to >> + * guarantee the CPU interrupt will be raised again even if we >> + * don't end up clearing all the VLV_IIR and >> GEN8_MASTER_IRQ_CONTROL >> + * bits this time around. >> + */ >> I915_WRITE(GEN8_MASTER_IRQ, 0); >> + ier = I915_READ(VLV_IER); >> + I915_WRITE(VLV_IER, 0); >> >> gen8_gt_irq_handler(dev_priv, master_ctl); >> >> @@ -1865,6 +1898,7 @@ static irqreturn_t cherryview_irq_handler(int irq, >> void *arg) >> if (iir) >> I915_WRITE(VLV_IIR, iir); >> >> + I915_WRITE(VLV_IER, ier); >> I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL); >> POSTING_READ(GEN8_MASTER_IRQ); >> } while (0); >> -- >> 2.7.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
