Re: [igt-dev] [PATCH v12 6/7] lib/igt_gt: use for_each_engine_class_instance to loop through active engines

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 18/03/2019 23:28, Andi Shyti wrote:
> Extend the 'for_each_engine_class_instance' so that it can loop
> only through active engines.
> 
> The 'for_each_engine_class_instance()' define starts its loop
> from generating a 'struct intel_engine_data' that contains only
> the current engines and it loops through the list.
> 
> A new parameter is added which refers to the context, to which
> engines ar bound.
> 
> Update tests/perf_pmu.c that uses the
> 'for_each_engine_class_instance()' loop.
> 
> Signed-off-by: Andi Shyti <andi.shyti@intel.com>
> ---
>   lib/igt_gt.h     |  9 ++++++---
>   tests/perf_pmu.c | 21 +++++++++++----------
>   2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/igt_gt.h b/lib/igt_gt.h
> index 475c0b3c3cc6..22baabce7bb0 100644
> --- a/lib/igt_gt.h
> +++ b/lib/igt_gt.h
> @@ -117,8 +117,11 @@ void gem_require_engine(int gem_fd,
>   #define __for_each_engine_class_instance(e__) \
>   	for ((e__) = intel_execution_engines2; (e__)->name; (e__)++)
>   
> -#define for_each_engine_class_instance(fd__, e__) \
> -	for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) \
> -		for_if (gem_has_engine((fd__), (e__)->class, (e__)->instance))
> +#include "i915/gem_engine_topology.h"
> +
> +#define for_each_engine_class_instance(fd__, ctx__, e__) \
> +	for (struct intel_engine_data i__ = intel_init_engine_list(fd__, ctx__); \
> +		((e__) = (i__.n < i__.nengines) ? &i__.engines[i__.n] : NULL); \
> +			i__.n++)
>   
>   #endif /* IGT_GT_H */
> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> index 45291298c021..1b02192201e8 100644
> --- a/tests/perf_pmu.c
> +++ b/tests/perf_pmu.c
> @@ -434,7 +434,7 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
>   
>   	i = 0;
>   	fd[0] = -1;
> -	for_each_engine_class_instance(gem_fd, e_) {
> +	for_each_engine_class_instance(gem_fd, 0, e_) {

How much churn it would be to drop for_each_engine_class_instance and 
convert the for_each_engine_physical macro to use the new scheme?

I am thinking about ending with two macros:

for_each_engine_physical(fd, e) // or not e, if it is in the iterator?

for_each_engine_context(fd, ctx, e) // when tests are using contexts

The former would just be:

#define for_each_engine_physical(fd) for_each_engine_context(fd, 0)

Chris your opinion?

Regards,

Tvrtko

>   		if (e == e_)
>   			busy_idx = i;
>   
> @@ -473,15 +473,16 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
>   static void
>   __submit_spin_batch(int gem_fd, igt_spin_t *spin,
>   		    const struct intel_execution_engine2 *e,
> -		    int offset)
> +		    int offset,
> +		    unsigned int i)
>   {
>   	struct drm_i915_gem_execbuffer2 eb = spin->execbuf;
>   
>   	eb.flags &= ~(0x3f | I915_EXEC_BSD_MASK);
> -	eb.flags |= e2ring(gem_fd, e) | I915_EXEC_NO_RELOC;
> +	eb.flags |= I915_EXEC_NO_RELOC;
>   	eb.batch_start_offset += offset;
>   
> -	gem_execbuf(gem_fd, &eb);
> +	gem_context_execbuf(gem_fd, &eb, *e, i, 0);
>   }
>   
>   static void
> @@ -497,11 +498,11 @@ most_busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
>   	unsigned int idle_idx, i;
>   
>   	i = 0;
> -	for_each_engine_class_instance(gem_fd, e_) {
> +	for_each_engine_class_instance(gem_fd, 0, e_) {
>   		if (e == e_)
>   			idle_idx = i;
>   		else if (spin)
> -			__submit_spin_batch(gem_fd, spin, e_, 64);
> +			__submit_spin_batch(gem_fd, spin, e_, 64, i + 1);
>   		else
>   			spin = __spin_poll(gem_fd, 0, e2ring(gem_fd, e_));
>   
> @@ -554,9 +555,9 @@ all_busy_check_all(int gem_fd, const unsigned int num_engines,
>   	unsigned int i;
>   
>   	i = 0;
> -	for_each_engine_class_instance(gem_fd, e) {
> +	for_each_engine_class_instance(gem_fd, 0, e) {
>   		if (spin)
> -			__submit_spin_batch(gem_fd, spin, e, 64);
> +			__submit_spin_batch(gem_fd, spin, e, 64, i + 1);
>   		else
>   			spin = __spin_poll(gem_fd, 0, e2ring(gem_fd, e));
>   
> @@ -1597,7 +1598,7 @@ accuracy(int gem_fd, const struct intel_execution_engine2 *e,
>   
>   				/* Restart the spinbatch. */
>   				__rearm_spin_batch(spin);
> -				__submit_spin_batch(gem_fd, spin, e, 0);
> +				__submit_spin_batch(gem_fd, spin, e, 0, 0);
>   
>   				/* PWM busy sleep. */
>   				loop_busy = igt_nsec_elapsed(&start);
> @@ -1683,7 +1684,7 @@ igt_main
>   		igt_require_gem(fd);
>   		igt_require(i915_type_id() > 0);
>   
> -		for_each_engine_class_instance(fd, e)
> +		for_each_engine_class_instance(fd, 0, e)
>   			num_engines++;
>   	}
>   
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev