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

[Daniel Vetter <daniel@ffwll.ch>]

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@keithp.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@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.

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev