Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > Quoting Chris Wilson (2017-10-23 22:32:35) >> When we park the engine (upon idling), we kill the irq tasklet. However, >> to be sure that it is not restarted by a final interrupt after doing so, >> flush the interrupt handler before parking. As we only park the engines >> when we believe the system is idle, there should not be any spurious >> interrupts later to distrub us; so flushing the final in-flight >> interrupt should be sufficient. >> >> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> >> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> >> Cc: Imre Deak <imre.deak@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/i915_gem.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index bb0e85043e01..b547a6327d34 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -3327,6 +3327,12 @@ i915_gem_idle_work_handler(struct work_struct *work) >> if (new_requests_since_last_retire(dev_priv)) >> goto out_unlock; >> >> + /* >> + * Be paranoid and flush a concurrent interrupt to make sure >> + * we don't reactivate any irq tasklets after parking. > > * FIXME: Note that even though we have waited for execlists to be idle, > * there may still be an in-flight interrupt even though the CSB > * is now empty. synchronize_irq() makes sure that the residual > * interrupt is completed before we continue, but it doesn't prevent > * the HW from raising that residual interrupt later. To complete > * the shield we should coordinate disabling the CS irq with > * flushing the residual interrupt. Apparently tasklet_kill is 'broken' in a sence that yeah new schedule reanimates the dead one. This seems weird guarantee for an api of such name and someone has even tried to fix it: https://patchwork.kernel.org/patch/9298717/ I am pondering that can we combine the tasklet_disable into the mix. We would just defer the tasklet until we are ready and willing to process again? -Mika > >> + */ >> + synchronize_irq(dev_priv->drm.irq); >> + >> /* >> * We are committed now to parking the engines, make sure there >> * will be no more interrupts arriving later. >> -- >> 2.15.0.rc1 >> _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
