Bug 106498 - Lenovo X1 Yoga (Gen 3) touchpad not suspended in tablet mode
Summary: Lenovo X1 Yoga (Gen 3) touchpad not suspended in tablet mode
Status: RESOLVED FIXED
Alias: None
Product: Wayland
Classification: Unclassified
Component: libinput (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Wayland bug list
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-13 04:44 UTC by Zachary Smith
Modified: 2018-05-18 04:01 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
libinput record (18.33 KB, text/x-log)
2018-05-13 04:44 UTC, Zachary Smith
Details

Description Zachary Smith 2018-05-13 04:44:03 UTC
Created attachment 139538 [details]
libinput record

Hardware: Lenovo Thinkpad X1 Yoga (Gen 3)
libinput version 1.10.5 (packaged with Fedora 28)


When entering tablet mode, the touchpad is not suspended/disabled and touch and click are still registered.

Tablet mode is entered by folding the screen back and around. The touchpad and clicks to the touchpad are not disabled. The pointer continues to move and also register clicks. The keyboard and trackpoint stick and associated buttons are disabled so it is only the touchpad that is not properly disabled.

output of 'libinput debug-events --verbose' shows the device entering tablet mode and announcing "event8  - tablet-mode: suspending touchpad", however you can also see a few gestures recorded soon after that message and before I fold the screen back to disable tablet mode.

--- libinput debug-events --verbose ---
event2  - Power Button: is tagged by udev as: Keyboard
event2  - Power Button: device is a keyboard
event7  - Video Bus: is tagged by udev as: Keyboard
event7  - Video Bus: device is a keyboard
event1  - Lid Switch: is tagged by udev as: Switch
event1  - Lid Switch: device is a switch device
event0  - Sleep Button: is tagged by udev as: Keyboard
event0  - Sleep Button: device is a keyboard
event5  - Wacom Pen and multitouch sensor Finger: is tagged by udev as: Touchscreen
event5  - Wacom Pen and multitouch sensor Finger: device is a touch device
event6  - Wacom Pen and multitouch sensor Pen: is tagged by udev as: Tablet
event6  - Wacom Pen and multitouch sensor Pen: tablet 'Wacom Pen and multitouch sensor Pen' unknown to libwacom
event6  - Wacom Pen and multitouch sensor Pen: device is a tablet
event4  - Integrated Camera: Integrated C: is tagged by udev as: Keyboard
event4  - Integrated Camera: Integrated C: device is a keyboard
event10 - HDA Intel PCH Mic: is tagged by udev as: Switch
event11 - HDA Intel PCH Headphone: is tagged by udev as: Switch
event12 - HDA Intel PCH HDMI/DP,pcm=3: is tagged by udev as: Switch
event13 - HDA Intel PCH HDMI/DP,pcm=7: is tagged by udev as: Switch
event14 - HDA Intel PCH HDMI/DP,pcm=8: is tagged by udev as: Switch
event15 - HDA Intel PCH HDMI/DP,pcm=9: is tagged by udev as: Switch
event16 - HDA Intel PCH HDMI/DP,pcm=10: is tagged by udev as: Switch
event3  - AT Translated Set 2 keyboard: is tagged by udev as: Keyboard
event3  - AT Translated Set 2 keyboard: device is a keyboard
event1  - lid: keyboard paired with Lid Switch<->AT Translated Set 2 keyboard
event9  - ThinkPad Extra Buttons: is tagged by udev as: Keyboard Switch
event9  - ThinkPad Extra Buttons: device is a keyboard
event9  - ThinkPad Extra Buttons: device is a switch device
event3  - tablet_mode_switch: activated for AT Translated Set 2 keyboard<->ThinkPad Extra Buttons
event8  - Synaptics TM3293-011: is tagged by udev as: Touchpad
event8  - using pressure-based touch detection (25:30)
event8  - palm: pressure threshold is 130
event8  - thumb: enabled thumb detection (+pressure)
event8  - Synaptics TM3293-011: device is a touchpad
event8  - lid_switch: activated for Synaptics TM3293-011<->Lid Switch
event8  - palm: dwt activated with Synaptics TM3293-011<->AT Translated Set 2 keyboard
event8  - tablet_mode_switch: activated for Synaptics TM3293-011<->ThinkPad Extra Buttons
event17 - TPPS/2 Elan TrackPoint: is tagged by udev as: Mouse Pointingstick
event17 - TPPS/2 Elan TrackPoint: trackpoint does not have a specified range, guessing... see https://wayland.freedesktop.org/libinput/doc/1.10.5/trackpoints.html
event17 - TPPS/2 Elan TrackPoint: trackpoint device set to range 20
event17 - TPPS/2 Elan TrackPoint: device is a pointer
event17 - tablet_mode_switch: activated for TPPS/2 Elan TrackPoint<->ThinkPad Extra Buttons
-event2   DEVICE_ADDED     Power Button                      seat0 default group1  cap:k
-event7   DEVICE_ADDED     Video Bus                         seat0 default group2  cap:k
-event1   DEVICE_ADDED     Lid Switch                        seat0 default group3  cap:S
-event0   DEVICE_ADDED     Sleep Button                      seat0 default group4  cap:k
-event5   DEVICE_ADDED     Wacom Pen and multitouch sensor Finger seat0 default group5  cap:t  size 309x174mm calib
-event6   DEVICE_ADDED     Wacom Pen and multitouch sensor Pen seat0 default group5  cap:T  size 309x174mm calib
-event4   DEVICE_ADDED     Integrated Camera: Integrated C   seat0 default group6  cap:k
-event10  DEVICE_ADDED     HDA Intel PCH Mic                 seat0 default group7  cap:
-event11  DEVICE_ADDED     HDA Intel PCH Headphone           seat0 default group7  cap:
-event12  DEVICE_ADDED     HDA Intel PCH HDMI/DP,pcm=3       seat0 default group7  cap:
-event13  DEVICE_ADDED     HDA Intel PCH HDMI/DP,pcm=7       seat0 default group7  cap:
-event14  DEVICE_ADDED     HDA Intel PCH HDMI/DP,pcm=8       seat0 default group7  cap:
-event15  DEVICE_ADDED     HDA Intel PCH HDMI/DP,pcm=9       seat0 default group7  cap:
-event16  DEVICE_ADDED     HDA Intel PCH HDMI/DP,pcm=10      seat0 default group7  cap:
-event3   DEVICE_ADDED     AT Translated Set 2 keyboard      seat0 default group8  cap:k
-event9   DEVICE_ADDED     ThinkPad Extra Buttons            seat0 default group9  cap:kS
-event8   DEVICE_ADDED     Synaptics TM3293-011              seat0 default group10 cap:pg  size 97x60mm tap(dl off) left scroll-nat scroll-2fg-edge click-buttonareas-clickfinger dwt-on
-event17  DEVICE_ADDED     TPPS/2 Elan TrackPoint            seat0 default group11 cap:p left scroll-nat scroll-button
event17 - tablet-mode: suspending device
event8  - tablet-mode: suspending touchpad
event3  - tablet-mode: suspending device
-event9   SWITCH_TOGGLE     +4.56s	switch tablet-mode state 1
event8  - pressure: begin touch
event8  - thumb state: THUMB_STATE_MAYBE → THUMB_STATE_NO
event8  - button state: from BUTTON_STATE_NONE, event BUTTON_EVENT_IN_AREA to BUTTON_STATE_AREA
-event8   POINTER_MOTION    +6.51s	 -3.22/ -1.29
 event8   POINTER_MOTION    +6.52s	-10.93/ -3.64
 event8   POINTER_MOTION    +6.53s	-14.57/ -5.10
 event8   POINTER_MOTION    +6.53s	-19.67/ -6.56
 event8   POINTER_MOTION    +6.54s	-18.21/ -5.83
 event8   POINTER_MOTION    +6.55s	-16.75/ -5.10
 event8   POINTER_MOTION    +6.56s	-16.02/ -4.37
 event8   POINTER_MOTION    +6.56s	-13.11/ -3.64
 event8   POINTER_MOTION    +6.57s	-12.38/ -2.91
 event8   POINTER_MOTION    +6.58s	-11.65/ -2.19
 event8   POINTER_MOTION    +6.59s	 -8.74/ -1.46
 event8   POINTER_MOTION    +6.59s	 -6.56/ -0.73
 event8   POINTER_MOTION    +6.60s	 -4.37/  0.00
 event8   POINTER_MOTION    +6.61s	 -3.64/  0.00
 event8   POINTER_MOTION    +6.62s	 -2.19/  0.00
 event8   POINTER_MOTION    +6.63s	 -1.46/  0.00
 event8   POINTER_MOTION    +6.63s	 -0.73/  0.00
 event8   POINTER_MOTION    +6.67s	  0.66/  0.00
 event8   POINTER_MOTION    +6.68s	  1.32/  0.00
 event8   POINTER_MOTION    +6.69s	  2.91/ -0.73
 event8   POINTER_MOTION    +6.70s	  3.64/ -0.73
 event8   POINTER_MOTION    +6.70s	  5.10/ -1.46
 event8   POINTER_MOTION    +6.71s	  6.56/ -2.91
 event8   POINTER_MOTION    +6.72s	 12.38/ -3.64
 event8   POINTER_MOTION    +6.73s	 15.30/ -4.37
 event8   POINTER_MOTION    +6.73s	 16.02/ -3.64
 event8   POINTER_MOTION    +6.74s	 18.94/ -2.91
 event8   POINTER_MOTION    +6.75s	 20.39/ -1.46
 event8   POINTER_MOTION    +6.76s	 24.04/  0.73
 event8   POINTER_MOTION    +6.76s	 21.85/  0.73
 event8   POINTER_MOTION    +6.77s	 22.58/  1.46
 event8   POINTER_MOTION    +6.78s	 23.31/  2.19
 event8   POINTER_MOTION    +6.79s	 16.02/  2.19
 event8   POINTER_MOTION    +6.80s	 14.57/  2.19
 event8   POINTER_MOTION    +6.80s	 15.30/  1.46
 event8   POINTER_MOTION    +6.81s	 14.57/  1.46
 event8   POINTER_MOTION    +6.82s	 12.38/  0.73
 event8   POINTER_MOTION    +6.83s	 10.20/  0.00
 event8   POINTER_MOTION    +6.83s	 10.93/ -0.73
event8  - button state: from BUTTON_STATE_AREA, event BUTTON_EVENT_UP to BUTTON_STATE_NONE
event17 - tablet-mode: resuming device
event8  - tablet-mode: resume touchpad
event1  - lid: keyboard paired with Lid Switch<->AT Translated Set 2 keyboard
event8  - palm: dwt activated with Synaptics TM3293-011<->AT Translated Set 2 keyboard
event3  - tablet-mode: resuming device
-event9   SWITCH_TOGGLE     +9.36s	switch tablet-mode state 0
-event3   KEYBOARD_KEY     +10.94s	*** (-1) pressed
 event3   KEYBOARD_KEY     +11.15s	*** (-1) pressed

event2  - Power Button: device removed
event7  - Video Bus: device removed
event1  - Lid Switch: device removed
event0  - Sleep Button: device removed
event5  - Wacom Pen and multitouch sensor Finger: device removed
event6  - Wacom Pen and multitouch sensor Pen: device removed
event4  - Integrated Camera: Integrated C: device removed
event10 - HDA Intel PCH Mic: device removed
event11 - HDA Intel PCH Headphone: device removed
event12 - HDA Intel PCH HDMI/DP,pcm=3: device removed
event13 - HDA Intel PCH HDMI/DP,pcm=7: device removed
event14 - HDA Intel PCH HDMI/DP,pcm=8: device removed
event15 - HDA Intel PCH HDMI/DP,pcm=9: device removed
event16 - HDA Intel PCH HDMI/DP,pcm=10: device removed
event3  - AT Translated Set 2 keyboard: device removed
event9  - ThinkPad Extra Buttons: device removed
event8  - Synaptics TM3293-011: device removed
event17 - TPPS/2 Elan TrackPoint: device removed
--- end ---

--- udevadm info ---
P: /devices/rmi4-00/input/input20/event8
N: input/event8
E: DEVNAME=/dev/input/event8
E: DEVPATH=/devices/rmi4-00/input/input20/event8
E: ID_BUS=rmi
E: ID_INPUT=1
E: ID_INPUT_HEIGHT_MM=59
E: ID_INPUT_TOUCHPAD=1
E: ID_INPUT_TOUCHPAD_INTEGRATION=internal
E: ID_INPUT_WIDTH_MM=96
E: ID_SERIAL=noserial
E: LIBINPUT_DEVICE_GROUP=1d/6cb/0:rmi4-00
E: MAJOR=13
E: MINOR=72
E: SUBSYSTEM=input
E: USEC_INITIALIZED=7032368
Comment 1 Peter Hutterer 2018-05-14 00:57:57 UTC
That is... strange. The code calls evdev_device_suspend() immediately and that should close the fd on the device. I'll need you to debug this because I can't see where this is going wrong. Simple enough if you're on the machine. Grep git master and build it:

https://wayland.freedesktop.org/libinput/doc/latest/building_libinput.html
No need to install it, just build it locally, then run:

sudo gdb ./builddir/libinput-debug-events

put a breakpoint at  tp_switch_event() and trigger the tablet mode switch. Single-step through to figure out where it's going wrong.




Speaking of Yoga, can you check bug 103749 and confirm whether this device needs the keyboard exception too?
Comment 2 Zachary Smith 2018-05-14 05:38:12 UTC
I was able to step through to where device->fd is step to -1 and confirmed in gdb that 'print device->fd' is -1 after being set. 

So it appears that the logic is correct to get through disabling the fd in evdev_device_suspend which is as expected.

I could use a bit more help on where to look next or what I should be on the look out for. Any more suggestions?

As for https://bugs.freedesktop.org/show_bug.cgi?id=103749 it does not apply to the Lenovo X1 Yoga (Gen 3) as there is no capacitive button on the bezel.
Comment 3 Peter Hutterer 2018-05-14 06:09:35 UTC
Ah, goood to know, I'll forget about the other bug then :)

I guess the next step would be to search for the if 0 in evdev.c and make it an if 1, this way you get the evdev events and printed (in --verbose mode). break on evdev_device_resume() and evdev_device_dispatch() to figure out where the events are coming from and why they're still coming in when the fd was closed. Check the fd value on the device too.
Comment 4 Zachary Smith 2018-05-14 14:56:57 UTC
I haven't been able to chase this all the way down yet but here is my current thought:

It appears that tp_resume_conditional is being called after evdev_device_suspend

evdev_device_suspend
evdev_notify_suspended_device 
tp_interface_device_removed
tp_resume_conditional

Inside tp_resume_conditional, both if conditionals fail which results in tp_resume being called which appears to resume the tp. 

My best guess is that one of those conditionals in tp_resume_conditional should be true resulting in a return OR a new conditional may be needed?

I can get back to this more tonight (west coast USA time) so feel free to give me some more breadcrumbs to follow.
Comment 5 Peter Hutterer 2018-05-14 22:54:04 UTC
Strange, and unfortunately the test devices I have here don't seem to reproduce this issue.

evdev_device_suspend() 
 - we expect that to be called with the device (if in doubt, print
   evdev_device->devname

evdev_notify_suspended_device()
 - this notifies other devices that a device has been suspended so they can 
   update their state (e.g. suspending a touchpad disables the disable-while-
   typing pairing with the keyboard)

tp_interface_device_removed()
 - this should not be called because it would imply that the device tries to 
   notify itself

So the next thing to check here is what the argument is to evdev_notify_suspended_device() and whether it's the touchpad. If not, then we probably have some memory corruption somewhere. If it is the touchpad, check the list_for_each loop and why the 'continue' condition doesn't trigger. That's likely the explanation.
Comment 6 Zachary Smith 2018-05-15 05:26:51 UTC
Thanks for the helpful comments, Peter.

I added a breakpoint to evdev_notify_suspended_device and stepped through all of the devices coming through. When the touchpad ("Synaptics TM3293-011") comes through, I watched it go all the way through and it does successfully hit the 'continue' in the list_for_each block so is not sending a dispatch to itself. It continues to be suspended and has its fd set to -1. All seems good there.

A little while later, the keyboard ("AT Translated Set 2 keyboard") comes through evdev_notify_suspend_device and loops over the devices_list. 

When it comes to the touchpad, if (d->dispatch->interface->device_suspended) evaluates true and d->dispatch->interface->device_suspended(d, device) is called (with 'd' == touchpad and device = keyboard) triggering tp_interface_device_removed which ultimately calls tp_resume_conditional which then re-enables the touchpad that was previously disabled. 

I confirmed that the touchpad fd was -1 when entering tp_resume_conditional and 23 (which was what it was prior to suspending) upon exit.


Here is the stack:
#0  0x00007ffff7bab526 in tp_resume_conditional (tp=
    0x6518f0, device=0x668380, excluded_device=0x65c000)
    at ../src/evdev-mt-touchpad.c:1955
#1  0x00007ffff7bac15f in tp_interface_device_removed (device=0x668380, removed_device=0x65c000) at ../src/evdev-mt-touchpad.c:2344
#2  0x00007ffff7b9ffc7 in evdev_notify_suspended_device (device=0x65c000)
    at ../src/evdev.c:2506
#3  0x00007ffff7ba00f9 in evdev_device_suspend (device=0x65c000) at ../src/evdev.c:2537
#4  0x00007ffff7ba4a8c in fallback_suspend (dispatch=0x66f730, device=0x65c000)
    at ../src/evdev-fallback.c:1149
#5  0x00007ffff7ba4b4b in fallback_tablet_mode_switch_event (time=2988977338, event=
    0x68bda0, data=0x66f730) at ../src/evdev-fallback.c:1175
#6  0x00007ffff7b96f75 in post_device_event (device=0x65a560, time=2988977338, type=
    LIBINPUT_EVENT_SWITCH_TOGGLE, event=0x68bda0) at ../src/libinput.c:2117
#7  0x00007ffff7b98349 in switch_notify_toggle (device=0x65a560, time=2988977338, sw=
    LIBINPUT_SWITCH_TABLET_MODE, state=LIBINPUT_SWITCH_STATE_ON) at ../src/libinput.c:2740
#8  0x00007ffff7ba3d1b in fallback_process_switch (dispatch=
    0x667120, device=0x65a560, e=0x7fffffffe790, time=2988977338)
    at ../src/evdev-fallback.c:705
#9  0x00007ffff7ba437b in fallback_interface_process (evdev_dispatch=
    0x667120, device=0x65a560, event=0x7fffffffe790, time=2988977338)
    at ../src/evdev-fallback.c:902
#10 0x00007ffff7b9c8ae in evdev_process_event (device=0x65a560, e=0x7fffffffe790)
    at ../src/evdev.c:863
#11 0x00007ffff7b9c8e4 in evdev_device_dispatch_one (device=0x65a560, ev=0x7fffffffe790)
    at ../src/evdev.c:871
#12 0x00007ffff7b9ca83 in evdev_device_dispatch (data=0x65a560) at ../src/evdev.c:930
#13 0x00007ffff7b96c14 in libinput_dispatch (libinput=0x619760) at ../src/libinput.c:2000
#14 0x0000000000407229 in handle_and_print_events (li=0x619760)
    at ../tools/libinput-debug-events.c:773
#15 0x00000000004076a2 in mainloop (li=0x619760) at ../tools/libinput-debug-events.c:899
#16 0x00000000004078aa in main (argc=2, argv=0x7fffffffeba8)
    at ../tools/libinput-debug-events.c:998
Comment 7 Peter Hutterer 2018-05-15 05:58:40 UTC
oh, thanks. that helps and it's a relatively simple problem: we only remember that we're suspended but not *why*. So at the next opportunity we resume, regardless of the original reason. This worked when it was only disable-while-typing (dwt) but now that we have a few different suspend conditions it breaks. We need some state/bitmask/whatever to remember why we suspended and then only resume when that reason clears.
Comment 9 Zachary Smith 2018-05-16 14:30:54 UTC
I cloned your repo, checked out your wip/touchpad-multiple-suspend-handler branch, and compiled successfully.

Next, I ran builddir/libinput-debug-events, opened the lid to initiate tablet mode, and moved my finger across the touchpad - unfortunately, the pointer still moved and clicks were registered. My first question is, do I need to take some other steps to test the ultimate functionality (install the compiled binaries, for example)?

Here are a few things I tried next to capture some information for you.

I changed #if 0 to 1 in evdev.c, rebuilt, and ran in gdb with breakpoints in tp_suspend, tp_resume, tp_tablet_mode_switch_event, and evdev_device_suspend. I set args to --verbose and stepped through the touchpad suspend and resume flow. It appears that the device fd is being set to -1 in suspend and back to its original value (23) in resume. There does not appear to be a resume request fired (ie, tp_resume is not called via a notify event from the keyboard for example). This all seems good and as far as I can tell the trigger check portion of the code you added appears to be working.

However, the touchpad pointer moves and clicks have an effect on the system in tablet mode. What may be interesting is that there are no messages from evdev_print_event displayed for the touchpad events until tablet mode is ended at which point I see them displayed again.

Let me know if I can provide anything else.
Comment 10 Zachary Smith 2018-05-16 21:07:05 UTC
After seeing that you had clear instructions for "Reverting to the system-provided libinput package" here: https://wayland.freedesktop.org/libinput/doc/latest/building_libinput.html, curiosity got the best of me I just went ahead and installed the build from your branch. 

After rebooting, it appears that the touchpad is disabled in tablet mode! 

Let me know if there is anything else I can do to help debug or test these patches for you.
Comment 11 Peter Hutterer 2018-05-16 23:08:23 UTC
ftr, libinput debug-events and all other libinput tools don't modify the existing context. so their output is the only information, regardless what the system pointer does. libinput debug-gui would've been better here because it does show the pointer movement as well (instead of just printing it).

but if the installed version works, that's fine with me ;) thanks for testing!
Comment 12 Peter Hutterer 2018-05-18 04:01:40 UTC
commit 6adb336829951d8d3acd5498d53ef1d505db3373
Author: Peter Hutterer <>
Date:   Wed May 16 09:40:50 2018 +1000

     touchpad: remember the suspend reason


https://wayland.freedesktop.org/libinput/doc/latest/reporting_bugs.html#fixed_bugs


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.