[igt-dev] [PATCH i-g-t] tests/i915/gem_exec_schedule.c: Switch to gem_sync and gem_read

[igt-dev] [PATCH i-g-t] tests/i915/gem_exec_schedule.c: Switch to gem_sync and gem_read

[Antonio Argenziano via igt-dev]

use IOCTLs gem_read for reading and, gem_sync to wait on a batch.

Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>

Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_exec_schedule.c | 113 +++++++++++++--------------------
 1 file changed, 45 insertions(+), 68 deletions(-)

diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 00f9528a..5df48960 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -152,7 +152,7 @@ static void fifo(int fd, unsigned ring)
 {
 	IGT_CORK_HANDLE(cork);
 	uint32_t scratch, plug;
-	uint32_t *ptr;
+	uint32_t result;
 
 	scratch = gem_create(fd, 4096);
 
@@ -165,13 +165,10 @@ static void fifo(int fd, unsigned ring)
 	unplug_show_queue(fd, &cork, ring);
 	gem_close(fd, plug);
 
-	ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ);
-	gem_set_domain(fd, scratch, /* no write hazard lies! */
-			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+	gem_read(fd, scratch, 0, &result, sizeof(result));
 	gem_close(fd, scratch);
 
-	igt_assert_eq_u32(ptr[0], 2);
-	munmap(ptr, 4096);
+	igt_assert_eq_u32(result, 2);
 }
 
 static void independent(int fd, unsigned int engine)
@@ -249,7 +246,7 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
 	unsigned nengine;
 	unsigned engine;
 	uint32_t scratch;
-	uint32_t *ptr;
+	uint32_t result[4096 / sizeof(uint32_t)];
 
 	nengine = 0;
 	if (ring == ALL_ENGINES) {
@@ -287,22 +284,19 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
 	}
 	igt_waitchildren();
 
-	ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ);
-	gem_set_domain(fd, scratch, /* no write hazard lies! */
-			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+	gem_read(fd, scratch, 0, result, 4096);
 	gem_close(fd, scratch);
 
 	for (unsigned n = 0; n < ncpus; n++) {
-		igt_assert_eq_u32(ptr[2*n], ~n);
+		igt_assert_eq_u32(result[2 * n], ~n);
 		/*
 		 * Note this count is approximate due to unconstrained
 		 * ordering of the dword writes between engines.
 		 *
 		 * Take the result with a pinch of salt.
 		 */
-		igt_info("Child[%d] completed %u cycles\n",  n, ptr[2*n+1]);
+		igt_info("Child[%d] completed %u cycles\n",  n, result[(2 * n) + 1]);
 	}
-	munmap(ptr, 4096);
 }
 
 static void reorder(int fd, unsigned ring, unsigned flags)
@@ -310,7 +304,7 @@ static void reorder(int fd, unsigned ring, unsigned flags)
 {
 	IGT_CORK_HANDLE(cork);
 	uint32_t scratch, plug;
-	uint32_t *ptr;
+	uint32_t result;
 	uint32_t ctx[2];
 
 	ctx[LO] = gem_context_create(fd);
@@ -334,23 +328,20 @@ static void reorder(int fd, unsigned ring, unsigned flags)
 	gem_context_destroy(fd, ctx[LO]);
 	gem_context_destroy(fd, ctx[HI]);
 
-	ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ);
-	gem_set_domain(fd, scratch, /* no write hazard lies! */
-			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+	gem_read(fd, scratch, 0, &result, sizeof(result));
 	gem_close(fd, scratch);
 
 	if (flags & EQUAL) /* equal priority, result will be fifo */
-		igt_assert_eq_u32(ptr[0], ctx[HI]);
+		igt_assert_eq_u32(result, ctx[HI]);
 	else
-		igt_assert_eq_u32(ptr[0], ctx[LO]);
-	munmap(ptr, 4096);
+		igt_assert_eq_u32(result, ctx[LO]);
 }
 
 static void promotion(int fd, unsigned ring)
 {
 	IGT_CORK_HANDLE(cork);
 	uint32_t result, dep;
-	uint32_t *ptr;
+	uint32_t result_read, dep_read;
 	uint32_t ctx[3];
 	uint32_t plug;
 
@@ -389,21 +380,14 @@ static void promotion(int fd, unsigned ring)
 	gem_context_destroy(fd, ctx[LO]);
 	gem_context_destroy(fd, ctx[HI]);
 
-	ptr = gem_mmap__gtt(fd, dep, 4096, PROT_READ);
-	gem_set_domain(fd, dep, /* no write hazard lies! */
-			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+	gem_read(fd, dep, 0, &dep_read, sizeof(dep_read));
 	gem_close(fd, dep);
 
-	igt_assert_eq_u32(ptr[0], ctx[HI]);
-	munmap(ptr, 4096);
-
-	ptr = gem_mmap__gtt(fd, result, 4096, PROT_READ);
-	gem_set_domain(fd, result, /* no write hazard lies! */
-			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+	gem_read(fd, result, 0, &result_read, sizeof(result_read));
 	gem_close(fd, result);
 
-	igt_assert_eq_u32(ptr[0], ctx[NOISE]);
-	munmap(ptr, 4096);
+	igt_assert_eq_u32(dep_read, ctx[HI]);
+	igt_assert_eq_u32(result_read, ctx[NOISE]);
 }
 
 #define NEW_CTX (0x1 << 0)
@@ -411,7 +395,7 @@ static void promotion(int fd, unsigned ring)
 static void preempt(int fd, unsigned ring, unsigned flags)
 {
 	uint32_t result = gem_create(fd, 4096);
-	uint32_t *ptr = gem_mmap__gtt(fd, result, 4096, PROT_READ);
+	uint32_t result_read;
 	igt_spin_t *spin[MAX_ELSP_QLEN];
 	uint32_t ctx[2];
 	igt_hang_t hang;
@@ -438,8 +422,8 @@ static void preempt(int fd, unsigned ring, unsigned flags)
 
 		store_dword(fd, ctx[HI], ring, result, 0, n + 1, 0, I915_GEM_DOMAIN_RENDER);
 
-		gem_set_domain(fd, result, I915_GEM_DOMAIN_GTT, 0);
-		igt_assert_eq_u32(ptr[0], n + 1);
+		gem_read(fd, result, 0, &result_read, sizeof(result_read));
+		igt_assert_eq_u32(result_read, n + 1);
 		igt_assert(gem_bo_busy(fd, spin[0]->handle));
 	}
 
@@ -452,7 +436,6 @@ static void preempt(int fd, unsigned ring, unsigned flags)
 	gem_context_destroy(fd, ctx[LO]);
 	gem_context_destroy(fd, ctx[HI]);
 
-	munmap(ptr, 4096);
 	gem_close(fd, result);
 }
 
@@ -493,7 +476,7 @@ static void __preempt_other(int fd,
 			    unsigned flags)
 {
 	uint32_t result = gem_create(fd, 4096);
-	uint32_t *ptr = gem_mmap__gtt(fd, result, 4096, PROT_READ);
+	uint32_t result_read[4096 / sizeof(uint32_t)];
 	unsigned int n, i, other;
 
 	n = 0;
@@ -519,10 +502,11 @@ static void __preempt_other(int fd,
 	gem_set_domain(fd, result, I915_GEM_DOMAIN_GTT, 0);
 
 	n++;
+
+	gem_read(fd, result, 0, result_read, 4096);
 	for (i = 0; i <= n; i++)
-		igt_assert_eq_u32(ptr[i], i);
+		igt_assert_eq_u32(result_read[i], i);
 
-	munmap(ptr, 4096);
 	gem_close(fd, result);
 }
 
@@ -570,7 +554,7 @@ static void __preempt_queue(int fd,
 			    unsigned depth, unsigned flags)
 {
 	uint32_t result = gem_create(fd, 4096);
-	uint32_t *ptr = gem_mmap__gtt(fd, result, 4096, PROT_READ);
+	uint32_t result_read[4096 / sizeof(4096)];
 	igt_spin_t *above = NULL, *below = NULL;
 	unsigned int other, n, i;
 	int prio = MAX_PRIO;
@@ -628,9 +612,11 @@ static void __preempt_queue(int fd,
 
 	gem_set_domain(fd, result, I915_GEM_DOMAIN_GTT, 0);
 
+	gem_read(fd, result, 0, result_read, 4096);
+
 	n++;
 	for (i = 0; i <= n; i++)
-		igt_assert_eq_u32(ptr[i], i);
+		igt_assert_eq_u32(result_read[i], i);
 
 	if (below) {
 		igt_assert(gem_bo_busy(fd, below->handle));
@@ -641,7 +627,6 @@ static void __preempt_queue(int fd,
 	gem_context_destroy(fd, ctx[NOISE]);
 	gem_context_destroy(fd, ctx[HI]);
 
-	munmap(ptr, 4096);
 	gem_close(fd, result);
 }
 
@@ -658,7 +643,7 @@ static void preempt_queue(int fd, unsigned ring, unsigned int flags)
 static void preempt_self(int fd, unsigned ring)
 {
 	uint32_t result = gem_create(fd, 4096);
-	uint32_t *ptr = gem_mmap__gtt(fd, result, 4096, PROT_READ);
+	uint32_t result_read[4096 / sizeof(uint32_t)];
 	igt_spin_t *spin[MAX_ELSP_QLEN];
 	unsigned int other;
 	unsigned int n, i;
@@ -699,14 +684,15 @@ static void preempt_self(int fd, unsigned ring)
 		igt_spin_batch_free(fd, spin[i]);
 	}
 
+	gem_read(fd, result, 0, result_read, 4096);
+
 	n++;
 	for (i = 0; i <= n; i++)
-		igt_assert_eq_u32(ptr[i], i);
+		igt_assert_eq_u32(result_read[i], i);
 
 	gem_context_destroy(fd, ctx[NOISE]);
 	gem_context_destroy(fd, ctx[HI]);
 
-	munmap(ptr, 4096);
 	gem_close(fd, result);
 }
 
@@ -755,8 +741,8 @@ static void deep(int fd, unsigned ring)
 	unsigned int nreq;
 	uint32_t plug;
 	uint32_t result, dep[XS];
+	uint32_t result_read[size], dep_read[size];
 	uint32_t expected = 0;
-	uint32_t *ptr;
 	uint32_t *ctx;
 	int dep_nreq;
 	int n;
@@ -879,25 +865,20 @@ static void deep(int fd, unsigned ring)
 		gem_context_destroy(fd, ctx[n]);
 
 	for (int m = 0; m < XS; m++) {
-		ptr = gem_mmap__gtt(fd, dep[m], size, PROT_READ);
-		gem_set_domain(fd, dep[m], /* no write hazard lies! */
-				I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+
+		gem_read(fd, dep[m], 0, dep_read, size);
 		gem_close(fd, dep[m]);
 
 		for (n = 0; n < dep_nreq; n++)
-			igt_assert_eq_u32(ptr[n], ctx[n % MAX_CONTEXTS]);
-		munmap(ptr, size);
+			igt_assert_eq_u32(dep_read[n], ctx[n % MAX_CONTEXTS]);
 	}
 
-	ptr = gem_mmap__gtt(fd, result, size, PROT_READ);
-	gem_set_domain(fd, result, /* no write hazard lies! */
-			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 	gem_close(fd, result);
 
+	gem_read(fd, result, 0, result_read, size);
 	/* No reordering due to PI on all contexts because of the common dep */
 	for (int m = 0; m < XS; m++)
-		igt_assert_eq_u32(ptr[m], expected);
-	munmap(ptr, size);
+		igt_assert_eq_u32(result_read[m], expected);
 
 	free(ctx);
 #undef XS
@@ -923,7 +904,7 @@ static void wide(int fd, unsigned ring)
 	IGT_CORK_HANDLE(cork);
 	uint32_t plug;
 	uint32_t result;
-	uint32_t *ptr;
+	uint32_t result_read[4*MAX_CONTEXTS];
 	uint32_t *ctx;
 	unsigned int count;
 
@@ -952,12 +933,9 @@ static void wide(int fd, unsigned ring)
 	for (int n = 0; n < MAX_CONTEXTS; n++)
 		gem_context_destroy(fd, ctx[n]);
 
-	ptr = gem_mmap__gtt(fd, result, 4*MAX_CONTEXTS, PROT_READ);
-	gem_set_domain(fd, result, /* no write hazard lies! */
-			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+	gem_read(fd, result, n, result_read, 4*MAX_CONTEXTS);
 	for (int n = 0; n < MAX_CONTEXTS; n++)
-		igt_assert_eq_u32(ptr[n], ctx[n]);
-	munmap(ptr, 4*MAX_CONTEXTS);
+		igt_assert_eq_u32(result_read[n], ctx[n]);
 
 	gem_close(fd, result);
 	free(ctx);
@@ -973,7 +951,8 @@ static void reorder_wide(int fd, unsigned ring)
 	unsigned int ring_size = gem_measure_ring_inflight(fd, ring, MEASURE_RING_NEW_CTX);
 	IGT_CORK_HANDLE(cork);
 	uint32_t result, target, plug;
-	uint32_t *found, *expected;
+	uint32_t result_read[4096 / sizeof(uint32_t)];
+	uint32_t *expected;
 
 	result = gem_create(fd, 4096);
 	target = gem_create(fd, 4096);
@@ -1053,12 +1032,10 @@ static void reorder_wide(int fd, unsigned ring)
 	unplug_show_queue(fd, &cork, ring);
 	gem_close(fd, plug);
 
-	found = gem_mmap__gtt(fd, result, 4096, PROT_READ);
-	gem_set_domain(fd, result, /* no write hazard lies! */
-			I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+	gem_read(fd, result, 0, result_read, 4096);
 	for (int n = 0; n < 1024; n++)
-		igt_assert_eq_u32(found[n], expected[n]);
-	munmap(found, 4096);
+		igt_assert_eq_u32(result_read[n], expected[n]);
+
 	munmap(expected, 4096);
 
 	gem_close(fd, result);
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_exec_schedule.c: Switch to gem_sync and gem_read

[Chris Wilson]

Quoting Antonio Argenziano (2019-02-14 01:22:50)
> use IOCTLs gem_read for reading and, gem_sync to wait on a batch.

At least give a hint as to why.
 
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/i915/gem_exec_schedule.c | 113 +++++++++++++--------------------
>  1 file changed, 45 insertions(+), 68 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> index 00f9528a..5df48960 100644
> --- a/tests/i915/gem_exec_schedule.c
> +++ b/tests/i915/gem_exec_schedule.c
> @@ -152,7 +152,7 @@ static void fifo(int fd, unsigned ring)
>  {
>         IGT_CORK_HANDLE(cork);
>         uint32_t scratch, plug;
> -       uint32_t *ptr;
> +       uint32_t result;
>  
>         scratch = gem_create(fd, 4096);
>  
> @@ -165,13 +165,10 @@ static void fifo(int fd, unsigned ring)
>         unplug_show_queue(fd, &cork, ring);
>         gem_close(fd, plug);
>  
> -       ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ);
> -       gem_set_domain(fd, scratch, /* no write hazard lies! */
> -                       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);

gem_sync.

> +       gem_read(fd, scratch, 0, &result, sizeof(result));

So I think something like a 

static inline uint32_t
sync_read_u32(int i915, uint32_t handle, uint64_t offset)
{
	uint32_t result;

	gem_sync(i915, handle); /* no write hazard lies! */
	gem_read(i915, handle, offset, &results, sizeof(result));

	return result;
}

result = sync_read_u32(fd, scratch, 0);

would tidy up most of the conversions.

>         gem_close(fd, scratch);
>  
> -       igt_assert_eq_u32(ptr[0], 2);
> -       munmap(ptr, 4096);
> +       igt_assert_eq_u32(result, 2);
>  }
>  
>  static void independent(int fd, unsigned int engine)
> @@ -249,7 +246,7 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
>         unsigned nengine;
>         unsigned engine;
>         uint32_t scratch;
> -       uint32_t *ptr;
> +       uint32_t result[4096 / sizeof(uint32_t)];

Is ncpus not known at this point, could it be?
uint32_t results[2 * ncpus];

>         nengine = 0;
>         if (ring == ALL_ENGINES) {
> @@ -287,22 +284,19 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
>         }
>         igt_waitchildren();
>  
> -       ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ);
> -       gem_set_domain(fd, scratch, /* no write hazard lies! */
> -                       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);

Anything that has /* no write hazard lies! */ must be replaced by
gem_sync() with the same warning.

> +       gem_read(fd, scratch, 0, result, 4096);
s/4096/sizeof(results))
>         gem_close(fd, scratch);

[snip more of the same]
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for tests/i915/gem_exec_schedule.c: Switch to gem_sync and gem_read

[Patchwork]

== Series Details ==

Series: tests/i915/gem_exec_schedule.c: Switch to gem_sync and gem_read
URL   : https://patchwork.freedesktop.org/series/56642/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
eea5cf40199c0d08ac654481b830d8a3d4408502 test: Add PSR2 selective update tests

[247/464] Linking target tests/gem_close_race.
[248/464] Linking target tests/gem_concurrent_blit.
[249/464] Linking target tests/gem_cs_prefetch.
[250/464] Linking target tests/gem_cpu_reloc.
[251/464] Linking target tests/gem_create.
[252/464] Linking target tests/gem_cs_tlb.
[253/464] Linking target tests/gem_ctx_create.
[254/464] Linking target tests/gem_ctx_bad_destroy.
[255/464] Linking target tests/gem_ctx_exec.
[256/464] Linking target tests/gem_ctx_param.
[257/464] Linking target tests/gem_ctx_isolation.
[258/464] Linking target tests/gem_ctx_thrash.
[259/464] Linking target tests/gem_ctx_switch.
[260/464] Linking target tests/gem_evict_alignment.
[261/464] Linking target tests/gem_double_irq_loop.
[262/464] Linking target tests/gem_exec_alignment.
[263/464] Linking target tests/gem_evict_everything.
[264/464] Linking target tests/gem_exec_async.
[265/464] Linking target tests/gem_exec_await.
[266/464] Linking target tests/gem_exec_big.
[267/464] Linking target tests/gem_exec_bad_domains.
[268/464] Linking target tests/gem_exec_basic.
[269/464] Linking target tests/gem_exec_blt.
[270/464] Linking target tests/gem_exec_create.
[271/464] Linking target tests/gem_exec_capture.
[272/464] Linking target tests/gem_exec_faulting_reloc.
[273/464] Linking target tests/gem_exec_fence.
[274/464] Linking target tests/gem_exec_flush.
[275/464] Linking target tests/gem_exec_gttfill.
[276/464] Linking target tests/gem_exec_latency.
[277/464] Linking target tests/gem_exec_nop.
[278/464] Linking target tests/gem_exec_lut_handle.
[279/464] Linking target tests/gem_exec_parallel.
[280/464] Linking target tests/gem_exec_reloc.
[281/464] Linking target tests/gem_exec_parse.
[282/464] Linking target tests/gem_exec_params.
[283/464] Linking target tests/gem_exec_reuse.
[284/464] Compiling C object 'tests/gem_exec_schedule@exe/i915_gem_exec_schedule.c.o'.
FAILED: tests/gem_exec_schedule@exe/i915_gem_exec_schedule.c.o 
ccache cc  -Itests/gem_exec_schedule@exe -Itests -I../tests -I. -I../ -I../lib/stubs/syscalls -Ilib -I../lib -I../include/drm-uapi -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/libpng16 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/opt/igt/include -I/opt/igt/include/libdrm -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/home/cidrm/kernel_headers/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O0 -g -D_GNU_SOURCE -include config.h -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -pthread -MD -MQ 'tests/gem_exec_schedule@exe/i915_gem_exec_schedule.c.o' -MF 'tests/gem_exec_schedule@exe/i915_gem_exec_schedule.c.o.d' -o 'tests/gem_exec_schedule@exe/i915_gem_exec_schedule.c.o' -c ../tests/i915/gem_exec_schedule.c
../tests/i915/gem_exec_schedule.c: In function ‘wide’:
../tests/i915/gem_exec_schedule.c:936:23: error: ‘n’ undeclared (first use in this function); did you mean ‘yn’?
  gem_read(fd, result, n, result_read, 4*MAX_CONTEXTS);
                       ^
                       yn
../tests/i915/gem_exec_schedule.c:936:23: note: each undeclared identifier is reported only once for each function it appears in
../tests/i915/gem_exec_schedule.c:937:11: warning: declaration of ‘n’ shadows previous non-variable [-Wshadow]
  for (int n = 0; n < MAX_CONTEXTS; n++)
           ^
ninja: build stopped: subcommand failed.

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

Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_exec_schedule.c: Switch to gem_sync and gem_read

[Antonio Argenziano via igt-dev]

On 14/02/19 00:59, Chris Wilson wrote:
> Quoting Antonio Argenziano (2019-02-14 01:22:50)
>> use IOCTLs gem_read for reading and, gem_sync to wait on a batch.
> 
> At least give a hint as to why.
>   
>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   tests/i915/gem_exec_schedule.c | 113 +++++++++++++--------------------
>>   1 file changed, 45 insertions(+), 68 deletions(-)
>>
>> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
>> index 00f9528a..5df48960 100644
>> --- a/tests/i915/gem_exec_schedule.c
>> +++ b/tests/i915/gem_exec_schedule.c
>> @@ -152,7 +152,7 @@ static void fifo(int fd, unsigned ring)
>>   {
>>          IGT_CORK_HANDLE(cork);
>>          uint32_t scratch, plug;
>> -       uint32_t *ptr;
>> +       uint32_t result;
>>   
>>          scratch = gem_create(fd, 4096);
>>   
>> @@ -165,13 +165,10 @@ static void fifo(int fd, unsigned ring)
>>          unplug_show_queue(fd, &cork, ring);
>>          gem_close(fd, plug);
>>   
>> -       ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ);
>> -       gem_set_domain(fd, scratch, /* no write hazard lies! */
>> -                       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> 
> gem_sync.
> 
>> +       gem_read(fd, scratch, 0, &result, sizeof(result));
> 
> So I think something like a
> 
> static inline uint32_t
> sync_read_u32(int i915, uint32_t handle, uint64_t offset)
> {
> 	uint32_t result;
> 
> 	gem_sync(i915, handle); /* no write hazard lies! */
> 	gem_read(i915, handle, offset, &results, sizeof(result));
> 
> 	return result;
> }
> 
> result = sync_read_u32(fd, scratch, 0);
> 
> would tidy up most of the conversions.
> 
>>          gem_close(fd, scratch);
>>   
>> -       igt_assert_eq_u32(ptr[0], 2);
>> -       munmap(ptr, 4096);
>> +       igt_assert_eq_u32(result, 2);
>>   }
>>   
>>   static void independent(int fd, unsigned int engine)
>> @@ -249,7 +246,7 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
>>          unsigned nengine;
>>          unsigned engine;
>>          uint32_t scratch;
>> -       uint32_t *ptr;
>> +       uint32_t result[4096 / sizeof(uint32_t)];
> 
> Is ncpus not known at this point, could it be?
> uint32_t results[2 * ncpus];

I went with the size of the source mainly because it is easier to find 
at a glance.

Antonio

> 
>>          nengine = 0;
>>          if (ring == ALL_ENGINES) {
>> @@ -287,22 +284,19 @@ static void smoketest(int fd, unsigned ring, unsigned timeout)
>>          }
>>          igt_waitchildren();
>>   
>> -       ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ);
>> -       gem_set_domain(fd, scratch, /* no write hazard lies! */
>> -                       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> 
> Anything that has /* no write hazard lies! */ must be replaced by
> gem_sync() with the same warning.
> 
>> +       gem_read(fd, scratch, 0, result, 4096);
> s/4096/sizeof(results))
>>          gem_close(fd, scratch);
> 
> [snip more of the same]
> -Chris
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev