[igt-dev] [PATCH i-g-t] intel/xe_exec_store.c: Fix the check for num of placements in store_all.

[Ch, Sai Gowtham]

>-----Original Message-----
>From: Kumar, Janga Rahul <janga.rahul.kumar at intel.com>
>Sent: Wednesday, November 29, 2023 5:25 PM
>To: Ch, Sai Gowtham <sai.gowtham.ch at intel.com>; igt-dev at lists.freedesktop.org;
>Ch, Sai Gowtham <sai.gowtham.ch at intel.com>
>Subject: RE: [igt-dev] [PATCH i-g-t] intel/xe_exec_store.c: Fix the check for num of
>placements in store_all.
>
>
>
>> -----Original Message-----
>> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
>> sai.gowtham.ch at intel.com
>> Sent: Wednesday, November 29, 2023 3:56 PM
>> To: igt-dev at lists.freedesktop.org; Ch, Sai Gowtham
>> <sai.gowtham.ch at intel.com>
>> Subject: [igt-dev] [PATCH i-g-t] intel/xe_exec_store.c: Fix the check
>> for num of placements in store_all.
>>
>> From: Sai Gowtham Ch <sai.gowtham.ch at intel.com>
>>
>> Adjust the store_all execution with dynamic subtest, so that test will
>> skip the engines with no instances and executes on the engines with instances.
>>
>> Signed-off-by: Sai Gowtham Ch <sai.gowtham.ch at intel.com>
>> ---
>>  tests/intel/xe_exec_store.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/intel/xe_exec_store.c b/tests/intel/xe_exec_store.c
>> index
>> 9c14bfd14..95c678bf7 100644
>> --- a/tests/intel/xe_exec_store.c
>> +++ b/tests/intel/xe_exec_store.c
>> @@ -245,7 +245,8 @@ static void store_all(int fd, int gt, int class)
>>  		eci[num_placements++] = *hwe;
>>  	}
>>
>> -	igt_require(num_placements);
>> +	igt_skip_on_f(!num_placements, "Engine class:%d gt:%d not enabled on
>> this platform\n",
>> +			class, gt);
>>
>>  	for (i = 0; i < num_placements; i++) {
>>  		struct drm_xe_exec_queue_create create = { @@ -304,10
>> +305,13 @@ igt_main
>>  	igt_subtest("basic-store")
>>  		store(fd);
>>
>> -	igt_subtest("basic-all") {
>> +	igt_subtest_with_dynamic("basic-all") {
>>  		xe_for_each_gt (fd, gt)
>> -			xe_for_each_hw_engine_class(class)
>> -				store_all(fd, gt, class);
>> +			xe_for_each_hw_engine_class (class) {
>The problem here is, test is failing as we are trying to check all the engine classes
>on all gt's. Test is skipping in cases where the engine class is not present on a
>particular engine or an hardware.
>
>Here the correct solution will be using " xe_for_each_hw_engine " instead of using
>xe_for_each_gt and xe_for_each_hw_engine_class.
>
>xe_for_each_hw_engine internally uses query ioctl to get the existing engines info
>on a hardware. Each engine id, class, instance , gt id is returned which can be used
>to pass gt and class data to store_all.
>
>JFYI
>xe_for_each_gt - iterates over all gt's
>xe_for_each_hw_engine_class - loop iterates over all the known engines classes
>including the ones which are not present on the GT or on the platform.
>
>By passing only existing engine class info, above check "
>igt_require(num_placements)" won't be required.
You are right, however If you look at the code in store_all we are querying num_placements 
>From xe_for_each_hw_engine. And we use that num_placements later for engine creation. 

Previously test was skipping the entire test when it hits gt with engine class has 0 instances, however intension of this patch is to 
Skip only if gt with engine class has 0 instances and execute rest of the test. 
 
Thanks,
Gowtham
>
>Thanks,
>Rahul
>> +				igt_dynamic_f("class-%d", class) {
>> +					store_all(fd, gt, class);
>> +				}
>> +			}
>>  	}
>>
>>  	igt_subtest("cachelines")
>> --
>> 2.39.1