[Intel-gfx] [PATCH 17/20] drm/i915/gt: Declare when we enabled timeslicing

[Chris Wilson]

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 at chris-wilson.co.uk>
> > Cc: Kenneth Graunke <kenneth at 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