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

[Lyude Paul <lyude@redhat.com>]

On Wed, 2019-02-20 at 22:17 +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.
> 
> v3: Lyude reported that we might get a few udev events on startup of
> the test. Drain those first.
> 
> v4: Use the igt hotplug library functions, they already take care of
> all the uevent special cases.
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Keith Packard <keithp@keithp.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  lib/igt_kms.c     | 42 +++++++++++++++++++++++++++++++-----------
>  lib/igt_kms.h     |  2 ++
>  tests/kms_lease.c | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 69 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 85a911e11060..8f0b79d7727b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -4019,16 +4019,8 @@ struct udev_monitor *igt_watch_hotplug(void)
>  	return mon;
>  }
>  
> -/**
> - * igt_hotplug_detected:
> - * @mon: A udev monitor initialized with #igt_watch_hotplug
> - * @timeout_secs: How long to wait for a hotplug event to occur.
> - *
> - * Assert that a hotplug event was received since we last checked the
> monitor.
> - *
> - * Returns: true if a sysfs hotplug event was received, false if we timed
> out
> - */
> -bool igt_hotplug_detected(struct udev_monitor *mon, int timeout_secs)
> +static bool event_detected(struct udev_monitor *mon, int timeout_secs,
> +			   const char *property)
>  {
>  	struct udev_device *dev;
>  	const char *hotplug_val;
> @@ -4046,7 +4038,7 @@ bool igt_hotplug_detected(struct udev_monitor *mon,
> int timeout_secs)
>  	while (!hotplug_received && poll(&fd, 1, timeout_secs * 1000)) {
>  		dev = udev_monitor_receive_device(mon);
>  
> -		hotplug_val = udev_device_get_property_value(dev, "HOTPLUG");
> +		hotplug_val = udev_device_get_property_value(dev, property);
>  		if (hotplug_val && atoi(hotplug_val) == 1)
>  			hotplug_received = true;
>  
> @@ -4056,6 +4048,34 @@ bool igt_hotplug_detected(struct udev_monitor *mon,
> int timeout_secs)
>  	return hotplug_received;
>  }
>  
> +/**
> + * igt_hotplug_detected:
> + * @mon: A udev monitor initialized with #igt_watch_hotplug
> + * @timeout_secs: How long to wait for a hotplug event to occur.
> + *
> + * Assert that a hotplug event was received since we last checked the
> monitor.
> + *
> + * Returns: true if a sysfs hotplug event was received, false if we timed
> out
> + */
> +bool igt_hotplug_detected(struct udev_monitor *mon, int timeout_secs)
> +{
> +	return event_detected(mon, timeout_secs, "HOTPLUG");
> +}
> +
> +/**
> + * igt_lease_change_detected:
> + * @mon: A udev monitor initialized with #igt_watch_hotplug
> + * @timeout_secs: How long to wait for a lease change event to occur.
> + *
> + * Assert that a lease change event was received since we last checked the
> monitor.
> + *
> + * Returns: true if a sysfs lease change event was received, false if we
> timed out
> + */
> +bool igt_lease_change_detected(struct udev_monitor *mon, int timeout_secs)
> +{
> +	return event_detected(mon, timeout_secs, "LEASE");
> +}
> +
>  /**
>   * igt_flush_hotplugs:
>   * @mon: A udev monitor initialized with #igt_watch_hotplug
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 679d4e84fab8..5755b160d13e 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -749,6 +749,8 @@ const unsigned char* igt_kms_get_alt_edid(void);
>  struct udev_monitor *igt_watch_hotplug(void);
>  bool igt_hotplug_detected(struct udev_monitor *mon,
>  			  int timeout_secs);
> +bool igt_lease_change_detected(struct udev_monitor *mon,
> +			       int timeout_secs);
>  void igt_flush_hotplugs(struct udev_monitor *mon);
>  void igt_cleanup_hotplug(struct udev_monitor *mon);
>  
> diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> index d082a7988eaa..600f6779561d 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,37 @@ 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;
> +
> +	uevent_monitor = igt_watch_hotplug();
> +
> +	igt_flush_hotplugs(uevent_monitor);
Shouldn't we rename
igt_watch_hotplug()/igt_flush_hotplugs()/igt_cleanup_hotplug()/etc. as well to
reflect that these are used for more then udev hotplug events now?

Otherwise, looks good to me

> +
> +	lease_fd = create_simple_lease(data->master.fd, data);
> +
> +	igt_assert(!igt_lease_change_detected(uevent_monitor, 1));
> +
> +	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);
> +
> +	igt_assert(igt_lease_change_detected(uevent_monitor, 1));
> +
> +	mll.lessees_ptr = 0;
> +	igt_assert_eq(list_lessees(data->master.fd, &mll), 0);
> +	igt_assert_eq(mll.count_lessees, 0);
> +
> +	igt_cleanup_hotplug(uevent_monitor);
> +}
> +
>  igt_main
>  {
>  	data_t data;
> @@ -1023,4 +1056,7 @@ igt_main
>  
>  	igt_subtest("implicit-plane-lease")
>  		implicit_plane_lease(&data);
> +
> +	igt_subtest("lease-uevent")
> +		lease_uevent(&data);
>  }
-- 
Cheers,
	Lyude Paul

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev