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

[Tvrtko Ursulin]

On 07/01/2019 13:02, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-07 12:58:39)
>>
>> On 07/01/2019 12:16, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-01-07 11:58:13)
>>>> On 05/01/2019 02:39, Carlos Santa wrote:
>>>>> +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;
>>>>> +     enum forcewake_domains fw_domains;
>>>>> +     u32 current_seqno;
>>>>> +
>>>>> +     switch (engine->id) {
>>>>> +     default:
>>>>> +             MISSING_CASE(engine->id);
>>>>> +             /* fall through */
>>>>> +     case RCS:
>>>>> +             fw_domains = FORCEWAKE_RENDER;
>>>>> +             break;
>>>>> +     case VCS:
>>>>> +     case VCS2:
>>>>> +     case VECS:
>>>>> +             fw_domains = FORCEWAKE_MEDIA;
>>>>> +             break;
>>>>> +     }
>>>>> +
>>>>> +     intel_uncore_forcewake_get(dev_priv, fw_domains);
>>>>
>>>> I'd be tempted to drop this and just use I915_WRITE. It doesn't feel
>>>> like there is any performance to be gained with it and it embeds too
>>>> much knowledge here.
>>>
>>> No, no, no. Let's not reintroduce a fw inside irq context on a frequent
>>> timer again.
>>
>> Tasklet and hopefully watchdog timeouts are not frequent. :)
> 
> I thought the typical value mentioned elsewhere was a 1ms watchdog. Some

Commit message to a patch from this series says 60ms is recommended.

> might say why even use a watchdog for longer than that as a hrtimer will
> be more efficient (coupling in with other timer activity) ;)

For the normal case (longish batches, very few timeouts) it feels more 
efficient to have ten or so extra dwords with each request than to 
fiddle with hrtimers, no?

But interactions with preemption and future time-slicing yeah don't 
know. Maybe a single solution will be simpler at that point.

Regards,

Tvrtko