[Intel-gfx] [PATCH 07/18] drm/i915: Combine tasklet_kill and tasklet_disable

[Chris Wilson]

Quoting Mika Kuoppala (2018-04-26 11:19:14)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2018-04-24 13:26:11)
> >> Chris Wilson <chris at chris-wilson.co.uk> writes:
> >> 
> >> > Ideally, we want to atomically flush and disable the tasklet before
> >> > resetting the GPU. At present, we rely on being the only part to touch
> >> > our tasklet and serialisation of the reset process to ensure that we can
> >> > suspend the tasklet from the mix of reset/wedge pathways. In this patch,
> >> > we move the tasklet abuse into its own function and tweak it such that
> >> > we only do a synchronous operation the first time it is disabled around
> >> > the reset. This allows us to avoid the sync inside a softirq context in
> >> > subsequent patches.
> >> >
> >> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> >> > Cc: Michał Winiarski <michal.winiarski at intel.com>
> >> > CC: Michel Thierry <michel.thierry at intel.com>
> >> > Cc: Jeff McGee <jeff.mcgee at intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_lrc.c | 14 +++++++++++---
> >> >  1 file changed, 11 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >> > index bbcc6439a2a1..d5640f3d5276 100644
> >> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> >> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> >> > @@ -1754,6 +1754,16 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine)
> >> >       return init_workarounds_ring(engine);
> >> >  }
> >> >  
> >> > +static void tasklet_kill_and_disable(struct tasklet_struct *t)
> >> > +{
> >> > +     if (!atomic_read(&t->count))
> >> > +             tasklet_kill(t);
> >> > +
> >> > +     if (atomic_inc_return(&t->count) == 1)
> >> > +             tasklet_unlock_wait(t);
> >> 
> >> You would spin only on the first try regardless. Is this
> >> just to prevent one extra spinlock on reset path?
> >
> > No, the end goal is to prevent a recursive lock.
> 
> Ok so you want to be able to call this from inside the
> tasklet itself.
> 
> On the bigger picture, if the preempt has has already
> timeouted and we want to reset, does it matter between
> resetting from wq or from timer irq. In another
> words what do we gain for this much of added complexity?

Yes. A reset and recovery is microseconds. Scheduling the wq is orders
of magnitude indeterminably worse latency. (Calling schedule() itself
will take as long as the reset.)
-Chris