touchpad: remember the suspend reason

There are 4 possible cases why a touchpad suspends right now: lid switch,
tablet mode switch, sendevents disabled and sendevents disabled when an
external mouse is present.

But these reasons can stack up, e.g. a lid switch may happen while send events
is disabled, disabling one should not re-enable the touchpad. This patch adds
a bitmask to remember the reasons we're current suspended, resuming only
happens once all reasons are back to 0.

https://bugs.freedesktop.org/show_bug.cgi?id=106498

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

src/evdev-mt-touchpad.c

@@ -1842,8 +1842,16 @@ tp_clear_state(struct tp_dispatch *tp)
 }
 
 static void
-tp_suspend(struct tp_dispatch *tp, struct evdev_device *device)
+tp_suspend(struct tp_dispatch *tp,
+	   struct evdev_device *device,
+	   enum suspend_trigger trigger)
 {
+	if (tp->suspend_reason & trigger)
+		return;
+
+	if (tp->suspend_reason != 0)
+		goto out;
+
 	tp_clear_state(tp);
 
 	/* On devices with top softwarebuttons we don't actually suspend the
@@ -1857,6 +1865,9 @@ tp_suspend(struct tp_dispatch *tp, struct evdev_device *device)
 	} else {
 		evdev_device_suspend(device);
 	}
+
+out:
+	tp->suspend_reason |= trigger;
 }
 
 static void
@@ -1917,8 +1928,14 @@ tp_sync_slots(struct tp_dispatch *tp,
 }
 
 static void
-tp_resume(struct tp_dispatch *tp, struct evdev_device *device)
+tp_resume(struct tp_dispatch *tp,
+	  struct evdev_device *device,
+	  enum suspend_trigger trigger)
 {
+	tp->suspend_reason &= ~trigger;
+	if (tp->suspend_reason != 0)
+		return;
+
 	if (tp->buttons.has_topbuttons) {
 		/* tap state-machine is offline while suspended, reset state */
 		tp_clear_state(tp);
@@ -1932,31 +1949,6 @@ tp_resume(struct tp_dispatch *tp, struct evdev_device *device)
 	tp_sync_slots(tp, device);
 }
 
-#define NO_EXCLUDED_DEVICE NULL
-static void
-tp_resume_conditional(struct tp_dispatch *tp,
-		      struct evdev_device *device,
-		      struct evdev_device *excluded_device)
-{
-	if (tp->sendevents.current_mode == LIBINPUT_CONFIG_SEND_EVENTS_DISABLED)
-		return;
-
-	if (tp->sendevents.current_mode ==
-		    LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE) {
-		struct libinput_device *dev;
-
-		list_for_each(dev, &device->base.seat->devices_list, link) {
-			struct evdev_device *d = evdev_device(dev);
-			if (d != excluded_device &&
-			    (d->tags & EVDEV_TAG_EXTERNAL_MOUSE)) {
-				return;
-			}
-		}
-	}
-
-	tp_resume(tp, device);
-}
-
 static void
 tp_trackpoint_timeout(uint64_t now, void *data)
 {
@@ -2215,11 +2207,11 @@ tp_lid_switch_event(uint64_t time, struct libinput_event *event, void *data)
 
 	switch (libinput_event_switch_get_switch_state(swev)) {
 	case LIBINPUT_SWITCH_STATE_OFF:
-		tp_resume_conditional(tp, tp->device, NO_EXCLUDED_DEVICE);
+		tp_resume(tp, tp->device, SUSPEND_LID);
 		evdev_log_debug(tp->device, "lid: resume touchpad\n");
 		break;
 	case LIBINPUT_SWITCH_STATE_ON:
-		tp_suspend(tp, tp->device);
+		tp_suspend(tp, tp->device, SUSPEND_LID);
 		evdev_log_debug(tp->device, "lid: suspending touchpad\n");
 		break;
 	}
@@ -2243,11 +2235,11 @@ tp_tablet_mode_switch_event(uint64_t time,
 
 	switch (libinput_event_switch_get_switch_state(swev)) {
 	case LIBINPUT_SWITCH_STATE_OFF:
-		tp_resume_conditional(tp, tp->device, NO_EXCLUDED_DEVICE);
+		tp_resume(tp, tp->device, SUSPEND_TABLET_MODE);
 		evdev_log_debug(tp->device, "tablet-mode: resume touchpad\n");
 		break;
 	case LIBINPUT_SWITCH_STATE_ON:
-		tp_suspend(tp, tp->device);
+		tp_suspend(tp, tp->device, SUSPEND_TABLET_MODE);
 		evdev_log_debug(tp->device, "tablet-mode: suspending touchpad\n");
 		break;
 	}
@@ -2300,7 +2292,7 @@ tp_pair_tablet_mode_switch(struct evdev_device *touchpad,
 	if (evdev_device_switch_get_state(tablet_mode_switch,
 					  LIBINPUT_SWITCH_TABLET_MODE)
 		    == LIBINPUT_SWITCH_STATE_ON) {
-		tp_suspend(tp, touchpad);
+		tp_suspend(tp, touchpad, SUSPEND_TABLET_MODE);
 	}
 }
 
@@ -2320,7 +2312,7 @@ tp_interface_device_added(struct evdev_device *device,
 		return;
 
 	if (added_device->tags & EVDEV_TAG_EXTERNAL_MOUSE)
-		tp_suspend(tp, device);
+		tp_suspend(tp, device, SUSPEND_EXTERNAL_MOUSE);
 }
 
 static void
@@ -2354,17 +2346,32 @@ tp_interface_device_removed(struct evdev_device *device,
 		libinput_device_remove_event_listener(
 					&tp->lid_switch.listener);
 		tp->lid_switch.lid_switch = NULL;
+		tp_resume(tp, device, SUSPEND_LID);
 	}
 
 	if (removed_device == tp->tablet_mode_switch.tablet_mode_switch) {
 		libinput_device_remove_event_listener(
 					&tp->tablet_mode_switch.listener);
 		tp->tablet_mode_switch.tablet_mode_switch = NULL;
+		tp_resume(tp, device, SUSPEND_TABLET_MODE);
 	}
 
-	/* removed_device is still in the device list at this point, so we
+	if (tp->sendevents.current_mode ==
-	 * need to exclude it from the tp_resume_conditional */
+		    LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE) {
-	tp_resume_conditional(tp, device, removed_device);
+		struct libinput_device *dev;
+		bool found = false;
+
+		list_for_each(dev, &device->base.seat->devices_list, link) {
+			struct evdev_device *d = evdev_device(dev);
+			if (d != removed_device &&
+			    (d->tags & EVDEV_TAG_EXTERNAL_MOUSE)) {
+				found = true;
+				break;
+			}
+		}
+		if (!found)
+			tp_resume(tp, device, SUSPEND_EXTERNAL_MOUSE);
+	}
 }
 
 static inline void
@@ -3334,8 +3341,8 @@ tp_suspend_conditional(struct tp_dispatch *tp,
 	list_for_each(dev, &device->base.seat->devices_list, link) {
 		struct evdev_device *d = evdev_device(dev);
 		if (d->tags & EVDEV_TAG_EXTERNAL_MOUSE) {
-			tp_suspend(tp, device);
+			tp_suspend(tp, device, SUSPEND_EXTERNAL_MOUSE);
-			return;
+			break;
 		}
 	}
 }
@@ -3357,13 +3364,16 @@ tp_sendevents_set_mode(struct libinput_device *device,
 
 	switch(mode) {
 	case LIBINPUT_CONFIG_SEND_EVENTS_ENABLED:
-		tp_resume(tp, evdev);
+		tp_resume(tp, evdev, SUSPEND_SENDEVENTS);
+		tp_resume(tp, evdev, SUSPEND_EXTERNAL_MOUSE);
 		break;
 	case LIBINPUT_CONFIG_SEND_EVENTS_DISABLED:
-		tp_suspend(tp, evdev);
+		tp_suspend(tp, evdev, SUSPEND_SENDEVENTS);
+		tp_resume(tp, evdev, SUSPEND_EXTERNAL_MOUSE);
 		break;
 	case LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE:
 		tp_suspend_conditional(tp, evdev);
+		tp_resume(tp, evdev, SUSPEND_SENDEVENTS);
 		break;
 	default:
 		return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;

src/evdev-mt-touchpad.h

@@ -236,6 +236,14 @@ struct tp_touch {
 	} speed;
 };
 
+enum suspend_trigger {
+	SUSPEND_NO_FLAG         = 0x0,
+	SUSPEND_EXTERNAL_MOUSE  = 0x1,
+	SUSPEND_SENDEVENTS      = 0x2,
+	SUSPEND_LID             = 0x4,
+	SUSPEND_TABLET_MODE     = 0x8,
+};
+
 struct tp_dispatch {
 	struct evdev_dispatch base;
 	struct evdev_device *device;
@@ -245,6 +253,8 @@ struct tp_dispatch {
 	bool has_mt;
 	bool semi_mt;
 
+	uint32_t suspend_reason;
+
 	/* pen/touch arbitration */
 	struct {
 		bool in_arbitration;

test/litest.h

@@ -1012,6 +1012,42 @@ litest_disable_middleemu(struct litest_device *dev)
 	litest_assert_int_eq(status, expected);
 }
 
+static inline void
+litest_sendevents_off(struct litest_device *dev)
+{
+	struct libinput_device *device = dev->libinput_device;
+	enum libinput_config_status status, expected;
+
+	expected = LIBINPUT_CONFIG_STATUS_SUCCESS;
+	status = libinput_device_config_send_events_set_mode(device,
+				    LIBINPUT_CONFIG_SEND_EVENTS_DISABLED);
+	litest_assert_int_eq(status, expected);
+}
+
+static inline void
+litest_sendevents_on(struct litest_device *dev)
+{
+	struct libinput_device *device = dev->libinput_device;
+	enum libinput_config_status status, expected;
+
+	expected = LIBINPUT_CONFIG_STATUS_SUCCESS;
+	status = libinput_device_config_send_events_set_mode(device,
+				    LIBINPUT_CONFIG_SEND_EVENTS_ENABLED);
+	litest_assert_int_eq(status, expected);
+}
+
+static inline void
+litest_sendevents_ext_mouse(struct litest_device *dev)
+{
+	struct libinput_device *device = dev->libinput_device;
+	enum libinput_config_status status, expected;
+
+	expected = LIBINPUT_CONFIG_STATUS_SUCCESS;
+	status = libinput_device_config_send_events_set_mode(device,
+				    LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE);
+	litest_assert_int_eq(status, expected);
+}
+
 static inline bool
 litest_touchpad_is_external(struct litest_device *dev)
 {

test/test-touchpad.c

@@ -5968,8 +5968,322 @@ START_TEST(touchpad_speed_ignore_finger_edgescroll)
 }
 END_TEST
 
+enum suspend {
+	SUSPEND_EXT_MOUSE = 1,
+	SUSPEND_SENDEVENTS,
+	SUSPEND_LID,
+	SUSPEND_TABLETMODE,
+	SUSPEND_COUNT,
+};
+
+static void
+assert_touchpad_moves(struct litest_device *tp)
+{
+	struct libinput *li = tp->libinput;
+
+	litest_touch_down(tp, 0, 50, 50);
+	litest_touch_move_to(tp, 0, 50, 50, 60, 80, 20, 0);
+	litest_touch_up(tp, 0);
+	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
+}
+
+static void
+assert_touchpad_does_not_move(struct litest_device *tp)
+{
+	struct libinput *li = tp->libinput;
+
+	litest_touch_down(tp, 0, 20, 20);
+	litest_touch_move_to(tp, 0, 20, 20, 60, 80, 20, 0);
+	litest_touch_up(tp, 0);
+	litest_assert_empty_queue(li);
+}
+
+START_TEST(touchpad_suspend_abba)
+{
+	struct litest_device *tp = litest_current_device();
+	struct litest_device *lid, *tabletmode, *extmouse;
+	struct libinput *li = tp->libinput;
+	enum suspend first = _i; /* ranged test */
+	enum suspend other;
+
+	if (first == SUSPEND_EXT_MOUSE && litest_touchpad_is_external(tp))
+		return;
+
+	lid = litest_add_device(li, LITEST_LID_SWITCH);
+	tabletmode = litest_add_device(li, LITEST_THINKPAD_EXTRABUTTONS);
+	extmouse = litest_add_device(li, LITEST_MOUSE);
+
+	litest_disable_tap(tp->libinput_device);
+
+	/* ABBA test for touchpad internal suspend:
+	 *  reason A on
+	 *  reason B on
+	 *  reason B off
+	 *  reason A off
+	 */
+	for (other = SUSPEND_EXT_MOUSE; other < SUSPEND_COUNT; other++) {
+		if (other == first)
+			continue;
+
+		if (other == SUSPEND_EXT_MOUSE && litest_touchpad_is_external(tp))
+			return;
+
+		/* That transition is tested elsewhere and has a different
+		 * behavior */
+		if ((other == SUSPEND_SENDEVENTS && first == SUSPEND_EXT_MOUSE) ||
+		    (first == SUSPEND_SENDEVENTS && other == SUSPEND_EXT_MOUSE))
+			continue;
+
+		litest_drain_events(li);
+		assert_touchpad_moves(tp);
+
+		/* First reason for suspend: on */
+		switch (first) {
+		case  SUSPEND_EXT_MOUSE:
+			litest_sendevents_ext_mouse(tp);
+			break;
+		case  SUSPEND_TABLETMODE:
+			litest_switch_action(tabletmode,
+					     LIBINPUT_SWITCH_TABLET_MODE,
+					     LIBINPUT_SWITCH_STATE_ON);
+			break;
+		case  SUSPEND_LID:
+			litest_switch_action(lid,
+					     LIBINPUT_SWITCH_LID,
+					     LIBINPUT_SWITCH_STATE_ON);
+			break;
+		case  SUSPEND_SENDEVENTS:
+			litest_sendevents_off(tp);
+			break;
+		default:
+			ck_abort();
+		}
+
+		litest_drain_events(li);
+
+		assert_touchpad_does_not_move(tp);
+
+		/* Second reason to suspend: on/off while first reason remains */
+		switch (other) {
+		case SUSPEND_EXT_MOUSE:
+			litest_sendevents_ext_mouse(tp);
+			litest_sendevents_on(tp);
+			break;
+		case SUSPEND_LID:
+			litest_switch_action(lid,
+					     LIBINPUT_SWITCH_LID,
+					     LIBINPUT_SWITCH_STATE_ON);
+			litest_drain_events(li);
+			litest_switch_action(lid,
+					     LIBINPUT_SWITCH_LID,
+					     LIBINPUT_SWITCH_STATE_OFF);
+			litest_drain_events(li);
+			break;
+		case SUSPEND_TABLETMODE:
+			litest_switch_action(tabletmode,
+					     LIBINPUT_SWITCH_TABLET_MODE,
+					     LIBINPUT_SWITCH_STATE_ON);
+			litest_drain_events(li);
+			litest_switch_action(tabletmode,
+					     LIBINPUT_SWITCH_TABLET_MODE,
+					     LIBINPUT_SWITCH_STATE_OFF);
+			litest_drain_events(li);
+			break;
+		case SUSPEND_SENDEVENTS:
+			litest_sendevents_off(tp);
+			litest_sendevents_on(tp);
+			break;
+		default:
+			ck_abort();
+		}
+
+		assert_touchpad_does_not_move(tp);
+
+		/* First reason for suspend: off */
+		switch (first) {
+		case  SUSPEND_EXT_MOUSE:
+			litest_sendevents_on(tp);
+			break;
+		case  SUSPEND_TABLETMODE:
+			litest_switch_action(tabletmode,
+					     LIBINPUT_SWITCH_TABLET_MODE,
+					     LIBINPUT_SWITCH_STATE_OFF);
+			break;
+		case  SUSPEND_LID:
+			litest_switch_action(lid,
+					     LIBINPUT_SWITCH_LID,
+					     LIBINPUT_SWITCH_STATE_OFF);
+			break;
+		case  SUSPEND_SENDEVENTS:
+			litest_sendevents_on(tp);
+			break;
+		default:
+			ck_abort();
+		}
+
+		litest_drain_events(li);
+		assert_touchpad_moves(tp);
+	}
+
+	litest_delete_device(lid);
+	litest_delete_device(tabletmode);
+	litest_delete_device(extmouse);
+}
+END_TEST
+
+START_TEST(touchpad_suspend_abab)
+{
+	struct litest_device *tp = litest_current_device();
+	struct litest_device *lid, *tabletmode, *extmouse;
+	struct libinput *li = tp->libinput;
+	enum suspend first = _i; /* ranged test */
+	enum suspend other;
+
+	if (first == SUSPEND_EXT_MOUSE && litest_touchpad_is_external(tp))
+		return;
+
+	lid = litest_add_device(li, LITEST_LID_SWITCH);
+	tabletmode = litest_add_device(li, LITEST_THINKPAD_EXTRABUTTONS);
+	extmouse = litest_add_device(li, LITEST_MOUSE);
+
+	litest_disable_tap(tp->libinput_device);
+
+	/* ABAB test for touchpad internal suspend:
+	 *  reason A on
+	 *  reason B on
+	 *  reason A off
+	 *  reason B off
+	 */
+	for (other = SUSPEND_EXT_MOUSE; other < SUSPEND_COUNT; other++) {
+		if (other == first)
+			continue;
+
+		if (other == SUSPEND_EXT_MOUSE && litest_touchpad_is_external(tp))
+			return;
+
+		/* That transition is tested elsewhere and has a different
+		 * behavior */
+		if ((other == SUSPEND_SENDEVENTS && first == SUSPEND_EXT_MOUSE) ||
+		    (first == SUSPEND_SENDEVENTS && other == SUSPEND_EXT_MOUSE))
+			continue;
+
+		litest_drain_events(li);
+		assert_touchpad_moves(tp);
+
+		/* First reason for suspend: on */
+		switch (first) {
+		case  SUSPEND_EXT_MOUSE:
+			litest_sendevents_ext_mouse(tp);
+			break;
+		case  SUSPEND_TABLETMODE:
+			litest_switch_action(tabletmode,
+					     LIBINPUT_SWITCH_TABLET_MODE,
+					     LIBINPUT_SWITCH_STATE_ON);
+			break;
+		case  SUSPEND_LID:
+			litest_switch_action(lid,
+					     LIBINPUT_SWITCH_LID,
+					     LIBINPUT_SWITCH_STATE_ON);
+			break;
+		case  SUSPEND_SENDEVENTS:
+			litest_sendevents_off(tp);
+			break;
+		default:
+			ck_abort();
+		}
+
+		litest_drain_events(li);
+
+		assert_touchpad_does_not_move(tp);
+
+		/* Second reason to suspend: on */
+		switch (other) {
+		case SUSPEND_EXT_MOUSE:
+			litest_sendevents_ext_mouse(tp);
+			break;
+		case SUSPEND_LID:
+			litest_switch_action(lid,
+					     LIBINPUT_SWITCH_LID,
+					     LIBINPUT_SWITCH_STATE_ON);
+			litest_drain_events(li);
+			break;
+		case SUSPEND_TABLETMODE:
+			litest_switch_action(tabletmode,
+					     LIBINPUT_SWITCH_TABLET_MODE,
+					     LIBINPUT_SWITCH_STATE_ON);
+			litest_drain_events(li);
+			break;
+		case SUSPEND_SENDEVENTS:
+			litest_sendevents_off(tp);
+			break;
+		default:
+			ck_abort();
+		}
+
+		assert_touchpad_does_not_move(tp);
+
+		/* First reason for suspend: off */
+		switch (first) {
+		case  SUSPEND_EXT_MOUSE:
+			litest_sendevents_on(tp);
+			break;
+		case  SUSPEND_TABLETMODE:
+			litest_switch_action(tabletmode,
+					     LIBINPUT_SWITCH_TABLET_MODE,
+					     LIBINPUT_SWITCH_STATE_OFF);
+			break;
+		case  SUSPEND_LID:
+			litest_switch_action(lid,
+					     LIBINPUT_SWITCH_LID,
+					     LIBINPUT_SWITCH_STATE_OFF);
+			break;
+		case  SUSPEND_SENDEVENTS:
+			litest_sendevents_on(tp);
+			break;
+		default:
+			ck_abort();
+		}
+
+		litest_drain_events(li);
+		assert_touchpad_does_not_move(tp);
+
+		/* Second reason to suspend: off */
+		switch (other) {
+		case SUSPEND_EXT_MOUSE:
+			litest_sendevents_on(tp);
+			break;
+		case SUSPEND_LID:
+			litest_switch_action(lid,
+					     LIBINPUT_SWITCH_LID,
+					     LIBINPUT_SWITCH_STATE_OFF);
+			litest_drain_events(li);
+			break;
+		case SUSPEND_TABLETMODE:
+			litest_switch_action(tabletmode,
+					     LIBINPUT_SWITCH_TABLET_MODE,
+					     LIBINPUT_SWITCH_STATE_OFF);
+			litest_drain_events(li);
+			break;
+		case SUSPEND_SENDEVENTS:
+			litest_sendevents_on(tp);
+			break;
+		default:
+			ck_abort();
+		}
+
+		litest_drain_events(li);
+		assert_touchpad_moves(tp);
+	}
+
+	litest_delete_device(lid);
+	litest_delete_device(tabletmode);
+	litest_delete_device(extmouse);
+}
+END_TEST
+
 TEST_COLLECTION(touchpad)
 {
+	struct range suspends = { SUSPEND_EXT_MOUSE, SUSPEND_COUNT };
 	struct range axis_range = {ABS_X, ABS_Y + 1};
 	struct range twice = {0, 2 };
 
@@ -6150,4 +6464,7 @@ TEST_COLLECTION(touchpad)
 	litest_add("touchpad:speed", touchpad_speed_ignore_finger, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
 	litest_add("touchpad:speed", touchpad_speed_allow_nearby_finger, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
 	litest_add("touchpad:speed", touchpad_speed_ignore_finger_edgescroll, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
+
+	litest_add_ranged("touchpad:suspend", touchpad_suspend_abba, LITEST_TOUCHPAD, LITEST_ANY, &suspends);
+	litest_add_ranged("touchpad:suspend", touchpad_suspend_abab, LITEST_TOUCHPAD, LITEST_ANY, &suspends);
 }