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

[Lionel Landwerlin]

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.


>
>> @@ -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?


>
>> +       } 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.


> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> -Chris
>