Bug 197991

Summary: Volume and rotation lock buttons do not work on the Wacom Mobile Studio Pro
Product: Platform Specific/Hardware Reporter: Jason Gerecke (killertofu)
Component: x86-64Assignee: drivers_platform_x86 (drivers_platform_x86)
Status: RESOLVED CODE_FIX    
Severity: normal CC: alex.hung, andy.shevchenko, jehan, stefan.bruens
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 4.14 Tree: Mainline
Regression: No
Attachments: DSDT table
Decompiled DSDT table
dmesg log
dmidecode output (16" pre-release)
intel-hid with dmi quirk
evemu.log
dmidecode output (16" release)
dmidecode output (13" release)
new intel-hid with DMI strings updated
new intel-hid with DMI strings + identifier updated

Description Jason Gerecke 2017-11-25 17:51:49 UTC
The Wacom MobileStudio Pro is a tablet-style PC. Its volume up/down and rotation lock buttons do not work in Linux 4.14. These buttons appear to be controlled by the "intel-hid" platform driver, which logs[1] the following debug messages to dmesg when the respective buttons are pressed and released:

intel-hid INT33D5:00: unknown event 0xc4
intel-hid INT33D5:00: unknown event 0xc5
intel-hid INT33D5:00: unknown event 0xc6
intel-hid INT33D5:00: unknown event 0xc7
intel-hid INT33D5:00: unknown event 0xc8
intel-hid INT33D5:00: unknown event 0xc9

It seems that a patch[2] was accepted in early 2017 which added support for a "5-button array" used by these buttons. The "intel_array_keymap" seems to properly describe the events that are sent. However, it seems that the driver does not realize that the "5-button array" is supported/used by this device's hardware. In particular, the message "platform supports 5 button array" is never printed out.

After decompiling the ACPI DSDT table, I see no mention of an "HEBC" device that this driver checks for (though I do see the "HDMM" device that is looked for immediately above). Perhaps the "5-button array" needs to be supported on hardware without an "HEBC" device?

[1]: `# echo 'file intel-hid.c +p' > /sys/kernel/debug/dynamic_debug/control`
[2]: https://patchwork.kernel.org/patch/9571353/
Comment 1 Jason Gerecke 2017-11-25 17:59:19 UTC
Created attachment 260849 [details]
DSDT table
Comment 2 Jason Gerecke 2017-11-25 18:00:07 UTC
Created attachment 260851 [details]
Decompiled DSDT table
Comment 3 Jason Gerecke 2017-11-25 18:03:54 UTC
Created attachment 260853 [details]
dmesg log
Comment 4 Jason Gerecke 2017-11-29 22:01:23 UTC
Created attachment 260939 [details]
dmidecode output (16" pre-release)
Comment 5 Alex Hung 2017-11-30 12:30:03 UTC
Created attachment 260947 [details]
intel-hid with dmi quirk

I still need to find a system to verify the modified intel-hid, but let's see whether this will setup 5 button array on Wacom Mobile Studio Pro and handle events events correctly.
Comment 6 Jason Gerecke 2017-12-01 18:22:09 UTC
Compiling the kernel with the replacement intel-hid.c file seems to do the trick on my MobileStudio Pro. There is now an "Intel HID 5 button array" input device which sends KEY_VOLUMEUP, KEY_VOLUMEDOWN, and SW_ROTATE_LOCK events.

I notice a potential issue with the SW_ROTATE_SWITCH, however. In particular, this device uses a spring-loaded momentary contact switch for the control rather than a true toggle switch (see the "Hardware & Tactile Buttons" section of [1]). The kernel sends an SW_ROTATE_LOCK event with value "1" only while the button is held. As soon as it is released, an SW_ROTATE_LOCK event with value "0" is sent.

I've tested the volume and rotation lock within GNOME and the former works perfectly, but the latter doesn't seem to do anything (even if I hold the switch while rotating). I'm not sure if GNOME actually supports the rotation lock switch, however, so this might be normal.


Let me know if there is anything that can/should be done about the momentary nature of the rotation lock "switch" on this device.


[1]: http://pocketnow.com/2017/03/06/wacom-mobilestudio-pro-13-review
Comment 7 Jehan 2017-12-01 18:43:56 UTC
> I'm not sure if GNOME actually supports the rotation lock switch, however, so
> this might be normal.

GNOME has a rotation lock.

On our MobileStudio Pro with GNOME 3.26, we have a lock button by default on the top-right menu of the desktop (next to the power-off/lock-screen/settings button; So I assume that when GNOME detects the gyroscope/accelerometer, it adds such a button; cf. for instance this screenshot: https://pbs.twimg.com/media/DPVcXHxXUAABq5L.jpg). So it has the feature to lock the rotation.

Unless you meant: does it supports receiving SW_ROTATE_LOCK event and locking rotation when it does. In this case, I don't know.
Is there no command to simulate such events and throw them at GNOME?
Comment 8 Jason Gerecke 2017-12-01 21:51:00 UTC
Created attachment 260987 [details]
evemu.log

(In reply to Jehan from comment #7)
> Unless you meant: does it supports receiving SW_ROTATE_LOCK event and
> locking rotation when it does. In this case, I don't know.
> Is there no command to simulate such events and throw them at GNOME?

Yes, this is what I meant. I've attached a recording from `evemu-record` of me using these buttons. If you have the necessary package[1] installed, you can replay it locally by running `sudo evemu-play /path/to/evemu.log`.

In the recording, I do the following:

1) Press the volume-up button
2) Press the rotation lock button
3) Wait 13 seconds (try rotating the tablet at this time)
4) Press the rotation lock button
5) Press the volume-down button
6) Wait 7 seconds (try rotating the tablet again)
7) Press the volume-up button
8) Hold the rotation lock button down
9) Wait 13 seconds (try rotating the tablet again)
10) Release the rotation lock button
11) Press the volume down button
[end of recording: might as well try rotating the tablet one last time]

