[igt-dev] [PATCH i-g-t] tests/kms_lease: exercise uevent

[Lyude Paul]

On Thu, 2019-02-21 at 00:33 +0100, Daniel Vetter wrote:
> On Wed, Feb 20, 2019 at 05:25:33PM -0500, Lyude Paul wrote:
> > On Wed, 2019-02-20 at 21:33 +0100, Daniel Vetter wrote:
> > > And make sure we get the LEASE=1 value, indicating a lessee change.
> > > 
> > > v2: Apparently netlink reading can leak EAGAIN out through
> > > udev_monitor_receive_device. No idea what's going on there, so let's
> > > wrap some duct tape around it.
> > > 
> > > Cc: Keith Packard <keithp at keithp.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > > ---
> > >  tests/kms_lease.c | 68
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 68 insertions(+)
> > > 
> > > diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> > > index d082a7988eaa..6fe91615a70d 100644
> > > --- a/tests/kms_lease.c
> > > +++ b/tests/kms_lease.c
> > > @@ -40,6 +40,8 @@
> > >  #include <sys/time.h>
> > >  #include <sys/wait.h>
> > >  
> > > +#include <libudev.h>
> > > +
> > >  #include <drm.h>
> > >  
> > >  IGT_TEST_DESCRIPTION("Test of CreateLease.");
> > > @@ -976,6 +978,69 @@ static void implicit_plane_lease(data_t *data)
> > >  	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > >  }
> > >  
> > > +static void lease_uevent(data_t *data)
> > > +{
> > > +	int lease_fd;
> > > +	struct local_drm_mode_list_lessees mll;
> > > +	struct udev_monitor *uevent_monitor;
> > > +	struct udev *udev;
> > > +	struct udev_device *dev;
> > > +	const char *lease;
> > > +	struct pollfd udev_fd;
> > > +
> > > +	udev = udev_new();
> > > +	igt_assert(udev);
> > > +
> > > +	uevent_monitor = udev_monitor_new_from_netlink(udev, "udev");
> > > +	igt_assert(uevent_monitor);
> > > +
> > > +	do_or_die(udev_monitor_filter_add_match_subsystem_devtype(uevent_monit
> > > or,
> > > +								  "drm",
> > > +								  "drm_minor")
> > > );
> > > +	do_or_die(udev_monitor_enable_receiving(uevent_monitor));
> > > +
> > > +	/* netlink is not synchronous ... */
> > > +	udev_fd.fd = udev_monitor_get_fd(uevent_monitor);
> > > +	udev_fd.revents = 0;
> > > +	udev_fd.events = POLLIN;
> > > +
> > > +	poll(&udev_fd, 1, 100);
> > > +	dev = udev_monitor_receive_device(uevent_monitor);
> > > +	igt_assert(!dev);
> > I think this is a sanity check to ensure there's no leftover events when
> > we
> > start the test, if I'm correct we should probably leave a comment here
> > indicating that.
> 
> v4 is much prettier, since it uses the library. Can you pls take a look at
> that and see whether it's fine? It's a bit a bigger change.

Whoops :), was wondering why I didn't see any igt_hotplug* calls there...

> 
> > > +
> > > +	lease_fd = create_simple_lease(data->master.fd, data);
> > > +
> > > +	poll(&udev_fd, 1, 100);
> > > +	dev = udev_monitor_receive_device(uevent_monitor);
> > > +	igt_assert(!dev);
> > Probably should have a comment explaining what this check for is as well
> 
> Yeah I can add a comment to v4 that we expect no event after create, but
> one when the lessee is closed.
> 
> Cheers, Daniel
> 
> > > +
> > > +	mll.pad = 0;
> > > +	mll.count_lessees = 0;
> > > +	mll.lessees_ptr = 0;
> > > +	igt_assert_eq(list_lessees(data->master.fd, &mll), 0);
> > > +	igt_assert_eq(mll.count_lessees, 1);
> > > +
> > > +	close(lease_fd);
> > > +
> > > +	poll(&udev_fd, 1, 100);
> > > +	do {
> > > +		dev = udev_monitor_receive_device(uevent_monitor);
> > > +	} while (!dev && errno == EAGAIN);
> > > +	igt_assert(dev);
> > > +
> > > +	lease = udev_device_get_property_value(dev, "LEASE");
> > > +	igt_assert(lease);
> > > +	igt_assert(strcmp(lease, "1") == 0);
> > > +
> > > +	udev_device_unref(dev);
> > > +
> > > +	mll.lessees_ptr = 0;
> > > +	igt_assert_eq(list_lessees(data->master.fd, &mll), 0);
> > > +	igt_assert_eq(mll.count_lessees, 0);
> > > +
> > > +	udev_monitor_unref(uevent_monitor);
> > > +}
> > > +
> > >  igt_main
> > >  {
> > >  	data_t data;
> > > @@ -1023,4 +1088,7 @@ igt_main
> > >  
> > >  	igt_subtest("implicit-plane-lease")
> > >  		implicit_plane_lease(&data);
> > > +
> > > +	igt_subtest("lease-uevent")
> > > +		lease_uevent(&data);
> > >  }
> > -- 
> > Cheers,
> > 	Lyude Paul
> > 
-- 
Cheers,
	Lyude Paul