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

[Tvrtko Ursulin]

On 08/03/2019 03:16, Carlos Santa wrote:
> On Fri, 2019-03-01 at 09:36 +0000, Chris Wilson wrote:
>>>
>> 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.
>>
> 
> Chris, Tvrtko, I need some guidance on how to find the quilty seqno
> during a hang, can you please advice here what to do?

When an interrupt fires you need to ascertain whether the same request 
which enabled the watchdog is running, correct?

So I think you would need this, with a disclaimer that I haven't thought 
about the details really:

1. Take a reference to timeline hwsp when setting up the watchdog for a 
request.

2. Store the initial seqno associated with this request.

3. Force enable user interrupts.

4. When timeout fires, inspect the HWSP seqno to see if the request 
completed or not.

5. Reset the engine if not completed.

6. Put the timeline/hwsp reference.

If the user interrupt fires with the request completed cancel the above 
operations.

There could be an inherent race between inspecting the seqno and 
deciding to reset. Not sure at the moment what to do. Maybe just call it 
bad luck?

I also think for the software implementation you need to force no 
request coalescing for contexts with timeout set. Because you want to 
have 100% defined borders for request in and out - since the timeout is 
defined per request.

In this case you don't need the user interrupt for the trailing edge 
signal but can use context complete. Maybe putting hooks into 
context_in/out in intel_lrc.c would work under these circumstances.

Also if preempted you need to cancel the timer setup and store elapsed 
execution time.

Or it may make sense to just disable preemption for these contexts. 
Otherwise there is no point in trying to mandate the timeout?

But it is also kind of bad since non-privileged contexts can make 
themselves non-preemptable by setting the watchdog timeout.

Maybe as a compromise we need to automatically apply an elevated 
priority level, but not as high to be completely non-preemptable. Sounds 
like a hard question.

Regards,

Tvrtko