Re: [PATCH i-g-t 01/39] lib/drmtest: Add VKMS as a known driver type

["José Expósito" <jose.exposito89@gmail.com>]

Hi Louis,

On Thu, Feb 27, 2025 at 02:12:13PM +0100, Louis Chauvet wrote:
> 
> 
> Le 18/02/2025 à 17:49, José Expósito a écrit :
> > As we are going to add VKMS specific tests, allow to check if it is
> > available.
> > 
> > Co-developed-by: Jim Shargo <jshargo@chromium.org>
> > Signed-off-by: Jim Shargo <jshargo@chromium.org>
> > Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
> > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> > ---
> >   lib/drmtest.c | 18 ++++++++++++++++++
> >   lib/drmtest.h |  8 ++++++--
> >   2 files changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index 436b6de78..2cc6e0c29 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -971,3 +971,21 @@ void igt_require_xe(int fd)
> >   {
> >   	igt_require(is_xe_device(fd));
> >   }
> > +
> > +void igt_require_vkms(void)
> > +{
> > +	/*
> > +	 * Since VKMS can create and destroy virtual drivers at will, instead
> > +	 * look to make sure the driver is installed.
> > +	 */
> > +	struct stat s = {};
> > +	int ret;
> > +	const char *vkms_module_dir = "/sys/module/vkms";
> > +
> > +	ret = stat(vkms_module_dir, &s);
> > +
> > +	igt_require_f(ret == 0, "VKMS stat of %s returned %d (%s)\n",
> > +		      vkms_module_dir, ret, strerror(ret));
> > +	igt_require_f(S_ISDIR(s.st_mode),
> > +		      "VKMS stat of %s was not a directory\n", vkms_module_dir);
> > +}
> > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > index 27e5a18e2..1b57e49ec 100644
> > --- a/lib/drmtest.h
> > +++ b/lib/drmtest.h
> > @@ -54,15 +54,18 @@ int __get_drm_device_name(int fd, char *name, int name_size);
> >   #define DRIVER_PANFROST	(1 << 5)
> >   #define DRIVER_MSM	(1 << 6)
> >   #define DRIVER_XE	(1 << 7)
> > -#define DRIVER_VMWGFX   (1 << 8)
> > +#define DRIVER_VKMS	(1 << 8)
> > +#define DRIVER_VMWGFX	(1 << 9)
> 
> Is it normal to change the DRIVER_VMWGFX value?

Ugh, not at all. This looks like poor conflict resolution by me during one of
the multiple rebases.

Thanks for noticing!

> I also found the `module` list in drmtest.c that use this value, I think
> this list is used to do the proper `modprobe` during a open_driver().
> 
> Maybe you can use it, so you will be able to do:
> 
> 	drm_load_module(DRIVER_VKMS); // (If I understand correctly, this fails
> silently)
> 	igt_require_vkms();
> 
> And future tests requiring a vkms device will be able to do:
> 
> 	open_driver(DRIVER_VKMS);

Good point, added the missing bits to drmtest.c and the call to
"drm_load_module(DRIVER_VKMS);" in "igt_fixture".

Sending v2 in a bit with this and other changes :)

Jose

> >   /*
> >    * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> >    * with vgem as well as a supported driver, you can end up with a
> >    * near-100% skip rate if you don't explicitly specify the device,
> >    * depending on device-load ordering.
> > + *
> > + * Exclude VKMS to prefer hardware drivers.
> >    */
> > -#define DRIVER_ANY 	~(DRIVER_VGEM)
> > +#define DRIVER_ANY	~(DRIVER_VGEM | DRIVER_VKMS)
> >   /*
> >    * Compile friendly enum for i915/xe.
> > @@ -135,6 +138,7 @@ void igt_require_i915(int fd);
> >   void igt_require_nouveau(int fd);
> >   void igt_require_vc4(int fd);
> >   void igt_require_xe(int fd);
> > +void igt_require_vkms(void);
> >   bool is_amdgpu_device(int fd);
> >   bool is_i915_device(int fd);
> 
> -- 
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 
>