[igt-dev] [PATCH v11 0/6] new engine discovery interface

[Andi Shyti <andi.shyti@intel.com>]

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.

@Tvrtko: your last comments have added items to my todo list, I
answered to your comments in the patches :)

Andi

v10 --> v11
===========
RFC v10: [12]
few cosmetic changes in v11 and minor architectural details.
Thanks Tvrtko.
- the 'query_engines()' functions are static as no one is using
  them yet.
- removed the 'gem_has_engine_topology()' function because it's
  very little used, 'get_active_engines()' can be used instead.
- a minor ring -> engine renaming coming from Chris. 

v9 --> v10
==========
RFC v9: [11]
also this time quite many changes, thanks Chris for the reviews,
here the most relevant of them.
- gem_query.[ch] have been renamed to gem_engine_topology.[ch]
  and all the functions ended up there as they are referring to
  the topology of the engines.
- the functions 'get_active_engines()',
  'gem_set_context_get_engines()' and
  'igt_require_gem_engine_list()' will be the main interface to
  the gem_engine_topology library, refer to patch 2 for details.
- the define 'for_each_engine2()' doesn't expose anymore the
  iterator.
- 'gem_context_has_engine()' has been moved from ioctl_wrappers.c
  to gem_context.c.
- the gem_exec_basic exec-ctx subtest does not abort if the new
  getparam/setparam and query apis are not implemented as it can
  work with both (as it was done at the beginning).

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
[11] https://lists.freedesktop.org/archives/igt-dev/2019-February/009277.html
[12] https://lists.freedesktop.org/archives/igt-dev/2019-March/010197.html

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

 include/drm-uapi/i915_drm.h    | 404 +++++++++++++++++++++++++++------
 lib/Makefile.sources           |   2 +
 lib/i915/gem_context.c         |  21 ++
 lib/i915/gem_context.h         |   2 +
 lib/i915/gem_engine_topology.c | 193 ++++++++++++++++
 lib/i915/gem_engine_topology.h |  33 +++
 lib/igt_gt.c                   |   2 +-
 lib/igt_gt.h                   |  10 +-
 lib/ioctl_wrappers.c           |  19 --
 lib/ioctl_wrappers.h           |   3 +-
 lib/meson.build                |   1 +
 tests/i915/gem_exec_basic.c    |  12 +
 12 files changed, 607 insertions(+), 95 deletions(-)
 create mode 100644 lib/i915/gem_engine_topology.c
 create mode 100644 lib/i915/gem_engine_topology.h

-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev