[igt-dev] [PATCH v13 7/9] lib/igt_gt: use for_each_engine_class_instance to loop through active engines

[Tvrtko Ursulin]

On 19/03/2019 23:44, 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.
> 
> For back compatibility the previous version of the loop
> definition has been renamed to 'for_each_engine_physical', which
> uses the default context 0.
> 
> Update tests/perf_pmu.c that uses the
> 'for_each_engine_physical()' loop.

Here we had a misunderstanding. :) When talking about 
for_each_engine_physical, I actually meant for_each_physical_engine.

I was suggesting to replace the existing implementation of the latter 
with the new scheme, and was asking if the churn to existing tests would 
be too big.

It is what we want I think, since that way we get coverage on all 
engines in existing test cases.

If you do this, and also add vcs2 to the static table in this series, 
and we send a series with complete media scalability patches to the 
list, then with some Test-with: magic you may see how perf_pmu, and even 
all the other for_each_physical_engined test create and exercise new 
subtests on Icelakes.

Regards,

Tvrtko

> 
> Signed-off-by: Andi Shyti <andi.shyti at intel.com>
> ---
>   lib/igt_gt.h     | 11 ++++++++---
>   tests/perf_pmu.c |  8 ++++----
>   2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_gt.h b/lib/igt_gt.h
> index 9f28af8cfb5c..691cbb11ee0a 100644
> --- a/lib/igt_gt.h
> +++ b/lib/igt_gt.h
> @@ -118,8 +118,13 @@ 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++)
> +
> +#define for_each_engine_physical(f, e) for_each_engine_class_instance(f, 0, e)
>   
>   #endif /* IGT_GT_H */
> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> index 45291298c021..79adeb2c8f3f 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_physical(gem_fd, e_) {
>   		if (e == e_)
>   			busy_idx = i;
>   
> @@ -497,7 +497,7 @@ 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_physical(gem_fd, e_) {
>   		if (e == e_)
>   			idle_idx = i;
>   		else if (spin)
> @@ -554,7 +554,7 @@ 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_physical(gem_fd, e) {
>   		if (spin)
>   			__submit_spin_batch(gem_fd, spin, e, 64);
>   		else
> @@ -1683,7 +1683,7 @@ igt_main
>   		igt_require_gem(fd);
>   		igt_require(i915_type_id() > 0);
>   
> -		for_each_engine_class_instance(fd, e)
> +		for_each_engine_physical(fd, e)
>   			num_engines++;
>   	}
>   
>