[PATCH i-g-t 2/2] tests/core_getversion: Added raw-cards subtest
Kamil Konieczny
kamil.konieczny at linux.intel.com
Fri Dec 20 18:21:18 UTC 2024
Hi Cavitt,,
On 2024-12-19 at 21:03:29 +0000, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Sent: Thursday, December 19, 2024 12:36 PM
> To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
> Cc: igt-dev at lists.freedesktop.org
> Subject: Re: [PATCH i-g-t 2/2] tests/core_getversion: Added raw-cards subtest
> >
> > 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
> > */
>
> That sounds good, though we'd probably want some way to pass
> back the opened file descriptor. I guess the prototype for the
> dri card path function might look like:
>
> """
> bool __drm_get_dri_card_path(int id, int *fd)
> {
> char fname[32];
>
> snprintf(fname, ARRAY_SIZE(fname), "/dev/dri/card%d", id);
> *fd = open(fname, O_RDWR);
> return *fd != -1;
> }
> """
> And, of course, there'd be a mirror for __drm_get_render_card_path.
>
Yes, something like that but avoiding to open an actual file,
leave that decision to caller.
Regards,
Kamil
> >
> > > > + 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.
>
> Understood. Thanks much!
> -Jonathan Cavitt
>
> >
> > 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
> > > >
> > > >
> >
More information about the igt-dev
mailing list