[igt-dev] [PATCH i-g-t v3 2/2] i915/gem_ctx_isolation:: change semantics of ctx isolation uAPI
Petri Latvala
petri.latvala at intel.com
Thu Sep 1 12:33:14 UTC 2022
On Fri, Aug 19, 2022 at 08:52:18PM +0100, Adrian Larumbe wrote:
> ioctl I915_PARAM_HAS_CONTEXT_ISOLATION param is meant to report whether all
> the engines in the system support context isolation, but the way the return
> value was being used did not respect the contract on the uAPI.
>
> Skip all engine tests for which context isolation is not supported. Also skip
> tests that involve an engine reset if both RCS and CCS engines are present in
> the system. This is because they belong to the same reset domain.
>
> Signed-Off-By: Adrian Larumbe <adrian.larumbe at collabora.com>
> ---
> include/drm-uapi/i915_drm.h | 6 +-
> tests/i915/gem_ctx_isolation.c | 100 +++++++++++++++++++++++----------
> 2 files changed, 72 insertions(+), 34 deletions(-)
>
> diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
> index b4efc96c2edc..0b8fa57b52f9 100644
> --- a/include/drm-uapi/i915_drm.h
> +++ b/include/drm-uapi/i915_drm.h
> @@ -691,11 +691,7 @@ typedef struct drm_i915_irq_wait {
> * rather than default HW values. If true, it also ensures (insofar as HW
> * supports) that all state set by this context will not leak to any other
> * context.
> - *
> - * As not every engine across every gen support contexts, the returned
> - * value reports the support of context isolation for individual engines by
> - * returning a bitmask of each engine class set to true if that class supports
> - * isolation.
> + *
Don't make edits to the drm-uapi files. They need to be direct copies from the kernel.
--
Petri Latvala
> */
> #define I915_PARAM_HAS_CONTEXT_ISOLATION 50
>
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 4233ea5784dc..273a0c9708bb 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -45,6 +45,7 @@ enum {
> VCS2 = ENGINE(I915_ENGINE_CLASS_VIDEO, 2),
> VCS3 = ENGINE(I915_ENGINE_CLASS_VIDEO, 3),
> VECS0 = ENGINE(I915_ENGINE_CLASS_VIDEO_ENHANCE, 0),
> + CCS0 = ENGINE(I915_ENGINE_CLASS_COMPUTE, 0),
> };
>
> #define ALL ~0u
> @@ -627,9 +628,40 @@ static void compare_regs(int fd, const struct intel_execution_engine2 *e,
> num_errors, who);
> }
>
> +static bool
> +engine_has_context_isolation(const struct intel_execution_engine2 *e,
> + int gen)
> +{
> + if (gen > 8)
> + return true;
> +
> + if (gen >= 6 && gen <= 8 && e->class == I915_ENGINE_CLASS_RENDER)
> + return true;
> +
> + return false;
> +}
> +
> +static bool
> +has_engine_class(const intel_ctx_cfg_t *cfg, unsigned int class)
> +{
> + const struct i915_engine_class_instance *eci;
> + unsigned int i;
> +
> + igt_require(class <= I915_ENGINE_CLASS_COMPUTE);
> +
> + for (i = 0; i < cfg->num_engines; i++) {
> + eci = &cfg->engines[i];
> + if (eci->engine_class == class)
> + return true;
> + }
> +
> + return false;
> +}
> +
> static void nonpriv(int fd, const intel_ctx_cfg_t *cfg,
> const struct intel_execution_engine2 *e,
> - unsigned int flags)
> + unsigned int flags,
> + int gen)
> {
> static const uint32_t values[] = {
> 0x0,
> @@ -647,6 +679,7 @@ static void nonpriv(int fd, const intel_ctx_cfg_t *cfg,
>
> /* Sigh -- hsw: we need cmdparser access to our own registers! */
> igt_skip_on(intel_gen(intel_get_drm_devid(fd)) < 8);
> + igt_require(engine_has_context_isolation(e, gen));
>
> gem_quiescent_gpu(fd);
>
> @@ -729,7 +762,8 @@ static void nonpriv(int fd, const intel_ctx_cfg_t *cfg,
>
> static void isolation(int fd, const intel_ctx_cfg_t *cfg,
> const struct intel_execution_engine2 *e,
> - unsigned int flags)
> + unsigned int flags,
> + int gen)
> {
> static const uint32_t values[] = {
> 0x0,
> @@ -743,6 +777,8 @@ static void isolation(int fd, const intel_ctx_cfg_t *cfg,
> unsigned int num_values =
> flags & (DIRTY1 | DIRTY2) ? ARRAY_SIZE(values) : 1;
>
> + igt_require(engine_has_context_isolation(e, gen));
> +
> gem_quiescent_gpu(fd);
>
> for (int v = 0; v < num_values; v++) {
> @@ -865,7 +901,8 @@ static void inject_reset_context(int fd, const intel_ctx_cfg_t *cfg,
>
> static void preservation(int fd, const intel_ctx_cfg_t *cfg,
> const struct intel_execution_engine2 *e,
> - unsigned int flags)
> + unsigned int flags,
> + int gen)
> {
> static const uint32_t values[] = {
> 0x0,
> @@ -882,6 +919,8 @@ static void preservation(int fd, const intel_ctx_cfg_t *cfg,
> uint64_t ahnd[num_values + 1];
> igt_spin_t *spin;
>
> + igt_require(engine_has_context_isolation(e, gen));
> +
> gem_quiescent_gpu(fd);
>
> ctx[num_values] = intel_ctx_create(fd, cfg);
> @@ -902,8 +941,12 @@ static void preservation(int fd, const intel_ctx_cfg_t *cfg,
> gem_close(fd, read_regs(fd, ahnd[num_values], ctx[num_values], e, flags));
> igt_spin_free(fd, spin);
>
> - if (flags & RESET)
> + if (flags & RESET) {
> + igt_skip_on(has_engine_class(cfg, I915_ENGINE_CLASS_RENDER) &&
> + has_engine_class(cfg, I915_ENGINE_CLASS_COMPUTE));
> +
> inject_reset_context(fd, cfg, e);
> + }
>
> switch (flags & SLEEP_MASK) {
> case NOSLEEP:
> @@ -962,7 +1005,7 @@ static unsigned int __has_context_isolation(int fd)
> int value = 0;
>
> memset(&gp, 0, sizeof(gp));
> - gp.param = 50; /* I915_PARAM_HAS_CONTEXT_ISOLATION */
> + gp.param = I915_PARAM_HAS_CONTEXT_ISOLATION;
> gp.value = &value;
>
> igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
> @@ -971,21 +1014,19 @@ static unsigned int __has_context_isolation(int fd)
> return value;
> }
>
> -#define test_each_engine(e, i915, cfg, mask) \
> +#define test_each_engine(e, i915, cfg) \
> for_each_ctx_cfg_engine(i915, cfg, e) \
> - for_each_if(mask & (1 << (e)->class)) \
> - igt_dynamic_f("%s", (e)->name)
> + igt_dynamic_f("%s", (e)->name)
>
> igt_main
> {
> - unsigned int has_context_isolation = 0;
> + bool has_context_isolation = 0;
> const struct intel_execution_engine2 *e;
> intel_ctx_cfg_t cfg;
> int i915 = -1;
> + int gen;
>
> igt_fixture {
> - int gen;
> -
> i915 = drm_open_driver(DRIVER_INTEL);
> igt_require_gem(i915);
> igt_require(gem_has_contexts(i915));
> @@ -995,6 +1036,7 @@ igt_main
> igt_require(has_context_isolation);
>
> gen = intel_gen(intel_get_drm_devid(i915));
> + igt_require(gen >= 2);
>
> igt_warn_on_f(gen > LAST_KNOWN_GEN,
> "GEN not recognized! Test needs to be updated to run.\n");
> @@ -1006,43 +1048,43 @@ igt_main
> }
>
> igt_subtest_with_dynamic("nonpriv") {
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - nonpriv(i915, &cfg, e, 0);
> + test_each_engine(e, i915, &cfg)
> + nonpriv(i915, &cfg, e, 0, gen);
> }
>
> igt_subtest_with_dynamic("nonpriv-switch") {
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - nonpriv(i915, &cfg, e, DIRTY2);
> + test_each_engine(e, i915, &cfg)
> + nonpriv(i915, &cfg, e, DIRTY2, gen);
> }
>
> igt_subtest_with_dynamic("clean") {
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - isolation(i915, &cfg, e, 0);
> + test_each_engine(e, i915, &cfg)
> + isolation(i915, &cfg, e, 0, gen);
> }
>
> igt_subtest_with_dynamic("dirty-create") {
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - isolation(i915, &cfg, e, DIRTY1);
> + test_each_engine(e, i915, &cfg)
> + isolation(i915, &cfg, e, DIRTY1, gen);
> }
>
> igt_subtest_with_dynamic("dirty-switch") {
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - isolation(i915, &cfg, e, DIRTY2);
> + test_each_engine(e, i915, &cfg)
> + isolation(i915, &cfg, e, DIRTY2, gen);
> }
>
> igt_subtest_with_dynamic("preservation") {
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - preservation(i915, &cfg, e, 0);
> + test_each_engine(e, i915, &cfg)
> + preservation(i915, &cfg, e, 0, gen);
> }
>
> igt_subtest_with_dynamic("preservation-S3") {
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - preservation(i915, &cfg, e, S3);
> + test_each_engine(e, i915, &cfg)
> + preservation(i915, &cfg, e, S3, gen);
> }
>
> igt_subtest_with_dynamic("preservation-S4") {
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - preservation(i915, &cfg, e, S4);
> + test_each_engine(e, i915, &cfg)
> + preservation(i915, &cfg, e, S4, gen);
> }
>
> igt_fixture {
> @@ -1052,8 +1094,8 @@ igt_main
> igt_subtest_with_dynamic("preservation-reset") {
> igt_hang_t hang = igt_allow_hang(i915, 0, 0);
>
> - test_each_engine(e, i915, &cfg, has_context_isolation)
> - preservation(i915, &cfg, e, RESET);
> + test_each_engine(e, i915, &cfg)
> + preservation(i915, &cfg, e, RESET, gen);
>
> igt_disallow_hang(i915, hang);
> }
> --
> 2.37.0
>
More information about the igt-dev
mailing list