[Intel-gfx] [PATCH v4 2/5] drm/i915: Watchdog timeout: IRQ handler for gen8+

[Chris Wilson]

Quoting Carlos Santa (2019-02-21 02:58:16)
> +#define GEN8_WATCHDOG_1000US(dev_priv) watchdog_to_clock_counts(dev_priv, 1000)
> +static void gen8_watchdog_irq_handler(unsigned long data)
> +{
> +       struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
> +       struct drm_i915_private *dev_priv = engine->i915;
> +       unsigned int hung = 0;
> +       u32 current_seqno=0;
> +       char msg[80];
> +       unsigned int tmp;
> +       int len;
> +
> +       /* Stop the counter to prevent further timeout interrupts */
> +       I915_WRITE_FW(RING_CNTR(engine->mmio_base), get_watchdog_disable(engine));
> +
> +       /* Read the heartbeat seqno once again to check if we are stuck? */
> +       current_seqno = intel_engine_get_hangcheck_seqno(engine);

I have said this before, but this doesn't exist either, it's just a
temporary glitch in the matrix.

> +    if (current_seqno == engine->current_seqno) {
> +               hung |= engine->mask;
> +
> +               len = scnprintf(msg, sizeof(msg), "%s on ", "watchdog timeout");
> +               for_each_engine_masked(engine, dev_priv, hung, tmp)
> +                       len += scnprintf(msg + len, sizeof(msg) - len,
> +                                        "%s, ", engine->name);
> +               msg[len-2] = '\0';
> +
> +               i915_handle_error(dev_priv, hung, 0, "%s", msg);
> +
> +               /* Reset timer in case GPU hangs without another request being added */
> +               i915_queue_hangcheck(dev_priv);

You still haven't explained why we are not just resetting the engine
immediately. Have you looked at the preempt-timeout patches that need to
do the same thing from timer-irq context?

Resending the same old stuff over and over again is just exasperating.
-Chris