[Intel-gfx] [PATCH v2] drm/i915: clear error registers after error capture

[Chris Wilson]

Quoting Lionel Landwerlin (2018-08-30 12:41:42)
> On 30/08/2018 12:23, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2018-08-30 12:15:07)
> >> We need to clear the register in order to get correct value after the
> >> next potential hang.
> >>
> >> v2: Centralize error register clearing in i915_irq.c (Chris)
> >>
> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> > Ok, I was thinking of move the code around the files a bit more, but
> > agree with what you've done as being the smallest change possible.
> 
> 
> If you have a suggestion, I can do a v3.

Basically pulling the fault reg out of i915_gem_gtt.c and everything
into i915_reset.c. And then removing the struct_mutex.

> >> @@ -3238,6 +3238,22 @@ static void i915_clear_error_registers(struct drm_i915_private *dev_priv)
> >>                  I915_WRITE(EMR, I915_READ(EMR) | eir);
> >>                  I915_WRITE(IIR, I915_MASTER_ERROR_INTERRUPT);
> >>          }
> >> +
> >> +       if (INTEL_GEN(dev_priv) < 8) {
> >> +               struct intel_engine_cs *engine;
> >> +               enum intel_engine_id id;
> >> +
> >> +               for_each_engine(engine, dev_priv, id) {
> >> +                       I915_WRITE(RING_FAULT_REG(engine),
> >> +                                  I915_READ(RING_FAULT_REG(engine)) &
> >> +                                  ~RING_FAULT_VALID);
> >> +               }
> >> +               POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS]));
> > Memory says this is gen6+, and i915_gpu_error.c has the same opinion.
> 
> 
> What is? The post read?
The comment below :)

> >> +       } else {
> >> +               I915_WRITE(GEN8_RING_FAULT_REG,
> >> +                          I915_READ(GEN8_RING_FAULT_REG) & ~RING_FAULT_VALID);
> >> +               POSTING_READ(GEN8_RING_FAULT_REG);
> >> +       }
> > I think you want:
> >
> >       if (INTEL_GEN > 8) {
> >               ...
> >       } else if (INTEL_GEN > 6) {
> >               ...
> >       } else {
> >               /* Are there any fault regs for earlier? */
> >       }
> >
> > With that tweak,
> 
> 
> Thanks, done locally.

Hopefully with >= :)
-Chris