[PATCH v2 xf86-input-libinput] Always delay hotplugging subdevices

Peter Hutterer peter.hutterer at who-t.net
Mon Aug 22 02:38:42 UTC 2016


Avoid creating new devices from within the input thread which was the case for
tablet tools. It requires a lot more care about locking and has a potential to
mess up things.

Instead, schedule a WorkProc and buffer all events until we have the device
created. Once that's done, replay the event sequence so far. If the device
comes into proximity and out again before we manage to create the new device
we just ditch the whole sequence and wait for the next proximity in.

Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
Changes to v1:
- switch from fixed array to xorg_list 
- remove the now unused hotplug_when
- rename event_handling enum and values to be more obvious

 src/xf86libinput.c | 272 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 216 insertions(+), 56 deletions(-)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index e1ca405..fe78f05 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -102,6 +102,16 @@ struct xf86libinput_device {
 	struct xorg_list unclaimed_tablet_tool_list;
 };
 
+struct xf86libinput_tablet_tool_queued_event {
+	struct xorg_list node;
+	struct libinput_event_tablet_tool *event;
+};
+
+struct xf86libinput_tablet_tool_event_queue {
+	bool need_to_queue;
+	struct xorg_list event_list;
+};
+
 struct xf86libinput_tablet_tool {
 	struct xorg_list node;
 	struct libinput_tablet_tool *tool;
@@ -162,20 +172,22 @@ struct xf86libinput {
 	bool allow_mode_group_updates;
 };
 
-enum hotplug_when {
-	HOTPLUG_LATER,
-	HOTPLUG_NOW,
+enum event_handling {
+	EVENT_QUEUED,
+	EVENT_HANDLED,
 };
 
-static DeviceIntPtr
+static void
 xf86libinput_create_subdevice(InputInfoPtr pInfo,
 			      uint32_t capabilities,
-			      enum hotplug_when,
 			      XF86OptionPtr extra_opts);
 static inline void
 update_mode_prop(InputInfoPtr pInfo,
 		 struct libinput_event_tablet_pad *event);
 
+static enum event_handling
+xf86libinput_handle_event(struct libinput_event *event);
+
 static inline int
 use_server_fd(const InputInfoPtr pInfo) {
 	return pInfo->fd > -1 && (pInfo->flags & XI86_SERVER_FD);
@@ -1387,27 +1399,120 @@ xf86libinput_pick_device(struct xf86libinput_device *shared_device,
 }
 
 static void
+xf86libinput_tool_destroy_queued_event(struct xf86libinput_tablet_tool_queued_event *qe)
+{
+	struct libinput_event *e;
+
+	e = libinput_event_tablet_tool_get_base_event(qe->event);
+	libinput_event_destroy(e);
+	xorg_list_del(&qe->node);
+	free(qe);
+}
+
+static void
+xf86libinput_tool_replay_events(struct xf86libinput_tablet_tool_event_queue *queue)
+{
+	struct xf86libinput_tablet_tool_queued_event *qe, *tmp;
+
+	xorg_list_for_each_entry_safe(qe, tmp, &queue->event_list, node) {
+		struct libinput_event *e;
+
+		e = libinput_event_tablet_tool_get_base_event(qe->event);
+		xf86libinput_handle_event(e);
+		xf86libinput_tool_destroy_queued_event(qe);
+	}
+}
+
+static bool
+xf86libinput_tool_queue_event(struct libinput_event_tablet_tool *event)
+{
+	struct libinput_event *e;
+	struct libinput_tablet_tool *tool;
+	struct xf86libinput_tablet_tool_event_queue *queue;
+	struct xf86libinput_tablet_tool_queued_event *qe;
+
+	tool = libinput_event_tablet_tool_get_tool(event);
+	if (!tool)
+		return true;
+
+	queue = libinput_tablet_tool_get_user_data(tool);
+	if (!queue)
+		return false;
+
+	if (!queue->need_to_queue) {
+		if (!xorg_list_is_empty(&queue->event_list)) {
+			libinput_tablet_tool_set_user_data(tool, NULL);
+			xf86libinput_tool_replay_events(queue);
+			free(queue);
+		}
+
+		return false;
+	}
+
+	/* We got the prox out while still queuing, just ditch the whole
+	 * series of events and the event queue with it. */
+	if (libinput_event_tablet_tool_get_proximity_state(event) ==
+	    LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_OUT) {
+		struct xf86libinput_tablet_tool_queued_event *tmp;
+
+		xorg_list_for_each_entry_safe(qe, tmp, &queue->event_list, node)
+			xf86libinput_tool_destroy_queued_event(qe);
+
+		libinput_tablet_tool_set_user_data(tool, NULL);
+		free(queue);
+
+		/* we destroy the event here but return true
+		 * to make sure the event looks like it got queued and the
+		 * caller doesn't destroy it for us
+		 */
+		e = libinput_event_tablet_tool_get_base_event(event);
+		libinput_event_destroy(e);
+		return true;
+	}
+
+	qe = calloc(1, sizeof(*qe));
+	if (!qe) {
+		e = libinput_event_tablet_tool_get_base_event(event);
+		libinput_event_destroy(e);
+		return true;
+	}
+
+	qe->event = event;
+	xorg_list_append(&qe->node, &queue->event_list);
+
+	return true;
+}
+
+static enum event_handling
 xf86libinput_handle_tablet_tip(InputInfoPtr pInfo,
 			       struct libinput_event_tablet_tool *event)
 {
 	enum libinput_tablet_tool_tip_state state;
 	const BOOL is_absolute = TRUE;
 
+	if (xf86libinput_tool_queue_event(event))
+		return EVENT_QUEUED;
+
 	state = libinput_event_tablet_tool_get_tip_state(event);
 
 	xf86PostButtonEventP(pInfo->dev,
 			     is_absolute, 1,
 			     state == LIBINPUT_TABLET_TOOL_TIP_DOWN ? 1 : 0,
 			     0, 0, NULL);
+
+	return EVENT_HANDLED;
 }
 
-static void
+static enum event_handling
 xf86libinput_handle_tablet_button(InputInfoPtr pInfo,
 				  struct libinput_event_tablet_tool *event)
 {
 	enum libinput_button_state state;
 	uint32_t button, b;
 
+	if (xf86libinput_tool_queue_event(event))
+		return EVENT_QUEUED;
+
 	button = libinput_event_tablet_tool_get_button(event);
 	state = libinput_event_tablet_tool_get_button_state(event);
 
@@ -1418,9 +1523,11 @@ xf86libinput_handle_tablet_button(InputInfoPtr pInfo,
 			     b,
 			     state == LIBINPUT_BUTTON_STATE_PRESSED ? 1 : 0,
 			     0, 0, NULL);
+
+	return EVENT_HANDLED;
 }
 
-static void
+static enum event_handling
 xf86libinput_handle_tablet_axis(InputInfoPtr pInfo,
 				struct libinput_event_tablet_tool *event)
 {
@@ -1430,6 +1537,9 @@ xf86libinput_handle_tablet_axis(InputInfoPtr pInfo,
 	struct libinput_tablet_tool *tool;
 	double value;
 
+	if (xf86libinput_tool_queue_event(event))
+		return EVENT_QUEUED;
+
 	value = libinput_event_tablet_tool_get_x_transformed(event,
 							TABLET_AXIS_MAX);
 	valuator_mask_set_double(mask, 0, value);
@@ -1477,13 +1587,15 @@ xf86libinput_handle_tablet_axis(InputInfoPtr pInfo,
 		default:
 			xf86IDrvMsg(pInfo, X_ERROR,
 				    "Invalid rotation axis on tool\n");
-			return;
+			return EVENT_HANDLED;
 		}
 
 		valuator_mask_set_double(mask, valuator, value);
 	}
 
 	xf86PostMotionEventM(dev, Absolute, mask);
+
+	return EVENT_HANDLED;
 }
 
 static inline const char *
@@ -1507,47 +1619,35 @@ tool_type_to_str(enum libinput_tablet_tool_type type)
 	return str;
 }
 
-static void
-xf86libinput_handle_tablet_proximity(InputInfoPtr pInfo,
-				     struct libinput_event_tablet_tool *event)
+static inline void
+xf86libinput_create_tool_subdevice(InputInfoPtr pInfo,
+				   struct libinput_event_tablet_tool *event)
 {
 	struct xf86libinput *driver_data = pInfo->private;
 	struct xf86libinput_device *shared_device = driver_data->shared_device;
-	struct xf86libinput *dev = pInfo->private;
-	struct libinput_tablet_tool *tool;
 	struct xf86libinput_tablet_tool *t;
+	struct xf86libinput_tablet_tool_event_queue *queue;
+	struct libinput_tablet_tool *tool;
 	uint64_t serial, tool_id;
 	XF86OptionPtr options = NULL;
-	DeviceIntPtr pDev = pInfo->dev;
 	char name[64];
-	ValuatorMask *mask = driver_data->valuators;
-	double x, y;
-	BOOL in_prox;
-
-	x = libinput_event_tablet_tool_get_x_transformed(event, TABLET_AXIS_MAX);
-	y = libinput_event_tablet_tool_get_y_transformed(event, TABLET_AXIS_MAX);
-	valuator_mask_set_double(mask, 0, x);
-	valuator_mask_set_double(mask, 1, y);
-
-	tool = libinput_event_tablet_tool_get_tool(event);
-	serial = libinput_tablet_tool_get_serial(tool);
-	tool_id = libinput_tablet_tool_get_tool_id(tool);
-
-	xorg_list_for_each_entry(dev,
-				 &shared_device->device_list,
-				 shared_device_link) {
-		if (dev->tablet_tool &&
-		    libinput_tablet_tool_get_serial(dev->tablet_tool) == serial &&
-		    libinput_tablet_tool_get_tool_id(dev->tablet_tool) == tool_id) {
-			pDev = dev->pInfo->dev;
-			goto out;
-		}
-	}
 
 	t = calloc(1, sizeof *t);
 	if (!t)
 		return;
 
+	queue = calloc(1, sizeof(*queue));
+	if (!queue) {
+		free(t);
+		return;
+	}
+	queue->need_to_queue = true;
+	xorg_list_init(&queue->event_list);
+
+	tool = libinput_event_tablet_tool_get_tool(event);
+	serial = libinput_tablet_tool_get_serial(tool);
+	tool_id = libinput_tablet_tool_get_tool_id(tool);
+
 	t->tool = libinput_tablet_tool_ref(tool);
 	xorg_list_append(&t->node, &shared_device->unclaimed_tablet_tool_list);
 
@@ -1562,12 +1662,70 @@ xf86libinput_handle_tablet_proximity(InputInfoPtr pInfo,
 		     (uint32_t)serial) > strlen(pInfo->name))
 		options = xf86ReplaceStrOption(options, "Name", name);
 
-	pDev = xf86libinput_create_subdevice(pInfo, CAP_TABLET_TOOL, HOTPLUG_NOW, options);
+	libinput_tablet_tool_set_user_data(tool, queue);
+	xf86libinput_tool_queue_event(event);
+
+	xf86libinput_create_subdevice(pInfo, CAP_TABLET_TOOL, options);
+}
+
+static inline DeviceIntPtr
+xf86libinput_find_device_for_tool(InputInfoPtr pInfo,
+				  struct libinput_tablet_tool *tool)
+{
+	struct xf86libinput *dev = pInfo->private;
+	struct xf86libinput *driver_data = pInfo->private;
+	struct xf86libinput_device *shared_device = driver_data->shared_device;
+	uint64_t serial = libinput_tablet_tool_get_serial(tool);
+	uint64_t tool_id = libinput_tablet_tool_get_tool_id(tool);
+
+	xorg_list_for_each_entry(dev,
+				 &shared_device->device_list,
+				 shared_device_link) {
+		if (dev->tablet_tool &&
+		    libinput_tablet_tool_get_serial(dev->tablet_tool) == serial &&
+		    libinput_tablet_tool_get_tool_id(dev->tablet_tool) == tool_id) {
+			return dev->pInfo->dev;
+		}
+	}
+
+	return NULL;
+}
+
+static enum event_handling
+xf86libinput_handle_tablet_proximity(InputInfoPtr pInfo,
+				     struct libinput_event_tablet_tool *event)
+{
+	struct xf86libinput *driver_data = pInfo->private;
+	struct libinput_tablet_tool *tool;
+	DeviceIntPtr pDev;
+	ValuatorMask *mask = driver_data->valuators;
+	double x, y;
+	BOOL in_prox;
+
+	tool = libinput_event_tablet_tool_get_tool(event);
+	pDev = xf86libinput_find_device_for_tool(pInfo, tool);
 
-out:
 	in_prox = libinput_event_tablet_tool_get_proximity_state(event) ==
 				LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_IN;
+
+	if (pDev == NULL && in_prox) {
+		xf86libinput_create_tool_subdevice(pInfo, event);
+		return EVENT_QUEUED;
+	}
+
+	if (xf86libinput_tool_queue_event(event))
+		return EVENT_QUEUED;
+
+	BUG_RETURN_VAL(pDev == NULL, EVENT_HANDLED);
+
+	x = libinput_event_tablet_tool_get_x_transformed(event, TABLET_AXIS_MAX);
+	y = libinput_event_tablet_tool_get_y_transformed(event, TABLET_AXIS_MAX);
+	valuator_mask_set_double(mask, 0, x);
+	valuator_mask_set_double(mask, 1, y);
+
 	xf86PostProximityEventM(pDev, in_prox, mask);
+
+	return EVENT_HANDLED;
 }
 
 static void
@@ -1646,12 +1804,13 @@ xf86libinput_handle_tablet_pad_ring(InputInfoPtr pInfo,
 	xf86PostMotionEventM(dev, Absolute, mask);
 }
 
-static void
+static enum event_handling
 xf86libinput_handle_event(struct libinput_event *event)
 {
 	struct libinput_device *device;
 	enum libinput_event_type type;
 	InputInfoPtr pInfo;
+	enum event_handling event_handling = EVENT_HANDLED;
 
 	type = libinput_event_get_type(event);
 	device = libinput_event_get_device(event);
@@ -1659,7 +1818,7 @@ xf86libinput_handle_event(struct libinput_event *event)
 					 event);
 
 	if (!pInfo || !pInfo->dev->public.on)
-		return;
+		goto out;
 
 	switch (type) {
 		case LIBINPUT_EVENT_NONE:
@@ -1705,19 +1864,19 @@ xf86libinput_handle_event(struct libinput_event *event)
 		case LIBINPUT_EVENT_GESTURE_PINCH_END:
 			break;
 		case LIBINPUT_EVENT_TABLET_TOOL_AXIS:
-			xf86libinput_handle_tablet_axis(pInfo,
+			event_handling = xf86libinput_handle_tablet_axis(pInfo,
 							libinput_event_get_tablet_tool_event(event));
 			break;
 		case LIBINPUT_EVENT_TABLET_TOOL_BUTTON:
-			xf86libinput_handle_tablet_button(pInfo,
+			event_handling = xf86libinput_handle_tablet_button(pInfo,
 							  libinput_event_get_tablet_tool_event(event));
 			break;
 		case LIBINPUT_EVENT_TABLET_TOOL_PROXIMITY:
-			xf86libinput_handle_tablet_proximity(pInfo,
+			event_handling = xf86libinput_handle_tablet_proximity(pInfo,
 							     libinput_event_get_tablet_tool_event(event));
 			break;
 		case LIBINPUT_EVENT_TABLET_TOOL_TIP:
-			xf86libinput_handle_tablet_tip(pInfo,
+			event_handling = xf86libinput_handle_tablet_tip(pInfo,
 						       libinput_event_get_tablet_tool_event(event));
 			break;
 		case LIBINPUT_EVENT_TABLET_PAD_BUTTON:
@@ -1733,6 +1892,9 @@ xf86libinput_handle_event(struct libinput_event *event)
 							     libinput_event_get_tablet_pad_event(event));
 			break;
 	}
+
+out:
+	return event_handling;
 }
 
 static void
@@ -1754,8 +1916,8 @@ xf86libinput_read_input(InputInfoPtr pInfo)
 	}
 
 	while ((event = libinput_get_event(libinput))) {
-		xf86libinput_handle_event(event);
-		libinput_event_destroy(event);
+		if (xf86libinput_handle_event(event) == EVENT_HANDLED)
+			libinput_event_destroy(event);
 	}
 }
 
@@ -2439,10 +2601,9 @@ xf86libinput_hotplug_device_cb(ClientPtr client, pointer closure)
 	return TRUE;
 }
 
