[Intel-gfx] [PATCH] drm/i915: Fix __wait_seqno to use true infinite timeouts

[Daniel Vetter]

On Fri, Aug 23, 2013 at 10:06 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>> Yeah, I think this approach should work. A few comments:
>> - I think we need a debugfs file to shut the safety quirk off - when
>> testing on a machine where we actually miss interrupts it might be
>> usful to get the warning output every time.
>
> I did consider a modparam. Exposing it would indeed necessitate some
> protection against concurrent modificatin.
>
>> - I'd go for a new bool dev_priv->unreliable_seqno_signalling or so to
>> avoid any races due to the rmw cycle you now do on dev_priv->quirks.
>
> There isn't a race during writing, as hangcheck should never be run
> concurrently (or at least any concurrent calls filtered out at the start
> of the function). The read side is inherently racey.

It's more that thus far we've used dev_priv->quirks only for stuff
which never changes, now we have a quirk which gets only set at
runtime. It just feels conceptually wrong ;-) And if we add a 2nd such
quirk it'll break a bit, hence why I'd prefer a distinct piece of
state tracking

>> - I'd have opted for a faster timeout of the fake irq, but one that rearms.
>
> Whoops, that was a mistake. The intention was to run at 100Hz, do you
> want even faster? We could switch to a hrtimer and kill two birds with
> one stone (as timer is singleshot only).

I'd simply go with a 1 jiffies rearming timer.

>> Also I'd love to be able to test all this (both the missed irq
>> detection stuff and the fake irq) but I don't have a good idea right
>> now ... So I guess we need to again hope that it won't break too
>> quickly (since it eventually will break again).
>
> Disable the call to ring->get_irq. Perhaps the high word of
> dev_priv->stop_rings?

Yeah, something like stop_ring_irqs or so could work, maybe by
wrapping the wake_up_all calls into a little helper like wake_up_ring
which noops if the respective bit is set in stop_ring_irqs. But I'm
not sure whether this is worth the fuzz. Otoh we've just proven that
for untested code the question isn't if it breaks, but when ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch