Re: [PATCH i-g-t 1/2] tests/intel/xe_drm_fdinfo: Group utilization tests and skip when no utilization data

["Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>]

On 3/5/2025 1:15 PM, Kamil Konieczny wrote:
> Hi Marcin,
> On 2025-03-04 at 19:56:39 +0100, Marcin Bernatowicz wrote:
>> Wrap all utilization-related subtests in an igt_subtest_group and add
>> a fixture that ensures utilization data is available before running
>> them.
>>
>> Link: https://lore.kernel.org/r/20250205191644.2550879-1-marcin.bernatowicz@linux.intel.com
>>
>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
>> Cc: Jakub Kolakowski <jakub1.kolakowski@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Lukasz Laguna <lukasz.laguna@intel.com>
>> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>> ---
>>   tests/intel/xe_drm_fdinfo.c | 122 ++++++++++++++++++++----------------
>>   1 file changed, 67 insertions(+), 55 deletions(-)
>>
>> diff --git a/tests/intel/xe_drm_fdinfo.c b/tests/intel/xe_drm_fdinfo.c
>> index 7330b4330..d18fe10d8 100644
>> --- a/tests/intel/xe_drm_fdinfo.c
>> +++ b/tests/intel/xe_drm_fdinfo.c
>> @@ -735,10 +735,6 @@ igt_main
>>   	igt_subtest("basic-mem")
>>   		basic_memory(xe);
>>   
>> -	igt_describe("Check if basic fdinfo content is present for engine utilization");
>> -	igt_subtest("basic-utilization")
>> -		basic_engine_utilization(xe);
>> -
>>   	igt_describe("Create and compare total and resident memory consumption by client");
>>   	igt_subtest("mem-total-resident")
>>   		mem_total_resident(xe);
>> @@ -751,57 +747,73 @@ igt_main
> [...]
> 
>> +	igt_subtest_group {
>> +		igt_fixture {
>> +			struct drm_client_fdinfo info = { };
>> +
>> +			igt_require(igt_parse_drm_fdinfo(xe, &info, engine_map,
>> +							 ARRAY_SIZE(engine_map),
>> +							 NULL, 0));
> 
> Problem with require placed here is that it will trigger also
> for all other test outside of group. That will clobber output.

Does it mean the igt_subtest_group is deprecated and the description
no more valid ?

"
/**
  * igt_subtest_group:

...

* This macro allows to group together a set of #igt_fixture and #igt_subtest
  * clauses. If any common setup in a fixture fails, only the subtests 
in this
  * group will fail or skip. Subtest groups can be arbitrarily nested.
"

> 
>> +			igt_require(info.num_engines);
> 
> Both these should be placed in each subtest wrapped in function
> (see below) but imho this looks like a global requires at begin
> of a test? Or am I missing something?
> 
>> +		}
>> +
> 
> Create function like:
> 
> void require_utilization(int xe, ... engine_map)
> {
> 	struct drm_client_fdinfo info = { };
> 
> 	igt_require(igt_parse_drm_fdinfo(xe, &info, engine_map,
> 					 ARRAY_SIZE(engine_map),
> 					 NULL, 0));
> 	igt_require(info.num_engines);
> }
> 
> and use it in each utulization subtest like:
> 
> 	igt_describe("Check if basic fdinfo content is present for engine utilization");
> 	igt_subtest("basic-utilization") {
> 		require_utilization(xe);
> 		basic_engine_utilization(xe);
> 	}

That is the repetition I wanted to avoid ;)

---
marcin

> 
> Regards,
> Kamil
> 
>> +		igt_describe("Check if basic fdinfo content is present for engine utilization");
>> +		igt_subtest("basic-utilization")
>> +			basic_engine_utilization(xe);
>> +
> 
> [...]
>