[Intel-gfx] [PATCH 24/57] drm/i915/gt: Only kick the scheduler on timeslice/preemption change

[Chris Wilson]

Quoting Tvrtko Ursulin (2021-02-04 14:09:22)
> 
> On 01/02/2021 08:56, Chris Wilson wrote:
> > Kick the scheduler to allow it to see the timeslice duration change,
> > don't peek into execlists.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/gt/sysfs_engines.c | 11 +++++------
> >   1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/sysfs_engines.c b/drivers/gpu/drm/i915/gt/sysfs_engines.c
> > index 57ef5383dd4e..526f8402cfb7 100644
> > --- a/drivers/gpu/drm/i915/gt/sysfs_engines.c
> > +++ b/drivers/gpu/drm/i915/gt/sysfs_engines.c
> > @@ -9,6 +9,7 @@
> >   #include "i915_drv.h"
> >   #include "intel_engine.h"
> >   #include "intel_engine_heartbeat.h"
> > +#include "intel_engine_pm.h"
> >   #include "sysfs_engines.h"
> >   
> >   struct kobj_engine {
> > @@ -222,9 +223,8 @@ timeslice_store(struct kobject *kobj, struct kobj_attribute *attr,
> >               return -EINVAL;
> >   
> >       WRITE_ONCE(engine->props.timeslice_duration_ms, duration);
> > -
> > -     if (execlists_active(&engine->execlists))
> > -             set_timer_ms(&engine->execlists.timer, duration);
> > +     if (intel_engine_pm_is_awake(engine))
> > +             intel_engine_kick_scheduler(engine);
> >   
> >       return count;
> >   }
> > @@ -326,9 +326,8 @@ preempt_timeout_store(struct kobject *kobj, struct kobj_attribute *attr,
> >               return -EINVAL;
> >   
> >       WRITE_ONCE(engine->props.preempt_timeout_ms, timeout);
> > -
> > -     if (READ_ONCE(engine->execlists.pending[0]))
> > -             set_timer_ms(&engine->execlists.preempt, timeout);
> > +     if (intel_engine_pm_is_awake(engine))
> > +             intel_engine_kick_scheduler(engine);
> >   
> >       return count;
> >   }
> > 
> 
> Almost feels like from sysfs layer intel_engine_kick_scheduler should 
> dtrt without the need to check intel_engine_pm_is_awake. Even if that 
> means having __intel_engine_kick_scheduler for more intimate callers, if 
> required.
>   But anyway it is no worse than it was.

Yeah. I still remember trying to track down rogue tasklet execution
outside of the engine wakeref, back when we reading the execlists status
with from mmio.

We can drop the check as the current and future tasklets should be safe.
-Chris