The on-screen notification that appears when the volume is changed should make it easier to keep track of where in the script things are without needing a stopwatch :)


[1]: "evemu-tools" (Debian, Ubuntu, etc.), "evemu" (Red Hat, Fedora, Arch, Manjaro, etc.)
Comment 9 Alex Hung 2017-12-04 13:28:58 UTC
(In reply to Jason Gerecke from comment #6)
> Compiling the kernel with the replacement intel-hid.c file seems to do the
> trick on my MobileStudio Pro. There is now an "Intel HID 5 button array"
> input device which sends KEY_VOLUMEUP, KEY_VOLUMEDOWN, and SW_ROTATE_LOCK
> events.

Thanks for testing. This is the expected behaviours. I am going to create a patch and submit to platform-driver mail list.

> 
> I notice a potential issue with the SW_ROTATE_SWITCH, however. In
> particular, this device uses a spring-loaded momentary contact switch for
> the control rather than a true toggle switch (see the "Hardware & Tactile
> Buttons" section of [1]). The kernel sends an SW_ROTATE_LOCK event with
> value "1" only while the button is held. As soon as it is released, an
> SW_ROTATE_LOCK event with value "0" is sent.
> 
> I've tested the volume and rotation lock within GNOME and the former works
> perfectly, but the latter doesn't seem to do anything (even if I hold the
> switch while rotating). I'm not sure if GNOME actually supports the rotation
> lock switch, however, so this might be normal.

It is sufficient for intel-hid to generate events, and userspace service should handle it. Having say that, I will find a system with rotation capability and see what else intel-hid can do, but this may take a while. As a result, I will send a patch to address 5 button array first.

> 
> 
> Let me know if there is anything that can/should be done about the momentary
> nature of the rotation lock "switch" on this device.

Thanks. I will keep you posted.

> 
> 
> [1]: http://pocketnow.com/2017/03/06/wacom-mobilestudio-pro-13-review
Comment 10 Jehan 2017-12-04 16:32:21 UTC
> If you have the necessary package[1] installed, you can replay it locally

Unfortunately I had to reinstall Windows to ask for an exchange (webcam device not working) so I can't test anything on this machine right now.

Also in your log, I can indeed see a SW_ROTATE_LOCK 0 immediately after each SW_ROTATE_LOCK 1, which is not good, even if the feature gets implemented in GNOME (or other DEs). It would be nice if such spring-loaded switch case be taken into account; basically only emit a single SW_ROTATE_LOCK for a back and forth on the switch. Of course this means the kernel has to keep a state of its value for longer (no idea if that's a problem).
Could it be done this way?

> 8) Hold the rotation lock button down
> 9) Wait 13 seconds (try rotating the tablet again)
> 10) Release the rotation lock button

Did you confirm indeed that the screen still rotated with GNOME? If so, I guess we can already open a bug report there (though I'm not sure if that's with GNOME directly; maybe libinput or something else).
Comment 11 Jason Gerecke 2017-12-04 17:53:22 UTC
(In reply to Jehan from comment #10)
> > If you have the necessary package[1] installed, you can replay it locally
> 
> Unfortunately I had to reinstall Windows to ask for an exchange (webcam
> device not working) so I can't test anything on this machine right now.
> 
Ah, sorry to hear that.

> Also in your log, I can indeed see a SW_ROTATE_LOCK 0 immediately after each
> SW_ROTATE_LOCK 1, which is not good, even if the feature gets implemented in
> GNOME (or other DEs). It would be nice if such spring-loaded switch case be
> taken into account; basically only emit a single SW_ROTATE_LOCK for a back
> and forth on the switch. Of course this means the kernel has to keep a state
> of its value for longer (no idea if that's a problem).
> Could it be done this way?
> 
> > 8) Hold the rotation lock button down
> > 9) Wait 13 seconds (try rotating the tablet again)
> > 10) Release the rotation lock button
> 
> Did you confirm indeed that the screen still rotated with GNOME? If so, I
> guess we can already open a bug report there (though I'm not sure if that's
> with GNOME directly; maybe libinput or something else).

Yes, GNOME continues to rotate even when I hold the switch down. Definitely sounding more like GNOME doesn't have support, though its also possible that I'm missing some crucial package (the "iio-sensor-proxy" it needs for accelerometer rotation wasn't installed out of the box with Manjaro, so its definitely possible...) I'll go poke someone at GNOME for details.
Comment 12 Jehan 2017-12-04 18:06:08 UTC
> Ah, sorry to hear that.

I actually sent you an email about this a few days ago, hoping for some help on the reinstalling-Windows part. Didn't you receive it? (sorry for the off-topic!)

> I'll go poke someone at GNOME for details.

Ok. So I'll hold making a bug report for now.
Comment 13 Jason Gerecke 2017-12-05 21:20:51 UTC
Created attachment 261025 [details]
dmidecode output (16" release)
Comment 14 Jason Gerecke 2017-12-05 21:21:12 UTC
Created attachment 261027 [details]
dmidecode output (13" release)
Comment 15 Jason Gerecke 2017-12-05 21:24:42 UTC
As mentioned on the LKML, this patch worked on the pre-release version of the 16" model but not on the release version of the 13" model which I just tested. I've attached the output of dmidecode from release versions of both the 13" and 16" models which should allow for a better quirk to be created.
Comment 16 Alex Hung 2017-12-06 01:43:48 UTC
Created attachment 261031 [details]
new intel-hid with DMI strings updated

DMI strings are updated according to two previous attachment. Please note the earlier DMI strings from pre-released systems are removed as they seem to be too general and may affect other systems.
Comment 17 Alex Hung 2017-12-06 01:50:27 UTC
Jason reminded me that there is another patch for a new keycode "KEY_ROTATE_LOCK_TOGGLE" waiting for approval right now. It is not in mainline kernel yet and I cannot use for the patch I am submitting, but I will try to create a separate patch for testing, if the one in comment 16 is working properly.
Comment 18 Alex Hung 2017-12-06 02:13:28 UTC
Created attachment 261033 [details]
new intel-hid with DMI strings + identifier updated

Mobile Studio is changed to MobileStudio according to Jason's comments in mailing list.
Comment 19 Jason Gerecke 2017-12-06 21:36:05 UTC
Both the 13" and 16" release units work fine with the latest version of the attachment.
Comment 20 Jehan 2018-01-24 17:25:42 UTC
Any news? We'd love it if this could make it to a stable kernel soon. ;-)
Comment 21 Jason Gerecke 2018-01-25 15:59:03 UTC
This patch was accepted into the "testing" branch of the "linux-platform-drivers-x86" tree on December 12 (see https://patchwork.kernel.org/patch/10097597/). It has since been merged into the "for-next" branch, which I assume will be pulled by Linus into what will become Linux 4.16 sometime in April.

There has been no apparent motion on the patch to add a "KEY_ROTATE_LOCK_TOGGLE" keycode mentioned in comment #17 (see https://patchwork.kernel.org/patch/10052039/). A pair of "SW_ROTATE_LOCK" keycodes are (still) sent instead, which will prevent the rotation toggle from working properly in desktops that support those buttons.

I have confirmed with a GNOME developer that "SW_ROTATE_LOCK" is not handled by their desktop. A bug will need to be filed at https://bugzilla.gnome.org/ to request that this be added. If the "KEY_ROTATE_LOCK_TOGGLE" keycode is accepted for upstream, GNOME will need to add support for it as well.
Comment 22 Jehan 2018-01-25 16:20:21 UTC
(In reply to Jason Gerecke from comment #21)
> I have confirmed with a GNOME developer that "SW_ROTATE_LOCK" is not handled
> by their desktop. A bug will need to be filed at https://bugzilla.gnome.org/
> to request that this be added. If the "KEY_ROTATE_LOCK_TOGGLE" keycode is
> accepted for upstream, GNOME will need to add support for it as well.

Done: https://gitlab.gnome.org/GNOME/gnome-shell/issues/2
Comment 23 Stefan BrĂ¼ns 2018-03-05 21:02:49 UTC
The KEY_ROTATE_LOCK_TOGGLE definition is in 4.16-rc1.
Comment 24 Andy Shevchenko 2018-04-23 14:30:02 UTC
Fixed in v4.17-rc1. Commit c454a99d4ce1 ("intel-hid: add a DMI quirk to support Wacom MobileStudio Pro").