Re: [Intel-gfx] [RFC 1/2] drm/i915: Engine discovery uAPI
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
Re: [Intel-gfx] [RFC 1/2] drm/i915: Engine discovery uAPI
>-Original Message- >From: Tvrtko Ursulin [mailto:tursu...@ursulin.net] >Sent: Wednesday, April 19, 2017 12:56 AM >To: Intel-gfx@lists.freedesktop.org >Cc: tursu...@ursulin.net; Ursulin, Tvrtko; Ben >Widawsky ; Chris Wilson ; Vetter, >Daniel ; Joonas Lahtinen > ; Bloomfield, Jon ; >Charles, Daniel ; Rogozhkin, Dmitry V > ; Mateo Lozano, Oscar ; >Gong, Zhipeng ; intel-vaapi-me...@lists.01.org; >mesa-...@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
Re: [Intel-gfx] [RFC 1/2] drm/i915: Engine discovery uAPI
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