Re: [PATCH 2/4] drm/i915: Report back whether the irq was armed when adding the waiter

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ma, 2017-06-05 at 11:26 +0100, Chris Wilson wrote:
> The important condition that we need to check after enabling the
> interrupt for signaling is whether the request completed in the process
> (and so we missed that interrupt). A large cost in enabling the
> signaling (rather than waiters) is in waking up the auxiliary signaling
> thread, but we only need to do so to catch that missed interrupt. If we
> know we didn't miss any interrupts (because we didn't arm the interrupt)
> then we can skip waking the auxiliary thread.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

<SNIP>

> @@ -390,6 +391,7 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
>  
>  	if (first) {
>  		spin_lock(&b->irq_lock);
> +		irq_armed = !b->irq_armed;

This line here is quite confusing, "armed_irq" for local variable would
be *much* clearer. Could be further amended if I'm the only with such
mindset.

With a less confusing local variable name;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

>  		b->irq_wait = wait;
>  		/* After assigning ourselves as the new bottom-half, we must
>  		 * perform a cursory check to prevent a missed interrupt.
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx