drm/i915/execlists: Flush GTIIR on clearing CS interrupts during reset

Submitted by Chris Wilson on Feb. 2, 2018, 2:54 p.m.

Details

Message ID 20180202145455.29876-1-chris@chris-wilson.co.uk
State Accepted
Commit 274de876065a34ca1ff093bfacb62e440a4491be
Headers show
Series "drm/i915/execlists: Flush GTIIR on clearing CS interrupts during reset" ( rev: 1 ) in Intel GFX

Browsing this patch as part of:
"drm/i915/execlists: Flush GTIIR on clearing CS interrupts during reset" rev 1 in Intel GFX
<< prev patch [1/1] next patch >>

Commit Message

Chris Wilson Feb. 2, 2018, 2:54 p.m.
Be paranoid and flush the GTIIR after clearing the CS interrupt to be
sure it has taken before we re-enable the interrupt handler. We still
see early interrupts following reset, the tasklet handling the mmio read
before it has been written by the CS. This hopefully reduces the
frequency to 0...

References: https://bugs.freedesktop.org/show_bug.cgi?id=104262
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 40dbeaee9dfa..deeedfc9fe44 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1527,6 +1527,7 @@  static int gen9_init_render_ring(struct intel_engine_cs *engine)
 static void reset_irq(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	int i;
 
 	/*
 	 * Clear any pending interrupt state.
@@ -1535,10 +1536,14 @@  static void reset_irq(struct intel_engine_cs *engine)
 	 * 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);
+	for (i = 0; i < 2; i++) {
+		I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
+			   GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
+		POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
+	}
+	GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
+		   (GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift));
+
 	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
 }
 

Comments

On 2/2/2018 6:54 AM, Chris Wilson wrote:
> Be paranoid and flush the GTIIR after clearing the CS interrupt to be
> sure it has taken before we re-enable the interrupt handler. We still
> see early interrupts following reset, the tasklet handling the mmio read
> before it has been written by the CS. This hopefully reduces the
> frequency to 0...
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=104262
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 40dbeaee9dfa..deeedfc9fe44 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1527,6 +1527,7 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine)
>   static void reset_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> +	int i;
>   
>   	/*
>   	 * Clear any pending interrupt state.
> @@ -1535,10 +1536,14 @@ static void reset_irq(struct intel_engine_cs *engine)
>   	 * 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);
> +	for (i = 0; i < 2; i++) {
> +		I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
> +			   GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
> +		POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
> +	}
> +	GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
> +		   (GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift));
> +
>   	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
>   }
>   
> 

Acked-by: Michel Thierry <michel.thierry@intel.com>
Quoting Michel Thierry (2018-02-02 16:49:49)
> On 2/2/2018 6:54 AM, Chris Wilson wrote:
> > Be paranoid and flush the GTIIR after clearing the CS interrupt to be
> > sure it has taken before we re-enable the interrupt handler. We still
> > see early interrupts following reset, the tasklet handling the mmio read
> > before it has been written by the CS. This hopefully reduces the
> > frequency to 0...
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=104262
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Michel Thierry <michel.thierry@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++----
> >   1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 40dbeaee9dfa..deeedfc9fe44 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -1527,6 +1527,7 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine)
> >   static void reset_irq(struct intel_engine_cs *engine)
> >   {
> >       struct drm_i915_private *dev_priv = engine->i915;
> > +     int i;
> >   
> >       /*
> >        * Clear any pending interrupt state.
> > @@ -1535,10 +1536,14 @@ static void reset_irq(struct intel_engine_cs *engine)
> >        * 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);
> > +     for (i = 0; i < 2; i++) {
> > +             I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
> > +                        GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
> > +             POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
> > +     }
> > +     GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
> > +                (GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift));
> > +
> >       clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
> >   }
> >   
> > 
> 
> Acked-by: Michel Thierry <michel.thierry@intel.com>

Ta. Afaict, it's a timing issue and this is helping paper over it. Not a
fix, but maybe an indicator for someone to find the real problem?
-Chris