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

[Yang, Fei]

> 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 at linux.intel.com>
>>> Signed-off-by: Andi Shyti <andi.shyti at linux.intel.com>
>>
>> Thanks for the fix.
>> Reviewed-by: Fei Yang <fei.yang at intel.com>
>>
>>> Cc: Fei Yang <fei.yang at intel.com>
>>> Cc: Kamil Konieczny <kamil.konieczny at 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.
>>> +    * 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?

Unfortunately yes. Too bad this has to be patched so many times.
Just sent another.

-Fei

> 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