On Mon, Feb 27, 2017 at 01:57:35PM +0000, Tvrtko Ursulin wrote:
> 
> On 27/02/2017 13:24, Chris Wilson wrote:
> >     if (b->hangcheck_interrupts != atomic_read(&engine->irq_count)) {
> >@@ -67,7 +76,7 @@ static void intel_breadcrumbs_hangcheck(unsigned long data)
> >      * to process the pending interrupt (e.g, low priority task on a loaded
> >      * system) and wait until it sleeps before declaring a missed interrupt.
> >      */
> >-    if (intel_engine_wakeup(engine) & ENGINE_WAKEUP_ACTIVE) {
> >+    if (!(intel_engine_wakeup(engine) & ENGINE_WAKEUP_ASLEEP)) {
> 
> I did not get the explanation from the previous round on why you had
> to reverse the active to asleep. Here it even looks wrong now,
> because I thought you don't want to re-queue the hangcheck when
> there are no waiters?

No waiters: result = 0
Running waiter: result = WAKEUP_WAITER
Sleeping waiter: result = WAKEUP_WAITER | WAKEUP_ASLEEP

We only want to declare a mised irq if we wake up a sleeping waiter, and
keep the hangcheck timer running until the device is idle (when the irq
is disarmed).

How about:

if (intel_engine_wakeup(engine) & ENGINE_WAKEUP_ASLEEP) {
        set_bit(missed_irq);
        mod_timer(&b->fake_irq, jiffies + 1);
} else {
        mod_timer(b->hangcheck, wait_timeout);
}
?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to