Michel Dänzer <[email protected]> writes: > This commit broke input for me with xf86-input-libinput. No keyboard or > mouse input is received. There are no errors in the log file. > > xf86-input-evdev works.
Ok, I spent some time yesterday figuring out what happened here. The issue is that xf86-input-libinput is using AddEnabledDevice to register file descriptors for input devices, and expecting that xf86Wakeup would call the driver when input became available. The problem is that without passing the set of ready file descriptors to xf86Wakeup (the whole point of this series), there's no way for xf86Wakeup to discover that input is available on a file descriptor registered behind it's back through AddEnabledDevice. Once the select mask is gone from the wakeup handler, AddEnabledDevice and AddGeneralSocket are no longer as useful -- there's no way to discover when the associated socket is ready for I/O without polling on the descriptor each time the server wakes up (which is actually done in several places in the server, including DMX and Xnest). With the NotifyFD interfaces in place, I think it would be best if we eliminated the AddEnabledDevice and AddGeneralSocket interfaces from the OS layer. Most code using these in the core server has been changed to use the NotifyFD interface instead and I've got patches for the remaining cases. This leaves open the question about how to deal with xf86 input drivers though. The reason they're using AddEnabledDevice directly and not calling xf86AddEnabledDevice is that they want to be handled in the normal execution flow of the X server and not asynchronously from the SIGIO handler. Instead of using that kludge, I'd like to suggest that we fix the xf86AddEnabledDevice function to do what the devices need instead, by adding a flag to the InputInfo indicating when the driver can tolerate reading events at SIGIO time. Then, xf86AddEnabledDevice can either register for SIGIO or call the NotifyFD intefaces as appropriate. Here's a patch which adds a new flag (set by default, to match the current interface) signaling which drivers can support device I/O at SIGIO time:
From 70f75a9d5e196b1722a04060f7947f1f2832e011 Mon Sep 17 00:00:00 2001 From: Keith Packard <[email protected]> Date: Mon, 7 Dec 2015 15:10:13 -0800 Subject: [PATCH xserver 03/15] hw/xfree86: Add XI86_SIGNAL_IO This flags devices which can perform I/O at SIGIO time; other devices will have I/O performed in the normal flow of execution. Signed-off-by: Keith Packard <[email protected]> --- hw/xfree86/common/xf86Events.c | 14 ++++++++------ hw/xfree86/common/xf86Xinput.c | 1 + hw/xfree86/common/xf86Xinput.h | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c index 709afd6..0417c72 100644 --- a/hw/xfree86/common/xf86Events.c +++ b/hw/xfree86/common/xf86Events.c @@ -312,9 +312,10 @@ xf86SigioReadInput(int fd, void *closure) void xf86AddEnabledDevice(InputInfoPtr pInfo) { - if (!xf86InstallSIGIOHandler(pInfo->fd, xf86SigioReadInput, pInfo)) { - AddEnabledDevice(pInfo->fd); - } + if (pInfo->flags & XI86_SIGNAL_IO) + if (xf86InstallSIGIOHandler(pInfo->fd, xf86SigioReadInput, pInfo)) + return; + AddEnabledDevice(pInfo->fd); } /* @@ -324,9 +325,10 @@ xf86AddEnabledDevice(InputInfoPtr pInfo) void xf86RemoveEnabledDevice(InputInfoPtr pInfo) { - if (!xf86RemoveSIGIOHandler(pInfo->fd)) { - RemoveEnabledDevice(pInfo->fd); - } + if (pInfo->flags & XI86_SIGNAL_IO) + if (xf86RemoveSIGIOHandler(pInfo->fd)) + return; + RemoveEnabledDevice(pInfo->fd); } static int *xf86SignalIntercept = NULL; diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c index c56a2b9..a8dff0b 100644 --- a/hw/xfree86/common/xf86Xinput.c +++ b/hw/xfree86/common/xf86Xinput.c @@ -727,6 +727,7 @@ xf86AllocateInput(void) pInfo->fd = -1; pInfo->type_name = "UNKNOWN"; + pInfo->flags = XI86_SIGNAL_IO; return pInfo; } diff --git a/hw/xfree86/common/xf86Xinput.h b/hw/xfree86/common/xf86Xinput.h index 0024053..d00251a 100644 --- a/hw/xfree86/common/xf86Xinput.h +++ b/hw/xfree86/common/xf86Xinput.h @@ -66,6 +66,7 @@ /* server-internal only */ #define XI86_DEVICE_DISABLED 0x10 /* device was disabled before vt switch */ #define XI86_SERVER_FD 0x20 /* fd is managed by xserver */ +#define XI86_SIGNAL_IO 0x40 /* driver can handle I/O at SIGIO time */ /* Input device driver capabilities */ #define XI86_DRV_CAP_SERVER_FD 0x01 -- 2.6.2
And here's the change to xf86-input-libinput that matches.
From d412624c09e63046bdde70339eb74b41ab16377c Mon Sep 17 00:00:00 2001 From: Keith Packard <[email protected]> Date: Mon, 7 Dec 2015 15:33:39 -0800 Subject: [PATCH] Use xf86AddEnabledDevice instead of AddEnabledDevice when XI86_SIGNAL_IO This lets the xfree86 layer change to the NotifyFd interface without affecting the driver. Signed-off-by: Keith Packard <[email protected]> --- src/xf86libinput.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/xf86libinput.c b/src/xf86libinput.c index ee2165a..91f0b06 100644 --- a/src/xf86libinput.c +++ b/src/xf86libinput.c @@ -480,8 +480,12 @@ xf86libinput_on(DeviceIntPtr dev) pInfo->fd = libinput_get_fd(libinput); if (driver_context.device_enabled_count == 0) { +#ifdef XI86_SIGNAL_IO + xf86AddEnabledDevice(pInfo); +#else /* Can't use xf86AddEnabledDevice on an epollfd */ AddEnabledDevice(pInfo->fd); +#endif } driver_context.device_enabled_count++; @@ -500,7 +504,11 @@ xf86libinput_off(DeviceIntPtr dev) struct xf86libinput_device *shared_device = driver_data->shared_device; if (--driver_context.device_enabled_count == 0) { +#ifdef XI86_SIGNAL_IO + xf86RemoveEnabledDevice(pInfo); +#else RemoveEnabledDevice(pInfo->fd); +#endif } if (use_server_fd(pInfo)) { @@ -1821,6 +1829,9 @@ xf86libinput_pre_init(InputDriverPtr drv, pInfo->read_input = xf86libinput_read_input; pInfo->control_proc = NULL; pInfo->switch_mode = NULL; +#ifdef XI86_SIGNAL_IO + pInfo->flags &= ~XI86_SIGNAL_IO; +#endif driver_data = calloc(1, sizeof(*driver_data)); if (!driver_data) -- 2.6.2
In terms of sequencing and validating this series, here's what I
suggest:
1) Add the X server support. No API or ABI breakages are involved
2) Update input drivers
3) Switch X server to use NotifyFd everywhere
4) Remove AddEnabledDevice and AddGeneralSocket from X server
At this point, drivers not using the new interface will not run with the
changed X server as they will fail to find AddEnabledDevices, so we'll
be able to catch them quickly.
I'm willing to make the changes for each of the input drivers that are
'interesting'; only those currently using AddEnabledDevice would need
changing. Here's the list of drivers on my machine which currently call
AddEnabledDevice; I haven't actually looked inside them to know if
they actually need changing or not:
xf86-input-calcomp
xf86-input-citron
xf86-input-digitaledge
xf86-input-dmc
xf86-input-dynapro
xf86-input-elo2300
xf86-input-elographics
xf86-input-hyperpen
xf86-input-keyboard
xf86-input-libinput
xf86-input-magellan
xf86-input-magictouch
xf86-input-microtouch
xf86-input-mutouch
xf86-input-palmax
xf86-input-penmount
xf86-input-spaceorb
xf86-input-summa
xf86-input-ur98
-keith
signature.asc
Description: PGP signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
