All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
@ 2019-10-17 14:30 ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-10-17 14:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev

Dynamic subtests!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/i915/gem_context.c                |  40 +++++
 lib/i915/gem_context.h                |   2 +
 tests/Makefile.sources                |   1 +
 tests/i915/sysfs_preemption_timeout.c | 203 ++++++++++++++++++++++++++
 tests/meson.build                     |   1 +
 5 files changed, 247 insertions(+)
 create mode 100644 tests/i915/sysfs_preemption_timeout.c

diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
index 1fae5191f..a627a5c7b 100644
--- a/lib/i915/gem_context.c
+++ b/lib/i915/gem_context.c
@@ -403,3 +403,43 @@ bool gem_context_has_engine(int fd, uint32_t ctx, uint64_t engine)
 
 	return __gem_execbuf(fd, &execbuf) == -ENOENT;
 }
+
+static int create_ext_ioctl(int i915,
+			    struct drm_i915_gem_context_create_ext *arg)
+{
+	int err;
+
+	err = 0;
+	if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
+		err = -errno;
+		igt_assume(err);
+	}
+
+	errno = 0;
+	return err;
+}
+
+uint32_t gem_context_create_for_engine(int i915, unsigned int class, unsigned int inst)
+{
+	I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = {
+		.engines = { { .engine_class = class, .engine_instance = inst } }
+	};
+	struct drm_i915_gem_context_create_ext_setparam p_engines = {
+		.base = {
+			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
+			.next_extension = 0, /* end of chain */
+		},
+		.param = {
+			.param = I915_CONTEXT_PARAM_ENGINES,
+			.value = to_user_pointer(&engines),
+			.size = sizeof(engines),
+		},
+	};
+	struct drm_i915_gem_context_create_ext create = {
+		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
+		.extensions = to_user_pointer(&p_engines),
+	};
+
+	igt_assert_eq(create_ext_ioctl(i915, &create), 0);
+	return create.ctx_id;
+}
diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
index c0d4c9615..9e0a083f0 100644
--- a/lib/i915/gem_context.h
+++ b/lib/i915/gem_context.h
@@ -34,6 +34,8 @@ int __gem_context_create(int fd, uint32_t *ctx_id);
 void gem_context_destroy(int fd, uint32_t ctx_id);
 int __gem_context_destroy(int fd, uint32_t ctx_id);
 
+uint32_t gem_context_create_for_engine(int fd, unsigned int class, unsigned int inst);
+
 int __gem_context_clone(int i915,
 			uint32_t src, unsigned int share,
 			unsigned int flags,
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 093eb57f3..dff7dac06 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -98,6 +98,7 @@ TESTS_progs = \
 	tools_test \
 	vgem_basic \
 	vgem_slow \
+	i915/sysfs_preemption_timeout \
 	$(NULL)
 
 TESTS_progs += gem_bad_reloc
diff --git a/tests/i915/sysfs_preemption_timeout.c b/tests/i915/sysfs_preemption_timeout.c
new file mode 100644
index 000000000..a798345b1
--- /dev/null
+++ b/tests/i915/sysfs_preemption_timeout.c
@@ -0,0 +1,203 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "drmtest.h" /* gem_quiescent_gpu()! */
+#include "igt_dummyload.h"
+#include "igt_sysfs.h"
+#include "ioctl_wrappers.h" /* igt_require_gem()! */
+#include "sw_sync.h"
+
+#include "igt_debugfs.h"
+
+static bool __enable_hangcheck(int dir, bool state)
+{
+	return igt_sysfs_set(dir, "enable_hangcheck", state ? "1" : "0");
+}
+
+static bool enable_hangcheck(int i915, bool state)
+{
+	bool success;
+	int dir;
+
+	dir = igt_sysfs_open_parameters(i915);
+	if (dir < 0) /* no parameters, must be default! */
+		return false;
+
+	success = __enable_hangcheck(dir, state);
+	close(dir);
+
+	return success;
+}
+
+static void test_idempotent(int i915, int engine)
+{
+	unsigned int saved, delay;
+
+	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
+	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", 1);
+	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
+	igt_assert_eq(delay, 1);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
+	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
+	igt_assert_eq(delay, saved);
+}
+
+static void test_invalid(int i915, int engine)
+{
+	unsigned int saved, delay;
+
+	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
+	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", PRIu64, -1);
+	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
+	igt_assert_eq(delay, saved);
+}
+
+static uint64_t __test_timeout(int i915, int engine, unsigned int timeout)
+{
+	unsigned int class, inst;
+	struct timespec ts = {};
+	igt_spin_t *spin[2];
+	uint64_t elapsed;
+	uint32_t ctx;
+
+	igt_assert(igt_sysfs_scanf(engine, "class", "%u", &class) == 1);
+	igt_assert(igt_sysfs_scanf(engine, "instance", "%u", &inst) == 1);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", timeout);
+
+	ctx = gem_context_create_for_engine(i915, class, inst);
+	gem_context_set_priority(i915, ctx, -1023);
+	spin[0] = igt_spin_new(i915, ctx,
+			       .flags = (IGT_SPIN_NO_PREEMPTION |
+					 IGT_SPIN_POLL_RUN |
+					 IGT_SPIN_FENCE_OUT));
+	igt_spin_busywait_until_started(spin[0]);
+	gem_context_destroy(i915, ctx);
+
+	ctx = gem_context_create_for_engine(i915, class, inst);
+	gem_context_set_priority(i915, ctx, 1023);
+	igt_nsec_elapsed(&ts);
+	spin[1] = igt_spin_new(i915, ctx, .flags = IGT_SPIN_POLL_RUN);
+	igt_spin_busywait_until_started(spin[1]);
+	elapsed = igt_nsec_elapsed(&ts);
+	gem_context_destroy(i915, ctx);
+
+	igt_assert_eq(sync_fence_status(spin[0]->out_fence), -EIO);
+
+	igt_spin_free(i915, spin[1]);
+	igt_spin_free(i915, spin[0]);
+	gem_quiescent_gpu(i915);
+
+	return elapsed;
+}
+
+static void test_timeout(int i915, int engine)
+{
+	int delays[] = { 1, 50, 100, 500 };
+	unsigned int saved, delay;
+
+	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
+	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
+
+	gem_quiescent_gpu(i915);
+	igt_require(enable_hangcheck(i915, false));
+
+	for (int i = 0; i < ARRAY_SIZE(delays); i++) {
+		uint64_t elapsed;
+
+		elapsed = __test_timeout(i915, engine, delays[i]);
+		igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
+			 delays[i], elapsed * 1e-6);
+	}
+
+	igt_assert(enable_hangcheck(i915, true));
+	gem_quiescent_gpu(i915);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
+	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
+	igt_assert_eq(delay, saved);
+}
+
+igt_main
+{
+	int i915, sys = -1;
+	struct dirent *de;
+	int engines;
+	DIR *dir;
+
+	igt_fixture {
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(i915);
+
+		sys = igt_sysfs_open(i915);
+		igt_require(sys != -1);
+	}
+
+	engines = openat(sys, "engine", O_RDONLY);
+	dir = fdopendir(engines);
+	while (dir && (de = readdir(dir))) {
+		int engine = openat(engines, de->d_name, O_RDONLY);
+		struct stat st;
+		char *name;
+
+		name = igt_sysfs_get(engine, "name");
+		if (!name)
+			continue;
+
+		igt_subtest_group {
+			igt_fixture {
+				igt_require(fstatat(engine,
+						    "preempt_timeout_ms",
+						    &st, 0) == 0);
+			}
+
+			igt_subtest_f("%s-idempotent", name)
+				test_idempotent(i915, engine);
+			igt_subtest_f("%s-invalid", name)
+				test_invalid(i915, engine);
+			igt_subtest_f("%s-timeout", name)
+				test_timeout(i915, engine);
+		}
+
+		free(name);
+		close(engine);
+	}
+
+	igt_fixture {
+		close(sys);
+		close(i915);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 3f3eee277..a699377e3 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -238,6 +238,7 @@ i915_progs = [
 	'i915_query',
 	'i915_selftest',
 	'i915_suspend',
+	'sysfs_preemption_timeout',
 ]
 
 test_deps = [ igt_deps ]
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
@ 2019-10-17 14:30 ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-10-17 14:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: igt-dev, petri.latvala, tvrtko.ursulin

Dynamic subtests!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/i915/gem_context.c                |  40 +++++
 lib/i915/gem_context.h                |   2 +
 tests/Makefile.sources                |   1 +
 tests/i915/sysfs_preemption_timeout.c | 203 ++++++++++++++++++++++++++
 tests/meson.build                     |   1 +
 5 files changed, 247 insertions(+)
 create mode 100644 tests/i915/sysfs_preemption_timeout.c

diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
index 1fae5191f..a627a5c7b 100644
--- a/lib/i915/gem_context.c
+++ b/lib/i915/gem_context.c
@@ -403,3 +403,43 @@ bool gem_context_has_engine(int fd, uint32_t ctx, uint64_t engine)
 
 	return __gem_execbuf(fd, &execbuf) == -ENOENT;
 }
+
+static int create_ext_ioctl(int i915,
+			    struct drm_i915_gem_context_create_ext *arg)
+{
+	int err;
+
+	err = 0;
+	if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
+		err = -errno;
+		igt_assume(err);
+	}
+
+	errno = 0;
+	return err;
+}
+
+uint32_t gem_context_create_for_engine(int i915, unsigned int class, unsigned int inst)
+{
+	I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = {
+		.engines = { { .engine_class = class, .engine_instance = inst } }
+	};
+	struct drm_i915_gem_context_create_ext_setparam p_engines = {
+		.base = {
+			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
+			.next_extension = 0, /* end of chain */
+		},
+		.param = {
+			.param = I915_CONTEXT_PARAM_ENGINES,
+			.value = to_user_pointer(&engines),
+			.size = sizeof(engines),
+		},
+	};
+	struct drm_i915_gem_context_create_ext create = {
+		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
+		.extensions = to_user_pointer(&p_engines),
+	};
+
+	igt_assert_eq(create_ext_ioctl(i915, &create), 0);
+	return create.ctx_id;
+}
diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
index c0d4c9615..9e0a083f0 100644
--- a/lib/i915/gem_context.h
+++ b/lib/i915/gem_context.h
@@ -34,6 +34,8 @@ int __gem_context_create(int fd, uint32_t *ctx_id);
 void gem_context_destroy(int fd, uint32_t ctx_id);
 int __gem_context_destroy(int fd, uint32_t ctx_id);
 
+uint32_t gem_context_create_for_engine(int fd, unsigned int class, unsigned int inst);
+
 int __gem_context_clone(int i915,
 			uint32_t src, unsigned int share,
 			unsigned int flags,
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 093eb57f3..dff7dac06 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -98,6 +98,7 @@ TESTS_progs = \
 	tools_test \
 	vgem_basic \
 	vgem_slow \
+	i915/sysfs_preemption_timeout \
 	$(NULL)
 
 TESTS_progs += gem_bad_reloc
diff --git a/tests/i915/sysfs_preemption_timeout.c b/tests/i915/sysfs_preemption_timeout.c
new file mode 100644
index 000000000..a798345b1
--- /dev/null
+++ b/tests/i915/sysfs_preemption_timeout.c
@@ -0,0 +1,203 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "drmtest.h" /* gem_quiescent_gpu()! */
+#include "igt_dummyload.h"
+#include "igt_sysfs.h"
+#include "ioctl_wrappers.h" /* igt_require_gem()! */
+#include "sw_sync.h"
+
+#include "igt_debugfs.h"
+
+static bool __enable_hangcheck(int dir, bool state)
+{
+	return igt_sysfs_set(dir, "enable_hangcheck", state ? "1" : "0");
+}
+
+static bool enable_hangcheck(int i915, bool state)
+{
+	bool success;
+	int dir;
+
+	dir = igt_sysfs_open_parameters(i915);
+	if (dir < 0) /* no parameters, must be default! */
+		return false;
+
+	success = __enable_hangcheck(dir, state);
+	close(dir);
+
+	return success;
+}
+
+static void test_idempotent(int i915, int engine)
+{
+	unsigned int saved, delay;
+
+	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
+	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", 1);
+	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
+	igt_assert_eq(delay, 1);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
+	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
+	igt_assert_eq(delay, saved);
+}
+
+static void test_invalid(int i915, int engine)
+{
+	unsigned int saved, delay;
+
+	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
+	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", PRIu64, -1);
+	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
+	igt_assert_eq(delay, saved);
+}
+
+static uint64_t __test_timeout(int i915, int engine, unsigned int timeout)
+{
+	unsigned int class, inst;
+	struct timespec ts = {};
+	igt_spin_t *spin[2];
+	uint64_t elapsed;
+	uint32_t ctx;
+
+	igt_assert(igt_sysfs_scanf(engine, "class", "%u", &class) == 1);
+	igt_assert(igt_sysfs_scanf(engine, "instance", "%u", &inst) == 1);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", timeout);
+
+	ctx = gem_context_create_for_engine(i915, class, inst);
+	gem_context_set_priority(i915, ctx, -1023);
+	spin[0] = igt_spin_new(i915, ctx,
+			       .flags = (IGT_SPIN_NO_PREEMPTION |
+					 IGT_SPIN_POLL_RUN |
+					 IGT_SPIN_FENCE_OUT));
+	igt_spin_busywait_until_started(spin[0]);
+	gem_context_destroy(i915, ctx);
+
+	ctx = gem_context_create_for_engine(i915, class, inst);
+	gem_context_set_priority(i915, ctx, 1023);
+	igt_nsec_elapsed(&ts);
+	spin[1] = igt_spin_new(i915, ctx, .flags = IGT_SPIN_POLL_RUN);
+	igt_spin_busywait_until_started(spin[1]);
+	elapsed = igt_nsec_elapsed(&ts);
+	gem_context_destroy(i915, ctx);
+
+	igt_assert_eq(sync_fence_status(spin[0]->out_fence), -EIO);
+
+	igt_spin_free(i915, spin[1]);
+	igt_spin_free(i915, spin[0]);
+	gem_quiescent_gpu(i915);
+
+	return elapsed;
+}
+
+static void test_timeout(int i915, int engine)
+{
+	int delays[] = { 1, 50, 100, 500 };
+	unsigned int saved, delay;
+
+	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
+	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
+
+	gem_quiescent_gpu(i915);
+	igt_require(enable_hangcheck(i915, false));
+
+	for (int i = 0; i < ARRAY_SIZE(delays); i++) {
+		uint64_t elapsed;
+
+		elapsed = __test_timeout(i915, engine, delays[i]);
+		igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
+			 delays[i], elapsed * 1e-6);
+	}
+
+	igt_assert(enable_hangcheck(i915, true));
+	gem_quiescent_gpu(i915);
+
+	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
+	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
+	igt_assert_eq(delay, saved);
+}
+
+igt_main
+{
+	int i915, sys = -1;
+	struct dirent *de;
+	int engines;
+	DIR *dir;
+
+	igt_fixture {
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(i915);
+
+		sys = igt_sysfs_open(i915);
+		igt_require(sys != -1);
+	}
+
+	engines = openat(sys, "engine", O_RDONLY);
+	dir = fdopendir(engines);
+	while (dir && (de = readdir(dir))) {
+		int engine = openat(engines, de->d_name, O_RDONLY);
+		struct stat st;
+		char *name;
+
+		name = igt_sysfs_get(engine, "name");
+		if (!name)
+			continue;
+
+		igt_subtest_group {
+			igt_fixture {
+				igt_require(fstatat(engine,
+						    "preempt_timeout_ms",
+						    &st, 0) == 0);
+			}
+
+			igt_subtest_f("%s-idempotent", name)
+				test_idempotent(i915, engine);
+			igt_subtest_f("%s-invalid", name)
+				test_invalid(i915, engine);
+			igt_subtest_f("%s-timeout", name)
+				test_timeout(i915, engine);
+		}
+
+		free(name);
+		close(engine);
+	}
+
+	igt_fixture {
+		close(sys);
+		close(i915);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 3f3eee277..a699377e3 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -238,6 +238,7 @@ i915_progs = [
 	'i915_query',
 	'i915_selftest',
 	'i915_suspend',
+	'sysfs_preemption_timeout',
 ]
 
 test_deps = [ igt_deps ]
-- 
2.23.0

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [igt-dev] ✗ GitLab.Pipeline: warning for i915: Exercise preemption timeout controls in sysfs
  2019-10-17 14:30 ` [igt-dev] " Chris Wilson
  (?)
@ 2019-10-17 15:08 ` Patchwork
  -1 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-10-17 15:08 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: i915: Exercise preemption timeout controls in sysfs
URL   : https://patchwork.freedesktop.org/series/68158/
State : warning

== Summary ==

Did not get list of undocumented tests for this run, something is wrong!

Other than that, pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/71326 for more details

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/71326
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
  2019-10-17 14:30 ` [igt-dev] " Chris Wilson
@ 2019-10-18 12:23   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-10-18 12:23 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 17/10/2019 15:30, Chris Wilson wrote:
> Dynamic subtests!

Ouch! :)

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   lib/i915/gem_context.c                |  40 +++++
>   lib/i915/gem_context.h                |   2 +
>   tests/Makefile.sources                |   1 +
>   tests/i915/sysfs_preemption_timeout.c | 203 ++++++++++++++++++++++++++
>   tests/meson.build                     |   1 +
>   5 files changed, 247 insertions(+)
>   create mode 100644 tests/i915/sysfs_preemption_timeout.c
> 
> diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
> index 1fae5191f..a627a5c7b 100644
> --- a/lib/i915/gem_context.c
> +++ b/lib/i915/gem_context.c
> @@ -403,3 +403,43 @@ bool gem_context_has_engine(int fd, uint32_t ctx, uint64_t engine)
>   
>   	return __gem_execbuf(fd, &execbuf) == -ENOENT;
>   }
> +
> +static int create_ext_ioctl(int i915,
> +			    struct drm_i915_gem_context_create_ext *arg)
> +{
> +	int err;
> +
> +	err = 0;
> +	if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> +		err = -errno;
> +		igt_assume(err);
> +	}
> +
> +	errno = 0;
> +	return err;
> +}
> +
> +uint32_t gem_context_create_for_engine(int i915, unsigned int class, unsigned int inst)
> +{
> +	I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = {
> +		.engines = { { .engine_class = class, .engine_instance = inst } }
> +	};
> +	struct drm_i915_gem_context_create_ext_setparam p_engines = {
> +		.base = {
> +			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> +			.next_extension = 0, /* end of chain */
> +		},
> +		.param = {
> +			.param = I915_CONTEXT_PARAM_ENGINES,
> +			.value = to_user_pointer(&engines),
> +			.size = sizeof(engines),
> +		},
> +	};
> +	struct drm_i915_gem_context_create_ext create = {
> +		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> +		.extensions = to_user_pointer(&p_engines),
> +	};
> +
> +	igt_assert_eq(create_ext_ioctl(i915, &create), 0);
> +	return create.ctx_id;
> +}
> diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
> index c0d4c9615..9e0a083f0 100644
> --- a/lib/i915/gem_context.h
> +++ b/lib/i915/gem_context.h
> @@ -34,6 +34,8 @@ int __gem_context_create(int fd, uint32_t *ctx_id);
>   void gem_context_destroy(int fd, uint32_t ctx_id);
>   int __gem_context_destroy(int fd, uint32_t ctx_id);
>   
> +uint32_t gem_context_create_for_engine(int fd, unsigned int class, unsigned int inst);
> +
>   int __gem_context_clone(int i915,
>   			uint32_t src, unsigned int share,
>   			unsigned int flags,
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 093eb57f3..dff7dac06 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -98,6 +98,7 @@ TESTS_progs = \
>   	tools_test \
>   	vgem_basic \
>   	vgem_slow \
> +	i915/sysfs_preemption_timeout \
>   	$(NULL)
>   
>   TESTS_progs += gem_bad_reloc
> diff --git a/tests/i915/sysfs_preemption_timeout.c b/tests/i915/sysfs_preemption_timeout.c
> new file mode 100644
> index 000000000..a798345b1
> --- /dev/null
> +++ b/tests/i915/sysfs_preemption_timeout.c
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "drmtest.h" /* gem_quiescent_gpu()! */
> +#include "igt_dummyload.h"
> +#include "igt_sysfs.h"
> +#include "ioctl_wrappers.h" /* igt_require_gem()! */
> +#include "sw_sync.h"
> +
> +#include "igt_debugfs.h"
> +
> +static bool __enable_hangcheck(int dir, bool state)
> +{
> +	return igt_sysfs_set(dir, "enable_hangcheck", state ? "1" : "0");
> +}
> +
> +static bool enable_hangcheck(int i915, bool state)
> +{
> +	bool success;
> +	int dir;
> +
> +	dir = igt_sysfs_open_parameters(i915);
> +	if (dir < 0) /* no parameters, must be default! */
> +		return false;
> +
> +	success = __enable_hangcheck(dir, state);
> +	close(dir);
> +
> +	return success;
> +}
> +
> +static void test_idempotent(int i915, int engine)
> +{
> +	unsigned int saved, delay;
> +
> +	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> +	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", 1);
> +	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> +	igt_assert_eq(delay, 1);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> +	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> +	igt_assert_eq(delay, saved);
> +}
> +
> +static void test_invalid(int i915, int engine)
> +{
> +	unsigned int saved, delay;
> +
> +	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> +	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", PRIu64, -1);
> +	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> +	igt_assert_eq(delay, saved);
> +}
> +
> +static uint64_t __test_timeout(int i915, int engine, unsigned int timeout)
> +{
> +	unsigned int class, inst;
> +	struct timespec ts = {};
> +	igt_spin_t *spin[2];
> +	uint64_t elapsed;
> +	uint32_t ctx;
> +
> +	igt_assert(igt_sysfs_scanf(engine, "class", "%u", &class) == 1);
> +	igt_assert(igt_sysfs_scanf(engine, "instance", "%u", &inst) == 1);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", timeout);
> +
> +	ctx = gem_context_create_for_engine(i915, class, inst);
> +	gem_context_set_priority(i915, ctx, -1023);
> +	spin[0] = igt_spin_new(i915, ctx,
> +			       .flags = (IGT_SPIN_NO_PREEMPTION |
> +					 IGT_SPIN_POLL_RUN |
> +					 IGT_SPIN_FENCE_OUT));
> +	igt_spin_busywait_until_started(spin[0]);
> +	gem_context_destroy(i915, ctx);
> +
> +	ctx = gem_context_create_for_engine(i915, class, inst);
> +	gem_context_set_priority(i915, ctx, 1023);
> +	igt_nsec_elapsed(&ts);
> +	spin[1] = igt_spin_new(i915, ctx, .flags = IGT_SPIN_POLL_RUN);
> +	igt_spin_busywait_until_started(spin[1]);
> +	elapsed = igt_nsec_elapsed(&ts);
> +	gem_context_destroy(i915, ctx);
> +
> +	igt_assert_eq(sync_fence_status(spin[0]->out_fence), -EIO);
> +
> +	igt_spin_free(i915, spin[1]);
> +	igt_spin_free(i915, spin[0]);
> +	gem_quiescent_gpu(i915);
> +
> +	return elapsed;
> +}
> +
> +static void test_timeout(int i915, int engine)
> +{
> +	int delays[] = { 1, 50, 100, 500 };
> +	unsigned int saved, delay;
> +
> +	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> +	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> +
> +	gem_quiescent_gpu(i915);
> +	igt_require(enable_hangcheck(i915, false));
> +
> +	for (int i = 0; i < ARRAY_SIZE(delays); i++) {
> +		uint64_t elapsed;
> +
> +		elapsed = __test_timeout(i915, engine, delays[i]);
> +		igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
> +			 delays[i], elapsed * 1e-6);

No checking that measured time relates to configured timeout?

> +	}
> +
> +	igt_assert(enable_hangcheck(i915, true));
> +	gem_quiescent_gpu(i915);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> +	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> +	igt_assert_eq(delay, saved);
> +}
> +
> +igt_main
> +{
> +	int i915, sys = -1;
> +	struct dirent *de;
> +	int engines;
> +	DIR *dir;
> +
> +	igt_fixture {
> +		i915 = drm_open_driver(DRIVER_INTEL);
> +		igt_require_gem(i915);
> +
> +		sys = igt_sysfs_open(i915);
> +		igt_require(sys != -1);

igt_assert_fd?

> +	}
> +
> +	engines = openat(sys, "engine", O_RDONLY);
> +	dir = fdopendir(engines);
> +	while (dir && (de = readdir(dir))) {
> +		int engine = openat(engines, de->d_name, O_RDONLY);
> +		struct stat st;
> +		char *name;
> +
> +		name = igt_sysfs_get(engine, "name");
> +		if (!name)
> +			continue;
> +
> +		igt_subtest_group {
> +			igt_fixture {
> +				igt_require(fstatat(engine,
> +						    "preempt_timeout_ms",
> +						    &st, 0) == 0);
> +			}
> +
> +			igt_subtest_f("%s-idempotent", name)
> +				test_idempotent(i915, engine);
> +			igt_subtest_f("%s-invalid", name)
> +				test_invalid(i915, engine);
> +			igt_subtest_f("%s-timeout", name)
> +				test_timeout(i915, engine);
> +		}
> +
> +		free(name);
> +		close(engine);
> +	}

You probably should use __for_each_static_engine and then open sysfs 
nodes based on that. Gets around the dynamic subtests no-no at least.

> +
> +	igt_fixture {
> +		close(sys);
> +		close(i915);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 3f3eee277..a699377e3 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -238,6 +238,7 @@ i915_progs = [
>   	'i915_query',
>   	'i915_selftest',
>   	'i915_suspend',
> +	'sysfs_preemption_timeout',
>   ]
>   
>   test_deps = [ igt_deps ]
> 

Regards,

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
@ 2019-10-18 12:23   ` Tvrtko Ursulin
  0 siblings, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-10-18 12:23 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev, petri.latvala, tvrtko.ursulin


On 17/10/2019 15:30, Chris Wilson wrote:
> Dynamic subtests!

Ouch! :)

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   lib/i915/gem_context.c                |  40 +++++
>   lib/i915/gem_context.h                |   2 +
>   tests/Makefile.sources                |   1 +
>   tests/i915/sysfs_preemption_timeout.c | 203 ++++++++++++++++++++++++++
>   tests/meson.build                     |   1 +
>   5 files changed, 247 insertions(+)
>   create mode 100644 tests/i915/sysfs_preemption_timeout.c
> 
> diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
> index 1fae5191f..a627a5c7b 100644
> --- a/lib/i915/gem_context.c
> +++ b/lib/i915/gem_context.c
> @@ -403,3 +403,43 @@ bool gem_context_has_engine(int fd, uint32_t ctx, uint64_t engine)
>   
>   	return __gem_execbuf(fd, &execbuf) == -ENOENT;
>   }
> +
> +static int create_ext_ioctl(int i915,
> +			    struct drm_i915_gem_context_create_ext *arg)
> +{
> +	int err;
> +
> +	err = 0;
> +	if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> +		err = -errno;
> +		igt_assume(err);
> +	}
> +
> +	errno = 0;
> +	return err;
> +}
> +
> +uint32_t gem_context_create_for_engine(int i915, unsigned int class, unsigned int inst)
> +{
> +	I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = {
> +		.engines = { { .engine_class = class, .engine_instance = inst } }
> +	};
> +	struct drm_i915_gem_context_create_ext_setparam p_engines = {
> +		.base = {
> +			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> +			.next_extension = 0, /* end of chain */
> +		},
> +		.param = {
> +			.param = I915_CONTEXT_PARAM_ENGINES,
> +			.value = to_user_pointer(&engines),
> +			.size = sizeof(engines),
> +		},
> +	};
> +	struct drm_i915_gem_context_create_ext create = {
> +		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> +		.extensions = to_user_pointer(&p_engines),
> +	};
> +
> +	igt_assert_eq(create_ext_ioctl(i915, &create), 0);
> +	return create.ctx_id;
> +}
> diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
> index c0d4c9615..9e0a083f0 100644
> --- a/lib/i915/gem_context.h
> +++ b/lib/i915/gem_context.h
> @@ -34,6 +34,8 @@ int __gem_context_create(int fd, uint32_t *ctx_id);
>   void gem_context_destroy(int fd, uint32_t ctx_id);
>   int __gem_context_destroy(int fd, uint32_t ctx_id);
>   
> +uint32_t gem_context_create_for_engine(int fd, unsigned int class, unsigned int inst);
> +
>   int __gem_context_clone(int i915,
>   			uint32_t src, unsigned int share,
>   			unsigned int flags,
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 093eb57f3..dff7dac06 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -98,6 +98,7 @@ TESTS_progs = \
>   	tools_test \
>   	vgem_basic \
>   	vgem_slow \
> +	i915/sysfs_preemption_timeout \
>   	$(NULL)
>   
>   TESTS_progs += gem_bad_reloc
> diff --git a/tests/i915/sysfs_preemption_timeout.c b/tests/i915/sysfs_preemption_timeout.c
> new file mode 100644
> index 000000000..a798345b1
> --- /dev/null
> +++ b/tests/i915/sysfs_preemption_timeout.c
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "drmtest.h" /* gem_quiescent_gpu()! */
> +#include "igt_dummyload.h"
> +#include "igt_sysfs.h"
> +#include "ioctl_wrappers.h" /* igt_require_gem()! */
> +#include "sw_sync.h"
> +
> +#include "igt_debugfs.h"
> +
> +static bool __enable_hangcheck(int dir, bool state)
> +{
> +	return igt_sysfs_set(dir, "enable_hangcheck", state ? "1" : "0");
> +}
> +
> +static bool enable_hangcheck(int i915, bool state)
> +{
> +	bool success;
> +	int dir;
> +
> +	dir = igt_sysfs_open_parameters(i915);
> +	if (dir < 0) /* no parameters, must be default! */
> +		return false;
> +
> +	success = __enable_hangcheck(dir, state);
> +	close(dir);
> +
> +	return success;
> +}
> +
> +static void test_idempotent(int i915, int engine)
> +{
> +	unsigned int saved, delay;
> +
> +	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> +	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", 1);
> +	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> +	igt_assert_eq(delay, 1);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> +	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> +	igt_assert_eq(delay, saved);
> +}
> +
> +static void test_invalid(int i915, int engine)
> +{
> +	unsigned int saved, delay;
> +
> +	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> +	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", PRIu64, -1);
> +	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> +	igt_assert_eq(delay, saved);
> +}
> +
> +static uint64_t __test_timeout(int i915, int engine, unsigned int timeout)
> +{
> +	unsigned int class, inst;
> +	struct timespec ts = {};
> +	igt_spin_t *spin[2];
> +	uint64_t elapsed;
> +	uint32_t ctx;
> +
> +	igt_assert(igt_sysfs_scanf(engine, "class", "%u", &class) == 1);
> +	igt_assert(igt_sysfs_scanf(engine, "instance", "%u", &inst) == 1);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", timeout);
> +
> +	ctx = gem_context_create_for_engine(i915, class, inst);
> +	gem_context_set_priority(i915, ctx, -1023);
> +	spin[0] = igt_spin_new(i915, ctx,
> +			       .flags = (IGT_SPIN_NO_PREEMPTION |
> +					 IGT_SPIN_POLL_RUN |
> +					 IGT_SPIN_FENCE_OUT));
> +	igt_spin_busywait_until_started(spin[0]);
> +	gem_context_destroy(i915, ctx);
> +
> +	ctx = gem_context_create_for_engine(i915, class, inst);
> +	gem_context_set_priority(i915, ctx, 1023);
> +	igt_nsec_elapsed(&ts);
> +	spin[1] = igt_spin_new(i915, ctx, .flags = IGT_SPIN_POLL_RUN);
> +	igt_spin_busywait_until_started(spin[1]);
> +	elapsed = igt_nsec_elapsed(&ts);
> +	gem_context_destroy(i915, ctx);
> +
> +	igt_assert_eq(sync_fence_status(spin[0]->out_fence), -EIO);
> +
> +	igt_spin_free(i915, spin[1]);
> +	igt_spin_free(i915, spin[0]);
> +	gem_quiescent_gpu(i915);
> +
> +	return elapsed;
> +}
> +
> +static void test_timeout(int i915, int engine)
> +{
> +	int delays[] = { 1, 50, 100, 500 };
> +	unsigned int saved, delay;
> +
> +	igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> +	igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> +
> +	gem_quiescent_gpu(i915);
> +	igt_require(enable_hangcheck(i915, false));
> +
> +	for (int i = 0; i < ARRAY_SIZE(delays); i++) {
> +		uint64_t elapsed;
> +
> +		elapsed = __test_timeout(i915, engine, delays[i]);
> +		igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
> +			 delays[i], elapsed * 1e-6);

No checking that measured time relates to configured timeout?

> +	}
> +
> +	igt_assert(enable_hangcheck(i915, true));
> +	gem_quiescent_gpu(i915);
> +
> +	igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> +	igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> +	igt_assert_eq(delay, saved);
> +}
> +
> +igt_main
> +{
> +	int i915, sys = -1;
> +	struct dirent *de;
> +	int engines;
> +	DIR *dir;
> +
> +	igt_fixture {
> +		i915 = drm_open_driver(DRIVER_INTEL);
> +		igt_require_gem(i915);
> +
> +		sys = igt_sysfs_open(i915);
> +		igt_require(sys != -1);

igt_assert_fd?

> +	}
> +
> +	engines = openat(sys, "engine", O_RDONLY);
> +	dir = fdopendir(engines);
> +	while (dir && (de = readdir(dir))) {
> +		int engine = openat(engines, de->d_name, O_RDONLY);
> +		struct stat st;
> +		char *name;
> +
> +		name = igt_sysfs_get(engine, "name");
> +		if (!name)
> +			continue;
> +
> +		igt_subtest_group {
> +			igt_fixture {
> +				igt_require(fstatat(engine,
> +						    "preempt_timeout_ms",
> +						    &st, 0) == 0);
> +			}
> +
> +			igt_subtest_f("%s-idempotent", name)
> +				test_idempotent(i915, engine);
> +			igt_subtest_f("%s-invalid", name)
> +				test_invalid(i915, engine);
> +			igt_subtest_f("%s-timeout", name)
> +				test_timeout(i915, engine);
> +		}
> +
> +		free(name);
> +		close(engine);
> +	}

You probably should use __for_each_static_engine and then open sysfs 
nodes based on that. Gets around the dynamic subtests no-no at least.

> +
> +	igt_fixture {
> +		close(sys);
> +		close(i915);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 3f3eee277..a699377e3 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -238,6 +238,7 @@ i915_progs = [
>   	'i915_query',
>   	'i915_selftest',
>   	'i915_suspend',
> +	'sysfs_preemption_timeout',
>   ]
>   
>   test_deps = [ igt_deps ]
> 

Regards,

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
  2019-10-18 12:23   ` Tvrtko Ursulin
@ 2019-10-18 12:35     ` Chris Wilson
  -1 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-10-18 12:35 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev

Quoting Tvrtko Ursulin (2019-10-18 13:23:53)
> 
> On 17/10/2019 15:30, Chris Wilson wrote:
> > Dynamic subtests!
> 
> Ouch! :)
> 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > +static void test_timeout(int i915, int engine)
> > +{
> > +     int delays[] = { 1, 50, 100, 500 };
> > +     unsigned int saved, delay;
> > +
> > +     igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> > +     igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> > +
> > +     gem_quiescent_gpu(i915);
> > +     igt_require(enable_hangcheck(i915, false));
> > +
> > +     for (int i = 0; i < ARRAY_SIZE(delays); i++) {
> > +             uint64_t elapsed;
> > +
> > +             elapsed = __test_timeout(i915, engine, delays[i]);
> > +             igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
> > +                      delays[i], elapsed * 1e-6);
> 
> No checking that measured time relates to configured timeout?

Have now. Just needed some soaking to decide on thresholds. I've 50ms
but that may change as CI tends to have more scheduling intolerance than
local machines.

> > +     }
> > +
> > +     igt_assert(enable_hangcheck(i915, true));
> > +     gem_quiescent_gpu(i915);
> > +
> > +     igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> > +     igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> > +     igt_assert_eq(delay, saved);
> > +}
> > +
> > +igt_main
> > +{
> > +     int i915, sys = -1;
> > +     struct dirent *de;
> > +     int engines;
> > +     DIR *dir;
> > +
> > +     igt_fixture {
> > +             i915 = drm_open_driver(DRIVER_INTEL);
> > +             igt_require_gem(i915);
> > +
> > +             sys = igt_sysfs_open(i915);
> > +             igt_require(sys != -1);
> 
> igt_assert_fd?

Do we guarantee that the sysadmin has mounted sysfs? We don't automount
it unlike debugfs.

> > +             igt_subtest_group {
> > +                     igt_fixture {
> > +                             igt_require(fstatat(engine,
> > +                                                 "preempt_timeout_ms",
> > +                                                 &st, 0) == 0);
> > +                     }
> > +
> > +                     igt_subtest_f("%s-idempotent", name)
> > +                             test_idempotent(i915, engine);
> > +                     igt_subtest_f("%s-invalid", name)
> > +                             test_invalid(i915, engine);
> > +                     igt_subtest_f("%s-timeout", name)
> > +                             test_timeout(i915, engine);
> > +             }
> > +
> > +             free(name);
> > +             close(engine);
> > +     }
> 
> You probably should use __for_each_static_engine and then open sysfs 
> nodes based on that. Gets around the dynamic subtests no-no at least.

Defeatist!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
@ 2019-10-18 12:35     ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-10-18 12:35 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: igt-dev, petri.latvala, tvrtko.ursulin

Quoting Tvrtko Ursulin (2019-10-18 13:23:53)
> 
> On 17/10/2019 15:30, Chris Wilson wrote:
> > Dynamic subtests!
> 
> Ouch! :)
> 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > +static void test_timeout(int i915, int engine)
> > +{
> > +     int delays[] = { 1, 50, 100, 500 };
> > +     unsigned int saved, delay;
> > +
> > +     igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> > +     igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> > +
> > +     gem_quiescent_gpu(i915);
> > +     igt_require(enable_hangcheck(i915, false));
> > +
> > +     for (int i = 0; i < ARRAY_SIZE(delays); i++) {
> > +             uint64_t elapsed;
> > +
> > +             elapsed = __test_timeout(i915, engine, delays[i]);
> > +             igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
> > +                      delays[i], elapsed * 1e-6);
> 
> No checking that measured time relates to configured timeout?

Have now. Just needed some soaking to decide on thresholds. I've 50ms
but that may change as CI tends to have more scheduling intolerance than
local machines.

> > +     }
> > +
> > +     igt_assert(enable_hangcheck(i915, true));
> > +     gem_quiescent_gpu(i915);
> > +
> > +     igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> > +     igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> > +     igt_assert_eq(delay, saved);
> > +}
> > +
> > +igt_main
> > +{
> > +     int i915, sys = -1;
> > +     struct dirent *de;
> > +     int engines;
> > +     DIR *dir;
> > +
> > +     igt_fixture {
> > +             i915 = drm_open_driver(DRIVER_INTEL);
> > +             igt_require_gem(i915);
> > +
> > +             sys = igt_sysfs_open(i915);
> > +             igt_require(sys != -1);
> 
> igt_assert_fd?

Do we guarantee that the sysadmin has mounted sysfs? We don't automount
it unlike debugfs.

> > +             igt_subtest_group {
> > +                     igt_fixture {
> > +                             igt_require(fstatat(engine,
> > +                                                 "preempt_timeout_ms",
> > +                                                 &st, 0) == 0);
> > +                     }
> > +
> > +                     igt_subtest_f("%s-idempotent", name)
> > +                             test_idempotent(i915, engine);
> > +                     igt_subtest_f("%s-invalid", name)
> > +                             test_invalid(i915, engine);
> > +                     igt_subtest_f("%s-timeout", name)
> > +                             test_timeout(i915, engine);
> > +             }
> > +
> > +             free(name);
> > +             close(engine);
> > +     }
> 
> You probably should use __for_each_static_engine and then open sysfs 
> nodes based on that. Gets around the dynamic subtests no-no at least.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
  2019-10-18 12:35     ` Chris Wilson
@ 2019-10-18 12:39       ` Tvrtko Ursulin
  -1 siblings, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-10-18 12:39 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev


On 18/10/2019 13:35, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-10-18 13:23:53)
>>
>> On 17/10/2019 15:30, Chris Wilson wrote:
>>> Dynamic subtests!
>>
>> Ouch! :)
>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> +static void test_timeout(int i915, int engine)
>>> +{
>>> +     int delays[] = { 1, 50, 100, 500 };
>>> +     unsigned int saved, delay;
>>> +
>>> +     igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
>>> +     igt_debug("Initial preempt_timeout_ms:%u\n", saved);
>>> +
>>> +     gem_quiescent_gpu(i915);
>>> +     igt_require(enable_hangcheck(i915, false));
>>> +
>>> +     for (int i = 0; i < ARRAY_SIZE(delays); i++) {
>>> +             uint64_t elapsed;
>>> +
>>> +             elapsed = __test_timeout(i915, engine, delays[i]);
>>> +             igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
>>> +                      delays[i], elapsed * 1e-6);
>>
>> No checking that measured time relates to configured timeout?
> 
> Have now. Just needed some soaking to decide on thresholds. I've 50ms
> but that may change as CI tends to have more scheduling intolerance than
> local machines.
> 
>>> +     }
>>> +
>>> +     igt_assert(enable_hangcheck(i915, true));
>>> +     gem_quiescent_gpu(i915);
>>> +
>>> +     igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
>>> +     igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
>>> +     igt_assert_eq(delay, saved);
>>> +}
>>> +
>>> +igt_main
>>> +{
>>> +     int i915, sys = -1;
>>> +     struct dirent *de;
>>> +     int engines;
>>> +     DIR *dir;
>>> +
>>> +     igt_fixture {
>>> +             i915 = drm_open_driver(DRIVER_INTEL);
>>> +             igt_require_gem(i915);
>>> +
>>> +             sys = igt_sysfs_open(i915);
>>> +             igt_require(sys != -1);
>>
>> igt_assert_fd?
> 
> Do we guarantee that the sysadmin has mounted sysfs? We don't automount
> it unlike debugfs.
> 
>>> +             igt_subtest_group {
>>> +                     igt_fixture {
>>> +                             igt_require(fstatat(engine,
>>> +                                                 "preempt_timeout_ms",
>>> +                                                 &st, 0) == 0);
>>> +                     }
>>> +
>>> +                     igt_subtest_f("%s-idempotent", name)
>>> +                             test_idempotent(i915, engine);
>>> +                     igt_subtest_f("%s-invalid", name)
>>> +                             test_invalid(i915, engine);
>>> +                     igt_subtest_f("%s-timeout", name)
>>> +                             test_timeout(i915, engine);
>>> +             }
>>> +
>>> +             free(name);
>>> +             close(engine);
>>> +     }
>>
>> You probably should use __for_each_static_engine and then open sysfs
>> nodes based on that. Gets around the dynamic subtests no-no at least.
> 
> Defeatist!

Well I have challenged this status quo a few times and now I am 
embracing it, or should I say disagreeing and committing, so bonus 
points all round. :)

Regards,

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
@ 2019-10-18 12:39       ` Tvrtko Ursulin
  0 siblings, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-10-18 12:39 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: igt-dev, petri.latvala, tvrtko.ursulin


On 18/10/2019 13:35, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-10-18 13:23:53)
>>
>> On 17/10/2019 15:30, Chris Wilson wrote:
>>> Dynamic subtests!
>>
>> Ouch! :)
>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> +static void test_timeout(int i915, int engine)
>>> +{
>>> +     int delays[] = { 1, 50, 100, 500 };
>>> +     unsigned int saved, delay;
>>> +
>>> +     igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
>>> +     igt_debug("Initial preempt_timeout_ms:%u\n", saved);
>>> +
>>> +     gem_quiescent_gpu(i915);
>>> +     igt_require(enable_hangcheck(i915, false));
>>> +
>>> +     for (int i = 0; i < ARRAY_SIZE(delays); i++) {
>>> +             uint64_t elapsed;
>>> +
>>> +             elapsed = __test_timeout(i915, engine, delays[i]);
>>> +             igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
>>> +                      delays[i], elapsed * 1e-6);
>>
>> No checking that measured time relates to configured timeout?
> 
> Have now. Just needed some soaking to decide on thresholds. I've 50ms
> but that may change as CI tends to have more scheduling intolerance than
> local machines.
> 
>>> +     }
>>> +
>>> +     igt_assert(enable_hangcheck(i915, true));
>>> +     gem_quiescent_gpu(i915);
>>> +
>>> +     igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
>>> +     igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
>>> +     igt_assert_eq(delay, saved);
>>> +}
>>> +
>>> +igt_main
>>> +{
>>> +     int i915, sys = -1;
>>> +     struct dirent *de;
>>> +     int engines;
>>> +     DIR *dir;
>>> +
>>> +     igt_fixture {
>>> +             i915 = drm_open_driver(DRIVER_INTEL);
>>> +             igt_require_gem(i915);
>>> +
>>> +             sys = igt_sysfs_open(i915);
>>> +             igt_require(sys != -1);
>>
>> igt_assert_fd?
> 
> Do we guarantee that the sysadmin has mounted sysfs? We don't automount
> it unlike debugfs.
> 
>>> +             igt_subtest_group {
>>> +                     igt_fixture {
>>> +                             igt_require(fstatat(engine,
>>> +                                                 "preempt_timeout_ms",
>>> +                                                 &st, 0) == 0);
>>> +                     }
>>> +
>>> +                     igt_subtest_f("%s-idempotent", name)
>>> +                             test_idempotent(i915, engine);
>>> +                     igt_subtest_f("%s-invalid", name)
>>> +                             test_invalid(i915, engine);
>>> +                     igt_subtest_f("%s-timeout", name)
>>> +                             test_timeout(i915, engine);
>>> +             }
>>> +
>>> +             free(name);
>>> +             close(engine);
>>> +     }
>>
>> You probably should use __for_each_static_engine and then open sysfs
>> nodes based on that. Gets around the dynamic subtests no-no at least.
> 
> Defeatist!

Well I have challenged this status quo a few times and now I am 
embracing it, or should I say disagreeing and committing, so bonus 
points all round. :)

Regards,

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
  2019-10-18 12:39       ` Tvrtko Ursulin
@ 2019-10-18 14:06         ` Petri Latvala
  -1 siblings, 0 replies; 11+ messages in thread
From: Petri Latvala @ 2019-10-18 14:06 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev, intel-gfx

On Fri, Oct 18, 2019 at 01:39:37PM +0100, Tvrtko Ursulin wrote:
> 
> On 18/10/2019 13:35, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-10-18 13:23:53)
> > > 
> > > On 17/10/2019 15:30, Chris Wilson wrote:
> > > > Dynamic subtests!
> > > 
> > > Ouch! :)
> > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > ---
> > > > +static void test_timeout(int i915, int engine)
> > > > +{
> > > > +     int delays[] = { 1, 50, 100, 500 };
> > > > +     unsigned int saved, delay;
> > > > +
> > > > +     igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> > > > +     igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> > > > +
> > > > +     gem_quiescent_gpu(i915);
> > > > +     igt_require(enable_hangcheck(i915, false));
> > > > +
> > > > +     for (int i = 0; i < ARRAY_SIZE(delays); i++) {
> > > > +             uint64_t elapsed;
> > > > +
> > > > +             elapsed = __test_timeout(i915, engine, delays[i]);
> > > > +             igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
> > > > +                      delays[i], elapsed * 1e-6);
> > > 
> > > No checking that measured time relates to configured timeout?
> > 
> > Have now. Just needed some soaking to decide on thresholds. I've 50ms
> > but that may change as CI tends to have more scheduling intolerance than
> > local machines.
> > 
> > > > +     }
> > > > +
> > > > +     igt_assert(enable_hangcheck(i915, true));
> > > > +     gem_quiescent_gpu(i915);
> > > > +
> > > > +     igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> > > > +     igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> > > > +     igt_assert_eq(delay, saved);
> > > > +}
> > > > +
> > > > +igt_main
> > > > +{
> > > > +     int i915, sys = -1;
> > > > +     struct dirent *de;
> > > > +     int engines;
> > > > +     DIR *dir;
> > > > +
> > > > +     igt_fixture {
> > > > +             i915 = drm_open_driver(DRIVER_INTEL);
> > > > +             igt_require_gem(i915);
> > > > +
> > > > +             sys = igt_sysfs_open(i915);
> > > > +             igt_require(sys != -1);
> > > 
> > > igt_assert_fd?
> > 
> > Do we guarantee that the sysadmin has mounted sysfs? We don't automount
> > it unlike debugfs.
> > 
> > > > +             igt_subtest_group {
> > > > +                     igt_fixture {
> > > > +                             igt_require(fstatat(engine,
> > > > +                                                 "preempt_timeout_ms",
> > > > +                                                 &st, 0) == 0);
> > > > +                     }
> > > > +
> > > > +                     igt_subtest_f("%s-idempotent", name)
> > > > +                             test_idempotent(i915, engine);
> > > > +                     igt_subtest_f("%s-invalid", name)
> > > > +                             test_invalid(i915, engine);
> > > > +                     igt_subtest_f("%s-timeout", name)
> > > > +                             test_timeout(i915, engine);
> > > > +             }
> > > > +
> > > > +             free(name);
> > > > +             close(engine);
> > > > +     }
> > > 
> > > You probably should use __for_each_static_engine and then open sysfs
> > > nodes based on that. Gets around the dynamic subtests no-no at least.
> > 
> > Defeatist!
> 
> Well I have challenged this status quo a few times and now I am embracing
> it, or should I say disagreeing and committing, so bonus points all round.
> :)


Perhaps next week I'll get around to reshaping the dynamic subtests
series. Watch this space!

(Meanwhile, I hope it goes without saying, dynamic subtests are indeed
a no-no)


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs
@ 2019-10-18 14:06         ` Petri Latvala
  0 siblings, 0 replies; 11+ messages in thread
From: Petri Latvala @ 2019-10-18 14:06 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: igt-dev, intel-gfx, tvrtko.ursulin

On Fri, Oct 18, 2019 at 01:39:37PM +0100, Tvrtko Ursulin wrote:
> 
> On 18/10/2019 13:35, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-10-18 13:23:53)
> > > 
> > > On 17/10/2019 15:30, Chris Wilson wrote:
> > > > Dynamic subtests!
> > > 
> > > Ouch! :)
> > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > ---
> > > > +static void test_timeout(int i915, int engine)
> > > > +{
> > > > +     int delays[] = { 1, 50, 100, 500 };
> > > > +     unsigned int saved, delay;
> > > > +
> > > > +     igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> > > > +     igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> > > > +
> > > > +     gem_quiescent_gpu(i915);
> > > > +     igt_require(enable_hangcheck(i915, false));
> > > > +
> > > > +     for (int i = 0; i < ARRAY_SIZE(delays); i++) {
> > > > +             uint64_t elapsed;
> > > > +
> > > > +             elapsed = __test_timeout(i915, engine, delays[i]);
> > > > +             igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
> > > > +                      delays[i], elapsed * 1e-6);
> > > 
> > > No checking that measured time relates to configured timeout?
> > 
> > Have now. Just needed some soaking to decide on thresholds. I've 50ms
> > but that may change as CI tends to have more scheduling intolerance than
> > local machines.
> > 
> > > > +     }
> > > > +
> > > > +     igt_assert(enable_hangcheck(i915, true));
> > > > +     gem_quiescent_gpu(i915);
> > > > +
> > > > +     igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> > > > +     igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> > > > +     igt_assert_eq(delay, saved);
> > > > +}
> > > > +
> > > > +igt_main
> > > > +{
> > > > +     int i915, sys = -1;
> > > > +     struct dirent *de;
> > > > +     int engines;
> > > > +     DIR *dir;
> > > > +
> > > > +     igt_fixture {
> > > > +             i915 = drm_open_driver(DRIVER_INTEL);
> > > > +             igt_require_gem(i915);
> > > > +
> > > > +             sys = igt_sysfs_open(i915);
> > > > +             igt_require(sys != -1);
> > > 
> > > igt_assert_fd?
> > 
> > Do we guarantee that the sysadmin has mounted sysfs? We don't automount
> > it unlike debugfs.
> > 
> > > > +             igt_subtest_group {
> > > > +                     igt_fixture {
> > > > +                             igt_require(fstatat(engine,
> > > > +                                                 "preempt_timeout_ms",
> > > > +                                                 &st, 0) == 0);
> > > > +                     }
> > > > +
> > > > +                     igt_subtest_f("%s-idempotent", name)
> > > > +                             test_idempotent(i915, engine);
> > > > +                     igt_subtest_f("%s-invalid", name)
> > > > +                             test_invalid(i915, engine);
> > > > +                     igt_subtest_f("%s-timeout", name)
> > > > +                             test_timeout(i915, engine);
> > > > +             }
> > > > +
> > > > +             free(name);
> > > > +             close(engine);
> > > > +     }
> > > 
> > > You probably should use __for_each_static_engine and then open sysfs
> > > nodes based on that. Gets around the dynamic subtests no-no at least.
> > 
> > Defeatist!
> 
> Well I have challenged this status quo a few times and now I am embracing
> it, or should I say disagreeing and committing, so bonus points all round.
> :)


Perhaps next week I'll get around to reshaping the dynamic subtests
series. Watch this space!

(Meanwhile, I hope it goes without saying, dynamic subtests are indeed
a no-no)


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-10-18 14:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-17 14:30 [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs Chris Wilson
2019-10-17 14:30 ` [igt-dev] " Chris Wilson
2019-10-17 15:08 ` [igt-dev] ✗ GitLab.Pipeline: warning for " Patchwork
2019-10-18 12:23 ` [igt-dev] [PATCH i-g-t] " Tvrtko Ursulin
2019-10-18 12:23   ` Tvrtko Ursulin
2019-10-18 12:35   ` Chris Wilson
2019-10-18 12:35     ` Chris Wilson
2019-10-18 12:39     ` Tvrtko Ursulin
2019-10-18 12:39       ` Tvrtko Ursulin
2019-10-18 14:06       ` Petri Latvala
2019-10-18 14:06         ` Petri Latvala

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.