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@xxxxxxxxx> > >>> > >>>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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
