[Spice-devel] [PATCH] qxl: add uevent handler support

[Alon Levy]

On Mon, 2013-07-01 at 14:22 +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> This allows the driver to process uevents from the kernel when the mode
> changes.
Power9Blow!Sharp
Looks good to me, one question below, ACK regardless.

> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  configure.ac      | 14 +++++++++++++
>  src/Makefile.am   |  6 +++++-
>  src/qxl_drmmode.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qxl_drmmode.h | 10 +++++++++
>  src/qxl_kms.c     |  3 +++
>  5 files changed, 94 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index d481cc1..8b2d450 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -135,6 +135,20 @@ fi
>  AM_CONDITIONAL(BUILD_XSPICE, test "x$enable_xspice" = "xyes")
>  AM_CONDITIONAL(BUILD_QXL, test "x$enable_qxl" = "xyes")
>  
> +AC_ARG_ENABLE([udev],
> +		AS_HELP_STRING([--disable-udev], [Disable libudev support [default=auto]]),
> +		[enable_udev="$enableval"],
> +		[enable_udev=auto])
> +if test "x$enable_udev" != "xno"; then
> +	PKG_CHECK_MODULES(LIBUDEV, [libudev], [LIBUDEV=yes], [LIBUDEV=no])
> +	if test "x$LIBUDEV" = xyes; then
> +		AC_DEFINE(HAVE_LIBUDEV, 1,[libudev support])
> +	elif test "x$enable_udev" != "xauto"; then
> +		AC_MSG_ERROR([Building with udev requested but libudev not found])
> +	fi
> +fi
> +AM_CONDITIONAL(LIBUDEV, test x$LIBUDEV = xyes)
> +
>  PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.12.0])
>  
>  # AC_CHECK_FILE is not supported when cross compiling
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 4349b4e..d6a5445 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -27,7 +27,7 @@
>  
>  SUBDIRS=uxa
>  
> -AM_CFLAGS = $(SPICE_PROTOCOL_CFLAGS) $(XORG_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(DRM_CFLAGS)
> +AM_CFLAGS = $(SPICE_PROTOCOL_CFLAGS) $(XORG_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(DRM_CFLAGS) @LIBUDEV_CFLAGS@

Any reason to use an autoconf variable instead of a make variable?

>  
>  if BUILD_QXL
>  qxl_drv_la_LTLIBRARIES = qxl_drv.la
> @@ -35,6 +35,10 @@ qxl_drv_la_LDFLAGS = -module -avoid-version
>  qxl_drv_ladir = @moduledir@/drivers
>  
>  qxl_drv_la_LIBADD = uxa/libuxa.la
> +if LIBUDEV
> +qxl_drv_la_LIBADD += $(LIBUDEV_LIBS)
> +endif
> +
>  
>  qxl_drv_la_SOURCES =				\
>  	qxl.h					\
> diff --git a/src/qxl_drmmode.c b/src/qxl_drmmode.c
> index c1f5c15..03b4124 100644
> --- a/src/qxl_drmmode.c
> +++ b/src/qxl_drmmode.c
> @@ -875,4 +875,66 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
>  	return TRUE;
>  }
>  
> +#ifdef HAVE_LIBUDEV
> +static void
> +drmmode_handle_uevents(int fd, void *closure)
> +{
> +	drmmode_ptr drmmode = closure;
> +	ScrnInfoPtr scrn = drmmode->scrn;
> +	struct udev_device *dev;
> +	dev = udev_monitor_receive_device(drmmode->uevent_monitor);
> +	if (!dev)
> +		return;
> +
> +	RRGetInfo(xf86ScrnToScreen(scrn), TRUE);
> +	udev_device_unref(dev);
> +}
> +#endif
> +
> +void qxl_drmmode_uevent_init(ScrnInfoPtr scrn, drmmode_ptr drmmode)
> +{
> +#ifdef HAVE_LIBUDEV
> +	struct udev *u;
> +	struct udev_monitor *mon;
> +
> +	u = udev_new();
> +	if (!u)
> +		return;
> +	mon = udev_monitor_new_from_netlink(u, "udev");
> +	if (!mon) {
> +		udev_unref(u);
> +		return;
> +	}
> +
> +	if (udev_monitor_filter_add_match_subsystem_devtype(mon,
> +							    "drm",
> +							    "drm_minor") < 0 ||
> +	    udev_monitor_enable_receiving(mon) < 0) {
> +		udev_monitor_unref(mon);
> +		udev_unref(u);
> +		return;
> +	}
> +
> +	drmmode->uevent_handler =
> +		xf86AddGeneralHandler(udev_monitor_get_fd(mon),
> +				      drmmode_handle_uevents,
> +				      drmmode);
> +
> +	drmmode->uevent_monitor = mon;
> +#endif
> +}
> +
> +void qxl_drmmode_uevent_fini(ScrnInfoPtr scrn, drmmode_ptr drmmode)
> +{
> +#ifdef HAVE_LIBUDEV
> +	if (drmmode->uevent_handler) {
> +		struct udev *u = udev_monitor_get_udev(drmmode->uevent_monitor);
> +		xf86RemoveGeneralHandler(drmmode->uevent_handler);
> +
> +		udev_monitor_unref(drmmode->uevent_monitor);
> +		udev_unref(u);
> +	}
> +#endif
> +}
> +
>  #endif
> diff --git a/src/qxl_drmmode.h b/src/qxl_drmmode.h
> index df076c7..04752ec 100644
> --- a/src/qxl_drmmode.h
> +++ b/src/qxl_drmmode.h
> @@ -34,6 +34,9 @@
>  #include "xf86str.h"
>  #include "randrstr.h"
>  #include "xf86Crtc.h"
> +#ifdef HAVE_LIBUDEV
> +#include "libudev.h"
> +#endif
>  
>  typedef struct {
>    int fd;
> @@ -42,6 +45,10 @@ typedef struct {
>    drmModeFBPtr mode_fb;
>    int cpp;
>    ScrnInfoPtr scrn;
> +#ifdef HAVE_LIBUDEV
> +  struct udev_monitor *uevent_monitor;
> +  InputHandlerProc uevent_handler;
> +#endif
>  } drmmode_rec, *drmmode_ptr;
>  
>  typedef struct {
> @@ -78,6 +85,9 @@ typedef struct {
>  } drmmode_output_private_rec, *drmmode_output_private_ptr;
>  
>  extern Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp);
> +
> +extern void qxl_drmmode_uevent_init(ScrnInfoPtr scrn, drmmode_ptr drmmode);
> +extern void qxl_drmmode_uevent_fini(ScrnInfoPtr scrn, drmmode_ptr drmmode);
>  #endif
>  
>  #endif
> diff --git a/src/qxl_kms.c b/src/qxl_kms.c
> index b673294..b091a12 100644
> --- a/src/qxl_kms.c
> +++ b/src/qxl_kms.c
> @@ -95,6 +95,7 @@ qxl_close_screen_kms (CLOSE_SCREEN_ARGS_DECL)
>      qxl_screen_t *qxl = pScrn->driverPrivate;
>      Bool result;
>  
> +    qxl_drmmode_uevent_fini(pScrn, &qxl->drmmode);
>      pScreen->CloseScreen = qxl->close_screen;
>  
>      result = pScreen->CloseScreen (CLOSE_SCREEN_ARGS);
> @@ -198,6 +199,8 @@ qxl_create_screen_resources_kms(ScreenPtr pScreen)
>      
>      set_surface (pPixmap, qxl->primary);
>  
> +    qxl_drmmode_uevent_init(pScrn, &qxl->drmmode);
> +
>      if (!uxa_resources_init (pScreen))
>  	return FALSE;
>