diff mbox

[i-g-t,v2,4/4] lib/i915: Extract context priority setparam to a helper

Message ID 20171013110020.32078-4-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michał Winiarski Oct. 13, 2017, 11 a.m. UTC 
Another example of something that is used across different tests, and
should be moved to lib.

v2: Break the trend of expanding ioctl_wrappers

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 lib/i915/gem_context.c    | 40 +++++++++++++++++++++++++++++++
 lib/i915/gem_context.h    |  6 +++++
 tests/gem_exec_nop.c      | 26 ++++----------------
 tests/gem_exec_schedule.c | 60 ++++++++++++++++-------------------------------
 tests/gem_exec_whisper.c  | 23 ++++--------------
 tests/gem_sync.c          | 27 ++++-----------------
 6 files changed, 78 insertions(+), 104 deletions(-)

Comments

Katarzyna Dec Oct. 13, 2017, 11:26 a.m. UTC | #1 
On Fri, Oct 13, 2017 at 01:00:20PM +0200, Michał Winiarski wrote:
> Another example of something that is used across different tests, and
> should be moved to lib.
> 
> v2: Break the trend of expanding ioctl_wrappers
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>

Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
Chris Wilson Oct. 13, 2017, 11:31 a.m. UTC | #2 
Quoting Michał Winiarski (2017-10-13 12:00:20)
> Another example of something that is used across different tests, and
> should be moved to lib.
> 
> v2: Break the trend of expanding ioctl_wrappers
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>

Sneaky 4/4 is ok as well, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox

Patch

diff --git a/lib/i915/gem_context.c b/lib/i915/gem_context.c
index ba826ae3..6d9edf5e 100644
--- a/lib/i915/gem_context.c
+++ b/lib/i915/gem_context.c
@@ -197,3 +197,43 @@  void gem_context_require_bannable(int fd)
 
 	igt_require(has_ban_period || has_bannable);
 }
+
+#define LOCAL_I915_CONTEXT_PARAM_PRIORITY 0x6
+
+/**
+ * __gem_context_set_priority:
+ * @fd: open i915 drm file descriptor
+ * @ctx_id: i915 context id
+ * @prio: desired context priority
+ *
+ * This function modifies priority property of the context.
+ * It is used by the scheduler to decide on the ordering of requests submitted
+ * to the hardware.
+ *
+ * Returns: An integer equal to zero for success and negative for failure
+ */
+int __gem_context_set_priority(int fd, uint32_t ctx_id, int prio)
+{
+	struct local_i915_gem_context_param p;
+
+	memset(&p, 0, sizeof(p));
+	p.context = ctx_id;
+	p.size = 0;
+	p.param = LOCAL_I915_CONTEXT_PARAM_PRIORITY;
+	p.value = prio;
+
+	return __gem_context_set_param(fd, &p);
+}
+
+/**
+ * gem_context_set_priority:
+ * @fd: open i915 drm file descriptor
+ * @ctx_id: i915 context id
+ * @prio: desired context priority
+ *
+ * Like __gem_context_set_priority(), except we assert on failure.
+ */
+void gem_context_set_priority(int fd, uint32_t ctx_id, int prio)
+{
+	igt_assert(__gem_context_set_priority(fd, ctx_id, prio) == 0);
+}
diff --git a/lib/i915/gem_context.h b/lib/i915/gem_context.h
index 06b2ca99..a2339c4b 100644
--- a/lib/i915/gem_context.h
+++ b/lib/i915/gem_context.h
@@ -45,4 +45,10 @@  void gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
 int __gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
 int __gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
 
+#define LOCAL_I915_CONTEXT_MAX_USER_PRIORITY	1023
+#define LOCAL_I915_CONTEXT_DEFAULT_PRIORITY	0
+#define LOCAL_I915_CONTEXT_MIN_USER_PRIORITY	-1023
+int __gem_context_set_priority(int fd, uint32_t ctx, int prio);
+void gem_context_set_priority(int fd, uint32_t ctx, int prio);
+
 #endif /* GEM_CONTEXT_H */
diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
index 78f2b1bd..2356f3c4 100644
--- a/tests/gem_exec_nop.c
+++ b/tests/gem_exec_nop.c
@@ -52,9 +52,8 @@ 
 
 #define ENGINE_FLAGS  (I915_EXEC_RING_MASK | LOCAL_I915_EXEC_BSD_MASK)
 
-#define LOCAL_CONTEXT_PARAM_PRIORITY 6
-#define   MAX_PRIO 1023
-#define   MIN_PRIO -1023
+#define MAX_PRIO LOCAL_I915_CONTEXT_MAX_USER_PRIORITY
+#define MIN_PRIO LOCAL_I915_CONTEXT_MIN_USER_PRIORITY
 
 #define FORKED 1
 #define CHAINED 2
@@ -585,23 +584,6 @@  static void fence_signal(int fd, uint32_t handle,
 	igt_info("Signal %s: %'lu cycles (%'lu signals): %.3fus\n",
 		 ring_name, count, signal, elapsed(&start, &now) * 1e6 / count);
 }
-static int __ctx_set_priority(int fd, uint32_t ctx, int prio)
-{
-	struct local_i915_gem_context_param param;
-
-	memset(&param, 0, sizeof(param));
-	param.context = ctx;
-	param.size = 0;
-	param.param = LOCAL_CONTEXT_PARAM_PRIORITY;
-	param.value = prio;
-
-	return __gem_context_set_param(fd, &param);
-}
-
-static void ctx_set_priority(int fd, uint32_t ctx, int prio)
-{
-	igt_assert_eq(__ctx_set_priority(fd, ctx, prio), 0);
-}
 
 static void preempt(int fd, uint32_t handle,
 		   unsigned ring_id, const char *ring_name)
@@ -615,10 +597,10 @@  static void preempt(int fd, uint32_t handle,
 	gem_require_ring(fd, ring_id);
 
 	ctx[0] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[0], MIN_PRIO);
+	gem_context_set_priority(fd, ctx[0], MIN_PRIO);
 
 	ctx[1] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[1], MAX_PRIO);
+	gem_context_set_priority(fd, ctx[1], MAX_PRIO);
 
 	memset(&obj, 0, sizeof(obj));
 	obj.handle = handle;
diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c
index d3df449a..6d3d725e 100644
--- a/tests/gem_exec_schedule.c
+++ b/tests/gem_exec_schedule.c
@@ -33,37 +33,17 @@ 
 #include "igt_rand.h"
 #include "igt_sysfs.h"
 
-#define LOCAL_CONTEXT_PARAM_PRIORITY 6
-
 #define LO 0
 #define HI 1
 #define NOISE 2
 
-#define MAX_PRIO 1023
-#define MIN_PRIO -1023
+#define MAX_PRIO LOCAL_I915_CONTEXT_MAX_USER_PRIORITY
+#define MIN_PRIO LOCAL_I915_CONTEXT_MIN_USER_PRIORITY
 
 #define BUSY_QLEN 8
 
 IGT_TEST_DESCRIPTION("Check that we can control the order of execution");
 
-static int __ctx_set_priority(int fd, uint32_t ctx, int prio)
-{
-	struct local_i915_gem_context_param param;
-
-	memset(&param, 0, sizeof(param));
-	param.context = ctx;
-	param.size = 0;
-	param.param = LOCAL_CONTEXT_PARAM_PRIORITY;
-	param.value = prio;
-
-	return __gem_context_set_param(fd, &param);
-}
-
-static void ctx_set_priority(int fd, uint32_t ctx, int prio)
-{
-	igt_assert_eq(__ctx_set_priority(fd, ctx, prio), 0);
-}
-
 static void store_dword(int fd, uint32_t ctx, unsigned ring,
 			uint32_t target, uint32_t offset, uint32_t value,
 			uint32_t cork, unsigned write_domain)
@@ -156,7 +136,7 @@  static uint32_t create_highest_priority(int fd)
 	 * priority (and therefore the max user priority), so no context
 	 * can overtake us, and we effectively can form a plug.
 	 */
-	__ctx_set_priority(fd, ctx, MAX_PRIO);
+	__gem_context_set_priority(fd, ctx, MAX_PRIO);
 
 	return ctx;
 }
@@ -245,7 +225,7 @@  static void smoketest(int fd, unsigned ring, unsigned timeout)
 			int prio;
 
 			prio = hars_petruska_f54_1_random_unsafe_max(MAX_PRIO - MIN_PRIO) + MIN_PRIO;
-			ctx_set_priority(fd, ctx, prio);
+			gem_context_set_priority(fd, ctx, prio);
 
 			engine = engines[hars_petruska_f54_1_random_unsafe_max(nengine)];
 			store_dword(fd, ctx, engine, scratch,
@@ -287,10 +267,10 @@  static void reorder(int fd, unsigned ring, unsigned flags)
 	uint32_t ctx[2];
 
 	ctx[LO] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[LO], MIN_PRIO);
+	gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
 
 	ctx[HI] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[HI], flags & EQUAL ? MIN_PRIO : 0);
+	gem_context_set_priority(fd, ctx[HI], flags & EQUAL ? MIN_PRIO : 0);
 
 	scratch = gem_create(fd, 4096);
 	plug(fd, &cork);
@@ -326,13 +306,13 @@  static void promotion(int fd, unsigned ring)
 	uint32_t ctx[3];
 
 	ctx[LO] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[LO], MIN_PRIO);
+	gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
 
 	ctx[HI] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[HI], 0);
+	gem_context_set_priority(fd, ctx[HI], 0);
 
 	ctx[NOISE] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[NOISE], MIN_PRIO/2);
+	gem_context_set_priority(fd, ctx[NOISE], MIN_PRIO/2);
 
 	result = gem_create(fd, 4096);
 	dep = gem_create(fd, 4096);
@@ -385,16 +365,16 @@  static void preempt(int fd, unsigned ring, unsigned flags)
 	uint32_t ctx[2];
 
 	ctx[LO] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[LO], MIN_PRIO);
+	gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
 
 	ctx[HI] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[HI], MAX_PRIO);
+	gem_context_set_priority(fd, ctx[HI], MAX_PRIO);
 
 	for (int n = 0; n < 16; n++) {
 		if (flags & NEW_CTX) {
 			gem_context_destroy(fd, ctx[LO]);
 			ctx[LO] = gem_context_create(fd);
-			ctx_set_priority(fd, ctx[LO], MIN_PRIO);
+			gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
 		}
 		spin[n] = __igt_spin_batch_new(fd, ctx[LO], ring, 0);
 		igt_debug("spin[%d].handle=%d\n", n, spin[n]->handle);
@@ -436,12 +416,12 @@  static void preempt_other(int fd, unsigned ring)
 	 */
 
 	ctx[LO] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[LO], MIN_PRIO);
+	gem_context_set_priority(fd, ctx[LO], MIN_PRIO);
 
 	ctx[NOISE] = gem_context_create(fd);
 
 	ctx[HI] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[HI], MAX_PRIO);
+	gem_context_set_priority(fd, ctx[HI], MAX_PRIO);
 
 	n = 0;
 	for_each_engine(fd, other) {
@@ -496,7 +476,7 @@  static void preempt_self(int fd, unsigned ring)
 	ctx[HI] = gem_context_create(fd);
 
 	n = 0;
-	ctx_set_priority(fd, ctx[HI], MIN_PRIO);
+	gem_context_set_priority(fd, ctx[HI], MIN_PRIO);
 	for_each_engine(fd, other) {
 		spin[n] = __igt_spin_batch_new(fd, ctx[NOISE], other, 0);
 		store_dword(fd, ctx[HI], other,
@@ -504,7 +484,7 @@  static void preempt_self(int fd, unsigned ring)
 			    0, I915_GEM_DOMAIN_RENDER);
 		n++;
 	}
-	ctx_set_priority(fd, ctx[HI], MAX_PRIO);
+	gem_context_set_priority(fd, ctx[HI], MAX_PRIO);
 	store_dword(fd, ctx[HI], ring,
 		    result, (n + 1)*sizeof(uint32_t), n + 1,
 		    0, I915_GEM_DOMAIN_RENDER);
@@ -542,7 +522,7 @@  static void deep(int fd, unsigned ring)
 	ctx = malloc(sizeof(*ctx) * nctx);
 	for (int n = 0; n < nctx; n++) {
 		ctx[n] = gem_context_create(fd);
-		ctx_set_priority(fd, ctx[n], MAX_PRIO - nctx + n);
+		gem_context_set_priority(fd, ctx[n], MAX_PRIO - nctx + n);
 	}
 
 	result = gem_create(fd, size);
@@ -784,7 +764,7 @@  static void reorder_wide(int fd, unsigned ring)
 		uint32_t *batch;
 
 		execbuf.rsvd1 = gem_context_create(fd);
-		ctx_set_priority(fd, execbuf.rsvd1, n);
+		gem_context_set_priority(fd, execbuf.rsvd1, n);
 
 		obj[2].handle = gem_create(fd, sz);
 		batch = gem_mmap__gtt(fd, obj[2].handle, sz, PROT_WRITE);
@@ -878,7 +858,7 @@  static void test_pi_ringfull(int fd, unsigned int engine)
 	execbuf.buffer_count = 1;
 	execbuf.flags = engine;
 	execbuf.rsvd1 = gem_context_create(fd);
-	ctx_set_priority(fd, execbuf.rsvd1, MIN_PRIO);
+	gem_context_set_priority(fd, execbuf.rsvd1, MIN_PRIO);
 
 	gem_execbuf(fd, &execbuf);
 	gem_sync(fd, obj[1].handle);
@@ -926,7 +906,7 @@  static void test_pi_ringfull(int fd, unsigned int engine)
 
 		igt_debug("Creating HP context\n");
 		execbuf.rsvd1 = gem_context_create(fd);
-		ctx_set_priority(fd, execbuf.rsvd1, MAX_PRIO);
+		gem_context_set_priority(fd, execbuf.rsvd1, MAX_PRIO);
 
 		kill(getppid(), SIGALRM);
 		sched_yield();
diff --git a/tests/gem_exec_whisper.c b/tests/gem_exec_whisper.c
index da5f26aa..18c7ea5b 100644
--- a/tests/gem_exec_whisper.c
+++ b/tests/gem_exec_whisper.c
@@ -191,25 +191,10 @@  static void fini_hang(struct hang *h)
 	close(h->fd);
 }
 
-#define LOCAL_CONTEXT_PARAM_PRIORITY 6
-
-static int __ctx_set_priority(int fd, uint32_t ctx, int prio)
-{
-	struct local_i915_gem_context_param param;
-
-	memset(&param, 0, sizeof(param));
-	param.context = ctx;
-	param.size = 0;
-	param.param = LOCAL_CONTEXT_PARAM_PRIORITY;
-	param.value = prio;
-
-	return __gem_context_set_param(fd, &param);
-}
-
-static void ctx_set_priority(int fd, uint32_t ctx)
+static void ctx_set_random_priority(int fd, uint32_t ctx)
 {
 	int prio = hars_petruska_f54_1_random_unsafe_max(1024) - 512;
-	igt_assert_eq(__ctx_set_priority(fd, ctx, prio), 0);
+	gem_context_set_priority(fd, ctx, prio);
 };
 
 static void whisper(int fd, unsigned engine, unsigned flags)
@@ -430,7 +415,7 @@  static void whisper(int fd, unsigned engine, unsigned flags)
 							gem_open(this_fd,
 									gem_flink(fd, handle[1]));
 						if (flags & PRIORITY)
-							ctx_set_priority(this_fd, 0);
+							ctx_set_random_priority(this_fd, 0);
 					}
 
 					if (!(flags & CHAIN)) {
@@ -440,7 +425,7 @@  static void whisper(int fd, unsigned engine, unsigned flags)
 					if (flags & CONTEXTS) {
 						execbuf.rsvd1 = contexts[rand() % 64];
 						if (flags & PRIORITY)
-							ctx_set_priority(this_fd, execbuf.rsvd1);
+							ctx_set_random_priority(this_fd, execbuf.rsvd1);
 					}
 
 					gem_execbuf(this_fd, &execbuf);
diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index fccc0c0e..e558682a 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -33,9 +33,8 @@ 
 #define LOCAL_I915_EXEC_BSD_SHIFT      (13)
 #define LOCAL_I915_EXEC_BSD_MASK       (3 << LOCAL_I915_EXEC_BSD_SHIFT)
 
-#define LOCAL_CONTEXT_PARAM_PRIORITY 6
-#define   MAX_PRIO 1023
-#define   MIN_PRIO -1023
+#define MAX_PRIO LOCAL_I915_CONTEXT_MAX_USER_PRIORITY
+#define MIN_PRIO LOCAL_I915_CONTEXT_MIN_USER_PRIORITY
 
 #define ENGINE_MASK  (I915_EXEC_RING_MASK | LOCAL_I915_EXEC_BSD_MASK)
 
@@ -690,24 +689,6 @@  store_all(int fd, int num_children, int timeout)
 	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
 }
 
-static int __ctx_set_priority(int fd, uint32_t ctx, int prio)
-{
-	struct local_i915_gem_context_param param;
-
-	memset(&param, 0, sizeof(param));
-	param.context = ctx;
-	param.size = 0;
-	param.param = LOCAL_CONTEXT_PARAM_PRIORITY;
-	param.value = prio;
-
-	return __gem_context_set_param(fd, &param);
-}
-
-static void ctx_set_priority(int fd, uint32_t ctx, int prio)
-{
-	igt_assert_eq(__ctx_set_priority(fd, ctx, prio), 0);
-}
-
 static void
 preempt(int fd, unsigned ring, int num_children, int timeout)
 {
@@ -746,10 +727,10 @@  preempt(int fd, unsigned ring, int num_children, int timeout)
 	}
 
 	ctx[0] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[0], MIN_PRIO);
+	gem_context_set_priority(fd, ctx[0], MIN_PRIO);
 
 	ctx[1] = gem_context_create(fd);
-	ctx_set_priority(fd, ctx[1], MAX_PRIO);
+	gem_context_set_priority(fd, ctx[1], MAX_PRIO);
 
 	intel_detect_and_clear_missed_interrupts(fd);
 	igt_fork(child, num_children) {