[Intel-gfx] [PATCH] drm/i915: Reset the breadcrumbs IRQ more carefully

[Chris Wilson]

On Thu, Oct 06, 2016 at 05:02:44PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > On Thu, Oct 06, 2016 at 04:32:37PM +0300, Mika Kuoppala wrote:
> >> Chris Wilson <chris at 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 at chris-wilson.co.uk>
> >> > Cc: Mika Kuoppala <mika.kuoppala at 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.
> 
> I was considering that irqs are enabled and we have a
> active breadcrumbs timer, triggering at the same time as
> reset happens. So we would enable the fake irq as a post reset
> race between reset/breadcrumbs hangcheck.
> 
> As in why not cancel and postpone the timer and only after
> clear the missed_irq?

So just picking up that we don't cancel the fake irq along with the
clear_bit() (currently justing for the wait to complete before
cancelling).

> >> 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.
> 
> But we queue hangcheck also from retire work. So it could be that
> we fallback to fake irqs, even if next hangcheck might have
> managed to kick the wait and make forward progress?

Below the level of care. The limited kicking that hangcheck does is
immaterial to deciding whether or not we might need fake user interrupts.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre