Re: [PATCH 2/4] drm/i915: Synchronize irq before parking each engine

Chris Wilson
2017-10-24T09:05:51Z

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@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> ---
> 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.

> + */
> + 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