Re: [PATCH i-g-t 1/3] tests/intel/xe_oa: Fix __for_one_render_engine

["Pottumuttu, Sai Teja" <sai.teja.pottumuttu@intel.com>]

On 14-03-2025 05:50, Ashutosh Dixit wrote:
> On platforms without render engines, __for_one_render_engine is an infinite
> loop. Fix and simplify the macro to reduce chance of such errors in the
> future.
> 
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

LGTM,

Reviewed-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>

> ---
>   tests/intel/xe_oa.c | 27 ++++++++++++++++++---------
>   1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index 9bc62c5b9d..245481f8a3 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -430,6 +430,18 @@ static u64 oa_format_fields(u64 name)
>   }
>   #define __ff oa_format_fields
>   
> +static struct drm_xe_engine_class_instance *hwe_of_class(int fd, int class)
> +{
> +	struct drm_xe_engine_class_instance *hwe;
> +
> +	for (int m = 0; m < xe_number_engines(fd); m++) {
> +		hwe = &xe_engine(drm_fd, m)->instance;
> +		if (hwe->engine_class == class)
> +			return hwe;
> +	}
> +	return NULL;
> +}
> +
>   static struct drm_xe_engine_class_instance *oa_unit_engine(int fd, int n)
>   {
>   	struct drm_xe_query_oa_units *qoa = xe_oa_units(fd);
> @@ -4645,13 +4657,10 @@ static const char *xe_engine_class_name(uint32_t engine_class)
>   		igt_dynamic_f("%s-%d-%s", xe_engine_class_name(hwe->engine_class), \
>   			      hwe->engine_instance, str)
>   
> -#define __for_one_render_engine(hwe)	      \
> -	for (int m = 0, done = 0; !done; m++) \
> -		for_each_if(m < xe_number_engines(drm_fd) && \
> -			    (hwe = &xe_engine(drm_fd, m)->instance) && \
> -			    hwe->engine_class == DRM_XE_ENGINE_CLASS_RENDER && \
> -			    (done = 1)) \
> -			igt_dynamic_f("rcs-%d", hwe->engine_instance)
> +#define __for_one_render_engine(hwe) \
> +	hwe = hwe_of_class(drm_fd, DRM_XE_ENGINE_CLASS_RENDER); \
> +	igt_require_f(hwe, "no render engine\n"); \
> +	igt_dynamic_f("rcs-%d", hwe->engine_instance)
>   
>   igt_main
>   {
> @@ -4895,10 +4904,10 @@ igt_main
>   		}
>   
>   		for (const struct sync_section *s = sync_sections; s->name; s++) {
> -			igt_subtest_with_dynamic_f("syncs-%s", s->name)
> +			igt_subtest_with_dynamic_f("syncs-%s", s->name) {
>   				__for_one_render_engine(hwe)
>   					test_syncs(hwe, s->sync_type, s->flags);
> -
> +			}
>   		}
>   	}
>