From 6adb336829951d8d3acd5498d53ef1d505db3373 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 16 May 2018 09:40:50 +1000 Subject: [PATCH] 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 --- src/evdev-mt-touchpad.c | 90 +++++++----- src/evdev-mt-touchpad.h | 10 ++ test/litest.h | 36 +++++ test/test-touchpad.c | 317 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 413 insertions(+), 40 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index ba19d842..69a4892b 100644 --- a/src/evdev-mt-touchpad.c +++ b/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 - * need to exclude it from the tp_resume_conditional */ - tp_resume_conditional(tp, device, removed_device); + if (tp->sendevents.current_mode == + LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE) { + 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); - return; + tp_suspend(tp, device, SUSPEND_EXTERNAL_MOUSE); + 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; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index f5daa338..230ed88d 100644 --- a/src/evdev-mt-touchpad.h +++ b/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; diff --git a/test/litest.h b/test/litest.h index 26b93657..2315764b 100644 --- a/test/litest.h +++ b/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) { diff --git a/test/test-touchpad.c b/test/test-touchpad.c index 8341e209..2294bb88 100644 --- a/test/test-touchpad.c +++ b/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); } -- GitLab