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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx