[Intel-gfx] [PATCH 1/8] drm/i915/execlists: Avoid sync calls during park

[Chris Wilson]

Quoting Mika Kuoppala (2019-08-12 10:27:16)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Since we allow ourselves to use non-process context during parking, we
> > cannot allow ourselves to sleep and in particular cannot call
> > del_timer_sync() -- but we can use a plain del_timer().
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111375
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index bb74954889dd..b97047d58d3d 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2728,7 +2728,7 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
> >  
> >  static void execlists_park(struct intel_engine_cs *engine)
> >  {
> > -     del_timer_sync(&engine->execlists.timer);
> > +     del_timer(&engine->execlists.timer);
> 
> There will be another sync point then somewhere else or not needed?

Not required, as it means the timer if currently running and will just
kick the tasklet (as it does today). The tasklet running after we park
is not a huge issue as it doesn't touch HW -- it checks a CPU mapping
and in the process drains the GT wakeref.
 
> Also are irq safe timers where we could do a sync deletion. 
> 
> So my question is why the need for a sync point disappeared?

We didn't use it correctly to begin with :) To complete the sync, we
should have put a tasklet_kill(&execlists->tasklet); afterwards.
-Chris