[Intel-gfx] [PATCH v2] drm/i915: Clear lost context-switch interrupts across reset

[Michel Thierry]

On 18/08/17 02:05, Chris Wilson wrote:
> During a global reset, we disable the irq. As we disable the irq, the
> hardware may be raising a GT interrupt that we then ignore, leaving it
> pending in the GTIIR. After the reset, we then re-enable the irq,
> triggering the pending interrupt. However, that interrupt was for the
> stale state from before the reset, and the contents of the CSB buffer
> are now invalid.
> 
> v2: Add a comment to make it clear that the double clear is purely my
> paranoia.

Or say I was the paranoid.

> 
> Reported-by: "Dong, Chuanxiao" <chuanxiao.dong at intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: "Dong, Chuanxiao" <chuanxiao.dong at intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Michal Winiarski <michal.winiarski at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20170807121919.30165-1-chris@chris-wilson.co.uk
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 23 ++++++++++++++++++++++-
>   1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index b0738d2b2a7f..6f972e6ec663 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1221,6 +1221,14 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
>   	return ret;
>   }
>   
> +static u8 gtiir[] = {
> +	[RCS] = 0,
> +	[BCS] = 0,
> +	[VCS] = 1,
> +	[VCS2] = 1,
> +	[VECS] = 3,
> +};
> +
>   static int gen8_init_common_ring(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> @@ -1245,9 +1253,22 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine)
>   
>   	DRM_DEBUG_DRIVER("Execlists enabled for %s\n", engine->name);
>   
> -	/* After a GPU reset, we may have requests to replay */
> +	GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
> +
> +	/*
> +	 * Clear any pending interrupt state.
> +	 *
> +	 * We do it twice out of paranoia that some of the IIR are double
> +	 * buffered, and if we only reset it once there may still be
> +	 * an interrupt pending.
> +	 */
> +	I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
> +		   GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
> +	I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
> +		   GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
>   	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
>   
> +	/* After a GPU reset, we may have requests to replay */
>   	submit = false;
>   	for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) {
>   		if (!port_isset(&port[n]))
> 

Reviewed-by: Michel Thierry <michel.thierry at intel.com>