Re: [PATCH] drm/i915: Reset the breadcrumbs IRQ more carefully

Chris Wilson
2016-10-06T13:40:54Z

On Thu, Oct 06, 2016 at 04:32:37PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > Along with the interrupt, we want to restore the fake-irq and
> > wait-timeout detection. If we use the breadcrumbs interface to setup the
> > interrupt as it wants, the auxiliary timers will also be restored.
> >
> > Fixes: 821ed7df6e2a ("drm/i915: Update reset path to fix incomplete requests")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_breadcrumbs.c | 17 +++++++++++++++++
> > drivers/gpu/drm/i915/intel_engine_cs.c | 15 ---------------
> > drivers/gpu/drm/i915/intel_lrc.c | 2 +-
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +-
> > 5 files changed, 20 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> > index 9dba4971fb1e..d27da6d69735 100644
> > --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> > +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> > @@ -584,6 +584,23 @@ int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
> > return 0;
> > }
> >
> > +void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine)
> > +{
> > + struct intel_breadcrumbs *b = &engine->breadcrumbs;
> > +
>
> Should we kill the timer before proceeding in here?

Which timer? In breadcrumbs.c, we are concerned with the fake_irq and
the wait-timeout. The wait-timeout is reset below, we should add the
code to cancel the fake_irq along with clearing the bit.

> Not relevant to this patch but I also noticed that the period
> is identical to hangcheck period. Multiple of hangcheck period
> would be better, as our kicking might help and we don't
> want to fallback to fake irqs just so easily.

?

The main GPU hangcheck is kicked off by the wait timeout. Keeping the
two pieces independent (fake-irq, hangcheck) is quite nice, and the
jiffie wake up serves as a backup, and either it is required or it will
be disabled by the reset.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre