On Fri, Feb 24, 2017 at 10:19:07AM +0000, Chris Wilson wrote: > On Fri, Feb 24, 2017 at 10:04:23AM +0000, Tvrtko Ursulin wrote: > > >diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c > > >index 027c93e34c97..5f2665aa817c 100644 > > >--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c > > >+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c > > >@@ -81,8 +81,11 @@ static void intel_breadcrumbs_fake_irq(unsigned long data) > > > * every jiffie in order to kick the oldest waiter to do the > > > * coherent seqno check. > > > */ > > >- if (intel_engine_wakeup(engine)) > > >- mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1); > > >+ if (!intel_engine_has_waiter(engine)) > > >+ return; > > >+ > > >+ intel_engine_wakeup(engine); > > >+ mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1); > > > > Not sure it is worth splitting this in two instead of just leaving > > it as it was. > > The problem with the previous code was the timer stopped if the waiter > was running at that moment. I am still playing to find something that > doesn't look horrible. Just going back to the bool intel_engine_wakeup, > which is now far too large to be an inline for an unimportant function. Ah, found the complication. Here we want intel_engine_wakeup() to report if there was a waiter, but in intel_breadcrumbs_hangcheck, we want to know if we failed to wakeup the waiter. Just grin and bear it for the moment. :| -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx