[igt-dev] [RFC PATCH v9 0/5] new engine discovery interface

Andi Shyti via igt-dev
2019-02-14T00:44:52Z

Hi,

In this patchset I propose an alternative way of engine discovery
thanks to the new interfaces developed by Tvrtko and Chris[4].

Thanks Tvrtko, Chris, Antonio and Petri for your comments in the
previous RFCs.

Andi

v8 --> v9
=========
RFC v8: [10]
quite many changes, please refer to the review in [10]. Thanks
Chris for the review. These are the most relevant:
- all the allocation in gem_query have been made in stack, not
anymore allocated dynamically.
- removed get/set_context as it was already implemented and I
didn't know.
- renamed some functions and variables to hopefully more
meaningful names.

V7 --> v8
=========
RFC v7: [9]
- all functions have been moved from lib/igt_gt.{c,h} and
lib/ioctl_wrappers.{c,h} to lib/i916/gem_query.{c,h}. (thanks
Chris)
- 'for_each_engine_ctx' has been renamed to 'for_each_engine2' to
be consistent with the '2' that indicates the new 'struct
intel_execution_engine2' data structure.

V6 --> V7
=========
RFC v6: [8]
- a new patch has been added (patch 3) which adds a new
requirement check through the igt_require_gem_engine_list()
function. (thanks Chris) This function will initialize the
engine list instead of the instead of igt_require_gem() as it
was in v6
- all the ioctls have been wrapped (thanks Chris and Antonio) and
new library functions have been added and assert the ioctls
- gem_init_engine_list() function returns the errno from the
GETPARAM ioctl in order to be used as a requirement. (thanks
Chris)
- fixed few requires/asserts
- The engine list "intel_active_engines2" is allocated of the
number of engines instead of a political 64 (thanks Antonio).
- some parameter renaming in gem_has_ring_by_idx(). (thanks
Chris).
- the original "intel_execution_engines2" has not been renamed,
because it is used to create subtests before even executing any
test/ioctl. By renaming it, some subtest generations failed.
(thanks Petri)

V5 --> V6
=========
RFC v5: [7]
- Chris implemented the getparam ioctl which allows to the test
to figure otu whether the new interface has been implemented.
This way the for_each_engine_ctx() is able to work with new and
old kernel uapi (thanks Chris)

V4 --> V5
=========
RFC v4: [6]
- the engine list is now built in 'igt_require_gem()' instead of
'__open_driver()' so that we keep this discovery method
specific to the i915 driver (thanks Chris).
- All the query/setparam structures dynamic allocation based on
the number of engines, now are politically allocated 64 times,
to avoid extra ioctl calls that retrieve the engine number
(thanks Chris)
- use igt_ioctl instead of ioctl (thanks Chris)
- allocate intel_execution_engines2 dynamically instead of
statically (thanks Tvrtko)
- simplify the test in 'gem_exec_basic()' so that simply checks
the presence of the engine instead of executing a buffer
(thank Chris)
- a new patch has been added (patch 3) that extends the
'gem_has_ring()' boolean function. The new version sets the
index as it's mapped in the kernel.The previous function is now
a wrapper to the new function.

V3 --> V4
=========
PATCH v3: [3]
- re-architectured the discovery mechanism based on Tvrtko's
sugestion and reviews.. In this version the discovery is done
during the device opening and stored in a NULL terminated
array, which replaces the existing intel_execution_engines2
that is mainly used as a reference.

V2 --> V3
=========
RFC v2: [2]
- removed a standalone gem_query_engines_demo test and added the
exec-ctx subtest inside gem_exec_basic (thanks Tvrtko).
- fixed most of Tvrtko's comments in [5], which consist in
putting the mallocs igt_assert and ictls in igt_require and few
refactoring (thanks Tvrtko).

V1 --> V2
=========
RFC v1: [1]
- added a demo test that simply queries the driver about the
engines and executes a buffer (thanks Tvrtko)
- refactored the for_each_engine_ctx() macro so that what in the
previous version was done by the "bind" function, now it's done
in the first iteration. (Thanks Crhis)
- removed the "gem_has_ring_ctx()" because it was out of the
scope.
- rename functions to more meaningful names

[1] RFC v1: https://lists.freedesktop.org/archives/igt-dev/2018-November/007025.html
[2] RFC v2: https://lists.freedesktop.org/archives/igt-dev/2018-November/007079.html
[3] PATCH v3: https://lists.freedesktop.org/archives/igt-dev/2018-November/007148.html
[4] https://cgit.freedesktop.org/~tursulin/drm-intel/log/?h=media
[5] https://lists.freedesktop.org/archives/igt-dev/2018-November/007100.html
[6] https://lists.freedesktop.org/archives/igt-dev/2019-January/008029.html
[7] https://lists.freedesktop.org/archives/igt-dev/2019-January/008165.html
[8] https://lists.freedesktop.org/archives/igt-dev/2019-February/008902.html
[9] https://lists.freedesktop.org/archives/igt-dev/2019-February/009185.html
[10] https://lists.freedesktop.org/archives/igt-dev/2019-February/009205.html

Andi Shyti (5):
include/drm-uapi: import i915_drm.h header file
lib/i915: add gem_query library
lib/igt_gt: use for_each_engine2 to loop through engines
lib: ioctl_wrappers: reach engines by index as well
tests: gem_exec_basic: add "exec-ctx" buffer execution demo test

include/drm-uapi/i915_drm.h | 292 +++++++++++++++++++++++++++---------
lib/Makefile.sources | 2 +
lib/i915/gem_query.c | 184 +++++++++++++++++++++++
lib/i915/gem_query.h | 38 +++++
lib/igt_gt.c | 2 +-
lib/igt_gt.h | 10 +-
lib/ioctl_wrappers.c | 4 +-
lib/ioctl_wrappers.h | 4 +-
lib/meson.build | 1 +
tests/i915/gem_exec_basic.c | 15 ++
10 files changed, 477 insertions(+), 75 deletions(-)
create mode 100644 lib/i915/gem_query.c
create mode 100644 lib/i915/gem_query.h

--
2.20.1