Re: [igt-dev] [PATCH i-g-t v2 2/5] test/i915: gem_exec_async: use the gem_engine_topology library

[Andi Shyti <andi.shyti@intel.com>]

Hi Ram,

On Thu, May 30, 2019 at 09:12:19PM +0530, Ramalingam C wrote:
> Replace the legacy for_each_engine* defines with the ones
> implemented in the gem_engine_topology library.
> 
> v2:
>   class and instance are compared for engine identification [Andi]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>

this looks good to me,

Reviewed-by: Andi Shyti <andi.shyti@intel.com>

Thanks,
Andi

> ---
>  tests/i915/gem_exec_async.c | 34 +++++++++++++++-------------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
> index 9a06af7e29cb..0da97f1f0506 100644
> --- a/tests/i915/gem_exec_async.c
> +++ b/tests/i915/gem_exec_async.c
> @@ -28,7 +28,7 @@
>  
>  IGT_TEST_DESCRIPTION("Check that we can issue concurrent writes across the engines.");
>  
> -static void store_dword(int fd, unsigned ring,
> +static void store_dword(int fd, const struct intel_execution_engine2 *e,
>  			uint32_t target, uint32_t offset, uint32_t value)
>  {
>  	const int gen = intel_gen(intel_get_drm_devid(fd));
> @@ -41,7 +41,7 @@ static void store_dword(int fd, unsigned ring,
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = to_user_pointer(obj);
>  	execbuf.buffer_count = 2;
> -	execbuf.flags = ring;
> +	execbuf.flags = e->flags;
>  	if (gen < 6)
>  		execbuf.flags |= I915_EXEC_SECURE;
>  
> @@ -80,15 +80,15 @@ static void store_dword(int fd, unsigned ring,
>  	gem_close(fd, obj[1].handle);
>  }
>  
> -static void one(int fd, unsigned ring, uint32_t flags)
> +static void one(int fd, const struct intel_execution_engine2 *e)
>  {
>  	const int gen = intel_gen(intel_get_drm_devid(fd));
>  	struct drm_i915_gem_exec_object2 obj[2];
> +	struct intel_execution_engine2 *engine;
>  #define SCRATCH 0
>  #define BATCH 1
>  	struct drm_i915_gem_relocation_entry reloc;
>  	struct drm_i915_gem_execbuffer2 execbuf;
> -	unsigned int other;
>  	uint32_t *batch;
>  	int i;
>  
> @@ -138,19 +138,21 @@ static void one(int fd, unsigned ring, uint32_t flags)
>  	memset(&execbuf, 0, sizeof(execbuf));
>  	execbuf.buffers_ptr = to_user_pointer(obj);
>  	execbuf.buffer_count = 2;
> -	execbuf.flags = ring | flags;
> +	execbuf.flags = e->flags;
>  	igt_require(__gem_execbuf(fd, &execbuf) == 0);
>  	gem_close(fd, obj[BATCH].handle);
>  
>  	i = 0;
> -	for_each_physical_engine(fd, other) {
> -		if (other == ring)
> +
> +	__for_each_physical_engine(fd, engine) {
> +		if (engine->class == e->class &&
> +		    engine->instance == e->instance)
>  			continue;
>  
> -		if (!gem_can_store_dword(fd, other))
> +		if (!gem_class_can_store_dword(fd, engine->class))
>  			continue;
>  
> -		store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
> +		store_dword(fd, engine, obj[SCRATCH].handle, 4*i, i);
>  		i++;
>  	}
>  
> @@ -185,7 +187,7 @@ static bool has_async_execbuf(int fd)
>  
>  igt_main
>  {
> -	const struct intel_execution_engine *e;
> +	const struct intel_execution_engine2 *e;
>  	int fd = -1;
>  
>  	igt_skip_on_simulation();
> @@ -199,17 +201,11 @@ igt_main
>  		igt_fork_hang_detector(fd);
>  	}
>  
> -	for (e = intel_execution_engines; e->name; e++) {
> -		/* default exec-id is purely symbolic */
> -		if (e->exec_id == 0)
> -			continue;
> -
> +	__for_each_physical_engine(fd, e)
>  		igt_subtest_f("concurrent-writes-%s", e->name) {
> -			igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags));
> -			igt_require(gem_can_store_dword(fd, e->exec_id | e->flags));
> -			one(fd, e->exec_id, e->flags);
> +			igt_require(gem_class_can_store_dword(fd, e->class));
> +			one(fd, e);
>  		}
> -	}
>  
>  	igt_fixture {
>  		igt_stop_hang_detector();
> -- 
> 2.19.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev