Re: [Intel-gfx] [RFC 1/2] drm/i915: Engine discovery uAPI

[Tvrtko Ursulin]

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

[Gong, Zhipeng]

>-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

[Chris Wilson]

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