Re: [PATCH] drm/i915: Always kick the execlists tasklet after reset

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-03-15 10:10:20)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> > +static inline bool __tasklet_enable(struct tasklet_struct *t)
>> > +{
>> > +     return atomic_dec_and_test(&t->count);
>> > +}
>> > +
>> >  #endif /* __I915_GEM_H__ */
>> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> > index dc3de09c7586..b2d0e16645c7 100644
>> > --- a/drivers/gpu/drm/i915/intel_lrc.c
>> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> > @@ -2030,7 +2030,8 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
>> >       if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
>> >               execlists->tasklet.func(execlists->tasklet.data);
>> >  
>> > -     tasklet_enable(&execlists->tasklet);
>> > +     if (__tasklet_enable(&execlists->tasklet))
>> > +             tasklet_hi_schedule(&execlists->tasklet);
>> 
>> Why not just go fully unconditional, enable and schedule?
>
> If we schedule before we finish the reset, the tasklet busyspins, get's
> kicked to ksoftirqd, which then busyspins for its timeslice and gets
> bumped around by the scheduler until finally ready.

Hmm stealing the whole timeslice is rude. Just wanted
to weight in the extra trickery. Random thought was
to advocate for execlists_tasklet_enable|disable,
but perhaps we are not there yet.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx