Re: [PATCH v6 2/4] tests/kms_async_flips: Check for atomic async flip cap

["André Almeida" <andrealmeid@igalia.com>]

Em 08/04/2025 13:01, Melissa Wen escreveu:
> On 04/07, André Almeida wrote:
>> If a driver doesn't support doing an async flip through the atomic uAPI,
>> the atomic tests fails. Instead of failing, create a function that
>> checks for this capability and skip the test if is unsupported.
>>
>> Signed-off-by: André Almeida <andrealmeid@igalia.com>
>> Reviewed-by: Melissa Wen <mwen@igalia.com>
>> ---
>>   tests/kms_async_flips.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
>> index 126b96d6b..fc9cbd3f1 100644
>> --- a/tests/kms_async_flips.c
>> +++ b/tests/kms_async_flips.c
>> @@ -209,6 +209,14 @@ static void require_monotonic_timestamp(int fd)
>>   	igt_require_f(ret, "Monotonic timestamps not supported\n");
>>   }
>>   
>> +static void require_atomic_async_cap(data_t *data)
>> +{
>> +	int ret = igt_has_drm_cap(data->drm_fd, DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP);
>> +
>> +	igt_assert(ret >= 0);
>> +	igt_require_f(ret, "Atomic async flip not supported\n");
> 
> Hmmm.. I don't think we want an assert here. Otherwise, the test aborts
> instead of skipping if the cap wasn't introduced yet in a target kernel
> version.
> 
> drm_get_cap return -EINVAL if this capability doesn't exist in a kernel
> version and zero if it exists but isn't supported by the specific
> driver. Perhaps two igt_require_f makes sense, as below?
> 
> +	igt_require_f(ret > 0, "Atomic async flip cap doesn't exist in
> this kernel\n");

Maybe should be like this:

igt_require_f(ret >= 0, "Atomic async flip cap doesn't exist in this 
kernel; or bad fd\n");

> +	igt_require_f(ret, "Atomic async flip not supported by this
> driver\n");
> 
> WDYT?
> 

You are right, I initially understood that drm_get_cap() would return 0 
if the CAP didn't exist but a quick look in the kernel function 
drm_getcap() shows that this is not the case. I will fix that.