[PATCH i-g-t 2/2] tests/core_getversion: Added raw-cards subtest

[Cavitt, Jonathan]

-----Original Message-----
From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Kamil Konieczny
Sent: Wednesday, December 18, 2024 11:04 AM
To: igt-dev at lists.freedesktop.org
Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
Subject: [PATCH i-g-t 2/2] tests/core_getversion: Added raw-cards subtest
> 
> Created new subtest raw-card for checking all cards without
> loading any driver, so it could show what is loaded on a system
> after booting.
> 
> Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

Minor nits below, but nothing blocking:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>

> ---
>  tests/core_getversion.c | 39 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/core_getversion.c b/tests/core_getversion.c
> index 0842b7db6..989ed2aba 100644
> --- a/tests/core_getversion.c
> +++ b/tests/core_getversion.c
> @@ -25,6 +25,7 @@
>   *
>   */
>  
> +#include <fcntl.h>
>  #include <string.h>
>  #include <sys/ioctl.h>
>  
> @@ -45,6 +46,9 @@
>   *
>   * SUBTEST: all-cards
>   * Description: Tests GET_VERSION ioctl for all drm devices.
> + *
> + * SUBTEST: raw-cards
> + * Description: Test GET_VERSION ioctl for all present drm devices without loading any driver.
>   */
>  
>  IGT_TEST_DESCRIPTION("Tests the DRM_IOCTL_GET_VERSION ioctl and libdrm's "
> @@ -83,26 +87,47 @@ static void check_all_drm(void)
>  	}
>  }
>  
> -igt_main
> +static void check_raw_drm_cards(void)
>  {
> +	char fname[32];
>  	char info[256];
> -	int fd;
> +	int fd2, count;
>  
> -	igt_fixture {
> -		fd = __drm_open_driver(DRIVER_ANY);
> -		igt_assert_fd(fd);
> +	count = 0;
> +	for (int i = 0; i < 128; i++) {

Nit:
128 seems a bit arbitrary.  Is there a max card count macro we could be referencing?
Or perhaps we can create that macro locally as a part of this new test?

> +		snprintf(fname, ARRAY_SIZE(fname), "/dev/dri/card%d", i);
> +		fd2 = open(fname, O_RDWR);
> +		if (fd2 == -1)
> +			continue;
> +
> +		++count;
> +		check(fd2, info, sizeof(info));
> +		igt_info("%d: %s\n", i, info);
> +		close(fd2);
>  	}
>  
> +	igt_info("Tested %d drm devices\n", count);
> +}
> +
> +igt_main
> +{
> +	char info[256];
> +	int fd;
> +
>  	igt_describe("Check GET_VERSION ioctl of the first drm device.");
>  	igt_subtest("basic") {
> +		fd = __drm_open_driver(DRIVER_ANY);
> +		igt_assert_fd(fd);
>  		check(fd, info, sizeof(info));
>  		igt_info("0: %s\n", info);
> +		__drm_close_driver(fd);

Nit:
I see we're moving the drm open and close calls out of the fixtures and
into the basic subtest.  Perhaps it would be better to do this as a part of
the previous patch, rather than here.
-Jonathan Cavitt

>  	}
>  
>  	igt_describe("Check GET_VERSION ioctl for all drm devices.");
>  	igt_subtest("all-cards")
>  		check_all_drm();
>  
> -	igt_fixture
> -		__drm_close_driver(fd);
> +	igt_describe("Check GET_VERSION ioctl for all present drm devices without loading any driver.");
> +	igt_subtest("raw-cards")
> +		check_raw_drm_cards();
>  }
> -- 
> 2.47.1
> 
>