[i-g-t] lib/gem_test_engine: Add no device reopen variant

Commit Message

Janusz Krzysztofik May 12, 2020, 2:07 p.m. UTC

Some tests may need to take care of closing all device file descriptors
even on subtest failure.  For example, if a test would like to
effectively unload the driver module after a subtest failure, that
wouldn't be possible if any device file descriptors were kept open.
That may happen if the subtest wants to use gem_test_engine() helper as
it reopens the device for its internal use and may certainly leave it
open on failure.

Provide a variant of the helper that doesn't reopen the device.

Signed-off-by: Janusz Krzysztofik <[e-mail]>
---
 lib/i915/gem_submission.c | 31 ++++++++++++++++++++++++++++---
 lib/i915/gem_submission.h |  1 +
 2 files changed, 29 insertions(+), 3 deletions(-)

Comments

Chris Wilson May 12, 2020, 4:11 p.m. UTC | #1

Quoting Janusz Krzysztofik (2020-05-12 15:07:36)
> Some tests may need to take care of closing all device file descriptors
> even on subtest failure.  For example, if a test would like to
> effectively unload the driver module after a subtest failure, that
> wouldn't be possible if any device file descriptors were kept open.
> That may happen if the subtest wants to use gem_test_engine() helper as
> it reopens the device for its internal use and may certainly leave it
> open on failure.

I'm not completely satisfied with gem_test_engine(). The 'test' is
always better defined by the callers, as what is it testing exactly?

We can kill the i915_pm_rpm waste of time, which leaves gem_eio and that
is also dubious.
-Chris

Patch

diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index 72de0c223..48defd4a0 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -186,14 +186,16 @@  static bool is_wedged(int i915)
 }
 
 /**
- * gem_test_engine:
+ * __gem_test_engine:
  * @i915: open i915 drm file descriptor
  * @engine: the engine (I915_EXEC_RING id) to exercise
  *
  * Execute a nop batch on the engine specified, or ALL_ENGINES for all,
  * and check it executes.
+ *
+ * Note: This version does not reopen the device.
  */
-void gem_test_engine(int i915, unsigned int engine)
+void __gem_test_engine(int i915, unsigned int engine)
 {
 	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_exec_object2 obj = { };
@@ -202,7 +204,6 @@  void gem_test_engine(int i915, unsigned int engine)
 		.buffer_count = 1,
 	};
 
-	i915 = gem_reopen_driver(i915);
 	igt_assert(!is_wedged(i915));
 
 	obj.handle = gem_create(i915, 4096);
@@ -223,6 +224,30 @@  void gem_test_engine(int i915, unsigned int engine)
 	gem_close(i915, obj.handle);
 
 	igt_assert(!is_wedged(i915));
+}
+
+/**
+ * gem_test_engine:
+ * @i915: open i915 drm file descriptor
+ * @engine: the engine (I915_EXEC_RING id) to exercise
+ *
+ * Reopen the device so the test is run in an isolated context, then
+ * execute a nop batch on the engine specified, or ALL_ENGINES for all,
+ * and check it executes.
+ */
+void gem_test_engine(int i915, unsigned int engine)
+{
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
+	struct drm_i915_gem_exec_object2 obj = { };
+	struct drm_i915_gem_execbuffer2 execbuf = {
+		.buffers_ptr = to_user_pointer(&obj),
+		.buffer_count = 1,
+	};
+
+	i915 = gem_reopen_driver(i915);
+
+	__gem_test_engine(i915, engine);
+
 	close(i915);
 }
 
diff --git a/lib/i915/gem_submission.h b/lib/i915/gem_submission.h
index acd24bcbf..32b737169 100644
--- a/lib/i915/gem_submission.h
+++ b/lib/i915/gem_submission.h
@@ -46,6 +46,7 @@  static inline bool gem_has_cmdparser(int i915, uint32_t engine)
 bool gem_has_blitter(int i915);
 void gem_require_blitter(int i915);
 
+void __gem_test_engine(int fd, unsigned int engine);
 void gem_test_engine(int fd, unsigned int engine);
 
 int gem_reopen_driver(int fd);