Re: [igt-dev] [PATCH v16 7/8] lib: igt_dummyload: use for_each_context_engine()

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Andi Shyti <andi.shyti@intel.com>,
	IGT dev <igt-dev@lists.freedesktop.org>
Cc: Andi Shyti <andi@etezian.org>
Subject: Re: [igt-dev] [PATCH v16 7/8] lib: igt_dummyload: use for_each_context_engine()
Date: Fri, 29 Mar 2019 12:33:35 +0000	[thread overview]
Message-ID: <b147f1c8-f4f3-72c8-bb9c-b374229a7e6b@linux.intel.com> (raw)
In-Reply-To: <20190328192206.1591-8-andi.shyti@intel.com>

On 28/03/2019 19:22, Andi Shyti wrote:
> With the new getparam/setparam api, engines are mapped to
> context. Use for_each_context_engine() to loop through existing
> engines.
> 
> Signed-off-by: Andi Shyti <andi.shyti@intel.com>
> ---
>   lib/igt_dummyload.c | 21 ++++++++-------------
>   1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 47f6b92b424b..4e94ccc448ba 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -39,6 +39,7 @@
>   #include "ioctl_wrappers.h"
>   #include "sw_sync.h"
>   #include "igt_vgem.h"
> +#include "i915/gem_engine_topology.h"
>   #include "i915/gem_mman.h"
>   
>   /**
> @@ -86,7 +87,7 @@ emit_recursive_batch(igt_spin_t *spin,
>   	struct drm_i915_gem_relocation_entry relocs[2], *r;
>   	struct drm_i915_gem_execbuffer2 *execbuf;
>   	struct drm_i915_gem_exec_object2 *obj;
> -	unsigned int engines[16];
> +	unsigned int flags[16];

Bump it to I915_EXEC_RING_MASK + 1 while at it.

>   	unsigned int nengine;
>   	int fence_fd = -1;
>   	uint32_t *batch, *batch_start;
> @@ -94,17 +95,17 @@ emit_recursive_batch(igt_spin_t *spin,
>   
>   	nengine = 0;
>   	if (opts->engine == ALL_ENGINES) {
> -		unsigned int engine;
> +		struct intel_execution_engine2 *engine;
>   
> -		for_each_physical_engine(fd, engine) {
> +		for_each_context_engine(fd, opts->ctx, engine) {
>   			if (opts->flags & IGT_SPIN_POLL_RUN &&
> -			    !gem_can_store_dword(fd, engine))
> +			    !gem_engine_can_store_dword(fd, engine))
>   				continue;
>   
> -			engines[nengine++] = engine;
> +			flags[nengine++] = engine->flags;
>   		}
>   	} else {
> -		engines[nengine++] = opts->engine;
> +		flags[nengine++] = opts->engine;

Single engine case needs the can_store_dword test as well.

You have to ioctl get_param.engines to figure out how to get engine 
class. And then just call either gem_class_can_store_dword or 
gem_can_store_dword depending on whether there is engine map on the 
context or not.

If there is no engine map also do gem_require_ring here (it was lost 
from the hunk below). In case of engine map I think you don't need to check.

Regards,

Tvrtko

>   	}
>   	igt_require(nengine);
>   
> @@ -234,7 +235,7 @@ emit_recursive_batch(igt_spin_t *spin,
>   
>   	for (i = 0; i < nengine; i++) {
>   		execbuf->flags &= ~ENGINE_MASK;
> -		execbuf->flags |= engines[i];
> +		execbuf->flags |= flags[i];
>   
>   		gem_execbuf_wr(fd, execbuf);
>   
> @@ -308,12 +309,6 @@ igt_spin_batch_factory(int fd, const struct igt_spin_factory *opts)
>   
>   	igt_require_gem(fd);
>   
> -	if (opts->engine != ALL_ENGINES) {
> -		gem_require_ring(fd, opts->engine);
> -		if (opts->flags & IGT_SPIN_POLL_RUN)
> -			igt_require(gem_can_store_dword(fd, opts->engine));
> -	}
> -
>   	spin = spin_batch_create(fd, opts);
>   
>   	igt_assert(gem_bo_busy(fd, spin->handle));
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev