Re: [igt-dev] [PATCH] test/i915/gem_create: Skip the test if the PAT index is set on a non MTL device

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

On 12/07/2023 17:20, Yang, Fei wrote:
> On 12/07/2023 09:37, Andi Shyti wrote:
>> If we try to set the PAT index on a device that is not a Meteor Lake, we expect an -ENODEV return value. But then, we forget to skip and soon after we assert on the return value being '0', forcing the error path.
>>
>> Skip, as expected, in case the return value is either EINVAL or ENODEV, the latter for meteor lake devices.
>>
>> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> 
> Thanks for the fix.
> Reviewed-by: Fei Yang <fei.yang@intel.com>
> 
>> Cc: Fei Yang <fei.yang@intel.com>
>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>> ---
>>   tests/i915/gem_create.c | 23 +++++++++++++++++------
>>   1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c index b7961d9ef2..4b00fe6aab 100644
>> --- a/tests/i915/gem_create.c
>> +++ b/tests/i915/gem_create.c
>> @@ -668,14 +668,25 @@ static void create_ext_set_pat(int fd)
>>   	ret = __gem_create_ext(fd, &size, 0, &handle, &setparam_pat.base);
>>   
>>   	/*
>> -	 * With a valid PAT index specified, returning -EINVAL here
>> -	 * indicates set_pat extension is not supported
>> +	 * Currently the PAT index is supported only for Meteor Lake, so that
>> +	 * we expect:
>> +	 *
>> +	 *  * -EINVAL if i915 does not support the PAT index, e.g. the kernel is
>> +	 *    too old to have the PAT index supported.
>> +	 *  * -ENODEV if we are trying to set the PAT index on a non Meteor Lake
>> +	 *    platform.
>> +	 *
>> +	 * In both cases, though, the I915_GEM_CREATE_EXT_SET_PAT flag is not
>> +	 * supported.
>>   	 */
>> -	if (ret == -EINVAL)
>> -		igt_skip("I915_GEM_CREATE_EXT_SET_PAT is not supported\n");
>> -	else if (!IS_METEORLAKE(devid))
>> -		igt_assert_eq(ret, -ENODEV);
>> +	igt_skip_on_f(ret == -EINVAL ||
>> +		      (ret == -ENODEV && IS_METEORLAKE(devid)),

Should IS_METEORLAKE actually be !IS_METEORLAKE here?

Because I notice it still seems failing:

https://intel-gfx-ci.01.org/tree/drm-intel-fixes/CI_DIF_778/shard-dg2-3/igt@gem_create@create-ext-set-pat.html

Regards,

Tvrtko

>> +		      "I915_GEM_CREATE_EXT_SET_PAT is not supported\n");
>>   
>> +	/*
>> +	 * This means that we are on a Meteor Lake and the PAT
>> +	 * index is already supported by the running i915
>> +	 */
>>   	igt_assert(ret == 0);
>>   
>>   	/*
>> --
>> 2.40.1