[removing xorg, adding xorg-devel, see http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches]
On Sun, Jul 17, 2011 at 02:33:14AM +0200, Lennart Poettering wrote: > Add support for multi-seat-aware input device hotplugging. This > implements the multi-seat scheme explained here: > > http://www.freedesktop.org/wiki/Software/systemd/multiseat > > This introduces a new X server switch "-seat" which allows configuration > of the seat to enumerate hotplugging devices on. If specified the value > of this parameter will also be exported as root window property Xorg_Seat. > > To properly support input hotplugging devices need to be tagged in udev > according to the seat they are on. Untagged devices are assumed to be on > the default seat "seat0". If no "-seat" parameter is passed only devices > on "seat0" are used. This means that the new scheme is perfectly > compatible with existing setups which have no tagged input devices. > > This also optimizes the udev code in away that is beneficial for systems > that do not support new udev/systemd style multi-seat: the patch makes > use of libudev functions to limit enumerating/monitoring of devices to > actual input devices. This should speed up start-up and minimize > wake-ups, since we will only enumerate/monitor the devices we are > interested in instead of all devices on the machine and all events they > generate. see this recent thread http://lists.x.org/archives/xorg-devel/2011-May/022332.html summary: we don't know which subsystems will supply input devices, hints are appreciated. either way, sneaking this into the multi-seat patch is a bit rough. Some devices would have stopped working and the blame would have been on the new multi-seat support. Making this a separate patch would be easier to review and debug. > Finally, this also unifies the code paths for the udev "change" and > "add" events, since these should be handled the same way in order to be > robust to "udevadm trigger" calls, or lost netlink messages. This also > simplifies the code. > > Note that the -seat switch takes a completely generic identifier, and > that it has no effect on non-Linux systems. In fact, on other OSes a > completely different identifier scheme for seats could be used but still > be exposed with the Xorg_Seat and -seat. > > I tried to follow the coding style of the surrounding code blocks if > there was any one could follow. that _is_ the coding style :) > --- > config/udev.c | 34 +++++++++++++++++++++++++++++----- > hw/xfree86/common/xf86Init.c | 19 +++++++++++++++++++ > include/globals.h | 2 +- > os/utils.c | 10 ++++++++++ > 4 files changed, 59 insertions(+), 6 deletions(-) > > diff --git a/config/udev.c b/config/udev.c > index 9ac34ee..b30f075 100644 > --- a/config/udev.c > +++ b/config/udev.c > @@ -35,6 +35,7 @@ > #include "hotplug.h" > #include "config-backends.h" > #include "os.h" > +#include "globals.h" > > #define UDEV_XKB_PROP_KEY "xkb" > > @@ -65,6 +66,7 @@ device_added(struct udev_device *udev_device) > struct udev_list_entry *set, *entry; > struct udev_device *parent; > int rc; > + const char *dev_seat; > > path = udev_device_get_devnode(udev_device); > > @@ -73,6 +75,16 @@ device_added(struct udev_device *udev_device) > if (!path || !syspath) > return; > > + dev_seat = udev_device_get_property_value(udev_device, "ID_SEAT"); > + if (!dev_seat) > + dev_seat = "seat0"; > + > + if (SeatId && strcmp(dev_seat, SeatId)) > + return; > + > + if (!SeatId && strcmp(dev_seat, "seat0")) > + return; > + > if (!udev_device_get_property_value(udev_device, "ID_INPUT")) { > LogMessageVerb(X_INFO, 10, > "config/udev: ignoring device %s without " > @@ -251,14 +263,12 @@ wakeup_handler(pointer data, int err, pointer read_mask) > return; > action = udev_device_get_action(udev_device); > if (action) { > - if (!strcmp(action, "add")) > - device_added(udev_device); > - else if (!strcmp(action, "remove")) > - device_removed(udev_device); > - else if (!strcmp(action, "change")) { > + if (!strcmp(action, "add") || !strcmp(action, "change")) { > device_removed(udev_device); > device_added(udev_device); > } > + else if (!strcmp(action, "remove")) > + device_removed(udev_device); > } > udev_device_unref(udev_device); > } this should be a separate patch, it doesn't really have anything to do with the multiseat patches. > @@ -283,6 +293,10 @@ config_udev_init(void) > if (!udev_monitor) > return 0; > > + udev_monitor_filter_add_match_subsystem_devtype(udev_monitor, "input", > NULL); > + if (strcmp(SeatId, "seat0")) > + udev_monitor_filter_add_match_tag(udev_monitor, SeatId); > + > if (udev_monitor_enable_receiving(udev_monitor)) { > ErrorF("config/udev: failed to bind the udev monitor\n"); > return 0; > @@ -291,11 +305,21 @@ config_udev_init(void) > enumerate = udev_enumerate_new(udev); > if (!enumerate) > return 0; > + > + udev_enumerate_add_match_subsystem(enumerate, "input"); > + if (strcmp(SeatId, "seat0")) > + udev_enumerate_add_match_tag(enumerate, SeatId); > + > udev_enumerate_scan_devices(enumerate); > devices = udev_enumerate_get_list_entry(enumerate); > udev_list_entry_foreach(device, devices) { > const char *syspath = udev_list_entry_get_name(device); > struct udev_device *udev_device = udev_device_new_from_syspath(udev, > syspath); > + > + /* Device might be gone by the time we try to open it */ > + if (!udev_device) > + continue; > + this seems unrelated as well > device_added(udev_device); > udev_device_unref(udev_device); > } > diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c > index 15fdbc3..e3769eb 100644 > --- a/hw/xfree86/common/xf86Init.c > +++ b/hw/xfree86/common/xf86Init.c > @@ -654,6 +654,25 @@ InitOutput(ScreenInfo *pScreenInfo, int argc, char > **argv) > } > } > > + if (SeatId) { > +#define SEAT_ATOM_NAME "Xorg_Seat" please move this define to include/xserver-properties.h Also, property names can include spaces, so the '_' isn't really necessary. Any reason we use SeadId in code, but Xorg_Seat in the atom? Why not use "Seat ID"? (with or without space) > + Atom SeatAtom; > + > + SeatAtom = MakeAtom(SEAT_ATOM_NAME, sizeof(SEAT_ATOM_NAME) - 1, > TRUE); i think strlen() is more common than sizeof() for this. > + > + for (i = 0; i < xf86NumScreens; i++) { > + int ret; > + > + ret = xf86RegisterRootWindowProperty(xf86Screens[i]->scrnIndex, > + SeatAtom, XA_STRING, 8, > + strlen(SeatId)+1, SeatId ); > + if (ret != Success) { > + xf86DrvMsg(xf86Screens[i]->scrnIndex, X_WARNING, > + "Failed to register seat property\n"); > + } > + } > + } > + > /* If a screen uses depth 24, show what the pixmap format is */ > for (i = 0; i < xf86NumScreens; i++) { > if (xf86Screens[i]->depth == 24) { > diff --git a/include/globals.h b/include/globals.h > index 8b80a65..17bca82 100644 > --- a/include/globals.h > +++ b/include/globals.h > @@ -21,7 +21,7 @@ extern _X_EXPORT int defaultColorVisualClass; > > extern _X_EXPORT int GrabInProgress; > extern _X_EXPORT Bool noTestExtensions; > - > +extern _X_EXPORT char *SeatId; > extern _X_EXPORT char *ConnectionInfo; > > #ifdef DPMSExtension > diff --git a/os/utils.c b/os/utils.c > index 36cb46f..17869f0 100644 > --- a/os/utils.c > +++ b/os/utils.c > @@ -201,6 +201,8 @@ Bool PanoramiXExtensionDisabledHack = FALSE; > > int auditTrailLevel = 1; > > +char *SeatId = NULL; > + > #if defined(SVR4) || defined(__linux__) || defined(CSRG_BASED) > #define HAS_SAVED_IDS_AND_SETEUID > #endif > @@ -511,6 +513,7 @@ void UseMsg(void) > ErrorF("-render [default|mono|gray|color] set render color alloc > policy\n"); > ErrorF("-retro start with classic stipple and cursor\n"); > ErrorF("-s # screen-saver timeout (minutes)\n"); > + ErrorF("-seat string seat to run on\n"); > ErrorF("-t # default pointer threshold (pixels/t)\n"); > ErrorF("-terminate terminate at server reset\n"); > ErrorF("-to # connection time out\n"); > @@ -802,6 +805,13 @@ ProcessCommandLine(int argc, char *argv[]) > else > UseMsg(); > } > + else if ( strcmp( argv[i], "-seat") == 0) > + { > + if(++i < argc) > + SeatId = argv[i]; > + else > + UseMsg(); > + } indentation. And this should be added to the man page as well, together with the "Linux only"-ness. Cheers, Peter > else if ( strcmp( argv[i], "-t") == 0) > { > if(++i < argc) > -- > 1.7.6 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
