| Message ID | 20190117233126.30165-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/breadcrumbs: Drop assertion that we've already enabled irqs | expand |
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@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com> Regards, Tvrtko
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)
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@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/intel_breadcrumbs.c | 28 ++++++++++-------------- 1 file changed, 11 insertions(+), 17 deletions(-)