[Intel-gfx] [PATCH v3 1/5] drm/i915: Report both waiters and success from intel_engine_wakeup()

[Chris Wilson]

On Mon, Feb 27, 2017 at 10:03:32AM +0000, Tvrtko Ursulin wrote:
> 
> On 24/02/2017 18:01, Chris Wilson wrote:
> >The two users of the return value from intel_engine_wakeup() are
> >expecting different results. In the breadcrumbs hangcheck, we are using
> >it to determine whether wake_up_process() detected the waiter was
> >currently running (and if so we presume that it hasn't yet missed the
> >interrupt). However, in the fake_irq path, we are using the return value
> >as a check as to whether there are any waiters, and so we may
> >incorrectly stop the fake-irq if that waiter was currently running.
> >
> >To handle the two different needs, return both bits of information!
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> >---
> > drivers/gpu/drm/i915/intel_breadcrumbs.c | 28 +++++++++++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_ringbuffer.h  | 26 +++-----------------------
> > 2 files changed, 30 insertions(+), 24 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >index 027c93e34c97..64e1b0c2d8b6 100644
> >--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> >@@ -26,6 +26,32 @@
> >
> > #include "i915_drv.h"
> >
> >+unsigned int intel_engine_wakeup(struct intel_engine_cs *engine)
> 
> How come you decided to un-inline it? Changes shouldn't have made a
> big difference to its size I would have thought.

Mainly in the later patches we move this away from any important paths.

> >+{
> >+	unsigned int ret = 0;
> 
> Optional bikeshed - maybe call it res or something, since ret is
> usually used for < 0 || 0.

Been adopting int err for those, but result is better than ret.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre