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

Mika Kuoppala
2017-10-24T14:27:06Z

Chris Wilson <chris@chris-wilson.co.uk> 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@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.

I would call it spurious instead of residual as there is no real
reason for it.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

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