[Intel-gfx] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2

[Chris Wilson]

On Thu, May 18, 2017 at 01:13:20PM +0100, Tvrtko Ursulin wrote:
> 
> On 18/05/2017 12:10, Chris Wilson wrote:
> >On Thu, May 18, 2017 at 01:55:59PM +0300, Joonas Lahtinen wrote:
> >>On ke, 2017-05-17 at 16:40 +0100, Tvrtko Ursulin wrote:
> >>>From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>>
> >>>Building on top of the previous patch which exported the concept
> >>>of engine classes and instances, we can also use this instead of
> >>>the current awkward engine selection uAPI.
> >>>
> >>>This is primarily interesting for the VCS engine selection which
> >>>is a) currently done via disjoint set of flags, and b) the
> >>>current I915_EXEC_BSD flags has different semantics depending on
> >>>the underlying hardware which is bad.
> >>>
> >>>Proposed idea here is to reserve 16-bits of flags, to pass in
> >>>the engine class and instance (8 bits each), and a new flag
> >>>named I915_EXEC_CLASS_INSTACE to tell the kernel this new engine
> >>>selection API is in use.
> >
> >Note this text doesn't describe the interface in v3.
> >
> >>Would it make sense to use bitmask for future proofing? Then we could
> >>allow passing multiple options in the future.
> >
> >No. The first use case has to be explicit control of engine. That's
> >orthogonal to asking to select any of a particular class.
> 
> Was the suggestion to allow instance=any and instance=N? Or even all
> the way to instance=N-or-M?
> 
> If not the latter, then I can think of two other possible approaches:
> 
> 1. Reserve 0xff as instance=any, aka 128 instances should be enough
> for everbody. :) Could simply enforce in the uAPI that instance ==
> I915_EXEC_INSTANCE_MASK = -EINVAL for now or something.
> 
> 2. Do nothing now and add say I915_EXEC_CLASS at a later point. This
> patch adds I915_EXEC_CLASS_INSTANCE so I915_EXEC_CLASS would not
> sound out of place.

Yes, I would argue to defer it until later. One problem I have at the
moment is that not all members of a class are equal, HEVC-capable
engines and the reset do not belong to the same class (i.e. my hope is
that we could just say <class> | [ <mask> | INSTANCE_MASK ] | LOAD_BALANCE)
So I can see the sense in having instance as a mask, or at least making
the current instance field large enough to store a mask in future. I
just feel uneasy as that field could grow quite large, and maybe it will
be better to set the constraint via a context param (all dependency on
frequency and tuning of the LOAD_BALANCE). Hmm, liking having the
instance-mask on the context atm.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre