* [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI @ 2017-04-18 16:56 Tvrtko Ursulin 2017-04-18 16:56 ` [RFC 1/2] drm/i915: Engine discovery uAPI Tvrtko Ursulin ` (6 more replies) 0 siblings, 7 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-18 16:56 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, intel-vaapi-media, mesa-dev, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Inspired by the recent introduction of the engine class and instance concept, and a chat with Chris Wilson about a potential unification of PCI id based device discovery across multiple userspace components, I have cooked up two patches to gather some opinions on whether this sort of uAPI would be interesting for a wider audience. First patch is basically and old idea by Jon Bloomfield on how to do engine discovery, while the second one is just a quick hack to show how a similar approach could work for execbuffer2. Both patches are compile tested only and intended only to start a discussion. Tvrtko Ursulin (2): drm/i915: Engine discovery uAPI drm/i915: Select engines via class and instance in execbuffer2 drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/i915_gem_execbuffer.c | 36 ++++++++++++++++++ drivers/gpu/drm/i915/intel_engine_cs.c | 59 ++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 47 +++++++++++++++++++++++- 5 files changed, 145 insertions(+), 1 deletion(-) -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* [RFC 1/2] drm/i915: Engine discovery uAPI 2017-04-18 16:56 [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI Tvrtko Ursulin @ 2017-04-18 16:56 ` Tvrtko Ursulin 2017-04-18 20:13 ` Chris Wilson ` (3 more replies) 2017-04-18 16:56 ` [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2 Tvrtko Ursulin ` (5 subsequent siblings) 6 siblings, 4 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-18 16:56 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, intel-vaapi-media, mesa-dev, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Engine discovery uAPI allows userspace to probe for engine configuration and features without needing to maintain the internal PCI id based database. This enables removal of code duplications across userspace components. Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl which returns the number and information on the specified engine class. Currently only general engine configuration and HEVC feature of the VCS engine can be probed but the uAPI is designed to be generic and extensible. Code is based almost exactly on the earlier proposal on the topic by Jon Bloomfield. Engine class and instance refactoring made recently by Daniele Ceraolo Spurio enabled this to be implemented in an elegant fashion. To probe configuration userspace sets the engine class it wants to query (struct drm_i915_gem_engine_info) and provides an array of drm_i915_engine_info structs which will be filled in by the driver. Userspace also has to tell i915 how many elements are in the array, and the driver will report back the total number of engine instances in any case. 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: Daniel Charles <daniel.charles@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 Cc: mesa-dev@lists.freedesktop.org --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_engine_cs.c | 59 ++++++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 40 +++++++++++++++++++++++ 4 files changed, 103 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index cc7393e65e99..b720c9468adf 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2607,6 +2607,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 357b6c6c2f04..6eed0e854561 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3547,6 +3547,9 @@ i915_gem_context_lookup_timeline(struct i915_gem_context *ctx, int i915_perf_open_ioctl(struct drm_device *dev, void *data, struct drm_file *file); +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); + /* i915_gem_evict.c */ int __must_check i915_gem_evict_something(struct i915_address_space *vm, u64 min_size, u64 alignment, diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 402769d9d840..6deaffd34ae0 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -25,6 +25,7 @@ #include "i915_drv.h" #include "intel_ringbuffer.h" #include "intel_lrc.h" +#include <uapi/drm/i915_drm.h> struct engine_class_info { const char *name; @@ -1189,6 +1190,64 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915) engine->set_default_submission(engine); } +u8 user_class_map[DRM_I915_ENGINE_CLASS_MAX] = { + [DRM_I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, + [DRM_I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, + [DRM_I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, + [DRM_I915_ENGINE_CLASS_VIDEO_DECODE] = VIDEO_DECODE_CLASS, + [DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, +}; + +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file) +{ + struct drm_i915_private *i915 = to_i915(dev); + struct drm_i915_gem_engine_info *args = data; + enum drm_i915_gem_engine_class engine_class = args->engine_class; + struct drm_i915_engine_info __user *user_info = + u64_to_user_ptr(args->info_ptr); + struct drm_i915_engine_info info; + struct intel_engine_cs *engine; + enum intel_engine_id id; + u8 class; + + if (args->rsvd) + return -EINVAL; + + switch (engine_class) { + case DRM_I915_ENGINE_CLASS_OTHER: + case DRM_I915_ENGINE_CLASS_RENDER: + case DRM_I915_ENGINE_CLASS_COPY: + case DRM_I915_ENGINE_CLASS_VIDEO_DECODE: + case DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE: + if (engine_class >= DRM_I915_ENGINE_CLASS_MAX) + return -EINVAL; + class = user_class_map[engine_class]; + break; + default: + return -EINVAL; + }; + + args->num_engines = 0; + for_each_engine(engine, i915, id) { + if (class != engine->class) + continue; + + if (++args->num_engines > args->info_size) + continue; + + memset(&info, 0, sizeof(info)); + info.instance = engine->instance; + if (INTEL_GEN(i915) > 8 && id == VCS) + info.info = DRM_I915_ENGINE_HAS_HEVC; + + if (copy_to_user(user_info++, &info, sizeof(info))) + return -EFAULT; + } + + return 0; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_engine.c" #endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index f24a80d2d42e..6058596a9f33 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -260,6 +260,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_CONTEXT_GETPARAM 0x34 #define DRM_I915_GEM_CONTEXT_SETPARAM 0x35 #define DRM_I915_PERF_OPEN 0x36 +#define DRM_I915_GEM_ENGINE_INFO 0x37 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -315,6 +316,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) +#define DRM_IOCTL_I915_GEM_ENGINE_INFO DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_ENGINE_INFO, struct drm_i915_gem_engine_info) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1439,6 +1441,44 @@ enum drm_i915_perf_record_type { DRM_I915_PERF_RECORD_MAX /* non-ABI */ }; +enum drm_i915_gem_engine_class { + DRM_I915_ENGINE_CLASS_OTHER = 0, + DRM_I915_ENGINE_CLASS_RENDER = 1, + DRM_I915_ENGINE_CLASS_COPY = 2, + DRM_I915_ENGINE_CLASS_VIDEO_DECODE = 3, + DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE = 4, + DRM_I915_ENGINE_CLASS_MAX /* non-ABI */ +}; + +struct drm_i915_engine_info { + /** Engine instance number. */ + __u32 instance; + __u32 rsvd; + + /** Engine specific info. */ +#define DRM_I915_ENGINE_HAS_HEVC BIT(0) + __u64 info; +}; + +struct drm_i915_gem_engine_info { + /** in: Engine class to probe (enum drm_i915_gem_engine_class). */ + __u32 engine_class; + + /** out: Actual number of hardware engines. */ + __u32 num_engines; + + /** + * in: Number of struct drm_i915_engine_ifo entries in the provided + * info array. + */ + __u32 info_size; + __u32 rsvd; + + /** in/out: Pointer to array of struct i915_engine_info elements. */ + __u64 info_ptr; + +}; + #if defined(__cplusplus) } #endif -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [RFC 1/2] drm/i915: Engine discovery uAPI 2017-04-18 16:56 ` [RFC 1/2] drm/i915: Engine discovery uAPI Tvrtko Ursulin @ 2017-04-18 20:13 ` Chris Wilson 2017-04-24 8:07 ` Tvrtko Ursulin 2017-04-19 5:22 ` Kenneth Graunke ` (2 subsequent siblings) 3 siblings, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-04-18 20:13 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media On Tue, Apr 18, 2017 at 05:56:14PM +0100, Tvrtko Ursulin wrote: > +enum drm_i915_gem_engine_class { > + DRM_I915_ENGINE_CLASS_OTHER = 0, > + DRM_I915_ENGINE_CLASS_RENDER = 1, > + DRM_I915_ENGINE_CLASS_COPY = 2, > + DRM_I915_ENGINE_CLASS_VIDEO_DECODE = 3, > + DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE = 4, > + DRM_I915_ENGINE_CLASS_MAX /* non-ABI */ > +}; > + > +struct drm_i915_engine_info { > + /** Engine instance number. */ > + __u32 instance; > + __u32 rsvd; > + > + /** Engine specific info. */ > +#define DRM_I915_ENGINE_HAS_HEVC BIT(0) > + __u64 info; > +}; So the main question is how can we extend this in future, keeping forwards/backwards compat? I think if we put a query version into info and then the kernel supplies an array matching that version (or reports the most recent version supported if the request is too modern.) The kernel has to keep all the old struct variants and exporters indefinitely. Another alternative would get an ENGINE_GETPARAM where we just have a switch of all possibily questions. Maybe better as a CONTEXT_GETPARAM if we start thinking about allowing CONTEXT_SETPARAM to fine tune individual clients. > +struct drm_i915_gem_engine_info { > + /** in: Engine class to probe (enum drm_i915_gem_engine_class). */ > + __u32 engine_class; __u32 [in/out] version ? (see above) > + > + /** out: Actual number of hardware engines. */ > + __u32 num_engines; > + > + /** > + * in: Number of struct drm_i915_engine_ifo entries in the provided > + * info array. > + */ > + __u32 info_size; This is superfluous with num_engines. The standard 2 pass, discovery of size, followed by allocation and final query. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC 1/2] drm/i915: Engine discovery uAPI 2017-04-18 20:13 ` Chris Wilson @ 2017-04-24 8:07 ` Tvrtko Ursulin 0 siblings, 0 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-24 8:07 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Joonas Lahtinen, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev On 18/04/2017 21:13, Chris Wilson wrote: > On Tue, Apr 18, 2017 at 05:56:14PM +0100, Tvrtko Ursulin wrote: >> +enum drm_i915_gem_engine_class { >> + DRM_I915_ENGINE_CLASS_OTHER = 0, >> + DRM_I915_ENGINE_CLASS_RENDER = 1, >> + DRM_I915_ENGINE_CLASS_COPY = 2, >> + DRM_I915_ENGINE_CLASS_VIDEO_DECODE = 3, >> + DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE = 4, >> + DRM_I915_ENGINE_CLASS_MAX /* non-ABI */ >> +}; >> + >> +struct drm_i915_engine_info { >> + /** Engine instance number. */ >> + __u32 instance; >> + __u32 rsvd; >> + >> + /** Engine specific info. */ >> +#define DRM_I915_ENGINE_HAS_HEVC BIT(0) >> + __u64 info; >> +}; > > So the main question is how can we extend this in future, keeping > forwards/backwards compat? > > I think if we put a query version into info and then the kernel supplies > an array matching that version (or reports the most recent version > supported if the request is too modern.) > > The kernel has to keep all the old struct variants and exporters > indefinitely. Versioning sounds good to me. > Another alternative would get an ENGINE_GETPARAM where we just have a > switch of all possibily questions. Maybe better as a CONTEXT_GETPARAM if > we start thinking about allowing CONTEXT_SETPARAM to fine tune > individual clients. This idea I did not get - what is the switch of all possible questions? You mean new ioctl like ENGINE_GETPARAM which would return a list of queries supported by CONTEXT_GETPARAM? Which would effectively be a dispatcher-in-dispatcher kind of thing? >> +struct drm_i915_gem_engine_info { >> + /** in: Engine class to probe (enum drm_i915_gem_engine_class). */ >> + __u32 engine_class; > > __u32 [in/out] version ? (see above) > >> + >> + /** out: Actual number of hardware engines. */ >> + __u32 num_engines; >> + >> + /** >> + * in: Number of struct drm_i915_engine_ifo entries in the provided >> + * info array. >> + */ >> + __u32 info_size; > > This is superfluous with num_engines. The standard 2 pass, discovery of > size, followed by allocation and final query. This is also fine. I was one the fence whether to actually condense it to one field in the first posting or not myself. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC 1/2] drm/i915: Engine discovery uAPI 2017-04-18 16:56 ` [RFC 1/2] drm/i915: Engine discovery uAPI Tvrtko Ursulin 2017-04-18 20:13 ` Chris Wilson @ 2017-04-19 5:22 ` Kenneth Graunke 2017-04-24 8:26 ` [Mesa-dev] " Tvrtko Ursulin 2017-04-19 8:32 ` Gong, Zhipeng 2017-04-27 9:10 ` [RFC v2 " Tvrtko Ursulin 3 siblings, 1 reply; 48+ messages in thread From: Kenneth Graunke @ 2017-04-19 5:22 UTC (permalink / raw) To: mesa-dev Cc: Oscar Mateo, Ben Widawsky, Joonas Lahtinen, Tvrtko Ursulin, Intel-gfx, Rogozhkin, Dmitry V, Tvrtko Ursulin, Jon Bloomfield, Daniel Vetter, intel-vaapi-media, Gong, Zhipeng [-- Attachment #1.1: Type: text/plain, Size: 1345 bytes --] On Tuesday, April 18, 2017 9:56:14 AM PDT Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Engine discovery uAPI allows userspace to probe for engine > configuration and features without needing to maintain the > internal PCI id based database. I don't understand why I would want to query the existence of engines from the kernel. As a userspace driver developer, I have to know how to program the specific generation of hardware I'm on. I better know what engines that GPU has, or else there's no way I can competently program it. In Mesa, we recently imported libdrm and deleted all the engine checks (a460e1eb51406e5ca54abda42112bfb8523ff046). All generations have an RCS, Gen6+ has a separate BLT, and we don't need to use the others. It's completely statically determinable with a simple check. Runtime checks make sense for optional things...but not "is there a 3D engine?". Plus, even if you add this to the kernel, we still support back to 3.6 (and ChromeOS needs us to continue supporting 3.8), so we won't be able to magically use the new uABI - we'd need to support both. Which, if the point is to delete code...we'd actually have /more/ code for a few years. Or, we could not use it...but then nobody would be testing it, and if a bug creeps in...that pushes it back more years still. Sorry :( [-- Attachment #1.2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Mesa-dev] [RFC 1/2] drm/i915: Engine discovery uAPI 2017-04-19 5:22 ` Kenneth Graunke @ 2017-04-24 8:26 ` Tvrtko Ursulin 0 siblings, 0 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-24 8:26 UTC (permalink / raw) To: Kenneth Graunke, mesa-dev Cc: Daniel Vetter, Ben Widawsky, intel-vaapi-media, Intel-gfx On 19/04/2017 06:22, Kenneth Graunke wrote: > On Tuesday, April 18, 2017 9:56:14 AM PDT Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Engine discovery uAPI allows userspace to probe for engine >> configuration and features without needing to maintain the >> internal PCI id based database. > > I don't understand why I would want to query the existence of engines > from the kernel. As a userspace driver developer, I have to know how to > program the specific generation of hardware I'm on. I better know what > engines that GPU has, or else there's no way I can competently program it. > > In Mesa, we recently imported libdrm and deleted all the engine checks > (a460e1eb51406e5ca54abda42112bfb8523ff046). All generations have an > RCS, Gen6+ has a separate BLT, and we don't need to use the others. > It's completely statically determinable with a simple check. Runtime > checks make sense for optional things...but not "is there a 3D engine?". > > Plus, even if you add this to the kernel, we still support back to 3.6 > (and ChromeOS needs us to continue supporting 3.8), so we won't be able > to magically use the new uABI - we'd need to support both. Which, if > the point is to delete code...we'd actually have /more/ code for a few > years. Or, we could not use it...but then nobody would be testing it, > and if a bug creeps in...that pushes it back more years still. Okay the argument of more code for a while is I suppose always true with these types of work. But in general, the idea is to consolidate the queries and avoid (may be only partially) duplicate PCI databases across components. Because I suspect today you do some device discovery via libpciaccess (or something) and some via i915 GET_PARAMs and so. So the idea is to consolidate all that and do it via i915. Since another argument you raise, is that you have to know how does the GPU looks like to be able to competently program it, in which case who knows better than the kernel driver? But I think the main part of the argument is that why collect and derive this information from various sources when perhaps it could only be one. Maybe the exact idea is not so interesting for Mesa, which I wouldn't be surprised at all since the idea was born from libva and the BSD engine usage there. In which case perhaps Mesa could become more interested if the proposal was exporting some other data to userspace? I don't think it is critical to find something like that in Mesa, but it may be interesting. I think Ben mentioned at one point he had some ideas in this area, or something was discussed in the past which may be similar. I forgot the exact details now. So I think for now, if there is nothing which would be interesting in Mesa along the lines described so far, please just keep an eye on this. Just to make sure if some other component will be interested, and we end up starting to implement something, it is at least not hindering you, if we cannot find anything useful for you in here. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC 1/2] drm/i915: Engine discovery uAPI 2017-04-18 16:56 ` [RFC 1/2] drm/i915: Engine discovery uAPI Tvrtko Ursulin 2017-04-18 20:13 ` Chris Wilson 2017-04-19 5:22 ` Kenneth Graunke @ 2017-04-19 8:32 ` Gong, Zhipeng 2017-04-27 9:10 ` [RFC v2 " Tvrtko Ursulin 3 siblings, 0 replies; 48+ messages in thread From: Gong, Zhipeng @ 2017-04-19 8:32 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx >-----Original Message----- >From: Tvrtko Ursulin [mailto:tursulin@ursulin.net] >Sent: Wednesday, April 19, 2017 12:56 AM >To: Intel-gfx@lists.freedesktop.org >Cc: tursulin@ursulin.net; Ursulin, Tvrtko <tvrtko.ursulin@intel.com>; Ben Widawsky <ben@bwidawsk.net>; Chris Wilson <chris@chris-wilson.co.uk>; Vetter, Daniel <daniel.vetter@intel.com>; Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Bloomfield, Jon <jon.bloomfield@intel.com>; Charles, Daniel <daniel.charles@intel.com>; Rogozhkin, Dmitry V <dmitry.v.rogozhkin@intel.com>; Mateo Lozano, Oscar <oscar.mateo@intel.com>; Gong, Zhipeng <zhipeng.gong@intel.com>; intel-vaapi-media@lists.01.org; mesa-dev@lists.freedesktop.org Subject: [RFC 1/2] drm/i915: Engine discovery uAPI >+u8 user_class_map[DRM_I915_ENGINE_CLASS_MAX] = { >+ [DRM_I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, >+ [DRM_I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, >+ [DRM_I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, >+ [DRM_I915_ENGINE_CLASS_VIDEO_DECODE] = VIDEO_DECODE_CLASS, >+ [DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, }; >+ Not sure whether VIDEO_DECODE is a suitable name, since the same engine is also used in Media Encode. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* [RFC v2 1/2] drm/i915: Engine discovery uAPI 2017-04-18 16:56 ` [RFC 1/2] drm/i915: Engine discovery uAPI Tvrtko Ursulin ` (2 preceding siblings ...) 2017-04-19 8:32 ` Gong, Zhipeng @ 2017-04-27 9:10 ` Tvrtko Ursulin 2017-05-18 14:57 ` [RFC v3 " Tvrtko Ursulin 3 siblings, 1 reply; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-27 9:10 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, intel-vaapi-media, mesa-dev, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Engine discovery uAPI allows userspace to probe for engine configuration and features without needing to maintain the internal PCI id based database. This enables removal of code duplications across userspace components. Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl which returns the number and information on the specified engine class. Currently only general engine configuration and HEVC feature of the VCS engine can be probed but the uAPI is designed to be generic and extensible. Code is based almost exactly on the earlier proposal on the topic by Jon Bloomfield. Engine class and instance refactoring made recently by Daniele Ceraolo Spurio enabled this to be implemented in an elegant fashion. To probe configuration userspace sets the engine class it wants to query (struct drm_i915_gem_engine_info) and provides an array of drm_i915_engine_info structs which will be filled in by the driver. Userspace also has to tell i915 how many elements are in the array, and the driver will report back the total number of engine instances in any case. v2: * Add a version field and consolidate to one engine count. (Chris Wilson) * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO. (Gong Zhipeng) 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: Daniel Charles <daniel.charles@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 Cc: mesa-dev@lists.freedesktop.org --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_engine_cs.c | 64 ++++++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 40 +++++++++++++++++++++ 4 files changed, 108 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index c7d68e789642..1a3f0859227b 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2609,6 +2609,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 357b6c6c2f04..6eed0e854561 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3547,6 +3547,9 @@ i915_gem_context_lookup_timeline(struct i915_gem_context *ctx, int i915_perf_open_ioctl(struct drm_device *dev, void *data, struct drm_file *file); +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); + /* i915_gem_evict.c */ int __must_check i915_gem_evict_something(struct i915_address_space *vm, u64 min_size, u64 alignment, diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 82a274b336c5..caed32dbd912 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -25,6 +25,7 @@ #include "i915_drv.h" #include "intel_ringbuffer.h" #include "intel_lrc.h" +#include <uapi/drm/i915_drm.h> struct engine_class_info { const char *name; @@ -1187,6 +1188,69 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915) engine->set_default_submission(engine); } +u8 user_class_map[DRM_I915_ENGINE_CLASS_MAX] = { + [DRM_I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, + [DRM_I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, + [DRM_I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, + [DRM_I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS, + [DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, +}; + +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file) +{ + struct drm_i915_private *i915 = to_i915(dev); + struct drm_i915_gem_engine_info *args = data; + struct drm_i915_engine_info __user *user_info = + u64_to_user_ptr(args->info_ptr); + unsigned int info_size = args->num_engines; + struct drm_i915_engine_info info; + struct intel_engine_cs *engine; + enum intel_engine_id id; + u8 class; + + if (args->rsvd) + return -EINVAL; + + switch (args->engine_class) { + case DRM_I915_ENGINE_CLASS_OTHER: + case DRM_I915_ENGINE_CLASS_RENDER: + case DRM_I915_ENGINE_CLASS_COPY: + case DRM_I915_ENGINE_CLASS_VIDEO: + case DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE: + class = user_class_map[args->engine_class]; + break; + case DRM_I915_ENGINE_CLASS_MAX: + default: + return -EINVAL; + }; + + args->num_engines = 0; + + if (args->version != 1) { + args->version = 1; + return 0; + } + + for_each_engine(engine, i915, id) { + if (class != engine->class) + continue; + + if (++args->num_engines > info_size) + continue; + + memset(&info, 0, sizeof(info)); + info.instance = engine->instance; + if (INTEL_GEN(i915) >= 8 && id == VCS) + info.info = DRM_I915_ENGINE_HAS_HEVC; + + if (copy_to_user(user_info++, &info, sizeof(info))) + return -EFAULT; + } + + return 0; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_engine.c" #endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index f24a80d2d42e..2ac6667e57ea 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -260,6 +260,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_CONTEXT_GETPARAM 0x34 #define DRM_I915_GEM_CONTEXT_SETPARAM 0x35 #define DRM_I915_PERF_OPEN 0x36 +#define DRM_I915_GEM_ENGINE_INFO 0x37 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -315,6 +316,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) +#define DRM_IOCTL_I915_GEM_ENGINE_INFO DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_ENGINE_INFO, struct drm_i915_gem_engine_info) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1439,6 +1441,44 @@ enum drm_i915_perf_record_type { DRM_I915_PERF_RECORD_MAX /* non-ABI */ }; +enum drm_i915_gem_engine_class { + DRM_I915_ENGINE_CLASS_OTHER = 0, + DRM_I915_ENGINE_CLASS_RENDER = 1, + DRM_I915_ENGINE_CLASS_COPY = 2, + DRM_I915_ENGINE_CLASS_VIDEO = 3, + DRM_I915_ENGINE_CLASS_VIDEO_ENHANCE = 4, + DRM_I915_ENGINE_CLASS_MAX /* non-ABI */ +}; + +struct drm_i915_engine_info { + /** Engine instance number. */ + __u32 instance; + __u32 rsvd; + + /** Engine specific info. */ +#define DRM_I915_ENGINE_HAS_HEVC BIT(0) + __u64 info; +}; + +struct drm_i915_gem_engine_info { + /** in/out: Protocol version requested/supported. */ + __u32 version; + + /** in: Engine class to probe (enum drm_i915_gem_engine_class). */ + __u32 engine_class; + + /** + * in/out: Number of struct drm_i915_engine_info entries in the provided + * @info_ptr array and actual number of supported hardware engines. + */ + __u32 num_engines; + __u32 rsvd; + + /** in/out: Pointer to array of struct i915_engine_info elements. */ + __u64 info_ptr; + +}; + #if defined(__cplusplus) } #endif -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [RFC v3 1/2] drm/i915: Engine discovery uAPI 2017-04-27 9:10 ` [RFC v2 " Tvrtko Ursulin @ 2017-05-18 14:57 ` Tvrtko Ursulin 2017-06-26 15:47 ` [RFC v4 " Tvrtko Ursulin 0 siblings, 1 reply; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-18 14:57 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, intel-vaapi-media, mesa-dev, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Engine discovery uAPI allows userspace to probe for engine configuration and features without needing to maintain the internal PCI id based database. This enables removal of code duplications across userspace components. Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl which returns the number and information on the specified engine class. Currently only general engine configuration and HEVC feature of the VCS engine can be probed but the uAPI is designed to be generic and extensible. Code is based almost exactly on the earlier proposal on the topic by Jon Bloomfield. Engine class and instance refactoring made recently by Daniele Ceraolo Spurio enabled this to be implemented in an elegant fashion. To probe configuration userspace sets the engine class it wants to query (struct drm_i915_gem_engine_info) and provides an array of drm_i915_engine_info structs which will be filled in by the driver. Userspace also has to tell i915 how many elements are in the array, and the driver will report back the total number of engine instances in any case. v2: * Add a version field and consolidate to one engine count. (Chris Wilson) * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO. (Gong Zhipeng) v3: * Drop the DRM_ prefix from uAPI enums. (Chris Wilson) * Only reserve 8-bits for the engine info for time being. 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: Daniel Charles <daniel.charles@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 Cc: mesa-dev@lists.freedesktop.org --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_engine_cs.c | 64 ++++++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 41 ++++++++++++++++++++++ 4 files changed, 109 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 72fb47a439d2..dc268f41a609 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2623,6 +2623,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 17883a84b8c1..1e08b82c4823 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3521,6 +3521,9 @@ i915_gem_context_lookup_timeline(struct i915_gem_context *ctx, int i915_perf_open_ioctl(struct drm_device *dev, void *data, struct drm_file *file); +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); + /* i915_gem_evict.c */ int __must_check i915_gem_evict_something(struct i915_address_space *vm, u64 min_size, u64 alignment, diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 413bfd8d4bf4..8d370c5d07bc 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -25,6 +25,7 @@ #include "i915_drv.h" #include "intel_ringbuffer.h" #include "intel_lrc.h" +#include <uapi/drm/i915_drm.h> /* Haswell does have the CXT_SIZE register however it does not appear to be * valid. Now, docs explain in dwords what is in the context object. The full @@ -1286,6 +1287,69 @@ void intel_engines_mark_idle(struct drm_i915_private *i915) } } +u8 user_class_map[I915_ENGINE_CLASS_MAX] = { + [I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, + [I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, + [I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, + [I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS, + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, +}; + +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file) +{ + struct drm_i915_private *i915 = to_i915(dev); + struct drm_i915_gem_engine_info *args = data; + struct drm_i915_engine_info __user *user_info = + u64_to_user_ptr(args->info_ptr); + unsigned int info_size = args->num_engines; + struct drm_i915_engine_info info; + struct intel_engine_cs *engine; + enum intel_engine_id id; + u8 class; + + if (args->rsvd) + return -EINVAL; + + switch (args->engine_class) { + case I915_ENGINE_CLASS_OTHER: + case I915_ENGINE_CLASS_RENDER: + case I915_ENGINE_CLASS_COPY: + case I915_ENGINE_CLASS_VIDEO: + case I915_ENGINE_CLASS_VIDEO_ENHANCE: + class = user_class_map[args->engine_class]; + break; + case I915_ENGINE_CLASS_MAX: + default: + return -EINVAL; + }; + + args->num_engines = 0; + + if (args->version != 1) { + args->version = 1; + return 0; + } + + for_each_engine(engine, i915, id) { + if (class != engine->class) + continue; + + if (++args->num_engines > info_size) + continue; + + memset(&info, 0, sizeof(info)); + info.instance = engine->instance; + if (INTEL_GEN(i915) >= 8 && id == VCS) + info.info = I915_ENGINE_HAS_HEVC; + + if (copy_to_user(user_info++, &info, sizeof(info))) + return -EFAULT; + } + + return 0; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_engine.c" #endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index f24a80d2d42e..8f0926edc44f 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -260,6 +260,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_CONTEXT_GETPARAM 0x34 #define DRM_I915_GEM_CONTEXT_SETPARAM 0x35 #define DRM_I915_PERF_OPEN 0x36 +#define DRM_I915_GEM_ENGINE_INFO 0x37 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -315,6 +316,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) +#define DRM_IOCTL_I915_GEM_ENGINE_INFO DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_ENGINE_INFO, struct drm_i915_gem_engine_info) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1439,6 +1441,45 @@ enum drm_i915_perf_record_type { DRM_I915_PERF_RECORD_MAX /* non-ABI */ }; +enum drm_i915_gem_engine_class { + I915_ENGINE_CLASS_OTHER = 0, + I915_ENGINE_CLASS_RENDER = 1, + I915_ENGINE_CLASS_COPY = 2, + I915_ENGINE_CLASS_VIDEO = 3, + I915_ENGINE_CLASS_VIDEO_ENHANCE = 4, + I915_ENGINE_CLASS_MAX /* non-ABI */ +}; + +struct drm_i915_engine_info { + /** Engine instance number. */ + __u8 instance; + + /** Engine specific info. */ +#define I915_ENGINE_HAS_HEVC BIT(0) + __u8 info; + + __u8 rsvd2[6]; +}; + +struct drm_i915_gem_engine_info { + /** in/out: Protocol version requested/supported. */ + __u32 version; + + /** in: Engine class to probe (enum drm_i915_gem_engine_class). */ + __u32 engine_class; + + /** + * in/out: Number of struct drm_i915_engine_info entries in the provided + * @info_ptr array and actual number of supported hardware engines. + */ + __u32 num_engines; + __u32 rsvd; + + /** in/out: Pointer to array of struct i915_engine_info elements. */ + __u64 info_ptr; + +}; + #if defined(__cplusplus) } #endif -- 2.9.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [RFC v4 1/2] drm/i915: Engine discovery uAPI 2017-05-18 14:57 ` [RFC v3 " Tvrtko Ursulin @ 2017-06-26 15:47 ` Tvrtko Ursulin 2017-06-28 11:27 ` Chris Wilson ` (2 more replies) 0 siblings, 3 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-06-26 15:47 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Engine discovery uAPI allows userspace to probe for engine configuration and features without needing to maintain the internal PCI id based database. This enables removal of code duplications across userspace components. Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl which returns the number and information on the specified engine class. Currently only general engine configuration and HEVC feature of the VCS engine can be probed but the uAPI is designed to be generic and extensible. Code is based almost exactly on the earlier proposal on the topic by Jon Bloomfield. Engine class and instance refactoring made recently by Daniele Ceraolo Spurio enabled this to be implemented in an elegant fashion. To probe configuration userspace sets the engine class it wants to query (struct drm_i915_gem_engine_info) and provides an array of drm_i915_engine_info structs which will be filled in by the driver. Userspace also has to tell i915 how many elements are in the array, and the driver will report back the total number of engine instances in any case. v2: * Add a version field and consolidate to one engine count. (Chris Wilson) * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO. (Gong Zhipeng) v3: * Drop the DRM_ prefix from uAPI enums. (Chris Wilson) * Only reserve 8-bits for the engine info for time being. v4: * Made user_class_map static. 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> --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_engine_cs.c | 64 ++++++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 41 ++++++++++++++++++++++ 4 files changed, 109 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index e9d8e9ee51b2..44dd2f37937f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2724,6 +2724,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 427d10c7eca5..496565e1753f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3598,6 +3598,9 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine, struct i915_gem_context *ctx, uint32_t *reg_state); +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); + /* i915_gem_evict.c */ int __must_check i915_gem_evict_something(struct i915_address_space *vm, u64 min_size, u64 alignment, diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 3b46c1f7b88b..a98669f6ad85 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -25,6 +25,7 @@ #include "i915_drv.h" #include "intel_ringbuffer.h" #include "intel_lrc.h" +#include <uapi/drm/i915_drm.h> /* Haswell does have the CXT_SIZE register however it does not appear to be * valid. Now, docs explain in dwords what is in the context object. The full @@ -1332,6 +1333,69 @@ void intel_engines_mark_idle(struct drm_i915_private *i915) } } +static u8 user_class_map[I915_ENGINE_CLASS_MAX] = { + [I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, + [I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, + [I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, + [I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS, + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, +}; + +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file) +{ + struct drm_i915_private *i915 = to_i915(dev); + struct drm_i915_gem_engine_info *args = data; + struct drm_i915_engine_info __user *user_info = + u64_to_user_ptr(args->info_ptr); + unsigned int info_size = args->num_engines; + struct drm_i915_engine_info info; + struct intel_engine_cs *engine; + enum intel_engine_id id; + u8 class; + + if (args->rsvd) + return -EINVAL; + + switch (args->engine_class) { + case I915_ENGINE_CLASS_OTHER: + case I915_ENGINE_CLASS_RENDER: + case I915_ENGINE_CLASS_COPY: + case I915_ENGINE_CLASS_VIDEO: + case I915_ENGINE_CLASS_VIDEO_ENHANCE: + class = user_class_map[args->engine_class]; + break; + case I915_ENGINE_CLASS_MAX: + default: + return -EINVAL; + }; + + args->num_engines = 0; + + if (args->version != 1) { + args->version = 1; + return 0; + } + + for_each_engine(engine, i915, id) { + if (class != engine->class) + continue; + + if (++args->num_engines > info_size) + continue; + + memset(&info, 0, sizeof(info)); + info.instance = engine->instance; + if (INTEL_GEN(i915) >= 8 && id == VCS) + info.info = I915_VCS_HAS_HEVC; + + if (copy_to_user(user_info++, &info, sizeof(info))) + return -EFAULT; + } + + return 0; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_engine.c" #endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 7ccbd6a2bbe0..fbb1f6b99959 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -260,6 +260,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_CONTEXT_GETPARAM 0x34 #define DRM_I915_GEM_CONTEXT_SETPARAM 0x35 #define DRM_I915_PERF_OPEN 0x36 +#define DRM_I915_GEM_ENGINE_INFO 0x37 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -315,6 +316,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) +#define DRM_IOCTL_I915_GEM_ENGINE_INFO DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_ENGINE_INFO, struct drm_i915_gem_engine_info) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1467,6 +1469,45 @@ enum drm_i915_perf_record_type { DRM_I915_PERF_RECORD_MAX /* non-ABI */ }; +enum drm_i915_gem_engine_class { + I915_ENGINE_CLASS_OTHER = 0, + I915_ENGINE_CLASS_RENDER = 1, + I915_ENGINE_CLASS_COPY = 2, + I915_ENGINE_CLASS_VIDEO = 3, + I915_ENGINE_CLASS_VIDEO_ENHANCE = 4, + I915_ENGINE_CLASS_MAX /* non-ABI */ +}; + +struct drm_i915_engine_info { + /** Engine instance number. */ + __u8 instance; + + /** Engine specific info. */ +#define I915_VCS_HAS_HEVC BIT(0) + __u8 info; + + __u8 rsvd2[6]; +}; + +struct drm_i915_gem_engine_info { + /** in/out: Protocol version requested/supported. */ + __u32 version; + + /** in: Engine class to probe (enum drm_i915_gem_engine_class). */ + __u32 engine_class; + + /** + * in/out: Number of struct drm_i915_engine_info entries in the provided + * @info_ptr array and actual number of supported hardware engines. + */ + __u32 num_engines; + __u32 rsvd; + + /** in/out: Pointer to array of struct i915_engine_info elements. */ + __u64 info_ptr; + +}; + #if defined(__cplusplus) } #endif -- 2.9.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [RFC v4 1/2] drm/i915: Engine discovery uAPI 2017-06-26 15:47 ` [RFC v4 " Tvrtko Ursulin @ 2017-06-28 11:27 ` Chris Wilson 2017-06-28 13:15 ` Tvrtko Ursulin 2017-06-28 11:33 ` Chris Wilson 2017-06-28 15:30 ` [RFC v5 " Tvrtko Ursulin 2 siblings, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-06-28 11:27 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx; +Cc: Ben Widawsky, Daniel Vetter Quoting Tvrtko Ursulin (2017-06-26 16:47:41) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Engine discovery uAPI allows userspace to probe for engine > configuration and features without needing to maintain the > internal PCI id based database. > > This enables removal of code duplications across userspace > components. > > Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl > which returns the number and information on the specified engine > class. > > Currently only general engine configuration and HEVC feature of > the VCS engine can be probed but the uAPI is designed to be > generic and extensible. > > Code is based almost exactly on the earlier proposal on the > topic by Jon Bloomfield. Engine class and instance refactoring > made recently by Daniele Ceraolo Spurio enabled this to be > implemented in an elegant fashion. > > To probe configuration userspace sets the engine class it wants > to query (struct drm_i915_gem_engine_info) and provides an array > of drm_i915_engine_info structs which will be filled in by the > driver. Userspace also has to tell i915 how many elements are in > the array, and the driver will report back the total number of > engine instances in any case. > > v2: > * Add a version field and consolidate to one engine count. > (Chris Wilson) > * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO. > (Gong Zhipeng) > > v3: > * Drop the DRM_ prefix from uAPI enums. (Chris Wilson) > * Only reserve 8-bits for the engine info for time being. > > v4: > * Made user_class_map static. > > 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> > --- > drivers/gpu/drm/i915/i915_drv.c | 1 + > drivers/gpu/drm/i915/i915_drv.h | 3 ++ > drivers/gpu/drm/i915/intel_engine_cs.c | 64 ++++++++++++++++++++++++++++++++++ > include/uapi/drm/i915_drm.h | 41 ++++++++++++++++++++++ > 4 files changed, 109 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index e9d8e9ee51b2..44dd2f37937f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -2724,6 +2724,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), > }; > > static struct drm_driver driver = { > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 427d10c7eca5..496565e1753f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3598,6 +3598,9 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine, > struct i915_gem_context *ctx, > uint32_t *reg_state); > > +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file); > + > /* i915_gem_evict.c */ > int __must_check i915_gem_evict_something(struct i915_address_space *vm, > u64 min_size, u64 alignment, > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 3b46c1f7b88b..a98669f6ad85 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -25,6 +25,7 @@ > #include "i915_drv.h" > #include "intel_ringbuffer.h" > #include "intel_lrc.h" > +#include <uapi/drm/i915_drm.h> > > /* Haswell does have the CXT_SIZE register however it does not appear to be > * valid. Now, docs explain in dwords what is in the context object. The full > @@ -1332,6 +1333,69 @@ void intel_engines_mark_idle(struct drm_i915_private *i915) > } > } > > +static u8 user_class_map[I915_ENGINE_CLASS_MAX] = { > + [I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, > + [I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, > + [I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, > + [I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS, > + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, > +}; > + > +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file) > +{ > + struct drm_i915_private *i915 = to_i915(dev); > + struct drm_i915_gem_engine_info *args = data; > + struct drm_i915_engine_info __user *user_info = > + u64_to_user_ptr(args->info_ptr); > + unsigned int info_size = args->num_engines; > + struct drm_i915_engine_info info; > + struct intel_engine_cs *engine; > + enum intel_engine_id id; > + u8 class; > + > + if (args->rsvd) > + return -EINVAL; > + > + switch (args->engine_class) { > + case I915_ENGINE_CLASS_OTHER: > + case I915_ENGINE_CLASS_RENDER: > + case I915_ENGINE_CLASS_COPY: > + case I915_ENGINE_CLASS_VIDEO: > + case I915_ENGINE_CLASS_VIDEO_ENHANCE: > + class = user_class_map[args->engine_class]; > + break; > + case I915_ENGINE_CLASS_MAX: > + default: > + return -EINVAL; > + }; > + > + args->num_engines = 0; > + > + if (args->version != 1) { > + args->version = 1; > + return 0; My gut feeling says we want to report both the version and count in the first pass. Otherwise we have to do a version check, followed by a count query, followed by the actual retrieval. If we can combine the version + count check into one pass, it may be a little less hassle? -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v4 1/2] drm/i915: Engine discovery uAPI 2017-06-28 11:27 ` Chris Wilson @ 2017-06-28 13:15 ` Tvrtko Ursulin 2017-06-28 13:19 ` Tvrtko Ursulin 2017-06-28 13:21 ` Chris Wilson 0 siblings, 2 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-06-28 13:15 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx; +Cc: Daniel Vetter, Ben Widawsky On 28/06/2017 12:27, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2017-06-26 16:47:41) >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Engine discovery uAPI allows userspace to probe for engine >> configuration and features without needing to maintain the >> internal PCI id based database. >> >> This enables removal of code duplications across userspace >> components. >> >> Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl >> which returns the number and information on the specified engine >> class. >> >> Currently only general engine configuration and HEVC feature of >> the VCS engine can be probed but the uAPI is designed to be >> generic and extensible. >> >> Code is based almost exactly on the earlier proposal on the >> topic by Jon Bloomfield. Engine class and instance refactoring >> made recently by Daniele Ceraolo Spurio enabled this to be >> implemented in an elegant fashion. >> >> To probe configuration userspace sets the engine class it wants >> to query (struct drm_i915_gem_engine_info) and provides an array >> of drm_i915_engine_info structs which will be filled in by the >> driver. Userspace also has to tell i915 how many elements are in >> the array, and the driver will report back the total number of >> engine instances in any case. >> >> v2: >> * Add a version field and consolidate to one engine count. >> (Chris Wilson) >> * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO. >> (Gong Zhipeng) >> >> v3: >> * Drop the DRM_ prefix from uAPI enums. (Chris Wilson) >> * Only reserve 8-bits for the engine info for time being. >> >> v4: >> * Made user_class_map static. >> >> 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> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 1 + >> drivers/gpu/drm/i915/i915_drv.h | 3 ++ >> drivers/gpu/drm/i915/intel_engine_cs.c | 64 ++++++++++++++++++++++++++++++++++ >> include/uapi/drm/i915_drm.h | 41 ++++++++++++++++++++++ >> 4 files changed, 109 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >> index e9d8e9ee51b2..44dd2f37937f 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -2724,6 +2724,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { >> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), >> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), >> DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), >> + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), >> }; >> >> static struct drm_driver driver = { >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 427d10c7eca5..496565e1753f 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -3598,6 +3598,9 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine, >> struct i915_gem_context *ctx, >> uint32_t *reg_state); >> >> +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, >> + struct drm_file *file); >> + >> /* i915_gem_evict.c */ >> int __must_check i915_gem_evict_something(struct i915_address_space *vm, >> u64 min_size, u64 alignment, >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c >> index 3b46c1f7b88b..a98669f6ad85 100644 >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >> @@ -25,6 +25,7 @@ >> #include "i915_drv.h" >> #include "intel_ringbuffer.h" >> #include "intel_lrc.h" >> +#include <uapi/drm/i915_drm.h> >> >> /* Haswell does have the CXT_SIZE register however it does not appear to be >> * valid. Now, docs explain in dwords what is in the context object. The full >> @@ -1332,6 +1333,69 @@ void intel_engines_mark_idle(struct drm_i915_private *i915) >> } >> } >> >> +static u8 user_class_map[I915_ENGINE_CLASS_MAX] = { >> + [I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, >> + [I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, >> + [I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, >> + [I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS, >> + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, >> +}; >> + >> +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, >> + struct drm_file *file) >> +{ >> + struct drm_i915_private *i915 = to_i915(dev); >> + struct drm_i915_gem_engine_info *args = data; >> + struct drm_i915_engine_info __user *user_info = >> + u64_to_user_ptr(args->info_ptr); >> + unsigned int info_size = args->num_engines; >> + struct drm_i915_engine_info info; >> + struct intel_engine_cs *engine; >> + enum intel_engine_id id; >> + u8 class; >> + >> + if (args->rsvd) >> + return -EINVAL; >> + >> + switch (args->engine_class) { >> + case I915_ENGINE_CLASS_OTHER: >> + case I915_ENGINE_CLASS_RENDER: >> + case I915_ENGINE_CLASS_COPY: >> + case I915_ENGINE_CLASS_VIDEO: >> + case I915_ENGINE_CLASS_VIDEO_ENHANCE: >> + class = user_class_map[args->engine_class]; >> + break; >> + case I915_ENGINE_CLASS_MAX: >> + default: >> + return -EINVAL; >> + }; >> + >> + args->num_engines = 0; >> + >> + if (args->version != 1) { >> + args->version = 1; >> + return 0; > > My gut feeling says we want to report both the version and count in the > first pass. Otherwise we have to do a version check, followed by a count > query, followed by the actual retrieval. If we can combine the version + > count check into one pass, it may be a little less hassle? Could do, I don't see any downsides to it at the moment. But the only benefit seems it would be in cases where userspace would want to allocate the info array to the exact size, rather than just using a large enough number. In both cases userspace has to check both return code from the ioctl and that the version number matches the requested one. And in both cases the whole operation can be done with just one ioctl. Or if you want to allocate the exact size info array in both cases you need two ioctls. info.version = 1; info.class = <some class>; info.num_engines = <a large number should be enough for everyone>; ret = ioctl(&info); if (ret || info.version != 1) goto try_a_lower_version; for_each_engine(info) { ... } Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v4 1/2] drm/i915: Engine discovery uAPI 2017-06-28 13:15 ` Tvrtko Ursulin @ 2017-06-28 13:19 ` Tvrtko Ursulin 2017-06-28 13:21 ` Chris Wilson 1 sibling, 0 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-06-28 13:19 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx; +Cc: Daniel Vetter, Ben Widawsky On 28/06/2017 14:15, Tvrtko Ursulin wrote: > On 28/06/2017 12:27, Chris Wilson wrote: >> Quoting Tvrtko Ursulin (2017-06-26 16:47:41) >>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> >>> Engine discovery uAPI allows userspace to probe for engine >>> configuration and features without needing to maintain the >>> internal PCI id based database. >>> >>> This enables removal of code duplications across userspace >>> components. >>> >>> Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl >>> which returns the number and information on the specified engine >>> class. >>> >>> Currently only general engine configuration and HEVC feature of >>> the VCS engine can be probed but the uAPI is designed to be >>> generic and extensible. >>> >>> Code is based almost exactly on the earlier proposal on the >>> topic by Jon Bloomfield. Engine class and instance refactoring >>> made recently by Daniele Ceraolo Spurio enabled this to be >>> implemented in an elegant fashion. >>> >>> To probe configuration userspace sets the engine class it wants >>> to query (struct drm_i915_gem_engine_info) and provides an array >>> of drm_i915_engine_info structs which will be filled in by the >>> driver. Userspace also has to tell i915 how many elements are in >>> the array, and the driver will report back the total number of >>> engine instances in any case. >>> >>> v2: >>> * Add a version field and consolidate to one engine count. >>> (Chris Wilson) >>> * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO. >>> (Gong Zhipeng) >>> >>> v3: >>> * Drop the DRM_ prefix from uAPI enums. (Chris Wilson) >>> * Only reserve 8-bits for the engine info for time being. >>> >>> v4: >>> * Made user_class_map static. >>> >>> 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> >>> --- >>> drivers/gpu/drm/i915/i915_drv.c | 1 + >>> drivers/gpu/drm/i915/i915_drv.h | 3 ++ >>> drivers/gpu/drm/i915/intel_engine_cs.c | 64 >>> ++++++++++++++++++++++++++++++++++ >>> include/uapi/drm/i915_drm.h | 41 ++++++++++++++++++++++ >>> 4 files changed, 109 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.c >>> b/drivers/gpu/drm/i915/i915_drv.c >>> index e9d8e9ee51b2..44dd2f37937f 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.c >>> +++ b/drivers/gpu/drm/i915/i915_drv.c >>> @@ -2724,6 +2724,7 @@ static const struct drm_ioctl_desc >>> i915_ioctls[] = { >>> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, >>> i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), >>> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, >>> i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), >>> DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, >>> DRM_RENDER_ALLOW), >>> + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, >>> i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), >>> }; >>> static struct drm_driver driver = { >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>> b/drivers/gpu/drm/i915/i915_drv.h >>> index 427d10c7eca5..496565e1753f 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -3598,6 +3598,9 @@ void i915_oa_init_reg_state(struct >>> intel_engine_cs *engine, >>> struct i915_gem_context *ctx, >>> uint32_t *reg_state); >>> +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, >>> + struct drm_file *file); >>> + >>> /* i915_gem_evict.c */ >>> int __must_check i915_gem_evict_something(struct i915_address_space >>> *vm, >>> u64 min_size, u64 alignment, >>> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c >>> b/drivers/gpu/drm/i915/intel_engine_cs.c >>> index 3b46c1f7b88b..a98669f6ad85 100644 >>> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >>> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >>> @@ -25,6 +25,7 @@ >>> #include "i915_drv.h" >>> #include "intel_ringbuffer.h" >>> #include "intel_lrc.h" >>> +#include <uapi/drm/i915_drm.h> >>> /* Haswell does have the CXT_SIZE register however it does not >>> appear to be >>> * valid. Now, docs explain in dwords what is in the context >>> object. The full >>> @@ -1332,6 +1333,69 @@ void intel_engines_mark_idle(struct >>> drm_i915_private *i915) >>> } >>> } >>> +static u8 user_class_map[I915_ENGINE_CLASS_MAX] = { >>> + [I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, >>> + [I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, >>> + [I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, >>> + [I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS, >>> + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, >>> +}; >>> + >>> +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, >>> + struct drm_file *file) >>> +{ >>> + struct drm_i915_private *i915 = to_i915(dev); >>> + struct drm_i915_gem_engine_info *args = data; >>> + struct drm_i915_engine_info __user *user_info = >>> + u64_to_user_ptr(args->info_ptr); >>> + unsigned int info_size = args->num_engines; >>> + struct drm_i915_engine_info info; >>> + struct intel_engine_cs *engine; >>> + enum intel_engine_id id; >>> + u8 class; >>> + >>> + if (args->rsvd) >>> + return -EINVAL; >>> + >>> + switch (args->engine_class) { >>> + case I915_ENGINE_CLASS_OTHER: >>> + case I915_ENGINE_CLASS_RENDER: >>> + case I915_ENGINE_CLASS_COPY: >>> + case I915_ENGINE_CLASS_VIDEO: >>> + case I915_ENGINE_CLASS_VIDEO_ENHANCE: >>> + class = user_class_map[args->engine_class]; >>> + break; >>> + case I915_ENGINE_CLASS_MAX: >>> + default: >>> + return -EINVAL; >>> + }; >>> + >>> + args->num_engines = 0; >>> + >>> + if (args->version != 1) { >>> + args->version = 1; >>> + return 0; >> >> My gut feeling says we want to report both the version and count in the >> first pass. Otherwise we have to do a version check, followed by a count >> query, followed by the actual retrieval. If we can combine the version + >> count check into one pass, it may be a little less hassle? > > Could do, I don't see any downsides to it at the moment. > > But the only benefit seems it would be in cases where userspace would > want to allocate the info array to the exact size, rather than just > using a large enough number. Sorry, that was one outdated paragraph. From the one below it follows that the only benefit is offering a little bit more info on a version mismatch. But since we can do the whole query with one ioctl as below, I don't see that it would be useful. But could implement it just as well since as I said I don't see a downside to it. Regards, Tvrtko > In both cases userspace has to check both return code from the ioctl and > that the version number matches the requested one. And in both cases the > whole operation can be done with just one ioctl. Or if you want to > allocate the exact size info array in both cases you need two ioctls. > > info.version = 1; > info.class = <some class>; > info.num_engines = <a large number should be enough for everyone>; > ret = ioctl(&info); > if (ret || info.version != 1) > goto try_a_lower_version; > for_each_engine(info) { > ... > } > > Regards, > > Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v4 1/2] drm/i915: Engine discovery uAPI 2017-06-28 13:15 ` Tvrtko Ursulin 2017-06-28 13:19 ` Tvrtko Ursulin @ 2017-06-28 13:21 ` Chris Wilson 1 sibling, 0 replies; 48+ messages in thread From: Chris Wilson @ 2017-06-28 13:21 UTC (permalink / raw) To: Tvrtko Ursulin, Tvrtko Ursulin, Intel-gfx; +Cc: Daniel Vetter, Ben Widawsky Quoting Tvrtko Ursulin (2017-06-28 14:15:05) > > On 28/06/2017 12:27, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2017-06-26 16:47:41) > >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >> Engine discovery uAPI allows userspace to probe for engine > >> configuration and features without needing to maintain the > >> internal PCI id based database. > >> > >> This enables removal of code duplications across userspace > >> components. > >> > >> Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl > >> which returns the number and information on the specified engine > >> class. > >> > >> Currently only general engine configuration and HEVC feature of > >> the VCS engine can be probed but the uAPI is designed to be > >> generic and extensible. > >> > >> Code is based almost exactly on the earlier proposal on the > >> topic by Jon Bloomfield. Engine class and instance refactoring > >> made recently by Daniele Ceraolo Spurio enabled this to be > >> implemented in an elegant fashion. > >> > >> To probe configuration userspace sets the engine class it wants > >> to query (struct drm_i915_gem_engine_info) and provides an array > >> of drm_i915_engine_info structs which will be filled in by the > >> driver. Userspace also has to tell i915 how many elements are in > >> the array, and the driver will report back the total number of > >> engine instances in any case. > >> > >> v2: > >> * Add a version field and consolidate to one engine count. > >> (Chris Wilson) > >> * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO. > >> (Gong Zhipeng) > >> > >> v3: > >> * Drop the DRM_ prefix from uAPI enums. (Chris Wilson) > >> * Only reserve 8-bits for the engine info for time being. > >> > >> v4: > >> * Made user_class_map static. > >> > >> 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> > >> --- > >> drivers/gpu/drm/i915/i915_drv.c | 1 + > >> drivers/gpu/drm/i915/i915_drv.h | 3 ++ > >> drivers/gpu/drm/i915/intel_engine_cs.c | 64 ++++++++++++++++++++++++++++++++++ > >> include/uapi/drm/i915_drm.h | 41 ++++++++++++++++++++++ > >> 4 files changed, 109 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > >> index e9d8e9ee51b2..44dd2f37937f 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.c > >> +++ b/drivers/gpu/drm/i915/i915_drv.c > >> @@ -2724,6 +2724,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { > >> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), > >> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), > >> DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), > >> + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), > >> }; > >> > >> static struct drm_driver driver = { > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> index 427d10c7eca5..496565e1753f 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -3598,6 +3598,9 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine, > >> struct i915_gem_context *ctx, > >> uint32_t *reg_state); > >> > >> +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, > >> + struct drm_file *file); > >> + > >> /* i915_gem_evict.c */ > >> int __must_check i915_gem_evict_something(struct i915_address_space *vm, > >> u64 min_size, u64 alignment, > >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > >> index 3b46c1f7b88b..a98669f6ad85 100644 > >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c > >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > >> @@ -25,6 +25,7 @@ > >> #include "i915_drv.h" > >> #include "intel_ringbuffer.h" > >> #include "intel_lrc.h" > >> +#include <uapi/drm/i915_drm.h> > >> > >> /* Haswell does have the CXT_SIZE register however it does not appear to be > >> * valid. Now, docs explain in dwords what is in the context object. The full > >> @@ -1332,6 +1333,69 @@ void intel_engines_mark_idle(struct drm_i915_private *i915) > >> } > >> } > >> > >> +static u8 user_class_map[I915_ENGINE_CLASS_MAX] = { > >> + [I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, > >> + [I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, > >> + [I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, > >> + [I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS, > >> + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, > >> +}; > >> + > >> +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, > >> + struct drm_file *file) > >> +{ > >> + struct drm_i915_private *i915 = to_i915(dev); > >> + struct drm_i915_gem_engine_info *args = data; > >> + struct drm_i915_engine_info __user *user_info = > >> + u64_to_user_ptr(args->info_ptr); > >> + unsigned int info_size = args->num_engines; > >> + struct drm_i915_engine_info info; > >> + struct intel_engine_cs *engine; > >> + enum intel_engine_id id; > >> + u8 class; > >> + > >> + if (args->rsvd) > >> + return -EINVAL; > >> + > >> + switch (args->engine_class) { > >> + case I915_ENGINE_CLASS_OTHER: > >> + case I915_ENGINE_CLASS_RENDER: > >> + case I915_ENGINE_CLASS_COPY: > >> + case I915_ENGINE_CLASS_VIDEO: > >> + case I915_ENGINE_CLASS_VIDEO_ENHANCE: > >> + class = user_class_map[args->engine_class]; > >> + break; > >> + case I915_ENGINE_CLASS_MAX: > >> + default: > >> + return -EINVAL; > >> + }; > >> + > >> + args->num_engines = 0; > >> + > >> + if (args->version != 1) { > >> + args->version = 1; > >> + return 0; > > > > My gut feeling says we want to report both the version and count in the > > first pass. Otherwise we have to do a version check, followed by a count > > query, followed by the actual retrieval. If we can combine the version + > > count check into one pass, it may be a little less hassle? > > Could do, I don't see any downsides to it at the moment. > > But the only benefit seems it would be in cases where userspace would > want to allocate the info array to the exact size, rather than just > using a large enough number. > > In both cases userspace has to check both return code from the ioctl and > that the version number matches the requested one. And in both cases the > whole operation can be done with just one ioctl. Or if you want to > allocate the exact size info array in both cases you need two ioctls. > > info.version = 1; > info.class = <some class>; > info.num_engines = <a large number should be enough for everyone>; > ret = ioctl(&info); > if (ret || info.version != 1) > goto try_a_lower_version; > for_each_engine(info) { > ... > } Even if you do preallocate, you need to be prepared for the kernel to report something is off. I agree that my recommended approach would be to go with a static allocation that is good enough and fallback to the multi-pass query with dynamic allocation. If you design with that in mind, we get the required behaviour correct and then include the optimistic preallocation as an optimisation. But spell out how userspace is meant to work, and remember the forward-compatibility problem :) -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v4 1/2] drm/i915: Engine discovery uAPI 2017-06-26 15:47 ` [RFC v4 " Tvrtko Ursulin 2017-06-28 11:27 ` Chris Wilson @ 2017-06-28 11:33 ` Chris Wilson 2017-06-28 15:30 ` [RFC v5 " Tvrtko Ursulin 2 siblings, 0 replies; 48+ messages in thread From: Chris Wilson @ 2017-06-28 11:33 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx; +Cc: Ben Widawsky, Daniel Vetter Quoting Tvrtko Ursulin (2017-06-26 16:47:41) > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Engine discovery uAPI allows userspace to probe for engine > configuration and features without needing to maintain the > internal PCI id based database. > > This enables removal of code duplications across userspace > components. > > Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl > which returns the number and information on the specified engine > class. > > Currently only general engine configuration and HEVC feature of > the VCS engine can be probed but the uAPI is designed to be > generic and extensible. > > Code is based almost exactly on the earlier proposal on the > topic by Jon Bloomfield. Engine class and instance refactoring > made recently by Daniele Ceraolo Spurio enabled this to be > implemented in an elegant fashion. > > To probe configuration userspace sets the engine class it wants > to query (struct drm_i915_gem_engine_info) and provides an array > of drm_i915_engine_info structs which will be filled in by the > driver. Userspace also has to tell i915 how many elements are in > the array, and the driver will report back the total number of > engine instances in any case. Include psuedo code to first show how to do the multipass query followed by an example hooking it to execbuf. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* [RFC v5 1/2] drm/i915: Engine discovery uAPI 2017-06-26 15:47 ` [RFC v4 " Tvrtko Ursulin 2017-06-28 11:27 ` Chris Wilson 2017-06-28 11:33 ` Chris Wilson @ 2017-06-28 15:30 ` Tvrtko Ursulin 2 siblings, 0 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-06-28 15:30 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Engine discovery uAPI allows userspace to probe for engine configuration and features without needing to maintain the internal PCI id based database. This enables removal of code duplications across userspace components. Probing is done via the new DRM_IOCTL_I915_GEM_ENGINE_INFO ioctl which returns the number and information on the specified engine class. Currently only general engine configuration and HEVC feature of the VCS engine can be probed but the uAPI is designed to be generic and extensible. Code is based almost exactly on the earlier proposal on the topic by Jon Bloomfield. Engine class and instance refactoring made recently by Daniele Ceraolo Spurio enabled this to be implemented in an elegant fashion. To probe configuration userspace sets the engine class it wants to query (struct drm_i915_gem_engine_info) and provides an array of drm_i915_engine_info structs which will be filled in by the driver. Userspace also has to tell i915 how many elements are in the array, and the driver will report back the total number of engine instances in any case. Pseudo-code example of userspace using the uAPI: struct drm_i915_gem_engine_info info = {}; struct drm_i915_engine_info engines[8]; int i, ret; info.version = 1; info.engine_class = I915_ENGINE_CLASS_VIDEO; info.num_engines = ARRAY_SIZE(engines); info.info_ptr = to_user_pointer(&engines[0]); ret = ioctl(..., &info); assert(ret == 0); if (info.version != 1) goto kernel_does_not_support_a_version_this_new; for (i = 0; i < info.num_engines; i++) printf("VCS%u: flags=%x\n", engines[i].instance, engines[i].info); If the userspace wants to allocate the exact number of drm_i915_engine_info structures instead of pesimistically over-allocating, it can submit the ioctl two times. First time with info.num_engines set to zero, so the kernel reports back the total number of available engines, and the second time with the field set to the number of allocated array elements. v2: * Add a version field and consolidate to one engine count. (Chris Wilson) * Rename uAPI flags for VCS engines to DRM_I915_ENGINE_CLASS_VIDEO. (Gong Zhipeng) v3: * Drop the DRM_ prefix from uAPI enums. (Chris Wilson) * Only reserve 8-bits for the engine info for time being. v4: * Made user_class_map static. v5: * Example usage added to commit msg. (Chris Wilson) * Report engine count in case of version mismatch. (Chris Wilson) 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> --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/intel_engine_cs.c | 64 ++++++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 41 ++++++++++++++++++++++ 4 files changed, 109 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index e9d8e9ee51b2..44dd2f37937f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2724,6 +2724,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_ENGINE_INFO, i915_gem_engine_info_ioctl, DRM_RENDER_ALLOW), }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 427d10c7eca5..496565e1753f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3598,6 +3598,9 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine, struct i915_gem_context *ctx, uint32_t *reg_state); +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); + /* i915_gem_evict.c */ int __must_check i915_gem_evict_something(struct i915_address_space *vm, u64 min_size, u64 alignment, diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 3b46c1f7b88b..d88198933262 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -25,6 +25,7 @@ #include "i915_drv.h" #include "intel_ringbuffer.h" #include "intel_lrc.h" +#include <uapi/drm/i915_drm.h> /* Haswell does have the CXT_SIZE register however it does not appear to be * valid. Now, docs explain in dwords what is in the context object. The full @@ -1332,6 +1333,69 @@ void intel_engines_mark_idle(struct drm_i915_private *i915) } } +static u8 user_class_map[I915_ENGINE_CLASS_MAX] = { + [I915_ENGINE_CLASS_OTHER] = OTHER_CLASS, + [I915_ENGINE_CLASS_RENDER] = RENDER_CLASS, + [I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS, + [I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS, + [I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS, +}; + +int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, + struct drm_file *file) +{ + struct drm_i915_private *i915 = to_i915(dev); + struct drm_i915_gem_engine_info *args = data; + struct drm_i915_engine_info __user *user_info = + u64_to_user_ptr(args->info_ptr); + unsigned int info_size = args->num_engines; + struct drm_i915_engine_info info; + struct intel_engine_cs *engine; + enum intel_engine_id id; + u8 class; + + if (args->rsvd) + return -EINVAL; + + switch (args->engine_class) { + case I915_ENGINE_CLASS_OTHER: + case I915_ENGINE_CLASS_RENDER: + case I915_ENGINE_CLASS_COPY: + case I915_ENGINE_CLASS_VIDEO: + case I915_ENGINE_CLASS_VIDEO_ENHANCE: + class = user_class_map[args->engine_class]; + break; + case I915_ENGINE_CLASS_MAX: + default: + return -EINVAL; + }; + + args->num_engines = 0; + + for_each_engine(engine, i915, id) { + if (class != engine->class) + continue; + + if (++args->num_engines > info_size) + continue; + + if (args->version != 1) + continue; + + memset(&info, 0, sizeof(info)); + info.instance = engine->instance; + if (INTEL_GEN(i915) >= 8 && id == VCS) + info.info = I915_VCS_HAS_HEVC; + + if (copy_to_user(user_info++, &info, sizeof(info))) + return -EFAULT; + } + + args->version = 1; + + return 0; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_engine.c" #endif diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 7ccbd6a2bbe0..fbb1f6b99959 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -260,6 +260,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_CONTEXT_GETPARAM 0x34 #define DRM_I915_GEM_CONTEXT_SETPARAM 0x35 #define DRM_I915_PERF_OPEN 0x36 +#define DRM_I915_GEM_ENGINE_INFO 0x37 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -315,6 +316,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct drm_i915_gem_context_param) #define DRM_IOCTL_I915_PERF_OPEN DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param) +#define DRM_IOCTL_I915_GEM_ENGINE_INFO DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_ENGINE_INFO, struct drm_i915_gem_engine_info) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -1467,6 +1469,45 @@ enum drm_i915_perf_record_type { DRM_I915_PERF_RECORD_MAX /* non-ABI */ }; +enum drm_i915_gem_engine_class { + I915_ENGINE_CLASS_OTHER = 0, + I915_ENGINE_CLASS_RENDER = 1, + I915_ENGINE_CLASS_COPY = 2, + I915_ENGINE_CLASS_VIDEO = 3, + I915_ENGINE_CLASS_VIDEO_ENHANCE = 4, + I915_ENGINE_CLASS_MAX /* non-ABI */ +}; + +struct drm_i915_engine_info { + /** Engine instance number. */ + __u8 instance; + + /** Engine specific info. */ +#define I915_VCS_HAS_HEVC BIT(0) + __u8 info; + + __u8 rsvd2[6]; +}; + +struct drm_i915_gem_engine_info { + /** in/out: Protocol version requested/supported. */ + __u32 version; + + /** in: Engine class to probe (enum drm_i915_gem_engine_class). */ + __u32 engine_class; + + /** + * in/out: Number of struct drm_i915_engine_info entries in the provided + * @info_ptr array and actual number of supported hardware engines. + */ + __u32 num_engines; + __u32 rsvd; + + /** in/out: Pointer to array of struct i915_engine_info elements. */ + __u64 info_ptr; + +}; + #if defined(__cplusplus) } #endif -- 2.9.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-04-18 16:56 [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI Tvrtko Ursulin 2017-04-18 16:56 ` [RFC 1/2] drm/i915: Engine discovery uAPI Tvrtko Ursulin @ 2017-04-18 16:56 ` Tvrtko Ursulin 2017-04-18 21:10 ` Chris Wilson 2017-04-27 9:10 ` [RFC v2 " Tvrtko Ursulin 2017-04-18 17:12 ` ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI Patchwork ` (4 subsequent siblings) 6 siblings, 2 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-18 16:56 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, intel-vaapi-media, mesa-dev, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@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. 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); 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: Daniel Charles <daniel.charles@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 Cc: mesa-dev@lists.freedesktop.org --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 36 ++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 7 +++++- 2 files changed, 42 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..7fc92ec839a1 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1153,6 +1153,10 @@ i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec) if (exec->flags & __I915_EXEC_UNKNOWN_FLAGS) return false; + if ((exec->flags & I915_EXEC_CLASS_INSTANCE) && + (exec->flags & I915_EXEC_RING_MASK)) + return false; + /* Kernel clipping was a DRI1 misfeature */ if (exec->num_cliprects || exec->cliprects_ptr) return false; @@ -1492,6 +1496,35 @@ 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; + u16 class_instance; + u8 user_class, class, instance; + + class_instance = (args->flags & I915_EXEC_CLASS_INSTANCE_MASK) >> + I915_EXEC_CLASS_INSTANCE_SHIFT; + + user_class = class_instance >> 8; + instance = class_instance & 0xff; + + if (user_class >= DRM_I915_ENGINE_CLASS_MAX) + return NULL; + class = user_class_map[user_class]; + + for_each_engine(engine, i915, id) { + if (engine->class == class && engine->instance == instance) + return engine; + } + + return NULL; +} + #define I915_USER_RINGS (4) static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { @@ -1510,6 +1543,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; struct intel_engine_cs *engine; + if (args->flags & I915_EXEC_CLASS_INSTANCE) + return eb_select_engine_class_instance(dev_priv, args); + if (user_ring_id > I915_USER_RINGS) { DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); return NULL; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 6058596a9f33..727a6dc4b029 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -906,7 +906,12 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_FENCE_OUT (1<<17) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1)) +#define I915_EXEC_CLASS_INSTANCE (1<<18) + +#define I915_EXEC_CLASS_INSTANCE_SHIFT (19) +#define I915_EXEC_CLASS_INSTANCE_MASK (0xffff << I915_EXEC_CLASS_INSTANCE_SHIFT) + +#define __I915_EXEC_UNKNOWN_FLAGS (-(35 << 1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-04-18 16:56 ` [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2 Tvrtko Ursulin @ 2017-04-18 21:10 ` Chris Wilson 2017-04-24 8:36 ` Tvrtko Ursulin 2017-04-27 9:10 ` [RFC v2 " Tvrtko Ursulin 1 sibling, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-04-18 21:10 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media On Tue, Apr 18, 2017 at 05:56:15PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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. > > 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); To save a bit of space, we can use the ring selector as a class selector if bit18 is set, with 19-27 as instance. That limits us to 64 classes - hopefully not a problem for near future. At least I might have you sold you on a flexible execbuf3 by then. (As a digression, some cryptic notes for an implementation I did over Easter: /* * Execbuf3! * * ringbuffer * - per context * - per engine * - PAGE_SIZE ctl [ro head, rw tai] + user pot * - kthread [i915/$ctx-$engine] (optional?) * - assumes NO_RELOC-esque awareness * * SYNC flags [wait/signal], handle [semaphore/fence] * * BIND handle, offset [user provided] * ALLOC[32,64] handle, flags, *offset [kernel provided, need RELOC] * RELOC[32,64] handle, target_handle, offset, delta * CLEAR flags, handle * UNBIND handle * * BATCH flags, handle, offset * [or SVM flags, address] * PIN flags (MAY_RELOC), count, handle[count] * FENCE flags, count, handle[count] * SUBMIT handle [fence/NULL with error] */ At the moment it is just trying to do execbuf2, but more compactly and with fewer ioctls. But one of the main selling points is that we can extend the information passed around more freely than execbuf2.) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-04-18 21:10 ` Chris Wilson @ 2017-04-24 8:36 ` Tvrtko Ursulin 0 siblings, 0 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-24 8:36 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Joonas Lahtinen, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev On 18/04/2017 22:10, Chris Wilson wrote: > On Tue, Apr 18, 2017 at 05:56:15PM +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@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. >> >> 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); > > To save a bit of space, we can use the ring selector as a class selector > if bit18 is set, with 19-27 as instance. That limits us to 64 classes - > hopefully not a problem for near future. At least I might have you sold > you on a flexible execbuf3 by then. I was considering re-using those bits yes. I was thinking about the pro of keeping it completely separate but I suppose there is not much value in that. So I can re-use the ring selector just as well and have a smaller impact on number of bits left over. > (As a digression, some cryptic notes for an implementation I did over Easter: > /* > * Execbuf3! > * > * ringbuffer > * - per context > * - per engine We have this already so I am missing something I guess. > * - PAGE_SIZE ctl [ro head, rw tai] + user pot > * - kthread [i915/$ctx-$engine] (optional?) No idea what these two are. :) > * - assumes NO_RELOC-esque awareness Ok ok NO_RELOC. :) > * > * SYNC flags [wait/signal], handle [semaphore/fence] Sync fence in out just as today, but probably more? > * > * BIND handle, offset [user provided] > * ALLOC[32,64] handle, flags, *offset [kernel provided, need RELOC] > * RELOC[32,64] handle, target_handle, offset, delta > * CLEAR flags, handle > * UNBIND handle Explicit VMA management? Separate ioctl maybe would be better? > * > * BATCH flags, handle, offset > * [or SVM flags, address] > * PIN flags (MAY_RELOC), count, handle[count] > * FENCE flags, count, handle[count] > * SUBMIT handle [fence/NULL with error] > */ No idea again. :) > At the moment it is just trying to do execbuf2, but more compactly and > with fewer ioctls. But one of the main selling points is that we can > extend the information passed around more freely than execbuf2.) I have nothing against a better eb since I trust you know much better it is needed and when. But I don't know how long it will take to get there. This class/instance idea could be implemented quickly to solve the sore point of VCS/VCS2 engine selection. But yeah, it is another uABI to keep in that case. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* [RFC v2 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-04-18 16:56 ` [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2 Tvrtko Ursulin 2017-04-18 21:10 ` Chris Wilson @ 2017-04-27 9:10 ` Tvrtko Ursulin 2017-04-27 9:25 ` Chris Wilson 1 sibling, 1 reply; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-27 9:10 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, intel-vaapi-media, mesa-dev, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@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. 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@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: Daniel Charles <daniel.charles@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 Cc: mesa-dev@lists.freedesktop.org --- 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; + } + + return NULL; +} + #define I915_USER_RINGS (4) static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { @@ -1510,6 +1536,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; struct intel_engine_cs *engine; + if (args->flags & I915_EXEC_CLASS_INSTANCE) + return eb_select_engine_class_instance(dev_priv, args); + if (user_ring_id > I915_USER_RINGS) { DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); return NULL; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 2ac6667e57ea..6a26bdf5e684 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -906,7 +906,12 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_FENCE_OUT (1<<17) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1)) +#define I915_EXEC_CLASS_INSTANCE (1<<18) + +#define I915_EXEC_INSTANCE_SHIFT (19) +#define I915_EXEC_INSTANCE_MASK (0xff << I915_EXEC_INSTANCE_SHIFT) + +#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 27) << 1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ @@ -914,6 +919,10 @@ struct drm_i915_gem_execbuffer2 { #define i915_execbuffer2_get_context_id(eb2) \ ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) +#define i915_execbuffer2_engine(class, instance) \ + (I915_EXEC_CLASS_INSTANCE | (class) | \ + ((instance) << I915_EXEC_INSTANCE_SHIFT)) + struct drm_i915_gem_pin { /** Handle of the buffer to be pinned. */ __u32 handle; -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [RFC v2 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-04-27 9:10 ` [RFC v2 " Tvrtko Ursulin @ 2017-04-27 9:25 ` Chris Wilson 2017-04-27 10:09 ` Tvrtko Ursulin 2017-05-17 15:40 ` [RFC v3] " Tvrtko Ursulin 0 siblings, 2 replies; 48+ messages in thread From: Chris Wilson @ 2017-04-27 9:25 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media On Thu, Apr 27, 2017 at 10:10:34AM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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. > > 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@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: Daniel Charles <daniel.charles@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 > Cc: mesa-dev@lists.freedesktop.org > --- > 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] ? Still, at what point do we kill busy-ioctl per-engine reporting? Should we update all tracepoints to use class:instance (I think that's a better abi going forward). -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v2 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-04-27 9:25 ` Chris Wilson @ 2017-04-27 10:09 ` Tvrtko Ursulin 2017-04-27 10:26 ` Chris Wilson 2017-05-17 15:40 ` [RFC v3] " Tvrtko Ursulin 1 sibling, 1 reply; 48+ messages in thread From: Tvrtko Ursulin @ 2017-04-27 10:09 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Joonas Lahtinen, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev 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@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. >> >> 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@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: Daniel Charles <daniel.charles@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 >> Cc: mesa-dev@lists.freedesktop.org >> --- >> 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v2 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-04-27 10:09 ` Tvrtko Ursulin @ 2017-04-27 10:26 ` Chris Wilson 0 siblings, 0 replies; 48+ messages in thread From: Chris Wilson @ 2017-04-27 10:26 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media On Thu, Apr 27, 2017 at 11:09:35AM +0100, Tvrtko Ursulin wrote: > > 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@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. > >> > >>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@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: Daniel Charles <daniel.charles@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 > >>Cc: mesa-dev@lists.freedesktop.org > >>--- > >> 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? Userspace does try to distinguish between RCS and !RCS. But it's a rough heuristic that I'm not going to cry much over since it depends upon on so many other factors outside of its control as to which placement is better. > If so you recommend we make it a real boolean? Once we cross the u16 threshold, yup. Just then a busy read/write pair. > >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? Works for me. There are still a few other places where we want an index into an array, so keeping a map to engine->uabi_id seems sensible. Or we always include engine.instance as part of our uABI for extensible structs. E.g. struct context_watchdog_param { u32 engine; u32 instance; u64 watchdog_ns; }; Then set/get_context_param return an array of those rather than an array of watchdog_ns. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-04-27 9:25 ` Chris Wilson 2017-04-27 10:09 ` Tvrtko Ursulin @ 2017-05-17 15:40 ` Tvrtko Ursulin 2017-05-17 16:44 ` Chris Wilson ` (2 more replies) 1 sibling, 3 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-17 15:40 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, intel-vaapi-media, mesa-dev, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@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. 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) v3: * Add a map for fast class-instance engine lookup. (Chris Wilson) 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: Daniel Charles <daniel.charles@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 Cc: mesa-dev@lists.freedesktop.org --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 30 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_engine_cs.c | 7 +++++++ include/uapi/drm/i915_drm.h | 11 ++++++++++- 5 files changed, 51 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5dfa4a12e647..7bf4fd42480c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2066,6 +2066,7 @@ struct drm_i915_private { struct pci_dev *bridge_dev; struct i915_gem_context *kernel_context; struct intel_engine_cs *engine[I915_NUM_ENGINES]; + struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1][MAX_ENGINE_INSTANCE + 1]; struct i915_vma *semaphore; struct drm_dma_handle *status_page_dmah; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index af1965774e7b..c1ad49ab64cd 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1492,6 +1492,33 @@ 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 *get_engine_class(struct drm_i915_private *i915, + u8 class, u8 instance) +{ + if (class > MAX_ENGINE_CLASS || instance > MAX_ENGINE_INSTANCE) + return NULL; + + return i915->engine_class[class][instance]; +} + +static struct intel_engine_cs * +eb_select_engine_class_instance(struct drm_i915_private *i915, + struct drm_i915_gem_execbuffer2 *args) +{ + u8 class, instance; + + class = args->flags & I915_EXEC_RING_MASK; + if (class >= DRM_I915_ENGINE_CLASS_MAX) + return NULL; + + instance = (args->flags >> I915_EXEC_INSTANCE_SHIFT) && + I915_EXEC_INSTANCE_MASK; + + return get_engine_class(i915, user_class_map[class], instance); +} + #define I915_USER_RINGS (4) static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { @@ -1510,6 +1537,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; struct intel_engine_cs *engine; + if (args->flags & I915_EXEC_CLASS_INSTANCE) + return eb_select_engine_class_instance(dev_priv, args); + if (user_ring_id > I915_USER_RINGS) { DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); return NULL; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ee144ec57935..a3b59043b991 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -92,6 +92,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define VIDEO_ENHANCEMENT_CLASS 2 #define COPY_ENGINE_CLASS 3 #define OTHER_CLASS 4 +#define MAX_ENGINE_CLASS 4 + +#define MAX_ENGINE_INSTANCE 1 /* PCI config space */ diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 7566cf48012f..c5ad51c43d23 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -225,7 +225,14 @@ intel_engine_setup(struct drm_i915_private *dev_priv, ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); + if (WARN_ON(info->class > MAX_ENGINE_CLASS || + info->instance > MAX_ENGINE_INSTANCE || + dev_priv->engine_class[info->class][info->instance])) + return -EINVAL; + + dev_priv->engine_class[info->class][info->instance] = engine; dev_priv->engine[id] = engine; + return 0; } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 2ac6667e57ea..6a26bdf5e684 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -906,7 +906,12 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_FENCE_OUT (1<<17) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1)) +#define I915_EXEC_CLASS_INSTANCE (1<<18) + +#define I915_EXEC_INSTANCE_SHIFT (19) +#define I915_EXEC_INSTANCE_MASK (0xff << I915_EXEC_INSTANCE_SHIFT) + +#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 27) << 1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ @@ -914,6 +919,10 @@ struct drm_i915_gem_execbuffer2 { #define i915_execbuffer2_get_context_id(eb2) \ ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) +#define i915_execbuffer2_engine(class, instance) \ + (I915_EXEC_CLASS_INSTANCE | (class) | \ + ((instance) << I915_EXEC_INSTANCE_SHIFT)) + struct drm_i915_gem_pin { /** Handle of the buffer to be pinned. */ __u32 handle; -- 2.9.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-17 15:40 ` [RFC v3] " Tvrtko Ursulin @ 2017-05-17 16:44 ` Chris Wilson 2017-05-18 13:30 ` Tvrtko Ursulin 2017-05-18 10:55 ` Joonas Lahtinen 2017-05-18 14:58 ` [RFC v4 2/2] " Tvrtko Ursulin 2 siblings, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-05-17 16:44 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media On Wed, May 17, 2017 at 04:40:57PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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. > > 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) > > v3: > * Add a map for fast class-instance engine lookup. (Chris Wilson) > > 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: Daniel Charles <daniel.charles@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 > Cc: mesa-dev@lists.freedesktop.org > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 30 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_engine_cs.c | 7 +++++++ > include/uapi/drm/i915_drm.h | 11 ++++++++++- > 5 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 5dfa4a12e647..7bf4fd42480c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2066,6 +2066,7 @@ struct drm_i915_private { > struct pci_dev *bridge_dev; > struct i915_gem_context *kernel_context; > struct intel_engine_cs *engine[I915_NUM_ENGINES]; > + struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1][MAX_ENGINE_INSTANCE + 1]; > struct i915_vma *semaphore; > > struct drm_dma_handle *status_page_dmah; > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index af1965774e7b..c1ad49ab64cd 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1492,6 +1492,33 @@ 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 *get_engine_class(struct drm_i915_private *i915, > + u8 class, u8 instance) > +{ > + if (class > MAX_ENGINE_CLASS || instance > MAX_ENGINE_INSTANCE) > + return NULL; > + > + return i915->engine_class[class][instance]; > +} Be bold make this this an intel_engine_lookup(), I forsee some other users appearing very shortly. > +static struct intel_engine_cs * > +eb_select_engine_class_instance(struct drm_i915_private *i915, > + struct drm_i915_gem_execbuffer2 *args) > +{ > + u8 class, instance; > + > + class = args->flags & I915_EXEC_RING_MASK; > + if (class >= DRM_I915_ENGINE_CLASS_MAX) > + return NULL; > + > + instance = (args->flags >> I915_EXEC_INSTANCE_SHIFT) && > + I915_EXEC_INSTANCE_MASK; > + > + return get_engine_class(i915, user_class_map[class], instance); > +} > + > #define I915_USER_RINGS (4) > > static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { > @@ -1510,6 +1537,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, > unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; > struct intel_engine_cs *engine; > > + if (args->flags & I915_EXEC_CLASS_INSTANCE) > + return eb_select_engine_class_instance(dev_priv, args); > + > if (user_ring_id > I915_USER_RINGS) { > DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); > return NULL; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index ee144ec57935..a3b59043b991 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -92,6 +92,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > #define VIDEO_ENHANCEMENT_CLASS 2 > #define COPY_ENGINE_CLASS 3 > #define OTHER_CLASS 4 > +#define MAX_ENGINE_CLASS 4 > + > +#define MAX_ENGINE_INSTANCE 1 We also need the names in the uapi.h as well. Do we want to mention OTHER_CLASS before it is defined? I trust your crystal ball. Amusingly I notice that you do claim that you have these names in the changelog. :) We don't need DRM_I915 all the time. I915_ENGINE_CLASS is going to be unique, at least for those users of i915_drm.h > /* PCI config space */ > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 7566cf48012f..c5ad51c43d23 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -225,7 +225,14 @@ intel_engine_setup(struct drm_i915_private *dev_priv, > > ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); > > + if (WARN_ON(info->class > MAX_ENGINE_CLASS || > + info->instance > MAX_ENGINE_INSTANCE || > + dev_priv->engine_class[info->class][info->instance])) Spread these out or add a message telling what was wrong. > + return -EINVAL; > + > + dev_priv->engine_class[info->class][info->instance] = engine; > dev_priv->engine[id] = engine; > + > return 0; > } > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 2ac6667e57ea..6a26bdf5e684 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -906,7 +906,12 @@ struct drm_i915_gem_execbuffer2 { > */ > #define I915_EXEC_FENCE_OUT (1<<17) > > -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1)) > +#define I915_EXEC_CLASS_INSTANCE (1<<18) > + > +#define I915_EXEC_INSTANCE_SHIFT (19) > +#define I915_EXEC_INSTANCE_MASK (0xff << I915_EXEC_INSTANCE_SHIFT) > + > +#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 27) << 1)) > > #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) > #define i915_execbuffer2_set_context_id(eb2, context) \ > @@ -914,6 +919,10 @@ struct drm_i915_gem_execbuffer2 { > #define i915_execbuffer2_get_context_id(eb2) \ > ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) > > +#define i915_execbuffer2_engine(class, instance) \ > + (I915_EXEC_CLASS_INSTANCE | (class) | \ > + ((instance) << I915_EXEC_INSTANCE_SHIFT)) (class | (instance) << I915_EXEC_INSTANCE_SHIFT | I915_EXEC_CLASS_INSTANCE) Just suggesting to spread it out over another line for better readability. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-17 16:44 ` Chris Wilson @ 2017-05-18 13:30 ` Tvrtko Ursulin 2017-05-18 13:50 ` [Intel-gfx] " Chris Wilson 0 siblings, 1 reply; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-18 13:30 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Joonas Lahtinen, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev On 17/05/2017 17:44, Chris Wilson wrote: > On Wed, May 17, 2017 at 04:40:57PM +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@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. >> >> 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) >> >> v3: >> * Add a map for fast class-instance engine lookup. (Chris Wilson) >> >> 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: Daniel Charles <daniel.charles@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 >> Cc: mesa-dev@lists.freedesktop.org >> --- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 30 ++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/i915_reg.h | 3 +++ >> drivers/gpu/drm/i915/intel_engine_cs.c | 7 +++++++ >> include/uapi/drm/i915_drm.h | 11 ++++++++++- >> 5 files changed, 51 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 5dfa4a12e647..7bf4fd42480c 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2066,6 +2066,7 @@ struct drm_i915_private { >> struct pci_dev *bridge_dev; >> struct i915_gem_context *kernel_context; >> struct intel_engine_cs *engine[I915_NUM_ENGINES]; >> + struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1][MAX_ENGINE_INSTANCE + 1]; >> struct i915_vma *semaphore; >> >> struct drm_dma_handle *status_page_dmah; >> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> index af1965774e7b..c1ad49ab64cd 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> @@ -1492,6 +1492,33 @@ 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 *get_engine_class(struct drm_i915_private *i915, >> + u8 class, u8 instance) >> +{ >> + if (class > MAX_ENGINE_CLASS || instance > MAX_ENGINE_INSTANCE) >> + return NULL; >> + >> + return i915->engine_class[class][instance]; >> +} > > Be bold make this this an intel_engine_lookup(), I forsee some other > users appearing very shortly. Still static or you want to export it straight away? Preference for where to put it? Here or intel_engine_cs.c? >> +static struct intel_engine_cs * >> +eb_select_engine_class_instance(struct drm_i915_private *i915, >> + struct drm_i915_gem_execbuffer2 *args) >> +{ >> + u8 class, instance; >> + >> + class = args->flags & I915_EXEC_RING_MASK; >> + if (class >= DRM_I915_ENGINE_CLASS_MAX) >> + return NULL; >> + >> + instance = (args->flags >> I915_EXEC_INSTANCE_SHIFT) && >> + I915_EXEC_INSTANCE_MASK; >> + >> + return get_engine_class(i915, user_class_map[class], instance); >> +} >> + >> #define I915_USER_RINGS (4) >> >> static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { >> @@ -1510,6 +1537,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, >> unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; >> struct intel_engine_cs *engine; >> >> + if (args->flags & I915_EXEC_CLASS_INSTANCE) >> + return eb_select_engine_class_instance(dev_priv, args); >> + >> if (user_ring_id > I915_USER_RINGS) { >> DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); >> return NULL; >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index ee144ec57935..a3b59043b991 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -92,6 +92,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) >> #define VIDEO_ENHANCEMENT_CLASS 2 >> #define COPY_ENGINE_CLASS 3 >> #define OTHER_CLASS 4 >> +#define MAX_ENGINE_CLASS 4 >> + >> +#define MAX_ENGINE_INSTANCE 1 > > We also need the names in the uapi.h as well. Do we want to mention > OTHER_CLASS before it is defined? I trust your crystal ball. I have mentioned it just by the virtue of it existing in i915_reg.h Crystal ball is otherwise quiet. > Amusingly I notice that you do claim that you have these names in the > changelog. :) We don't need DRM_I915 all the time. I915_ENGINE_CLASS is > going to be unique, at least for those users of i915_drm.h They are all there courtesy of the previous patch. I will drop the DRM_ prefix throughout. > >> /* PCI config space */ >> >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c >> index 7566cf48012f..c5ad51c43d23 100644 >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >> @@ -225,7 +225,14 @@ intel_engine_setup(struct drm_i915_private *dev_priv, >> >> ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); >> >> + if (WARN_ON(info->class > MAX_ENGINE_CLASS || >> + info->instance > MAX_ENGINE_INSTANCE || >> + dev_priv->engine_class[info->class][info->instance])) > > Spread these out or add a message telling what was wrong. Can do, but I considered these to be such a basic programming errors which should be caught during development. Only thing which prevented me from putting in under the GEM_BUG_ON is the fact someone could not have it enabled and that this function can already handle failures. >> + return -EINVAL; >> + >> + dev_priv->engine_class[info->class][info->instance] = engine; >> dev_priv->engine[id] = engine; >> + >> return 0; >> } >> >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >> index 2ac6667e57ea..6a26bdf5e684 100644 >> --- a/include/uapi/drm/i915_drm.h >> +++ b/include/uapi/drm/i915_drm.h >> @@ -906,7 +906,12 @@ struct drm_i915_gem_execbuffer2 { >> */ >> #define I915_EXEC_FENCE_OUT (1<<17) >> >> -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1)) >> +#define I915_EXEC_CLASS_INSTANCE (1<<18) >> + >> +#define I915_EXEC_INSTANCE_SHIFT (19) >> +#define I915_EXEC_INSTANCE_MASK (0xff << I915_EXEC_INSTANCE_SHIFT) >> + >> +#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 27) << 1)) >> >> #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) >> #define i915_execbuffer2_set_context_id(eb2, context) \ >> @@ -914,6 +919,10 @@ struct drm_i915_gem_execbuffer2 { >> #define i915_execbuffer2_get_context_id(eb2) \ >> ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) >> >> +#define i915_execbuffer2_engine(class, instance) \ >> + (I915_EXEC_CLASS_INSTANCE | (class) | \ >> + ((instance) << I915_EXEC_INSTANCE_SHIFT)) > > (class | > (instance) << I915_EXEC_INSTANCE_SHIFT | > I915_EXEC_CLASS_INSTANCE) > > Just suggesting to spread it out over another line for better > readability. Okay but I'd like for the flag to come first, followed by class and then instance. Okay with that? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Intel-gfx] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 13:30 ` Tvrtko Ursulin @ 2017-05-18 13:50 ` Chris Wilson 0 siblings, 0 replies; 48+ messages in thread From: Chris Wilson @ 2017-05-18 13:50 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Oscar Mateo, Ben Widawsky, Rogozhkin, Dmitry V, Tvrtko Ursulin, Intel-gfx, Joonas Lahtinen, Tvrtko Ursulin, Jon Bloomfield, mesa-dev, Daniel Vetter, intel-vaapi-media, Gong, Zhipeng On Thu, May 18, 2017 at 02:30:56PM +0100, Tvrtko Ursulin wrote: > > On 17/05/2017 17:44, Chris Wilson wrote: > >On Wed, May 17, 2017 at 04:40:57PM +0100, Tvrtko Ursulin wrote: > >>+static struct intel_engine_cs *get_engine_class(struct drm_i915_private *i915, > >>+ u8 class, u8 instance) > >>+{ > >>+ if (class > MAX_ENGINE_CLASS || instance > MAX_ENGINE_INSTANCE) > >>+ return NULL; > >>+ > >>+ return i915->engine_class[class][instance]; > >>+} > > > >Be bold make this this an intel_engine_lookup(), I forsee some other > >users appearing very shortly. > > Still static or you want to export it straight away? Preference for > where to put it? Here or intel_engine_cs.c? Let's go with intel_engine_cs.c. We know we're going to move it otherwise. > >>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > >>index ee144ec57935..a3b59043b991 100644 > >>--- a/drivers/gpu/drm/i915/i915_reg.h > >>+++ b/drivers/gpu/drm/i915/i915_reg.h > >>@@ -92,6 +92,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) > >> #define VIDEO_ENHANCEMENT_CLASS 2 > >> #define COPY_ENGINE_CLASS 3 > >> #define OTHER_CLASS 4 > >>+#define MAX_ENGINE_CLASS 4 > >>+ > >>+#define MAX_ENGINE_INSTANCE 1 > > > >We also need the names in the uapi.h as well. Do we want to mention > >OTHER_CLASS before it is defined? I trust your crystal ball. > > I have mentioned it just by the virtue of it existing in i915_reg.h > Crystal ball is otherwise quiet. > > >Amusingly I notice that you do claim that you have these names in the > >changelog. :) We don't need DRM_I915 all the time. I915_ENGINE_CLASS is > >going to be unique, at least for those users of i915_drm.h > > They are all there courtesy of the previous patch. Hey, definitely only one v3 patch in my inbox and no 2/2 to clue me in :) > I will drop the DRM_ prefix throughout. > > > > >> /* PCI config space */ > >> > >>diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > >>index 7566cf48012f..c5ad51c43d23 100644 > >>--- a/drivers/gpu/drm/i915/intel_engine_cs.c > >>+++ b/drivers/gpu/drm/i915/intel_engine_cs.c > >>@@ -225,7 +225,14 @@ intel_engine_setup(struct drm_i915_private *dev_priv, > >> > >> ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); > >> > >>+ if (WARN_ON(info->class > MAX_ENGINE_CLASS || > >>+ info->instance > MAX_ENGINE_INSTANCE || > >>+ dev_priv->engine_class[info->class][info->instance])) > > > >Spread these out or add a message telling what was wrong. > > Can do, but I considered these to be such a basic programming errors > which should be caught during development. Only thing which > prevented me from putting in under the GEM_BUG_ON is the fact > someone could not have it enabled and that this function can already > handle failures. Spread them out and use GEM_WARN_ON? > > >>+ return -EINVAL; > >>+ > >>+ dev_priv->engine_class[info->class][info->instance] = engine; > >> dev_priv->engine[id] = engine; > >>+ > >> return 0; > >> } > >> > >>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > >>index 2ac6667e57ea..6a26bdf5e684 100644 > >>--- a/include/uapi/drm/i915_drm.h > >>+++ b/include/uapi/drm/i915_drm.h > >>@@ -906,7 +906,12 @@ struct drm_i915_gem_execbuffer2 { > >> */ > >> #define I915_EXEC_FENCE_OUT (1<<17) > >> > >>-#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1)) > >>+#define I915_EXEC_CLASS_INSTANCE (1<<18) > >>+ > >>+#define I915_EXEC_INSTANCE_SHIFT (19) > >>+#define I915_EXEC_INSTANCE_MASK (0xff << I915_EXEC_INSTANCE_SHIFT) > >>+ > >>+#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 27) << 1)) > >> > >> #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) > >> #define i915_execbuffer2_set_context_id(eb2, context) \ > >>@@ -914,6 +919,10 @@ struct drm_i915_gem_execbuffer2 { > >> #define i915_execbuffer2_get_context_id(eb2) \ > >> ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) > >> > >>+#define i915_execbuffer2_engine(class, instance) \ > >>+ (I915_EXEC_CLASS_INSTANCE | (class) | \ > >>+ ((instance) << I915_EXEC_INSTANCE_SHIFT)) > > > >(class | > > (instance) << I915_EXEC_INSTANCE_SHIFT | > > I915_EXEC_CLASS_INSTANCE) > > > >Just suggesting to spread it out over another line for better > >readability. > > Okay but I'd like for the flag to come first, followed by class and > then instance. Okay with that? Ok. I don't think we can stop it forming a triangle however we reorder them. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-17 15:40 ` [RFC v3] " Tvrtko Ursulin 2017-05-17 16:44 ` Chris Wilson @ 2017-05-18 10:55 ` Joonas Lahtinen 2017-05-18 11:10 ` Chris Wilson 2017-05-18 14:58 ` [RFC v4 2/2] " Tvrtko Ursulin 2 siblings, 1 reply; 48+ messages in thread From: Joonas Lahtinen @ 2017-05-18 10:55 UTC (permalink / raw) To: Tvrtko Ursulin, Intel-gfx Cc: Ben Widawsky, mesa-dev, Daniel Vetter, intel-vaapi-media On ke, 2017-05-17 at 16:40 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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. Would it make sense to use bitmask for future proofing? Then we could allow passing multiple options in the future. Other than that, looks good. Chris already commented some on the code itself. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 10:55 ` Joonas Lahtinen @ 2017-05-18 11:10 ` Chris Wilson 2017-05-18 12:13 ` Tvrtko Ursulin 0 siblings, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-05-18 11:10 UTC (permalink / raw) To: Joonas Lahtinen Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media 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@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. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 11:10 ` Chris Wilson @ 2017-05-18 12:13 ` Tvrtko Ursulin 2017-05-18 12:24 ` Chris Wilson 0 siblings, 1 reply; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-18 12:13 UTC (permalink / raw) To: Chris Wilson, Joonas Lahtinen, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev 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@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. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 12:13 ` Tvrtko Ursulin @ 2017-05-18 12:24 ` Chris Wilson 2017-05-18 13:06 ` Tvrtko Ursulin 0 siblings, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-05-18 12:24 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media 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@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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 12:24 ` Chris Wilson @ 2017-05-18 13:06 ` Tvrtko Ursulin 2017-05-18 13:37 ` Chris Wilson 2017-05-18 14:10 ` [Intel-gfx] " Emil Velikov 0 siblings, 2 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-18 13:06 UTC (permalink / raw) To: Chris Wilson, Joonas Lahtinen, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev 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@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. 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 13:06 ` Tvrtko Ursulin @ 2017-05-18 13:37 ` Chris Wilson 2017-05-18 16:20 ` Tvrtko Ursulin 2017-05-18 14:10 ` [Intel-gfx] " Emil Velikov 1 sibling, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-05-18 13:37 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media On Thu, May 18, 2017 at 02:06:35PM +0100, Tvrtko Ursulin wrote: > > On 18/05/2017 13:24, Chris Wilson wrote: > >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. Contexts are not thread-friendly. About the only way you can make them safe (wrt execbuf) is through the use of userspace GTT allocation (i.e. assigning an address on creation and making it permanent with softpin). So in general you end up serialising around execbuf and copying the state in/out of the ioctl. That gives a window of opportunity to use context_setparam as an extension for irregular parameter updates. It is not as nice as providing space in the execbuf ioctl, because of the extra state being carried around in the context. > But this problem in general can also be solved separately from > class-instance addressing via engine feature masking. But imo all members of a class should have the same features. That would be my definition of a class! -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 13:37 ` Chris Wilson @ 2017-05-18 16:20 ` Tvrtko Ursulin 2017-05-18 17:00 ` [Intel-gfx] " Chris Wilson 0 siblings, 1 reply; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-18 16:20 UTC (permalink / raw) To: Chris Wilson, Joonas Lahtinen, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev On 18/05/2017 14:37, Chris Wilson wrote: > On Thu, May 18, 2017 at 02:06:35PM +0100, Tvrtko Ursulin wrote: >> >> On 18/05/2017 13:24, Chris Wilson wrote: >>> 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. > > Contexts are not thread-friendly. About the only way you can make them > safe (wrt execbuf) is through the use of userspace GTT allocation (i.e. > assigning an address on creation and making it permanent with softpin). > > So in general you end up serialising around execbuf and copying the > state in/out of the ioctl. That gives a window of opportunity to use > context_setparam as an extension for irregular parameter updates. > > It is not as nice as providing space in the execbuf ioctl, because of > the extra state being carried around in the context. Yes not nice, I can't say that I like this very much. >> But this problem in general can also be solved separately from >> class-instance addressing via engine feature masking. > > But imo all members of a class should have the same features. That would > be my definition of a class! That sounds very totalitarian! :)) To me a class is a group of some entities which share some common characteristics - not necessarily completely uniform. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Intel-gfx] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 16:20 ` Tvrtko Ursulin @ 2017-05-18 17:00 ` Chris Wilson 2017-05-24 11:28 ` Tvrtko Ursulin 0 siblings, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-05-18 17:00 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Oscar Mateo, Ben Widawsky, Rogozhkin, Dmitry V, Tvrtko Ursulin, Intel-gfx, Joonas Lahtinen, Tvrtko Ursulin, Jon Bloomfield, mesa-dev, Daniel Vetter, intel-vaapi-media, Gong, Zhipeng On Thu, May 18, 2017 at 05:20:38PM +0100, Tvrtko Ursulin wrote: > > On 18/05/2017 14:37, Chris Wilson wrote: > >On Thu, May 18, 2017 at 02:06:35PM +0100, Tvrtko Ursulin wrote: > >> > >>But this problem in general can also be solved separately from > >>class-instance addressing via engine feature masking. > > > >But imo all members of a class should have the same features. That would > >be my definition of a class! > > That sounds very totalitarian! :)) To me a class is a group of some > entities which share some common characteristics - not necessarily > completely uniform. The problem otherwise is that we then have to define yet another interface based on features. To me that sounds like too much duplication, that we could avoid from the beginning. Curse the hw for being asymmetical! -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 17:00 ` [Intel-gfx] " Chris Wilson @ 2017-05-24 11:28 ` Tvrtko Ursulin 2017-05-25 14:14 ` [Intel-gfx] " Chris Wilson 0 siblings, 1 reply; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-24 11:28 UTC (permalink / raw) To: Chris Wilson, Joonas Lahtinen, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev On 18/05/2017 18:00, Chris Wilson wrote: > On Thu, May 18, 2017 at 05:20:38PM +0100, Tvrtko Ursulin wrote: >> >> On 18/05/2017 14:37, Chris Wilson wrote: >>> On Thu, May 18, 2017 at 02:06:35PM +0100, Tvrtko Ursulin wrote: >>>> >>>> But this problem in general can also be solved separately from >>>> class-instance addressing via engine feature masking. >>> >>> But imo all members of a class should have the same features. That would >>> be my definition of a class! >> >> That sounds very totalitarian! :)) To me a class is a group of some >> entities which share some common characteristics - not necessarily >> completely uniform. > > The problem otherwise is that we then have to define yet another > interface based on features. To me that sounds like too much > duplication, that we could avoid from the beginning. Curse the hw for > being asymmetical! Hm I don't see a problem with the feature base engine selection on top. You still do because of the desire classes were equal in features? To sum up what I (and we) talked about in various parts of the thread(s): Step 1a: New execbuf engine selection uAPI. - execbuf class=VCS instance=1 Step 1b: Engine discovery uAPI. Same as above but userpace can figure out how many VCS engines there are without PCI probing. I didn't get much feedback on this one. :( Step 2: Feature masks for execbuf. - execbuf class=VCS instance=0 features=HEVC = OK - execbuf class=VCS instance=1 features=HEVC = FAIL But userspace can use engine discovery to figure out which are the valid combinations. This could be a simpler, but less featureful and not very elegant alternative to step 2. Otherwise just a prep step for the subsequent steps below. Step 3a: (One day maybe) userspace selects a class, i915 picks the engine - execbuf class=VCS instance=any Step 3b: userspace selected class and features - execbuf class=VCS instance=any features=HEVC This RFC proposed steps 1a and 1b. The rest we leave for later. How does that sound? Acceptable? In case of engine discovery useful enough or what other features could we put it in to make it more useful for userspace? Potentially enable dropping PCI id probing altogether and enable libva/mesa/??? to probe everything using i915 ioctls. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Intel-gfx] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-24 11:28 ` Tvrtko Ursulin @ 2017-05-25 14:14 ` Chris Wilson 2017-06-15 8:08 ` Tvrtko Ursulin 0 siblings, 1 reply; 48+ messages in thread From: Chris Wilson @ 2017-05-25 14:14 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Oscar Mateo, Ben Widawsky, Rogozhkin, Dmitry V, Tvrtko Ursulin, Intel-gfx, Joonas Lahtinen, Tvrtko Ursulin, Jon Bloomfield, mesa-dev, Daniel Vetter, intel-vaapi-media, Gong, Zhipeng On Wed, May 24, 2017 at 12:28:07PM +0100, Tvrtko Ursulin wrote: > > On 18/05/2017 18:00, Chris Wilson wrote: > >On Thu, May 18, 2017 at 05:20:38PM +0100, Tvrtko Ursulin wrote: > >> > >>On 18/05/2017 14:37, Chris Wilson wrote: > >>>On Thu, May 18, 2017 at 02:06:35PM +0100, Tvrtko Ursulin wrote: > >>>> > >>>>But this problem in general can also be solved separately from > >>>>class-instance addressing via engine feature masking. > >>> > >>>But imo all members of a class should have the same features. That would > >>>be my definition of a class! > >> > >>That sounds very totalitarian! :)) To me a class is a group of some > >>entities which share some common characteristics - not necessarily > >>completely uniform. > > > >The problem otherwise is that we then have to define yet another > >interface based on features. To me that sounds like too much > >duplication, that we could avoid from the beginning. Curse the hw for > >being asymmetical! > > Hm I don't see a problem with the feature base engine selection on > top. You still do because of the desire classes were equal in > features? Ok, having another think about it, I agree that the features define a subclass. Basically it comes down to VCS | HEVC being a subset of the VCS group and we need a way to express any of VCS | HEVC and any of VCS cleanly. > To sum up what I (and we) talked about in various parts of the thread(s): > > Step 1a: New execbuf engine selection uAPI. > > - execbuf class=VCS instance=1 > > Step 1b: Engine discovery uAPI. > > Same as above but userpace can figure out how many VCS engines there > are without PCI probing. > > I didn't get much feedback on this one. :( Mainly because we didn't emphasis that this would be the only way to discover the parameters for execbuf / svm and it lost in the idea that this would replace their device_info tables. We are suggesting that we can phase out those tables (but we should put some effort into making sure we discard unwanted ones after init) since we will have to provide the information anyway. > Step 2: Feature masks for execbuf. > > - execbuf class=VCS instance=0 features=HEVC = OK > - execbuf class=VCS instance=1 features=HEVC = FAIL > > But userspace can use engine discovery to figure out which are the > valid combinations. > > This could be a simpler, but less featureful and not very elegant > alternative to step 2. > > Otherwise just a prep step for the subsequent steps below. > > Step 3a: (One day maybe) userspace selects a class, i915 picks the engine > > - execbuf class=VCS instance=any > > Step 3b: userspace selected class and features > > - execbuf class=VCS instance=any features=HEVC > > This RFC proposed steps 1a and 1b. The rest we leave for later. > > How does that sound? Acceptable? I want "1c: extensible interface for per-engine settings elsewhere in the uAPI" as well. We also need an answer to the i915_gem_busy_ioctl conundrum - if/when we should deprecated the engine id being exposed. It's the same question more or less as for how long (or whether we should) continue to support I915_EXEC_RING. Plus how to expose these via tracepoints. The quick answer is while we have less than 16 engines, we don't have to worry about the uABI breakage. > In case of engine discovery useful enough or what other features > could we put it in to make it more useful for userspace? Potentially > enable dropping PCI id probing altogether and enable libva/mesa/??? > to probe everything using i915 ioctls. To be convenient we have to beat the ease of device_info[PCIID], and being futureproof. The resistance will be because then they are dependent on the kernel for features that are not dependent on that kernel for execution, i.e. being able to use new userspace on old kernels and remain feature complete. To counter that we need to have a complete description of a new device the moment we enable it. That makes it a hard sell. The benefit is basically only a reduction in RO library mmappings, a very small improvement of start up speed at best. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-25 14:14 ` [Intel-gfx] " Chris Wilson @ 2017-06-15 8:08 ` Tvrtko Ursulin 0 siblings, 0 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-06-15 8:08 UTC (permalink / raw) To: Chris Wilson, Joonas Lahtinen, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Ben Widawsky, Daniel Vetter, Jon Bloomfield, Daniel Charles, Rogozhkin, Dmitry V, Oscar Mateo, Gong, Zhipeng, intel-vaapi-media, mesa-dev On 25/05/2017 15:14, Chris Wilson wrote: > On Wed, May 24, 2017 at 12:28:07PM +0100, Tvrtko Ursulin wrote: >> >> On 18/05/2017 18:00, Chris Wilson wrote: >>> On Thu, May 18, 2017 at 05:20:38PM +0100, Tvrtko Ursulin wrote: >>>> >>>> On 18/05/2017 14:37, Chris Wilson wrote: >>>>> On Thu, May 18, 2017 at 02:06:35PM +0100, Tvrtko Ursulin wrote: >>>>>> >>>>>> But this problem in general can also be solved separately from >>>>>> class-instance addressing via engine feature masking. >>>>> >>>>> But imo all members of a class should have the same features. That would >>>>> be my definition of a class! >>>> >>>> That sounds very totalitarian! :)) To me a class is a group of some >>>> entities which share some common characteristics - not necessarily >>>> completely uniform. >>> >>> The problem otherwise is that we then have to define yet another >>> interface based on features. To me that sounds like too much >>> duplication, that we could avoid from the beginning. Curse the hw for >>> being asymmetical! >> >> Hm I don't see a problem with the feature base engine selection on >> top. You still do because of the desire classes were equal in >> features? > > Ok, having another think about it, I agree that the features define a > subclass. Basically it comes down to VCS | HEVC being a subset of the > VCS group and we need a way to express any of VCS | HEVC and any of VCS > cleanly. Cool, I'll respin with engine features on the top to see how that looks. And I am also thinking about dropping the GT discovery bits. More on that below. >> To sum up what I (and we) talked about in various parts of the thread(s): >> >> Step 1a: New execbuf engine selection uAPI. >> >> - execbuf class=VCS instance=1 >> >> Step 1b: Engine discovery uAPI. >> >> Same as above but userpace can figure out how many VCS engines there >> are without PCI probing. >> >> I didn't get much feedback on this one. :( > > Mainly because we didn't emphasis that this would be the only way to > discover the parameters for execbuf / svm and it lost in the idea that > this would replace their device_info tables. We are suggesting that we > can phase out those tables (but we should put some effort into making > sure we discard unwanted ones after init) since we will have to provide > the information anyway. It has become more unlikely we would get libva (my hopeful first user) on board for this in the short/medium term. So as said above, I am thinking to drop this for now. If at some point we go for "step 2" below that would also enable some sort of engine enumeration/probing via execbuf null batches. >> Step 2: Feature masks for execbuf. >> >> - execbuf class=VCS instance=0 features=HEVC = OK >> - execbuf class=VCS instance=1 features=HEVC = FAIL >> >> But userspace can use engine discovery to figure out which are the >> valid combinations. >> >> This could be a simpler, but less featureful and not very elegant >> alternative to step 2. >> >> Otherwise just a prep step for the subsequent steps below. >> >> Step 3a: (One day maybe) userspace selects a class, i915 picks the engine >> >> - execbuf class=VCS instance=any >> >> Step 3b: userspace selected class and features >> >> - execbuf class=VCS instance=any features=HEVC >> >> This RFC proposed steps 1a and 1b. The rest we leave for later. >> >> How does that sound? Acceptable? > > I want "1c: extensible interface for per-engine settings elsewhere in the > uAPI" as well. What do you have in mind here, what other uAPI deals with engines? Regards, Tvrtko > We also need an answer to the i915_gem_busy_ioctl conundrum - if/when we > should deprecated the engine id being exposed. It's the same question > more or less as for how long (or whether we should) continue to support > I915_EXEC_RING. Plus how to expose these via tracepoints. > > The quick answer is while we have less than 16 engines, we don't have to > worry about the uABI breakage. > >> In case of engine discovery useful enough or what other features >> could we put it in to make it more useful for userspace? Potentially >> enable dropping PCI id probing altogether and enable libva/mesa/??? >> to probe everything using i915 ioctls. > > To be convenient we have to beat the ease of device_info[PCIID], and > being futureproof. The resistance will be because then they are > dependent on the kernel for features that are not dependent on that > kernel for execution, i.e. being able to use new userspace on old > kernels and remain feature complete. To counter that we need to > have a complete description of a new device the moment we enable it. > That makes it a hard sell. The benefit is basically only a reduction in > RO library mmappings, a very small improvement of start up speed at > best. > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Intel-gfx] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 13:06 ` Tvrtko Ursulin 2017-05-18 13:37 ` Chris Wilson @ 2017-05-18 14:10 ` Emil Velikov 2017-05-18 14:55 ` Tvrtko Ursulin 1 sibling, 1 reply; 48+ messages in thread From: Emil Velikov @ 2017-05-18 14:10 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Oscar Mateo, Ben Widawsky, Rogozhkin, Dmitry V, Tvrtko Ursulin, intel-vaapi-media, intel-gfx@lists.freedesktop.org, Joonas Lahtinen, Tvrtko Ursulin, Jon Bloomfield, ML mesa-dev, Daniel Vetter, Gong, Zhipeng Hi Tvrtko, On 18 May 2017 at 14:06, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > 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; > }; > To spare yourself from writing a compat ioctl, you want to have the members at the same offset on both 32 and 64bit builds. At the same times the whole struct size should be multiple of 64bit. This and a few others are covered in Daniel Vetter's "Botching up ioctls" [1] -Emil [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ioctl/botching-up-ioctls.txt _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [RFC v3] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 14:10 ` [Intel-gfx] " Emil Velikov @ 2017-05-18 14:55 ` Tvrtko Ursulin 0 siblings, 0 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-18 14:55 UTC (permalink / raw) To: Emil Velikov Cc: Ben Widawsky, intel-vaapi-media, intel-gfx@lists.freedesktop.org, ML mesa-dev, Daniel Vetter On 18/05/2017 15:10, Emil Velikov wrote: > Hi Tvrtko, > > On 18 May 2017 at 14:06, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > >> 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; >> }; >> > To spare yourself from writing a compat ioctl, you want to have the > members at the same offset on both 32 and 64bit builds. > At the same times the whole struct size should be multiple of 64bit. > > This and a few others are covered in Daniel Vetter's "Botching up ioctls" [1] Yes, thanks, I failed to add up to 64. :) Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* [RFC v4 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-05-17 15:40 ` [RFC v3] " Tvrtko Ursulin 2017-05-17 16:44 ` Chris Wilson 2017-05-18 10:55 ` Joonas Lahtinen @ 2017-05-18 14:58 ` Tvrtko Ursulin 2017-05-18 15:13 ` Chris Wilson 2017-06-26 15:48 ` [RFC v6 " Tvrtko Ursulin 2 siblings, 2 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-05-18 14:58 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, intel-vaapi-media, mesa-dev, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@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 8-bits of flags, to pass in the engine instance, re-use the existing engine selection bits for the class selection, and a new flag named I915_EXEC_CLASS_INSTANCE 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(I915_ENGINE_CLASS_VIDEO_DECODE, 0); Or to send a command to VCS1: eb.flags = i915_execbuffer2_engine(I915_ENGINE_CLASS_VIDEO_DECODE, 1); v2: * Fix unknown flags mask. * Use I915_EXEC_RING_MASK for class. (Chris Wilson) v3: * Add a map for fast class-instance engine lookup. (Chris Wilson) v4: * Update commit to reflect v3. * Export intel_engine_lookup for other users. (Chris Wilson) * Split out some warns. (Chris Wilson) 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: Daniel Charles <daniel.charles@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 Cc: mesa-dev@lists.freedesktop.org --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 ++++++++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_engine_cs.c | 20 ++++++++++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +++ include/uapi/drm/i915_drm.h | 12 +++++++++++- 6 files changed, 58 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1e08b82c4823..53b41963f672 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2115,6 +2115,7 @@ struct drm_i915_private { struct pci_dev *bridge_dev; struct i915_gem_context *kernel_context; struct intel_engine_cs *engine[I915_NUM_ENGINES]; + struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1][MAX_ENGINE_INSTANCE + 1]; struct i915_vma *semaphore; struct drm_dma_handle *status_page_dmah; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index af1965774e7b..006c8046af5f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1492,6 +1492,23 @@ gen8_dispatch_bsd_engine(struct drm_i915_private *dev_priv, return file_priv->bsd_engine; } +static struct intel_engine_cs * +eb_select_engine_class_instance(struct drm_i915_private *i915, + struct drm_i915_gem_execbuffer2 *args) +{ + u8 class = args->flags & I915_EXEC_RING_MASK; + extern u8 user_class_map[I915_ENGINE_CLASS_MAX]; + u8 instance; + + if (class >= ARRAY_SIZE(user_class_map)) + return NULL; + + instance = (args->flags >> I915_EXEC_INSTANCE_SHIFT) && + I915_EXEC_INSTANCE_MASK; + + return intel_engine_lookup(i915, user_class_map[class], instance); +} + #define I915_USER_RINGS (4) static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { @@ -1510,6 +1527,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; struct intel_engine_cs *engine; + if (args->flags & I915_EXEC_CLASS_INSTANCE) + return eb_select_engine_class_instance(dev_priv, args); + if (user_ring_id > I915_USER_RINGS) { DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); return NULL; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 89888adb9af1..fa4a3841c4af 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -92,6 +92,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define VIDEO_ENHANCEMENT_CLASS 2 #define COPY_ENGINE_CLASS 3 #define OTHER_CLASS 4 +#define MAX_ENGINE_CLASS 4 + +#define MAX_ENGINE_INSTANCE 1 /* PCI config space */ diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 8d370c5d07bc..8f31514b01d5 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -198,6 +198,15 @@ intel_engine_setup(struct drm_i915_private *dev_priv, GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes)); class_info = &intel_engine_classes[info->class]; + if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS)) + return -EINVAL; + + if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE)) + return -EINVAL; + + if (GEM_WARN_ON(dev_priv->engine_class[info->class][info->instance])) + return -EINVAL; + GEM_BUG_ON(dev_priv->engine[id]); engine = kzalloc(sizeof(*engine), GFP_KERNEL); if (!engine) @@ -225,7 +234,9 @@ intel_engine_setup(struct drm_i915_private *dev_priv, ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); + dev_priv->engine_class[info->class][info->instance] = engine; dev_priv->engine[id] = engine; + return 0; } @@ -1350,6 +1361,15 @@ int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, return 0; } +struct intel_engine_cs * +intel_engine_lookup(struct drm_i915_private *i915, u8 class, u8 instance) +{ + if (class > MAX_ENGINE_CLASS || instance > MAX_ENGINE_INSTANCE) + return NULL; + + return i915->engine_class[class][instance]; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_engine.c" #endif diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 6aa20ac8cde3..692823b33d75 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -734,4 +734,7 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv); void intel_engines_mark_idle(struct drm_i915_private *i915); void intel_engines_reset_default_submission(struct drm_i915_private *i915); +struct intel_engine_cs * +intel_engine_lookup(struct drm_i915_private *i915, u8 class, u8 instance); + #endif /* _INTEL_RINGBUFFER_H_ */ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 8f0926edc44f..a48ef426f60d 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -906,7 +906,12 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_FENCE_OUT (1<<17) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1)) +#define I915_EXEC_CLASS_INSTANCE (1<<18) + +#define I915_EXEC_INSTANCE_SHIFT (19) +#define I915_EXEC_INSTANCE_MASK (0xff << I915_EXEC_INSTANCE_SHIFT) + +#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 27) << 1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ @@ -914,6 +919,11 @@ struct drm_i915_gem_execbuffer2 { #define i915_execbuffer2_get_context_id(eb2) \ ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) +#define i915_execbuffer2_engine(class, instance) \ + (I915_EXEC_CLASS_INSTANCE | \ + (class) | \ + ((instance) << I915_EXEC_INSTANCE_SHIFT)) + struct drm_i915_gem_pin { /** Handle of the buffer to be pinned. */ __u32 handle; -- 2.9.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [RFC v4 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 14:58 ` [RFC v4 2/2] " Tvrtko Ursulin @ 2017-05-18 15:13 ` Chris Wilson 2017-06-26 15:48 ` [RFC v6 " Tvrtko Ursulin 1 sibling, 0 replies; 48+ messages in thread From: Chris Wilson @ 2017-05-18 15:13 UTC (permalink / raw) To: Tvrtko Ursulin Cc: Ben Widawsky, Intel-gfx, mesa-dev, Daniel Vetter, intel-vaapi-media On Thu, May 18, 2017 at 03:58:26PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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 8-bits of flags, to pass in the > engine instance, re-use the existing engine selection bits for > the class selection, and a new flag named > I915_EXEC_CLASS_INSTANCE 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(I915_ENGINE_CLASS_VIDEO_DECODE, 0); > > Or to send a command to VCS1: > > eb.flags = i915_execbuffer2_engine(I915_ENGINE_CLASS_VIDEO_DECODE, 1); > > v2: > * Fix unknown flags mask. > * Use I915_EXEC_RING_MASK for class. (Chris Wilson) > > v3: > * Add a map for fast class-instance engine lookup. (Chris Wilson) > > v4: > * Update commit to reflect v3. > * Export intel_engine_lookup for other users. (Chris Wilson) > * Split out some warns. (Chris Wilson) > > 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: Daniel Charles <daniel.charles@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 > Cc: mesa-dev@lists.freedesktop.org > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 ++++++++++++++++++++ > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_engine_cs.c | 20 ++++++++++++++++++++ > drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +++ > include/uapi/drm/i915_drm.h | 12 +++++++++++- > 6 files changed, 58 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 1e08b82c4823..53b41963f672 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2115,6 +2115,7 @@ struct drm_i915_private { > struct pci_dev *bridge_dev; > struct i915_gem_context *kernel_context; > struct intel_engine_cs *engine[I915_NUM_ENGINES]; > + struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1][MAX_ENGINE_INSTANCE + 1]; > struct i915_vma *semaphore; > > struct drm_dma_handle *status_page_dmah; > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index af1965774e7b..006c8046af5f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1492,6 +1492,23 @@ gen8_dispatch_bsd_engine(struct drm_i915_private *dev_priv, > return file_priv->bsd_engine; > } > > +static struct intel_engine_cs * > +eb_select_engine_class_instance(struct drm_i915_private *i915, > + struct drm_i915_gem_execbuffer2 *args) > +{ > + u8 class = args->flags & I915_EXEC_RING_MASK; > + extern u8 user_class_map[I915_ENGINE_CLASS_MAX]; > + u8 instance; > + > + if (class >= ARRAY_SIZE(user_class_map)) > + return NULL; > + > + instance = (args->flags >> I915_EXEC_INSTANCE_SHIFT) && > + I915_EXEC_INSTANCE_MASK; > + > + return intel_engine_lookup(i915, user_class_map[class], instance); The class mapping is going to be the same for all users of intel_engine_lookup() presumably? (At least I hope we plan on using the same id everywhere!) intel_engine_lookup_from_user(), just to reinforce where those ids are coming from? So other than the conversation of what class means, looks good to me. To give some context for other users, one example is a context param for setting per-engine settings (e.g. sseu, watchdog thresholds): struct drm_i915_gem_context_param_sseu { __u32 engine; __u32 instance; __u32 slice_mask; __u32 subslice_mask; __u32 min_eu_per_subslice; __u32 max_eu_per_subslice; }; static int i915_gem_context_setparam__sseu(struct i915_gem_context *ctx, const struct drm_i915_gem_context_param *args) { const struct drm_i915_gem_context_param_sseu *user = u64_to_user_ptr(args->value); const unsigned int size = args->size; unsigned int total; total = 0; while (total + sizeof(*user) <= size) { struct drm_i915_gem_context_param_sseu sseu; int ret; if (copy_from_user(&sseu, user++, sizeof(sseu))) return -EFAULT; ret = intel_lr_context_set_sseu(ctx, &sseu); if (ret) return ret; total += sizeof(sseu); } return 0; } int intel_lr_context_set_sseu(struct i915_gem_context *ctx, const struct drm_i915_gem_context_param_sseu *user) { struct drm_i915_private *i915 = ctx->i915; struct intel_engine_cs *engine; struct intel_context *ce; struct sseu_dev_info sseu; int ret; lockdep_assert_held(&i915->drm.struct_mutex); engine = intel_engine_lookup(i915, user->engine, user->instance); if (IS_ERR(engine)) return ERR_PTR(engine); ... My goal is that we only have to use one set of class/instance defines! -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* [RFC v6 2/2] drm/i915: Select engines via class and instance in execbuffer2 2017-05-18 14:58 ` [RFC v4 2/2] " Tvrtko Ursulin 2017-05-18 15:13 ` Chris Wilson @ 2017-06-26 15:48 ` Tvrtko Ursulin 1 sibling, 0 replies; 48+ messages in thread From: Tvrtko Ursulin @ 2017-06-26 15:48 UTC (permalink / raw) To: Intel-gfx; +Cc: Ben Widawsky, Daniel Vetter From: Tvrtko Ursulin <tvrtko.ursulin@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 8-bits of flags, to pass in the engine instance, re-use the existing engine selection bits for the class selection, and a new flag named I915_EXEC_CLASS_INSTANCE 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(I915_ENGINE_CLASS_VIDEO_DECODE, 0); Or to send a command to VCS1: eb.flags = i915_execbuffer2_engine(I915_ENGINE_CLASS_VIDEO_DECODE, 1); v2: * Fix unknown flags mask. * Use I915_EXEC_RING_MASK for class. (Chris Wilson) v3: * Add a map for fast class-instance engine lookup. (Chris Wilson) v4: * Update commit to reflect v3. * Export intel_engine_lookup for other users. (Chris Wilson) * Split out some warns. (Chris Wilson) v5: * Fixed shift and mask logic. * Rebased to be standalone. v6: * Rebased back to follow engine info ioctl. * Rename helper to intel_engine_lookup_user. (Chris Wilson) 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> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 ++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_engine_cs.c | 28 ++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +++ include/uapi/drm/i915_drm.h | 13 ++++++++++++- 6 files changed, 61 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 496565e1753f..4950de8c3de5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2137,6 +2137,7 @@ struct drm_i915_private { struct pci_dev *bridge_dev; struct i915_gem_context *kernel_context; struct intel_engine_cs *engine[I915_NUM_ENGINES]; + struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1][MAX_ENGINE_INSTANCE + 1]; struct i915_vma *semaphore; struct drm_dma_handle *status_page_dmah; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index ec33b358fba9..63241840f00a 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2050,6 +2050,17 @@ gen8_dispatch_bsd_engine(struct drm_i915_private *dev_priv, return file_priv->bsd_engine; } +static struct intel_engine_cs * +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 instance = (args->flags & I915_EXEC_INSTANCE_MASK) >> + I915_EXEC_INSTANCE_SHIFT; + + return intel_engine_lookup_user(i915, class, instance); +} + #define I915_USER_RINGS (4) static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { @@ -2068,6 +2079,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; struct intel_engine_cs *engine; + if (args->flags & I915_EXEC_CLASS_INSTANCE) + return eb_select_engine_class_instance(dev_priv, args); + if (user_ring_id > I915_USER_RINGS) { DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); return NULL; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c8647cfa81ba..834673c9ee6b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -95,6 +95,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define VIDEO_ENHANCEMENT_CLASS 2 #define COPY_ENGINE_CLASS 3 #define OTHER_CLASS 4 +#define MAX_ENGINE_CLASS 4 + +#define MAX_ENGINE_INSTANCE 1 /* PCI config space */ diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a98669f6ad85..1089b22d2090 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -198,6 +198,15 @@ intel_engine_setup(struct drm_i915_private *dev_priv, GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes)); class_info = &intel_engine_classes[info->class]; + if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS)) + return -EINVAL; + + if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE)) + return -EINVAL; + + if (GEM_WARN_ON(dev_priv->engine_class[info->class][info->instance])) + return -EINVAL; + GEM_BUG_ON(dev_priv->engine[id]); engine = kzalloc(sizeof(*engine), GFP_KERNEL); if (!engine) @@ -225,7 +234,9 @@ intel_engine_setup(struct drm_i915_private *dev_priv, ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier); + dev_priv->engine_class[info->class][info->instance] = engine; dev_priv->engine[id] = engine; + return 0; } @@ -1396,6 +1407,23 @@ int i915_gem_engine_info_ioctl(struct drm_device *dev, void *data, return 0; } +struct intel_engine_cs * +intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance) +{ + if (class >= ARRAY_SIZE(user_class_map)) + return NULL; + + class = user_class_map[class]; + + if (WARN_ON_ONCE(class > MAX_ENGINE_CLASS)) + return NULL; + + if (instance > MAX_ENGINE_INSTANCE) + return NULL; + + return i915->engine_class[class][instance]; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_engine.c" #endif diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index d33c93444c0d..283444f9be10 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -735,4 +735,7 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv); void intel_engines_mark_idle(struct drm_i915_private *i915); void intel_engines_reset_default_submission(struct drm_i915_private *i915); +struct intel_engine_cs * +intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance); + #endif /* _INTEL_RINGBUFFER_H_ */ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index fbb1f6b99959..c3fabb89e1c2 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -929,7 +929,13 @@ struct drm_i915_gem_execbuffer2 { * element). */ #define I915_EXEC_BATCH_FIRST (1<<18) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_BATCH_FIRST<<1)) + +#define I915_EXEC_CLASS_INSTANCE (1<<19) + +#define I915_EXEC_INSTANCE_SHIFT (20) +#define I915_EXEC_INSTANCE_MASK (0xff << I915_EXEC_INSTANCE_SHIFT) + +#define __I915_EXEC_UNKNOWN_FLAGS (-((1 << 27) << 1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ @@ -937,6 +943,11 @@ struct drm_i915_gem_execbuffer2 { #define i915_execbuffer2_get_context_id(eb2) \ ((eb2).rsvd1 & I915_EXEC_CONTEXT_ID_MASK) +#define i915_execbuffer2_engine(class, instance) \ + (I915_EXEC_CLASS_INSTANCE | \ + (class) | \ + ((instance) << I915_EXEC_INSTANCE_SHIFT)) + struct drm_i915_gem_pin { /** Handle of the buffer to be pinned. */ __u32 handle; -- 2.9.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 48+ messages in thread
* ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI 2017-04-18 16:56 [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI Tvrtko Ursulin 2017-04-18 16:56 ` [RFC 1/2] drm/i915: Engine discovery uAPI Tvrtko Ursulin 2017-04-18 16:56 ` [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2 Tvrtko Ursulin @ 2017-04-18 17:12 ` Patchwork 2017-04-27 10:16 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev3) Patchwork ` (3 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Patchwork @ 2017-04-18 17:12 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: New engine discovery and execbuffer2 engine selection uAPI URL : https://patchwork.freedesktop.org/series/23189/ State : failure == Summary == Series 23189v1 New engine discovery and execbuffer2 engine selection uAPI https://patchwork.freedesktop.org/api/1.0/series/23189/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload: pass -> FAIL (fi-ilk-650) Subgroup basic-reload-final: pass -> FAIL (fi-ilk-650) Test gem_cpu_reloc: Subgroup basic: pass -> FAIL (fi-hsw-4770) pass -> FAIL (fi-bxt-t5700) pass -> FAIL (fi-kbl-7500u) pass -> FAIL (fi-skl-6700hq) pass -> FAIL (fi-skl-gvtdvm) pass -> FAIL (fi-byt-n2820) pass -> FAIL (fi-bsw-n3050) pass -> FAIL (fi-hsw-4770r) pass -> FAIL (fi-bxt-j4205) pass -> FAIL (fi-byt-j1900) pass -> FAIL (fi-skl-6770hq) pass -> FAIL (fi-ivb-3520m) pass -> FAIL (fi-bdw-gvtdvm) pass -> FAIL (fi-snb-2600) pass -> FAIL (fi-skl-6260u) pass -> FAIL (fi-skl-6700k) pass -> FAIL (fi-ivb-3770) pass -> FAIL (fi-kbl-7560u) pass -> FAIL (fi-bdw-5557u) pass -> FAIL (fi-snb-2520m) Test gem_ctx_switch: Subgroup basic-default: pass -> SKIP (fi-hsw-4770) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-skl-gvtdvm) pass -> SKIP (fi-byt-n2820) pass -> SKIP (fi-bsw-n3050) pass -> SKIP (fi-hsw-4770r) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-byt-j1900) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-ivb-3520m) pass -> SKIP (fi-bdw-gvtdvm) pass -> SKIP (fi-snb-2600) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-ivb-3770) pass -> SKIP (fi-kbl-7560u) pass -> SKIP (fi-bdw-5557u) pass -> SKIP (fi-snb-2520m) Subgroup basic-default-heavy: pass -> SKIP (fi-hsw-4770) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-skl-gvtdvm) pass -> SKIP (fi-byt-n2820) pass -> SKIP (fi-bsw-n3050) pass -> SKIP (fi-hsw-4770r) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-byt-j1900) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-ivb-3520m) pass -> SKIP (fi-bdw-gvtdvm) pass -> SKIP (fi-snb-2600) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-ivb-3770) pass -> SKIP (fi-kbl-7560u) pass -> SKIP (fi-bdw-5557u) pass -> SKIP (fi-snb-2520m) Test gem_exec_basic: Subgroup basic-blt: pass -> SKIP (fi-hsw-4770) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-kbl-7500u) pass -> SKIP (fi-skl-6700hq) pass -> SKIP (fi-skl-gvtdvm) pass -> SKIP (fi-byt-n2820) pass -> SKIP (fi-bsw-n3050) pass -> SKIP (fi-hsw-4770r) pass -> SKIP (fi-bxt-j4205) pass -> SKIP (fi-byt-j1900) pass -> SKIP (fi-skl-6770hq) pass -> SKIP (fi-ivb-3520m) pass -> SKIP (fi-bdw-gvtdvm) pass -> SKIP (fi-snb-2600) pass -> SKIP (fi-skl-6260u) pass -> SKIP (fi-skl-6700k) pass -> SKIP (fi-ivb-3770) pass -> SKIP (fi-kbl-7560u) pass -> SKIP (fi-bdw-5557u) pass -> SKIP (fi-snb-2520m) Subgroup basic-bsd: pass -> SKIP (fi-hsw-4770) pass -> SKIP (fi-bxt-t5700) pass -> SKIP (fi-ilk-650) pass -> SKIP (fi-kbl-7500u) WARNING: Long output truncated 7204beb80dcdfd1f8b1cff6a448301407f91a99c drm-tip: 2017y-04m-18d-16h-09m-11s UTC integration manifest b8a7a65 drm/i915: Select engines via class and instance in execbuffer2 f68c903 drm/i915: Engine discovery uAPI == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4513/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev3) 2017-04-18 16:56 [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI Tvrtko Ursulin ` (2 preceding siblings ...) 2017-04-18 17:12 ` ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI Patchwork @ 2017-04-27 10:16 ` Patchwork 2017-05-18 17:11 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev6) Patchwork ` (2 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Patchwork @ 2017-04-27 10:16 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: New engine discovery and execbuffer2 engine selection uAPI (rev3) URL : https://patchwork.freedesktop.org/series/23189/ State : success == Summary == Series 23189v3 New engine discovery and execbuffer2 engine selection uAPI https://patchwork.freedesktop.org/api/1.0/series/23189/revisions/3/mbox/ Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (fi-skl-6770hq) fdo#99739 fdo#99739 https://bugs.freedesktop.org/show_bug.cgi?id=99739 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:415s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:423s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:522s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:475s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:563s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:481s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:488s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:406s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:414s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:492s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:487s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:440s fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:556s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:438s fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:552s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:448s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:473s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:432s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:539s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:407s a0363fea4a56eef81e8cda759a24d8951dd7ac73 drm-tip: 2017y-04m-27d-08h-13m-25s UTC integration manifest 02b92bc drm/i915: Select engines via class and instance in execbuffer2 90aa3b5 drm/i915: Engine discovery uAPI == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4564/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev6) 2017-04-18 16:56 [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI Tvrtko Ursulin ` (3 preceding siblings ...) 2017-04-27 10:16 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev3) Patchwork @ 2017-05-18 17:11 ` Patchwork 2017-06-26 16:04 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev8) Patchwork 2017-06-28 15:47 ` ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI (rev9) Patchwork 6 siblings, 0 replies; 48+ messages in thread From: Patchwork @ 2017-05-18 17:11 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: New engine discovery and execbuffer2 engine selection uAPI (rev6) URL : https://patchwork.freedesktop.org/series/23189/ State : success == Summary == Series 23189v6 New engine discovery and execbuffer2 engine selection uAPI https://patchwork.freedesktop.org/api/1.0/series/23189/revisions/6/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:445s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:435s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:596s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:511s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:500s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:485s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:416s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:411s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:496s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:461s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:463s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:466s fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:583s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:466s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:505s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:440s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:534s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:404s ab08cb2750e769d074b2f147c8298ccd0cd08340 drm-tip: 2017y-05m-18d-15h-36m-17s UTC integration manifest d5f300b drm/i915: Select engines via class and instance in execbuffer2 128602d drm/i915: Engine discovery uAPI == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4743/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev8) 2017-04-18 16:56 [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI Tvrtko Ursulin ` (4 preceding siblings ...) 2017-05-18 17:11 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev6) Patchwork @ 2017-06-26 16:04 ` Patchwork 2017-06-28 15:47 ` ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI (rev9) Patchwork 6 siblings, 0 replies; 48+ messages in thread From: Patchwork @ 2017-06-26 16:04 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: New engine discovery and execbuffer2 engine selection uAPI (rev8) URL : https://patchwork.freedesktop.org/series/23189/ State : success == Summary == Series 23189v8 New engine discovery and execbuffer2 engine selection uAPI https://patchwork.freedesktop.org/api/1.0/series/23189/revisions/8/mbox/ Test gem_exec_suspend: Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-r) fdo#100125 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: pass -> FAIL (fi-snb-2600) fdo#100215 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (fi-blb-e6850) fdo#101599 Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-b: dmesg-warn -> PASS (fi-pnv-d510) fdo#101597 Subgroup suspend-read-crc-pipe-b: dmesg-warn -> PASS (fi-byt-j1900) fdo#101516 +1 fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fdo#101599 https://bugs.freedesktop.org/show_bug.cgi?id=101599 fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597 fdo#101516 https://bugs.freedesktop.org/show_bug.cgi?id=101516 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:450s fi-bdw-gvtdvm total:279 pass:257 dwarn:8 dfail:0 fail:0 skip:14 time:433s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:355s fi-bsw-n3050 total:279 pass:242 dwarn:1 dfail:0 fail:0 skip:36 time:523s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:512s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:489s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:486s fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:587s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:440s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:420s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:498s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:468s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:468s fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:572s fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:586s fi-pnv-d510 total:279 pass:222 dwarn:2 dfail:0 fail:0 skip:55 time:555s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:455s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:465s fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:480s fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:438s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:542s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:406s 21d74e215ad650f0e8e30de609bd65601f0aa11d drm-tip: 2017y-06m-26d-09h-12m-14s UTC integration manifest f1a5a50 drm/i915: Select engines via class and instance in execbuffer2 14e0807 drm/i915: Engine discovery uAPI == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5043/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
* ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI (rev9) 2017-04-18 16:56 [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI Tvrtko Ursulin ` (5 preceding siblings ...) 2017-06-26 16:04 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev8) Patchwork @ 2017-06-28 15:47 ` Patchwork 6 siblings, 0 replies; 48+ messages in thread From: Patchwork @ 2017-06-28 15:47 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: New engine discovery and execbuffer2 engine selection uAPI (rev9) URL : https://patchwork.freedesktop.org/series/23189/ State : failure == Summary == Series 23189v9 New engine discovery and execbuffer2 engine selection uAPI https://patchwork.freedesktop.org/api/1.0/series/23189/revisions/9/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: fail -> PASS (fi-snb-2600) fdo#100007 Test gem_exec_parse: Subgroup basic-allowed: pass -> FAIL (fi-byt-j1900) Subgroup basic-rejected: pass -> FAIL (fi-byt-j1900) Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: pass -> FAIL (fi-snb-2600) fdo#100215 fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007 fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:438s fi-bdw-gvtdvm total:279 pass:257 dwarn:8 dfail:0 fail:0 skip:14 time:430s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:357s fi-bsw-n3050 total:279 pass:242 dwarn:1 dfail:0 fail:0 skip:36 time:529s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:504s fi-byt-j1900 total:279 pass:251 dwarn:2 dfail:0 fail:2 skip:24 time:439s fi-byt-n2820 total:279 pass:249 dwarn:2 dfail:0 fail:0 skip:28 time:486s fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:602s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:433s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:417s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:501s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:471s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:464s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:574s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:587s fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:562s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:454s fi-skl-6700hq total:279 pass:223 dwarn:1 dfail:0 fail:30 skip:24 time:343s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:462s fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:479s fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:435s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:548s fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:403s 85a692e2c6a7cf93082044d776e838cb9e9b2146 drm-tip: 2017y-06m-28d-14h-24m-59s UTC integration manifest b3ca660 drm/i915: Select engines via class and instance in execbuffer2 6e37821 drm/i915: Engine discovery uAPI == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5060/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2017-06-28 15:47 UTC | newest] Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-18 16:56 [RFC 0/2] New engine discovery and execbuffer2 engine selection uAPI Tvrtko Ursulin 2017-04-18 16:56 ` [RFC 1/2] drm/i915: Engine discovery uAPI Tvrtko Ursulin 2017-04-18 20:13 ` Chris Wilson 2017-04-24 8:07 ` Tvrtko Ursulin 2017-04-19 5:22 ` Kenneth Graunke 2017-04-24 8:26 ` [Mesa-dev] " Tvrtko Ursulin 2017-04-19 8:32 ` Gong, Zhipeng 2017-04-27 9:10 ` [RFC v2 " Tvrtko Ursulin 2017-05-18 14:57 ` [RFC v3 " Tvrtko Ursulin 2017-06-26 15:47 ` [RFC v4 " Tvrtko Ursulin 2017-06-28 11:27 ` Chris Wilson 2017-06-28 13:15 ` Tvrtko Ursulin 2017-06-28 13:19 ` Tvrtko Ursulin 2017-06-28 13:21 ` Chris Wilson 2017-06-28 11:33 ` Chris Wilson 2017-06-28 15:30 ` [RFC v5 " Tvrtko Ursulin 2017-04-18 16:56 ` [RFC 2/2] drm/i915: Select engines via class and instance in execbuffer2 Tvrtko Ursulin 2017-04-18 21:10 ` Chris Wilson 2017-04-24 8:36 ` Tvrtko Ursulin 2017-04-27 9:10 ` [RFC v2 " Tvrtko Ursulin 2017-04-27 9:25 ` Chris Wilson 2017-04-27 10:09 ` Tvrtko Ursulin 2017-04-27 10:26 ` Chris Wilson 2017-05-17 15:40 ` [RFC v3] " Tvrtko Ursulin 2017-05-17 16:44 ` Chris Wilson 2017-05-18 13:30 ` Tvrtko Ursulin 2017-05-18 13:50 ` [Intel-gfx] " Chris Wilson 2017-05-18 10:55 ` Joonas Lahtinen 2017-05-18 11:10 ` Chris Wilson 2017-05-18 12:13 ` Tvrtko Ursulin 2017-05-18 12:24 ` Chris Wilson 2017-05-18 13:06 ` Tvrtko Ursulin 2017-05-18 13:37 ` Chris Wilson 2017-05-18 16:20 ` Tvrtko Ursulin 2017-05-18 17:00 ` [Intel-gfx] " Chris Wilson 2017-05-24 11:28 ` Tvrtko Ursulin 2017-05-25 14:14 ` [Intel-gfx] " Chris Wilson 2017-06-15 8:08 ` Tvrtko Ursulin 2017-05-18 14:10 ` [Intel-gfx] " Emil Velikov 2017-05-18 14:55 ` Tvrtko Ursulin 2017-05-18 14:58 ` [RFC v4 2/2] " Tvrtko Ursulin 2017-05-18 15:13 ` Chris Wilson 2017-06-26 15:48 ` [RFC v6 " Tvrtko Ursulin 2017-04-18 17:12 ` ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI Patchwork 2017-04-27 10:16 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev3) Patchwork 2017-05-18 17:11 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev6) Patchwork 2017-06-26 16:04 ` ✓ Fi.CI.BAT: success for New engine discovery and execbuffer2 engine selection uAPI (rev8) Patchwork 2017-06-28 15:47 ` ✗ Fi.CI.BAT: failure for New engine discovery and execbuffer2 engine selection uAPI (rev9) Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.