| Message ID | 20200403091300.14734-10-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 03/04/2020 10:13, Chris Wilson wrote: > Let userspace know if they can trust timeslicing by including it as part > of the I915_PARAM_HAS_SCHEDULER::I915_SCHEDULER_CAP_TIMESLICING > > v2: Only declare timeslicing if we can safely preempt userspace. > > Fixes: 8ee36e048c98 ("drm/i915/execlists: Minimalistic timeslicing") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Kenneth Graunke <kenneth@whitecape.org> > --- > drivers/gpu/drm/i915/gt/intel_engine.h | 3 ++- > drivers/gpu/drm/i915/gt/intel_engine_user.c | 5 +++++ > include/uapi/drm/i915_drm.h | 1 + > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h > index b469de0dd9b6..424672ee7874 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h > @@ -339,7 +339,8 @@ intel_engine_has_timeslices(const struct intel_engine_cs *engine) > if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION)) > return false; > > - return intel_engine_has_semaphores(engine); > + return (intel_engine_has_semaphores(engine) && > + intel_engine_has_preemption(engine)); This is turning off timeslicing on Gen8? Well it wouldn't have worked anyway, outside the batch boundaries.. so it does sound technically correct. > } > > #endif /* _INTEL_RINGBUFFER_H_ */ > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c > index 848decee9066..b84fdd722781 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c > @@ -121,6 +121,11 @@ static void set_scheduler_caps(struct drm_i915_private *i915) > else > disabled |= BIT(map[i].sched); > } > + > + if (intel_engine_has_timeslices(engine)) > + enabled |= I915_SCHEDULER_CAP_TIMESLICING; > + else > + disabled |= I915_SCHEDULER_CAP_TIMESLICING; > } > > i915->caps.scheduler = enabled & ~disabled; > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 704dd0e3bc1d..1ee227b5131a 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -523,6 +523,7 @@ typedef struct drm_i915_irq_wait { > #define I915_SCHEDULER_CAP_PREEMPTION (1ul << 2) > #define I915_SCHEDULER_CAP_SEMAPHORES (1ul << 3) > #define I915_SCHEDULER_CAP_ENGINE_BUSY_STATS (1ul << 4) > +#define I915_SCHEDULER_CAP_TIMESLICING (1ul << 5) Split uapi from Gen8 fix? Regards, Tvrtko > > #define I915_PARAM_HUC_STATUS 42 > >
Quoting Tvrtko Ursulin (2020-04-07 11:50:31) > > On 03/04/2020 10:13, Chris Wilson wrote: > > Let userspace know if they can trust timeslicing by including it as part > > of the I915_PARAM_HAS_SCHEDULER::I915_SCHEDULER_CAP_TIMESLICING > > > > v2: Only declare timeslicing if we can safely preempt userspace. > > > > Fixes: 8ee36e048c98 ("drm/i915/execlists: Minimalistic timeslicing") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Kenneth Graunke <kenneth@whitecape.org> > > --- > > drivers/gpu/drm/i915/gt/intel_engine.h | 3 ++- > > drivers/gpu/drm/i915/gt/intel_engine_user.c | 5 +++++ > > include/uapi/drm/i915_drm.h | 1 + > > 3 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h > > index b469de0dd9b6..424672ee7874 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine.h > > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h > > @@ -339,7 +339,8 @@ intel_engine_has_timeslices(const struct intel_engine_cs *engine) > > if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION)) > > return false; > > > > - return intel_engine_has_semaphores(engine); > > + return (intel_engine_has_semaphores(engine) && > > + intel_engine_has_preemption(engine)); > > This is turning off timeslicing on Gen8? Well it wouldn't have worked > anyway, outside the batch boundaries.. so it does sound technically correct. ... > Split uapi from Gen8 fix? I don't regard gen8 as broken per se, for the kernel could preempt between batches -- but under the spotlight of "can userspace use this", it clearly cannot. Hence why I put them both together, it is not until userspace needs to control itself, that it becomes a problem. The igt that test this purposefully do not run on gen8 because I was aware of the limitations. That should have been a big clue, but it wasn't until I looked at it from an actual user's perspective did I realise how important that little detail was. :( -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index b469de0dd9b6..424672ee7874 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -339,7 +339,8 @@ intel_engine_has_timeslices(const struct intel_engine_cs *engine) if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION)) return false; - return intel_engine_has_semaphores(engine); + return (intel_engine_has_semaphores(engine) && + intel_engine_has_preemption(engine)); } #endif /* _INTEL_RINGBUFFER_H_ */ diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c index 848decee9066..b84fdd722781 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c @@ -121,6 +121,11 @@ static void set_scheduler_caps(struct drm_i915_private *i915) else disabled |= BIT(map[i].sched); } + + if (intel_engine_has_timeslices(engine)) + enabled |= I915_SCHEDULER_CAP_TIMESLICING; + else + disabled |= I915_SCHEDULER_CAP_TIMESLICING; } i915->caps.scheduler = enabled & ~disabled; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 704dd0e3bc1d..1ee227b5131a 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -523,6 +523,7 @@ typedef struct drm_i915_irq_wait { #define I915_SCHEDULER_CAP_PREEMPTION (1ul << 2) #define I915_SCHEDULER_CAP_SEMAPHORES (1ul << 3) #define I915_SCHEDULER_CAP_ENGINE_BUSY_STATS (1ul << 4) +#define I915_SCHEDULER_CAP_TIMESLICING (1ul << 5) #define I915_PARAM_HUC_STATUS 42
Let userspace know if they can trust timeslicing by including it as part of the I915_PARAM_HAS_SCHEDULER::I915_SCHEDULER_CAP_TIMESLICING v2: Only declare timeslicing if we can safely preempt userspace. Fixes: 8ee36e048c98 ("drm/i915/execlists: Minimalistic timeslicing") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Kenneth Graunke <kenneth@whitecape.org> --- drivers/gpu/drm/i915/gt/intel_engine.h | 3 ++- drivers/gpu/drm/i915/gt/intel_engine_user.c | 5 +++++ include/uapi/drm/i915_drm.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)