[Intel-gfx] [PATCH] drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs

[Tvrtko Ursulin]

On 17/01/2019 23:31, Chris Wilson wrote:
> The motivation for introducing the check that we only enable breadcrumb
> irqs if the device's irq was installed was once upon a time we waited
> during suspend after disabling interrupts (which was quite slow until
> the bug was discovered). Since then we have the notion of pinning the
> breadcrumb irq, broadening it from the sole purpose of user interrupt
> notification and waiting, and more importantly decoupling it from a very
> defined time period during which enabling the irq was expected. So stop
> insisting the irq is installed before we setup our IMR masks, if the IER
> isn't yet enabled, nothing will happen and we will timeout instead,
> revealing the lack of irq in the hang debug messages.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_breadcrumbs.c | 28 ++++++++++--------------
>   1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index 4ed7105d7ff5..bfbff04c16aa 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -158,30 +158,24 @@ static void intel_breadcrumbs_fake_irq(struct timer_list *t)
>   
>   static void irq_enable(struct intel_engine_cs *engine)
>   {
> -	/*
> -	 * FIXME: Ideally we want this on the API boundary, but for the
> -	 * sake of testing with mock breadcrumbs (no HW so unable to
> -	 * enable irqs) we place it deep within the bowels, at the point
> -	 * of no return.
> -	 */
> -	GEM_BUG_ON(!intel_irqs_enabled(engine->i915));
> +	if (!engine->irq_enable)
> +		return;
>   
>   	/* Caller disables interrupts */
> -	if (engine->irq_enable) {
> -		spin_lock(&engine->i915->irq_lock);
> -		engine->irq_enable(engine);
> -		spin_unlock(&engine->i915->irq_lock);
> -	}
> +	spin_lock(&engine->i915->irq_lock);
> +	engine->irq_enable(engine);
> +	spin_unlock(&engine->i915->irq_lock);
>   }
>   
>   static void irq_disable(struct intel_engine_cs *engine)
>   {
> +	if (!engine->irq_disable)
> +		return;
> +
>   	/* Caller disables interrupts */
> -	if (engine->irq_disable) {
> -		spin_lock(&engine->i915->irq_lock);
> -		engine->irq_disable(engine);
> -		spin_unlock(&engine->i915->irq_lock);
> -	}
> +	spin_lock(&engine->i915->irq_lock);
> +	engine->irq_disable(engine);
> +	spin_unlock(&engine->i915->irq_lock);
>   }
>   
>   void __intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine)
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko