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@xxxxxxxxxxxxxxxxxx>
> > Cc: Kenneth Graunke <kenneth@xxxxxxxxxxxxx>
> > ---
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
