On 27/04/2017 10:25, Chris Wilson wrote:
On Thu, Apr 27, 2017 at 10:10:34AM +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. The new uAPI also removes access to the weak VCS engine balancing as currently existing in the driver. Example usage to send a command to VCS0: eb.flags = i915_execbuffer2_engine(DRM_I915_ENGINE_CLASS_VIDEO_DECODE, 0); Or to send a command to VCS1: eb.flags = i915_execbuffer2_engine(DRM_I915_ENGINE_CLASS_VIDEO_DECODE, 1); v2: * Fix unknown flags mask. * Use I915_EXEC_RING_MASK for class. (Chris Wilson) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Cc: Ben Widawsky <ben@xxxxxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> Cc: Jon Bloomfield <jon.bloomfield@xxxxxxxxx> Cc: Daniel Charles <daniel.charles@xxxxxxxxx> Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@xxxxxxxxx> Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx> Cc: "Gong, Zhipeng" <zhipeng.gong@xxxxxxxxx> Cc: intel-vaapi-media@xxxxxxxxxxxx Cc: mesa-dev@xxxxxxxxxxxxxxxxxxxxx --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 29 +++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 11 ++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index af1965774e7b..ecd1486642a7 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1492,6 +1492,32 @@ gen8_dispatch_bsd_engine(struct drm_i915_private *dev_priv, return file_priv->bsd_engine; } +extern u8 user_class_map[DRM_I915_ENGINE_CLASS_MAX]; + +static struct intel_engine_cs * +eb_select_engine_class_instance(struct drm_i915_private *i915, + struct drm_i915_gem_execbuffer2 *args) +{ + struct intel_engine_cs *engine; + enum intel_engine_id id; + u8 class, instance; + + class = args->flags & I915_EXEC_RING_MASK; + if (class >= DRM_I915_ENGINE_CLASS_MAX) + return NULL; + class = user_class_map[class]; + + instance = (args->flags >> I915_EXEC_INSTANCE_SHIFT) && + I915_EXEC_INSTANCE_MASK; + + for_each_engine(engine, i915, id) { + if (engine->class == class && engine->instance == instance) + return engine; + }I am underwhelmed. No, i915->class_engine[class][instance] ?
Hey it's just an RFC for the uAPI proposal! Implementation efficiency only comes later! :)
Still, at what point do we kill busy-ioctl per-engine reporting? Should
It's the one we already broke before without no one noticing, where it userspace only effectively cares about a boolean value?
If so you recommend we make it a real boolean?
we update all tracepoints to use class:instance (I think that's a better abi going forward).
I can't think of any big problems doing so. Could rename ring= to engine= there as well. engine=<class>.<instance> for example?
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
