Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > Quoting Mika Kuoppala (2019-08-12 10:27:16) >> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> >> > --- >> > 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. Ok, So no need for fancey irq safe timers either. Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx