[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 at intel.com>
> 
> Cc: Chris Wilson <chris at 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