Message ID | 20200227085723.1961649-17-chris@chris-wilson.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [01/20] drm/i915: Skip barriers inside waits | expand |
On 27/02/2020 08:57, 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 29c8c03c5caa..a32dc82a90d4 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h > @@ -326,7 +326,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 could be the fixes bit. AFAIR has_semaphores is can we preempt between batches and has_preemption is can we preempt inside them. What was the consequence of not considering this in has_timeslices? > } > > #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 2813e579b480..4f903431a3fe 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) And this would be the new uapi bit. Split into two patches? Regards, Tvrtko > > #define I915_PARAM_HUC_STATUS 42 > >
Quoting Tvrtko Ursulin (2020-02-28 12:45:42) > > On 27/02/2020 08:57, 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 29c8c03c5caa..a32dc82a90d4 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine.h > > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h > > @@ -326,7 +326,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 could be the fixes bit. Nah, I quite liked timeslicing between requests -- but I decided that if we are to make a statement userspace can rely on, it has to be timeslicing works inside userspace batches. > AFAIR has_semaphores is can we preempt between batches and > has_preemption is can we preempt inside them. What was the consequence > of not considering this in has_timeslices? Right. It meant the timeslice could only take effect in the MI_ARB_ENABLE window in between requests. That is we could break up long amalgamations of contexts, but not the user. So I don't think this bit is the fixes. > > #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 2813e579b480..4f903431a3fe 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) > > And this would be the new uapi bit. > > Split into two patches? Nah. This is the Fixes: that I need to use semaphores in iris :) And I think CAP_TIMESLICING has to mean userspace is timesliced (to the granularity of objects at least) for it to mean anything userspace can take advantage of. -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index 29c8c03c5caa..a32dc82a90d4 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -326,7 +326,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 2813e579b480..4f903431a3fe 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(-)