-static DeviceIntPtr
+static void
 xf86libinput_create_subdevice(InputInfoPtr pInfo,
 			      uint32_t capabilities,
-			      enum hotplug_when when,
 			      XF86OptionPtr extra_options)
 {
 	struct xf86libinput *driver_data = pInfo->private;
@@ -2483,18 +2644,14 @@ xf86libinput_create_subdevice(InputInfoPtr pInfo,
 
 	hotplug = calloc(1, sizeof(*hotplug));
 	if (!hotplug)
-		return NULL;
+		return;
 
 	hotplug->input_options = iopts;
 	hotplug->attrs = DuplicateInputAttributes(pInfo->attrs);
 
 	xf86IDrvMsg(pInfo, X_INFO, "needs a virtual subdevice\n");
-	if (when == HOTPLUG_LATER)
-		QueueWorkProc(xf86libinput_hotplug_device_cb, serverClient, hotplug);
-	else
-		return xf86libinput_hotplug_device(hotplug);
 
-	return NULL;
+	QueueWorkProc(xf86libinput_hotplug_device_cb, serverClient, hotplug);
 }
 
 static inline uint32_t
@@ -2519,6 +2676,7 @@ claim_tablet_tool(InputInfoPtr pInfo)
 {
 	struct xf86libinput *driver_data = pInfo->private;
 	struct xf86libinput_device *shared_device = driver_data->shared_device;
+	struct xf86libinput_tablet_tool_event_queue *queue;
 	struct xf86libinput_tablet_tool *t;
 	uint64_t serial, tool_id;
 
@@ -2531,6 +2689,9 @@ claim_tablet_tool(InputInfoPtr pInfo)
 		if (libinput_tablet_tool_get_serial(t->tool) == serial &&
 		    libinput_tablet_tool_get_tool_id(t->tool) == tool_id) {
 			driver_data->tablet_tool = t->tool;
+			queue = libinput_tablet_tool_get_user_data(t->tool);
+			if (queue)
+				queue->need_to_queue = false;
 			xorg_list_del(&t->node);
 			free(t);
 			return TRUE;
@@ -2662,7 +2823,6 @@ xf86libinput_pre_init(InputDriverPtr drv,
 		driver_data->capabilities &= ~CAP_KEYBOARD;
 		xf86libinput_create_subdevice(pInfo,
 					      CAP_KEYBOARD,
-					      HOTPLUG_LATER,
 					      NULL);
 	}
 
-- 
2.7.4



More information about the xorg-devel mailing list