[PATCH v6 1/4] lib/ioctl_wrappers: let the caller handle capability check result

[Melissa Wen]

On 04/07, André Almeida wrote:
> From: Melissa Wen <mwen at igalia.com>
> 
> Rework igt_has_drm_cap to just check if a DRM capability is supported
> and let the called decide what to do from this check. It prevents the
> test fails because of an assert done when it's called in
> igt_subtest_with_dynamics.
> 
> Signed-off-by: Melissa Wen <mwen at igalia.com>
Nice to have your `Co-dev: André` here.
> Signed-off-by: André Almeida <andrealmeid at igalia.com>
> ---
>  lib/ioctl_wrappers.c    | 13 ++++++++-----
>  lib/ioctl_wrappers.h    |  2 +-
>  tests/kms_async_flips.c | 13 +++++++++----
>  3 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 146973f0d..33f593295 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -1288,14 +1288,17 @@ int __kms_addfb(int fd, uint32_t handle,
>   * This helper verifies if the passed capability is
>   * supported by the kernel
>   *
> - * Returns: Whether the capability is supported or not.
> + * Returns: negative value if error, 0 if cap is not supported, 1 if cap is

Returns: negative if the given cap doesn't exist

> + * supported.
>   */
> -bool igt_has_drm_cap(int fd, uint64_t capability)
> +int igt_has_drm_cap(int fd, uint64_t capability)
>  {
> -	struct drm_get_cap cap = { .capability = capability };
> +	uint64_t value = 0;
> +
> +	if (drmGetCap(fd, capability, &value))
> +		return -errno;
>  
> -	igt_assert(drmIoctl(fd, DRM_IOCTL_GET_CAP, &cap) == 0);
> -	return cap.value;
> +	return value ? 1 : 0;
>  }
>  
>  /**
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index b7d7c2ad9..7cf05e626 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -144,7 +144,7 @@ void prime_sync_end(int dma_buf_fd, bool write);
>  
>  bool igt_has_fb_modifiers(int fd);
>  void igt_require_fb_modifiers(int fd);
> -bool igt_has_drm_cap(int fd, uint64_t capability);
> +int igt_has_drm_cap(int fd, uint64_t capability);
>  bool igt_has_set_caching(uint32_t devid);
>  
>  /**
> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
> index da426f753..126b96d6b 100644
> --- a/tests/kms_async_flips.c
> +++ b/tests/kms_async_flips.c
> @@ -203,8 +203,10 @@ static void make_fb(data_t *data, struct igt_fb *fb,
>  
>  static void require_monotonic_timestamp(int fd)
>  {
> -	igt_require_f(igt_has_drm_cap(fd, DRM_CAP_TIMESTAMP_MONOTONIC),
> -		      "Monotonic timestamps not supported\n");
> +	int ret = igt_has_drm_cap(fd, DRM_CAP_TIMESTAMP_MONOTONIC);
> +
> +	igt_assert(ret >= 0);
> +	igt_require_f(ret, "Monotonic timestamps not supported\n");
>  }
>  
>  static void test_init(data_t *data)
> @@ -747,13 +749,16 @@ igt_main
>  	int i;
>  
>  	igt_fixture {
> +		int ret;
> +
>  		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>  		kmstest_set_vt_graphics_mode();
>  		igt_display_require(&data.display, data.drm_fd);
>  		igt_display_require_output(&data.display);
>  
> -		igt_require_f(igt_has_drm_cap(data.drm_fd, DRM_CAP_ASYNC_PAGE_FLIP),
> -			      "Async Flip is not supported\n");
> +		ret = igt_has_drm_cap(data.drm_fd, DRM_CAP_ASYNC_PAGE_FLIP);
> +		igt_assert(ret >= 0);
> +		igt_require_f(ret, "Async Flip is not supported\n");

Same thing I mentioned in the atomic async cap patch. But in this case I think it's
a matter of taste. I prefer if we don't put an assert here,
otherwise the test fails instead of skipping in a kernel that async flip
wasn't introduced yet, but no strong opinion for this case, failing
early sounds fine too.

Melissa

>  
>  		if (is_intel_device(data.drm_fd))
>  			data.bops = buf_ops_create(data.drm_fd);
> -- 
> 2.49.0
>