| Message ID | 20210201085715.27435-24-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/57] drm/i915/gt: Restrict the GT clock override to just Icelake | expand |
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@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. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
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@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
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; }
Kick the scheduler to allow it to see the timeslice duration change, don't peek into execlists. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gt/sysfs_engines.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)