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

[Kamil Konieczny]

Hi Jonathan,
On 2024-12-18 at 20:37:06 +0000, Cavitt, Jonathan wrote:
> -----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?
> 

It is Linux kernel and/or DRM constant (as for now),
look into lib/drmtest.c

I could make it into a lib function like:

bool __drm_get_dri_card_path(path, size, idx);
bool __drm_get_dri_render_path(path, size, idx);

/* fill in system specific path to card, return false
 * in case of path too small or idx out of bound
 */

> > +		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
> 

Hmm, right, I will respin new version.

Regards,
Kamil

> >  	}
> >  
> >  	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
> > 
> >