| Message ID | 20170621091357.1408-2-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Tvrtko Ursulin (2017-06-21 10:13:57) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > This is a lighter-weight alternative to the previously posted > RFC titled "drm/i915: Engine discovery uAPI" which still allows > some engine configuration probing without depending on PCI ids. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Ben Widawsky <ben@bwidawsk.net> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Jon Bloomfield <jon.bloomfield@intel.com> > Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com> > Cc: Oscar Mateo <oscar.mateo@intel.com> > Cc: "Gong, Zhipeng" <zhipeng.gong@intel.com> > Cc: intel-vaapi-media@lists.01.org > -- > Floating as an alternative to the heavier engine discovery API > sent previously which did not manage to gain much interest from > userspace clients. > > With this one enumeration and feature discovery would be done by > sending null batches to all engine instances. Downside is less > extensibility if we are using a fixed and smaller number of eb > flags. But we lose out on features? Just after you convinced me that features was what we wanted! :-p -Chris
On 21/06/2017 10:45, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2017-06-21 10:13:57) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> This is a lighter-weight alternative to the previously posted >> RFC titled "drm/i915: Engine discovery uAPI" which still allows >> some engine configuration probing without depending on PCI ids. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Ben Widawsky <ben@bwidawsk.net> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Daniel Vetter <daniel.vetter@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Jon Bloomfield <jon.bloomfield@intel.com> >> Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com> >> Cc: Oscar Mateo <oscar.mateo@intel.com> >> Cc: "Gong, Zhipeng" <zhipeng.gong@intel.com> >> Cc: intel-vaapi-media@lists.01.org >> -- >> Floating as an alternative to the heavier engine discovery API >> sent previously which did not manage to gain much interest from >> userspace clients. >> >> With this one enumeration and feature discovery would be done by >> sending null batches to all engine instances. Downside is less >> extensibility if we are using a fixed and smaller number of eb >> flags. > > But we lose out on features? Just after you convinced me that features > was what we wanted! :-p We lose the features but get the capabilities :), if that was the joke! Or a concern that we might really want more data about stuff when probing? We could still add that later if we wanted since it is really a different thing altogether. This caps thing is actually 2nd part from another experiment I had, where the first part allowed userspace to tell us they do not care about the state, so we would be able to pick a VCS engine per-batch buffer, and not only statically per context. Maybe I add that one to this series as well and then it all becomes even more useful? Regards, Tvrtko
Quoting Tvrtko Ursulin (2017-06-23 12:58:19) > > On 21/06/2017 10:45, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2017-06-21 10:13:57) > >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >> This is a lighter-weight alternative to the previously posted > >> RFC titled "drm/i915: Engine discovery uAPI" which still allows > >> some engine configuration probing without depending on PCI ids. > >> > >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> Cc: Ben Widawsky <ben@bwidawsk.net> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Cc: Daniel Vetter <daniel.vetter@intel.com> > >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > >> Cc: Jon Bloomfield <jon.bloomfield@intel.com> > >> Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com> > >> Cc: Oscar Mateo <oscar.mateo@intel.com> > >> Cc: "Gong, Zhipeng" <zhipeng.gong@intel.com> > >> Cc: intel-vaapi-media@lists.01.org > >> -- > >> Floating as an alternative to the heavier engine discovery API > >> sent previously which did not manage to gain much interest from > >> userspace clients. > >> > >> With this one enumeration and feature discovery would be done by > >> sending null batches to all engine instances. Downside is less > >> extensibility if we are using a fixed and smaller number of eb > >> flags. > > > > But we lose out on features? Just after you convinced me that features > > was what we wanted! :-p > > We lose the features but get the capabilities :), if that was the joke! > > Or a concern that we might really want more data about stuff when > probing? We could still add that later if we wanted since it is really a > different thing altogether. > > This caps thing is actually 2nd part from another experiment I had, > where the first part allowed userspace to tell us they do not care about > the state, so we would be able to pick a VCS engine per-batch buffer, > and not only statically per context. > > Maybe I add that one to this series as well and then it all becomes even > more useful? My minimum requirement for testing execbuf is to be able to explicitly select an engine. So long as you do not exclude that possibility, I don't mind. I was also happy if that requires us to query the set of engines to figure out the mapping for the new interface. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 00c363a801cb..d34db9dc5870 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2062,6 +2062,9 @@ eb_select_engine_class_instance(struct drm_i915_private *i915, struct drm_i915_gem_execbuffer2 *args) { u8 class = args->flags & I915_EXEC_RING_MASK; + u8 caps = (args->flags & I915_EXEC_ENGINE_CAP_MASK) >> + I915_EXEC_ENGINE_CAP_SHIFT; + struct intel_engine_cs *engine; u8 instance; if (class >= ARRAY_SIZE(user_class_map)) @@ -2070,7 +2073,12 @@ eb_select_engine_class_instance(struct drm_i915_private *i915, instance = (args->flags & I915_EXEC_INSTANCE_MASK) >> I915_EXEC_INSTANCE_SHIFT; - return intel_engine_lookup(i915, user_class_map[class], instance); + engine = intel_engine_lookup(i915, user_class_map[class], instance); + + if (!engine || ((caps & engine->caps) != caps)) + return NULL; + + return engine; } #define I915_USER_RINGS (4) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 93466761ebd6..77dcc1150f7d 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -222,6 +222,9 @@ intel_engine_setup(struct drm_i915_private *dev_priv, engine->irq_shift = info->irq_shift; engine->class = info->class; engine->instance = info->instance; + if (INTEL_GEN(dev_priv) >= 8 && engine->class == VIDEO_DECODE_CLASS && + engine->instance == 0) + engine->caps = I915_BSD_CAP_HEVC; engine->context_size = __intel_engine_context_size(dev_priv, engine->class); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index c408d3d5cc80..19a27b57417d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -196,6 +196,8 @@ struct intel_engine_cs { u8 class; u8 instance; + u8 caps; + u32 context_size; u32 mmio_base; unsigned int irq_shift; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 30fec2c8fc95..e473d50cade9 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -933,7 +933,18 @@ struct drm_i915_gem_execbuffer2 { #define I915_EXEC_INSTANCE_SHIFT (20) #define I915_EXEC_INSTANCE_MASK (0xff << I915_EXEC_INSTANCE_SHIFT) -#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 27) << 1)) +/* + * Inform the kernel of what engine capabilities this batch buffer + * requires. For example only the first VCS engine has the HEVC block. + * + * We reserve four bits for the capabilities where each can be shared + * between different engines. Eg. first bit can mean one feature for + * one engine and something else for the other. + */ +#define I915_EXEC_ENGINE_CAP_SHIFT (28) +#define I915_EXEC_ENGINE_CAP_MASK (0xf << I915_EXEC_ENGINE_CAP_SHIFT) + +#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 31) << 1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ @@ -941,6 +952,10 @@ struct drm_i915_gem_execbuffer2 { #define i915_execbuffer2_get_context_id(eb2) \ ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) +#define I915_BSD_CAP_HEVC (1 << 0) + +#define I915_ENGINE_CAP_FLAG(v) ((v) << I915_EXEC_ENGINE_CAP_SHIFT) + enum drm_i915_gem_engine_class { I915_ENGINE_CLASS_OTHER = 0, I915_ENGINE_CLASS_RENDER = 1,