Re: [PATCH 1/4] drm/i915: introduce query info uAPI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/11/2017 16:15, Lionel Landwerlin wrote:
On 09/11/17 15:57, Joonas Lahtinen wrote:
On Wed, 2017-11-08 at 16:22 +0000, Lionel Landwerlin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Query info uAPI allows userspace to probe for a number of properties
of the GPU. This partially removes the need for userspace to maintain
the internal PCI id based database (as well as code duplication across
userspace components).

This first version enables engine configuration and features to be
queried.

Probing is done via the new DRM_IOCTL_I915_QUERY_INFO ioctl which
takes a query ID as well as query arguments.

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_query_info info = {};
   struct drm_i915_engine_info *engines;
   int i, ret;

   info.version = 1;
   info.query = I915_QUERY_INFO_ENGINE;
   info.query_params[0] = I915_ENGINE_CLASS_VIDEO;
   info.info_ptr_len = 0;
   ret = ioctl(..., &info);
   assert(ret == 0);

   engines = malloc(info.info_ptr_len);
   info.info_ptr = to_user_pointer(engines);
   ret = ioctl(..., &info);
   assert(ret == 0);

   for (i = 0; i < info.info_ptr / sizeof(*engines); i++)
       printf("VCS%u: flags=%x\n",
              engines[i].instance,
              engines[i].info);

First time with info.info_ptr_len set to zero, so the kernel reports
back the size of the data to be written into info.info_ptr. Then a
second time with the info.info_ptr_len field set to the correct
length.

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)

v6:
  * Return API to query_info to make it more generic (Lionel)
  * Add query ID & query params (Lionel)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
Cc: Ben Widawsky <ben@xxxxxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Jon Bloomfield <jon.bloomfield@xxxxxxxxx>
Cc: "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: "Gong, Zhipeng" <zhipeng.gong@xxxxxxxxx>
<SNIP>

+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2776,6 +2776,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {       DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW),       DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),       DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), +    DRM_IOCTL_DEF_DRV(I915_QUERY_INFO, i915_query_info_ioctl, DRM_RENDER_ALLOW),
I'm not a fan. This is bit much to the direction of I915_META_IOCTL.

So you would prefer a different ioctl for every bit of information we would want to query from the kernel?


So can we keep this as engine info query without versioning, and if you
need to query something else, lets have another ioctl for that.

(I heard a rumour of this being about RCS topology, which could be in
the engine info.).

I'm not sure to understand what you mean by "in the engine info". Would you mind to give an example?

We chatted about this internally a bit and no one seems to like a multiplexer within a multiplexer approach.

So one option is a completely separate ioctl, but before that the question is if everything you need to export is called RCS topology, so logically belongs to the render engine, could it be exported under the engine info name after all?

Like maybe adding drm_i915_rcs_engine_info, containing all the extra fields you need on top of the existing info, to be returned when the render class is queried.

One problem I spotted is that your new data is only valid on gen8+. So that would mean having a flag saying what data is valid.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux