Quoting Chris Wilson (2018-05-09 15:02:12)
> Quoting Mika Kuoppala (2018-05-09 14:54:04)
> > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > > Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> > > CC: Michel Thierry <michel.thierry@xxxxxxxxx>
> > > Cc: Jeff McGee <jeff.mcgee@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/i915_gem.c | 2 --
> > > drivers/gpu/drm/i915/i915_tasklet.h | 14 ++++++++++++++
> > > drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++--
> > > 3 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 98481e150e61..8d27e78b052c 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -3043,8 +3043,6 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine)
> > > * common case of recursively being called from set-wedged from inside
> > > * i915_reset.
> > > */
> > > - if (i915_tasklet_is_enabled(&engine->execlists.tasklet))
> > > - i915_tasklet_flush(&engine->execlists.tasklet);
> > > i915_tasklet_disable(&engine->execlists.tasklet);
> > >
> > > /*
> > > diff --git a/drivers/gpu/drm/i915/i915_tasklet.h b/drivers/gpu/drm/i915/i915_tasklet.h
> > > index c9f41a5ebb91..d8263892f385 100644
> > > --- a/drivers/gpu/drm/i915/i915_tasklet.h
> > > +++ b/drivers/gpu/drm/i915/i915_tasklet.h
> > > @@ -59,10 +59,24 @@ static inline void i915_tasklet_enable(struct i915_tasklet *t)
> > > }
> > >
> > > static inline void i915_tasklet_disable(struct i915_tasklet *t)
> > > +{
> > > + if (i915_tasklet_is_enabled(t))
> > > + i915_tasklet_flush(t);
> >
> > This needs a comment to explain how we can get away with
> > the race.
>
> It doesn't, we rely on exclusivity that is provided by it is only us
> that controls the tasklet. The rationale comes from the caller. That we
> call flush here at all, is just that it helps avoid spinning, it is not
> required. If the race is too much, we just need the next line.
Rather than ever worry, let's just kill it.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
