[igt-dev] [PATCH i-g-t 00/93] Stop depending on context mutation (v4)
Jason Ekstrand
jason at jlekstrand.net
Wed Jun 9 17:35:43 UTC 2021
I'm trying to clean up some of our uAPI technical debt in i915. One of the
biggest areas we have right now is context mutability. There's no good
reason why things like the set of engines or the VM should be able to be
changed on the fly and no "real" userspace actually relies on this
functionality. It does, however, make for a good excuse for tests and lots
of bug reports as things like swapping out the set of engines under load
break randomly. The solution here is to stop allowing that behavior and
simplify the i915 internals.
In particular, we'd like to remove the following from the i915 API:
1. I915_CONTEXT_CLONE_*. These are only used by IGT and have never been
used by any "real" userspace.
2. Changing the VM or set of engines via SETPARAM after they've been
"used" by an execbuf or similar. This would effectively make those
parameters create params rather than mutable state. We can't drop
setparam entirely for those because media does use it but we can
enforce some rules.
3. Unused (by non-IGT userspace) GETPARAM for things like engines.
As much as we'd love to do that, we have a bit of a problem in IGT. The
way we handle multi-engine testing today relies heavily on this soon-to-be-
deprecated functionality. In particular, the standard flow is usually
something like this:
static void run_test1(int fd, uint32_t engine)
{
igt_spin_t *spin;
ctx = = gem_context_clone_with_engines(fd, 0);
__igt_spin_new(fd, ctx, .engine = engine);
/* do some testing with ctx */
igt_spin_free(fd, spin);
gem_destroy_context(fd, ctx);
}
igt_main
{
struct intel_execution_engine2 *e;
/* Usual fixture code */
__for_each_physical_engine(fd, e)
run_test1(fd, e->flags);
__for_each_physical_engine(fd, e)
run_test2(fd, e->flags);
}
Let's walk through what this does:
1. __for_each_physical_engine calls intel_init_engine_list() which resets
the set of engines on ctx0 to the full set of engines available as per
the engine query. On older kernels/hardware where we don't have the
engines query, it leaves the set alone.
2. intel_init_engine_list() also returns a set of engines for iteration
and __for_each_physical_engine() sets up a for loop to walk the set.
3. gem_context_clone_with_engines() creates a new context using
I915_CONTEXT_CONTEXT_CLONE_ENGINES (not used by anything other than
IGT) to ask that the newly created context has the same set of engines
as ctx0. Remember we changed that at the start of loop iteration!
4. When the context is passed to __igt_spin_new(), it calls
gem_context_lookup_engine which does a GETPARAM to introspet the set of
engines on the context and figure out the engine class.
If you've been keeping track, this trivial and extremely common example
uses every single one of these soon-to-be-deprecated APIs even though the
test author may be completely obvious to it. It also means that getting
rid of IGT's use of them is going to require some fairly deep surgery.
The approach proposed and partially implemented here is to add a new
wrapper struct intel_ctx_t which wraps a GEM context handle as well as the
full set of parameters used to create it, represented by intel_ctx_cfg_t.
We can then use the context anywhere we would regularly use a context, we
just have to do ctx->id. If we want to clone it, we can do so by re-using
the create parameters by calling intel_ctx_create(fd, &old_ctx->cfg);
Along with the above rework (which got long, sorry) I've got a few other
patches in here which delete tests which exist expressly to test APIs that
are on the chopping block.
v2 (Jason Ekstrand):
- Better documentation
- Two more patches at the end to clean up query APIs even more
v3 (Jason Ekstrand):
- Improved intel_ctx interface
- More tests converted to intel_ctx_t
- Misc. bug fixes
v4 (Jason Ekstrand):
- Rebased
- Add tests for new error conditions
v4 (Jason Ekstrand):
- Apply review feedback from Zbigniew
- Fix some issues caused by updates to the kernel series
--Jason
Jason Ekstrand (93):
tests/i915: Drop gem_ctx_ringsize
tests/i915/gem_exec_balancer: Drop the ringsize subtest
tests/i915/gem_exec_endless: Stop setting the ring size
tests/i915/gem_ctx_param: Drop the zeromap subtests
tests/i915: Drop gem_ctx_clone
docs: Add gem_engine_topology.h to the docs
lib/i915/gem_engine_topology: Expose the __query_engines helper (v2)
lib/i915/gem_context: Add gem_context_create_ext helpers
lib: Add an intel_ctx wrapper struct and helpers (v5)
lib/i915/gem_engine_topology: Rework query_engine_list()
lib/i915/gem_engine_topology: Factor out static engine listing
lib/i915/gem_engine_topology: Add an iterator which doesn't munge
contexts (v2)
lib/i915/gem_engine_topology: Add an iterator for intel_ctx_t (v3)
tests/i915/gem_exec_basic: Convert to intel_ctx_t (v2)
lib/dummyload: Better document igt_spin_factory
lib/dummyload: Rename igt_spin_factory::ctx to ctx_id
lib/dummyload: Support intel_ctx_t (v2)
tests/i915/gem_exec_fence: Move the engine data into
inter_engine_context
tests/i915/gem_exec_fence: Convert to intel_ctx_t
tests/i915/gem_exec_schedule: Convert to intel_ctx_t
tests/i915/perf_pmu: Convert to intel_ctx_t
tests/i915/gem_exec_nop: Convert to intel_ctx_t
tests/i915/gem_exec_reloc: Convert to intel_ctx_t
tests/i915/gem_busy: Convert to intel_ctx_t
tests/i915/gem_ctx_isolation: Convert to intel_ctx_t
tests/i915/gem_exec_async: Convert to intel_ctx_t
tests/i915/sysfs_clients: Convert to intel_ctx_t
tests/i915/gem_exec_fair: Convert to intel_ctx_t
tests/i915/gem_spin_batch: Convert to intel_ctx_t
tests/i915/gem_exec_store: Convert to intel_ctx_t
tests/amdgpu/amd_prime: Convert to intel_ctx_t
tests/i915/i915_hangman: Convert to intel_ctx_t
tests/i915/gem_ringfill: Convert to intel_ctx_t
tests/prime_busy: Convert to intel_ctx_t
tests/prime_vgem: Convert to intel_ctx_t
tests/gem_exec_whisper: Convert to intel_ctx_t
tests/i915/gem_ctx_exec: Stop cloning contexts in close_race
tests/i915/gem_ctx_exec: Convert to intel_ctx_t
tests/i915/gem_exec_suspend: Convert to intel_ctx_t
tests/i915/gem_sync: Convert to intel_ctx_t
tests/i915/gem_userptr_blits: Convert to intel_ctx_t
tests/i915/gem_wait: Convert to intel_ctx_t
tests/i915/gem_request_retire: Convert to intel_ctx_t
tests/i915/gem_ctx_shared: Convert to intel_ctx_t
tests/i915/gem_ctx_shared: Stop cloning contexts
tests/i915/gem_create: Convert to intel_ctx_t
tests/i915/gem_ctx_switch: Convert to intel_ctx_t
tests/i915/gem_exec_parallel: Convert to intel_ctx_t
tests/i915/gem_exec_latency: Convert to intel_ctx_t
tests/i915/gem_watchdog: Convert to intel_ctx_t
tests/i915/gem_shrink: Convert to intel_ctx_t
tests/i915/gem_exec_params: Convert to intel_ctx_t
tests/i915/gem_exec_gttfill: Convert to intel_ctx_t
tests/i915/gem_exec_capture: Convert to intel_ctx_t
tests/i915/gem_exec_create: Convert to intel_ctx_t
tests/i915/gem_exec_await: Convert to intel_ctx_t
tests/i915/gem_ctx_persistence: Drop the clone subtest
tests/i915/gem_ctx_persistence: Drop the engine replace subtests
tests/i915/gem_ctx_persistence: Convert to intel_ctx_t
tests/i915/module_load: Convert to intel_ctx_t
tests/i915/pm_rc6_residency: Convert to intel_ctx_t
tests/i915/gem_cs_tlb: Convert to intel_ctx_t
tests/core_hotplug: Convert to intel_ctx_t
tests/i915/gem_exec_balancer: Stop cloning engines
tests/i915/gem_exec_balancer: Don't reset engines on a context
tests/i915/gem_exec_balancer: Stop munging ctx0 engines
tests/i915/gem_exec_balancer: Drop bonded tests
lib/intel_ctx: Add load balancing support (v2)
tests/i915/gem_exec_balancer: Convert to intel_ctx_t
tests/i915/gem_exec_endless: Stop munging ctx0 engines
lib/i915: Use for_each_physical_ring for submission tests
tests/i915/gem_ctx_engines: Rework execute-one*
tests/i915/gem_ctx_engines: Use better engine iteration
tests/i915/gem_ctx_engines: Drop the idempotent subtest
tests/i915/gem_ctx_create: Convert benchmarks to intel_ctx_t
tests/i915/gem_vm_create: Delete destroy racing tests
tests/i915/gem_vm_create: Use intel_ctx_t in the execbuf test
tests/i915/sysfs: Convert to intel_ctx_t
tests/i915/gem_workarounds: Convert to intel_ctx_t
lib/i915/gem_context: Delete all the context clone/copy stuff
tests/i915/gem_ctx_engines: Delete the libapi subtest
lib/igt_dummyload: Stop supporting ALL_ENGINES without an intel_ctx_t
lib/i915/gem_engine_topology: Delete the old physical engine iterators
tests/i915/gem_mmap_gtt: Convert to intel_ctx_t
igt/dummyload: Require an intel_ctx_t for POLL_RUN and !ALL_ENGINES
lib/i915: Rework engine API availability checks (v2)
lib/intel_bb: Remove intel_bb_assign_vm and tests (v2)
tests/i915/gem_ctx_param: Stop setting VMs on old contexts
tests/i915/gen9_exec_parse: Convert to intel_ctx_t
tests/i915/gem_ctx_param: Add tests for recently removed params
tests/i915/gem_ctx_param: Add a couple invalid PARAM_VM cases
tests/i915/gem_ctx_engines: Fix the invalid subtest for the new rules
tests/i915/gem_exec_balancer: Add a test for combind balancing and
bonding
.../igt-gpu-tools/igt-gpu-tools-docs.xml | 2 +
lib/i915/gem_context.c | 238 ++---
lib/i915/gem_context.h | 20 +-
lib/i915/gem_engine_topology.c | 278 +++---
lib/i915/gem_engine_topology.h | 78 +-
lib/i915/gem_submission.c | 13 +-
lib/igt_dummyload.c | 25 +-
lib/igt_dummyload.h | 27 +-
lib/igt_gt.c | 2 +-
lib/intel_batchbuffer.c | 65 --
lib/intel_batchbuffer.h | 4 -
lib/intel_ctx.c | 310 ++++++
lib/intel_ctx.h | 61 ++
lib/meson.build | 1 +
tests/amdgpu/amd_prime.c | 10 +-
tests/core_hotunplug.c | 6 +-
tests/i915/api_intel_bb.c | 104 --
tests/i915/gem_busy.c | 77 +-
tests/i915/gem_create.c | 14 +-
tests/i915/gem_cs_tlb.c | 10 +-
tests/i915/gem_ctx_clone.c | 451 ---------
tests/i915/gem_ctx_create.c | 76 +-
tests/i915/gem_ctx_engines.c | 356 ++-----
tests/i915/gem_ctx_exec.c | 54 +-
tests/i915/gem_ctx_isolation.c | 112 ++-
tests/i915/gem_ctx_param.c | 108 ++-
tests/i915/gem_ctx_persistence.c | 457 +++------
tests/i915/gem_ctx_ringsize.c | 345 -------
tests/i915/gem_ctx_shared.c | 335 ++++---
tests/i915/gem_ctx_switch.c | 115 ++-
tests/i915/gem_eio.c | 2 +-
tests/i915/gem_exec_async.c | 32 +-
tests/i915/gem_exec_await.c | 20 +-
tests/i915/gem_exec_balancer.c | 913 +++++-------------
tests/i915/gem_exec_basic.c | 7 +-
tests/i915/gem_exec_capture.c | 30 +-
tests/i915/gem_exec_create.c | 9 +-
tests/i915/gem_exec_endless.c | 14 +-
tests/i915/gem_exec_fair.c | 112 ++-
tests/i915/gem_exec_fence.c | 303 +++---
tests/i915/gem_exec_gttfill.c | 15 +-
tests/i915/gem_exec_latency.c | 118 ++-
tests/i915/gem_exec_nop.c | 156 +--
tests/i915/gem_exec_parallel.c | 29 +-
tests/i915/gem_exec_params.c | 4 +-
tests/i915/gem_exec_reloc.c | 102 +-
tests/i915/gem_exec_schedule.c | 887 +++++++++--------
tests/i915/gem_exec_store.c | 36 +-
tests/i915/gem_exec_suspend.c | 52 +-
tests/i915/gem_exec_whisper.c | 83 +-
tests/i915/gem_mmap_gtt.c | 18 +-
tests/i915/gem_request_retire.c | 17 +-
tests/i915/gem_ringfill.c | 48 +-
tests/i915/gem_shrink.c | 37 +-
tests/i915/gem_spin_batch.c | 79 +-
tests/i915/gem_sync.c | 159 +--
tests/i915/gem_userptr_blits.c | 25 +-
tests/i915/gem_vm_create.c | 125 +--
tests/i915/gem_wait.c | 20 +-
tests/i915/gem_watchdog.c | 167 +---
tests/i915/gem_workarounds.c | 11 +-
tests/i915/gen9_exec_parse.c | 106 +-
tests/i915/i915_hangman.c | 37 +-
tests/i915/i915_module_load.c | 7 +-
tests/i915/i915_pm_rc6_residency.c | 7 +-
tests/i915/i915_query.c | 17 +-
tests/i915/perf_pmu.c | 226 +++--
tests/i915/sysfs_clients.c | 87 +-
tests/i915/sysfs_heartbeat_interval.c | 40 +-
tests/i915/sysfs_preempt_timeout.c | 39 +-
tests/i915/sysfs_timeslice_duration.c | 51 +-
tests/meson.build | 2 -
tests/prime_busy.c | 19 +-
tests/prime_vgem.c | 35 +-
74 files changed, 3454 insertions(+), 4603 deletions(-)
create mode 100644 lib/intel_ctx.c
create mode 100644 lib/intel_ctx.h
delete mode 100644 tests/i915/gem_ctx_clone.c
delete mode 100644 tests/i915/gem_ctx_ringsize.c
--
2.31.1
More information about the igt-dev
mailing list