On 18/05/2017 13:24, Chris Wilson wrote:
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.
I don't think per context mask would work unless you won't to mandate
multi-contexts where they wouldn't otherwise be needed.
But this problem in general can also be solved separately from
class-instance addressing via engine feature masking.
As the GEM_ENGINE_INFO ioctl proposal defines a set of engine flags, at
a later point execbuf could be extended with a new flag. For example:
eb.flags = I915_EXEC_CLASS | I915_ENGINE_CLASS_VIDEO |
I915_EXEC_ENGINE_FEATURES | I915_ENGINE_HAS_HEVC;
Only problem I see that engine flags in the current proposal are u64 so
it would be a bit challenging to fit that in eb.flags.
Not sure if it would make sense to split the engine info flags into a
smaller and larger set. u8 which would be flags "compatible" with
I915_EXEC_ENGINE_FEATURES, and the remainder which would be something
else, or unused/reserved? Like:
struct drm_i915_engine_info {
/** Engine instance number. */
__u32 instance;
__u32 rsvd;
/** Engine specific features and info. */
#define DRM_I915_ENGINE_HAS_HEVC BIT(0)
__u8 features;
__u8 rsvd;
__32 info;
};
Or at that point you bring on eb3.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
