[Intel-gfx] [PATCH 3/3] drm/i915: Log catastrophic errors on gen11

[Chris Wilson]

Quoting Mika Kuoppala (2019-04-12 16:47:11)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2019-04-12 16:37:23)
> >> Add a log entry to indicate if engine reported an intr
> >> condition that is a sign of halted command streamer.
> >
> > It's a user error. Isn't that we spam the "there has been an error;
> > resetting the gpu" message enough?
> >
> > You could just look at the iir in the error capture for the same effect.
> >
> 
> Yeah it is debatable if this adds any value at all in this form.
> Could drop this and introduce only if I get motivation
> to handle reset faster straight from encountering error
> intr.
> 
> >> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_irq.c | 24 +++++++++++++++++++++---
> >>  drivers/gpu/drm/i915/i915_reg.h |  4 ++++
> >>  2 files changed, 25 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index d934545445e1..c64ef1291d62 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -1480,7 +1480,7 @@ static void snb_gt_irq_handler(struct drm_i915_private *dev_priv,
> >>  }
> >>  
> >>  static void
> >> -gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir)
> >> +gen8_cs_irq_handler(struct intel_engine_cs *engine, const u32 iir)
> >>  {
> >>         bool tasklet = false;
> >>  
> >> @@ -1496,6 +1496,20 @@ gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir)
> >>                 tasklet_hi_schedule(&engine->execlists.tasklet);
> >>  }
> >>  
> >> +static void
> >> +gen11_cs_irq_handler(struct intel_engine_cs *engine, const u32 iir)
> >> +{
> >> +       const u32 error_intrs =
> >> +               GT_LEGACY_CONTEXT_PPGTT_PAGE_FAULT |
> >> +                GT_CATASTROPHIC_ERROR_INTERRUPT |
> >> +               GT_CS_MASTER_ERROR_INTERRUPT;
> >> +
> >> +       if (likely(!(error_intrs & iir)))
> >
> > Likely?
> >
> 
> Likely to not have errors.

Oh, this is the CS/USER_INTERRUPT handler. Yeah, much more likely. So
much more, I'd push more for this to be polled by the error handler :)
-Chris