[igt-dev] [i-g-t V3 1/2] lib/i915: find scheduler caps for timeslicing

[Chris Wilson]

Quoting Tvrtko Ursulin (2021-06-01 11:24:00)
> 
> On 01/06/2021 11:10, Surendrakumar Upadhyay, TejaskumarX wrote:
> >> -----Original Message-----
> >> From: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> >> Sent: 01 June 2021 15:38
> >> To: Surendrakumar Upadhyay, TejaskumarX
> >> <tejaskumarx.surendrakumar.upadhyay at intel.com>; igt-
> >> dev at lists.freedesktop.org
> >> Subject: Re: [igt-dev] [i-g-t V3 1/2] lib/i915: find scheduler caps for timeslicing
> >>
> >>
> >> On 31/05/2021 14:41, Tejas Upadhyay wrote:
> >>> Check scheduler capability for timeslicing in appropriate way, which
> >>> requires wrappers to be created for IGT.
> >>>
> >>> V3:
> >>>       removed conditional error - petri
> >>> V2: has_timeslice = has_semaphore && has_preemption ||
> >>>       has_guc_submission - Tvrtko
> >>>
> >>> Signed-off-by: Tejas Upadhyay
> >>> <tejaskumarx.surendrakumar.upadhyay at intel.com>
> >>> ---
> >>>    lib/i915/gem_scheduler.c | 21 +++++++++++++++++++++
> >>>    lib/i915/gem_scheduler.h |  1 +
> >>>    2 files changed, 22 insertions(+)
> >>>
> >>> diff --git a/lib/i915/gem_scheduler.c b/lib/i915/gem_scheduler.c index
> >>> 184da843..f59c451f 100644
> >>> --- a/lib/i915/gem_scheduler.c
> >>> +++ b/lib/i915/gem_scheduler.c
> >>> @@ -29,6 +29,7 @@
> >>>    #include "ioctl_wrappers.h"
> >>>
> >>>    #include "i915/gem_scheduler.h"
> >>> +#include "i915/gem_submission.h"
> >>>
> >>>    /**
> >>>     * SECTION:gem_scheduler
> >>> @@ -129,6 +130,22 @@ bool gem_scheduler_has_engine_busy_stats(int
> >> fd)
> >>>             I915_SCHEDULER_CAP_ENGINE_BUSY_STATS;
> >>>    }
> >>>
> >>> +/**
> >>> + * gem_scheduler_has_timeslicing:
> >>> + * @fd: open i915 drm file descriptor
> >>> + *
> >>> + * Feature test macro to query whether the driver supports using HW
> >>> +preemption
> >>> + * to implement timeslicing of userspace batches. This allows
> >>> +userspace to
> >>> + * implement micro-level scheduling within their own batches.
> >>> + */
> >>> +bool gem_scheduler_has_timeslicing(int fd) {
> >>> +   return ((gem_scheduler_capability(fd) &
> >>> +           (I915_SCHEDULER_CAP_PREEMPTION |
> >>> +            I915_SCHEDULER_CAP_SEMAPHORES))

This bitwise or is incorrect, both are required.

> >>> +           || gem_has_guc_submission(fd));

This is not always true.

And it removes the clarity of being a simple wrapper.
-Chris