[PATCH 0/4] platform/x86: intel-vbtn: Add support for several more switches/buttons

[PATCH 0/4] platform/x86: intel-vbtn: Add support for several more switches/buttons

[Stefan Brüns]

Currently, intel-vbtn only supports the most relevant buttons, although
there are several more events, mostly in use by convertibles.

This series adds support for three more events. One of these events
is a switch (SW_*) event, which is currently not working when using
sparse keymaps. The first patch fixes this combination.

The second patch adds support for the SW_TABLET_MODE switch, which
is used by current convertibles.

The third patch adds support for the KEY_ROTATE_DISPLAY. On the Dell
XPS 12 (9Q33), rotation lock is implemented as a button to toggle
between locked and unlocked state. In locked state, the accelerometer
should be ignored, while in unlocked the screen contents should
autorotate based on the tablet orientation. The same functionality
is likely implemented as a switch (SW_ROTATE_LOCK event) on different
hardware.

The fourth patch adds support for the "Windows logo" button/key found on
the XPS 12 display (i.e. in tablet mode, it is the only key reachable).
The Lenovo Helix 2 has an equivalent touch button. The event currently
uses KEY_MENU, although a distinct key code may be a better choice.


Stefan Brüns (4):
  Input: sparse-keymap - send sync event for KE_SW/KW_VSW
  platform/x86: intel-vbtn: support SW_TABLET_MODE
  platform/x86: intel-vbtn: support KEY_ROTATE_DISPLAY
  platform/x86: intel-vbtn: support panel front button

 drivers/input/sparse-keymap.c     | 1 +
 drivers/platform/x86/intel-vbtn.c | 6 ++++++
 2 files changed, 7 insertions(+)

-- 
2.14.3

Re: [PATCH 0/4] platform/x86: intel-vbtn: Add support for several more switches/buttons

[Andy Shevchenko]

On Fri, Nov 3, 2017 at 9:03 PM, Stefan Brüns
<stefan.bruens@rwth-aachen.de> wrote:
> Currently, intel-vbtn only supports the most relevant buttons, although
> there are several more events, mostly in use by convertibles.
>
> This series adds support for three more events. One of these events
> is a switch (SW_*) event, which is currently not working when using
> sparse keymaps. The first patch fixes this combination.
>
> The second patch adds support for the SW_TABLET_MODE switch, which
> is used by current convertibles.
>
> The third patch adds support for the KEY_ROTATE_DISPLAY. On the Dell
> XPS 12 (9Q33), rotation lock is implemented as a button to toggle
> between locked and unlocked state. In locked state, the accelerometer
> should be ignored, while in unlocked the screen contents should
> autorotate based on the tablet orientation. The same functionality
> is likely implemented as a switch (SW_ROTATE_LOCK event) on different
> hardware.
>
> The fourth patch adds support for the "Windows logo" button/key found on
> the XPS 12 display (i.e. in tablet mode, it is the only key reachable).
> The Lenovo Helix 2 has an equivalent touch button. The event currently
> uses KEY_MENU, although a distinct key code may be a better choice.
>

All, except first, are applied to my review and testing queue, thanks!

>
> Stefan Brüns (4):
>   Input: sparse-keymap - send sync event for KE_SW/KW_VSW
>   platform/x86: intel-vbtn: support SW_TABLET_MODE
>   platform/x86: intel-vbtn: support KEY_ROTATE_DISPLAY
>   platform/x86: intel-vbtn: support panel front button
>
>  drivers/input/sparse-keymap.c     | 1 +
>  drivers/platform/x86/intel-vbtn.c | 6 ++++++
>  2 files changed, 7 insertions(+)
>
> --
> 2.14.3
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] platform/x86: intel-vbtn: Add support for several more switches/buttons

[Bastien Nocera]

On Sun, 2017-11-05 at 14:31 +0200, Andy Shevchenko wrote:
> On Fri, Nov 3, 2017 at 9:03 PM, Stefan Brüns
> <stefan.bruens@rwth-aachen.de> wrote:
> > Currently, intel-vbtn only supports the most relevant buttons,
> > although
> > there are several more events, mostly in use by convertibles.
> > 
> > This series adds support for three more events. One of these events
> > is a switch (SW_*) event, which is currently not working when using
> > sparse keymaps. The first patch fixes this combination.
> > 
> > The second patch adds support for the SW_TABLET_MODE switch, which
> > is used by current convertibles.
> > 
> > The third patch adds support for the KEY_ROTATE_DISPLAY. On the
> > Dell
> > XPS 12 (9Q33), rotation lock is implemented as a button to toggle
> > between locked and unlocked state. In locked state, the
> > accelerometer
> > should be ignored, while in unlocked the screen contents should
> > autorotate based on the tablet orientation. The same functionality
> > is likely implemented as a switch (SW_ROTATE_LOCK event) on
> > different
> > hardware.

KEY_ROTATE_DISPLAY will resolve to XF86RotateWindows. This is a button
for devices without accelerometers (such as old Thinkpad tablet models
with Wacom builtin), to request the display to be rotated. It's not a
request to the OS to toggle the acceleration lock.

The acceleration lock is currently handled in user-space if "Win+O" is
pressed, as some BIOSes will generate this key combination directly,
which Windows will handle.

Please revert this patch, it should send out Win+O instead (or it needs
another keycode added).

(Also, even if the code ultimately lands in the platform tree, it would
be nice to send the linux-input list a copy of those types of patches,
where this problem would have been caught earlier).

Cheers

Re: [PATCH 0/4] platform/x86: intel-vbtn: Add support for several more switches/buttons

[Bastien Nocera]

On Sun, 2017-11-05 at 14:31 +0200, Andy Shevchenko wrote:
> On Fri, Nov 3, 2017 at 9:03 PM, Stefan Brüns
> <stefan.bruens@rwth-aachen.de> wrote:
> > Currently, intel-vbtn only supports the most relevant buttons,
> > although
> > there are several more events, mostly in use by convertibles.
> > 
> > This series adds support for three more events. One of these events
> > is a switch (SW_*) event, which is currently not working when using
> > sparse keymaps. The first patch fixes this combination.
> > 
> > The second patch adds support for the SW_TABLET_MODE switch, which
> > is used by current convertibles.
> > 
> > The third patch adds support for the KEY_ROTATE_DISPLAY. On the
> > Dell
> > XPS 12 (9Q33), rotation lock is implemented as a button to toggle
> > between locked and unlocked state. In locked state, the
> > accelerometer
> > should be ignored, while in unlocked the screen contents should
> > autorotate based on the tablet orientation. The same functionality
> > is likely implemented as a switch (SW_ROTATE_LOCK event) on
> > different
> > hardware.
> > 
> > The fourth patch adds support for the "Windows logo" button/key
> > found on
> > the XPS 12 display (i.e. in tablet mode, it is the only key
> > reachable).
> > The Lenovo Helix 2 has an equivalent touch button. The event
> > currently
> > uses KEY_MENU, although a distinct key code may be a better choice.
> > 
> 
> All, except first, are applied to my review and testing queue,
> thanks!
> 
> > 
> > Stefan Brüns (4):
> >   Input: sparse-keymap - send sync event for KE_SW/KW_VSW
> >   platform/x86: intel-vbtn: support SW_TABLET_MODE
> >   platform/x86: intel-vbtn: support KEY_ROTATE_DISPLAY
> >   platform/x86: intel-vbtn: support panel front button

KEY_MENU is the key for the contextual menu. You need to use
KEY_LEFTMETA. See 791738be57473fddaf393dcedcef31b577231aaa which does
this for soc_button_array.

Cheers

Re: [PATCH 0/4] platform/x86: intel-vbtn: Add support for several more switches/buttons

[Brüns, Stefan]

On Montag, 6. November 2017 13:54:25 CET Bastien Nocera wrote:
> On Sun, 2017-11-05 at 14:31 +0200, Andy Shevchenko wrote:
> > On Fri, Nov 3, 2017 at 9:03 PM, Stefan Brüns
> > 
> > <stefan.bruens@rwth-aachen.de> wrote:
> > > Currently, intel-vbtn only supports the most relevant buttons,
> > > although
> > > there are several more events, mostly in use by convertibles.
> > > 
> > > This series adds support for three more events. One of these events
> > > is a switch (SW_*) event, which is currently not working when using
> > > sparse keymaps. The first patch fixes this combination.
> > > 
> > > The second patch adds support for the SW_TABLET_MODE switch, which
> > > is used by current convertibles.
> > > 
> > > The third patch adds support for the KEY_ROTATE_DISPLAY. On the
> > > Dell
> > > XPS 12 (9Q33), rotation lock is implemented as a button to toggle
> > > between locked and unlocked state. In locked state, the
> > > accelerometer
> > > should be ignored, while in unlocked the screen contents should
> > > autorotate based on the tablet orientation. The same functionality
> > > is likely implemented as a switch (SW_ROTATE_LOCK event) on
> > > different
> > > hardware.
> > > 
> > > The fourth patch adds support for the "Windows logo" button/key
> > > found on
> > > the XPS 12 display (i.e. in tablet mode, it is the only key
> > > reachable).
> > > The Lenovo Helix 2 has an equivalent touch button. The event
> > > currently
> > > uses KEY_MENU, although a distinct key code may be a better choice.
> > 
> > All, except first, are applied to my review and testing queue,
> > thanks!
> > 
> > > Stefan Brüns (4):
> > >   Input: sparse-keymap - send sync event for KE_SW/KW_VSW
> > >   platform/x86: intel-vbtn: support SW_TABLET_MODE
> > >   platform/x86: intel-vbtn: support KEY_ROTATE_DISPLAY
> > >   platform/x86: intel-vbtn: support panel front button
> 
> KEY_MENU is the key for the contextual menu. You need to use
> KEY_LEFTMETA. See 791738be57473fddaf393dcedcef31b577231aaa which does
> this for soc_button_array.

IMHO LEFTMETA is a bad idea for several reasons:

- LEFTMETA aka Windows aka RightGUI key is used as a modifier/flag (see e.g. 
USB HID HUT, Keyboard Page 0x07), while on a tablet, it is the only regular 
button (save special funtions like power, volume).

- on a regular keyboard, I expect the LEFTMETA key to be handled/usable as a 
modifier key. I would not expect it to be used as a shortcut key.

So if KEY_MENU is not acceptable, a new keycode IMHO is a much better option.

Kind regards,

Stefan

Re: [PATCH 0/4] platform/x86: intel-vbtn: Add support for several more switches/buttons

[Bastien Nocera]

On Mon, 2017-11-06 at 15:25 +0000, Brüns, Stefan wrote:
> On Montag, 6. November 2017 13:54:25 CET Bastien Nocera wrote:
> > On Sun, 2017-11-05 at 14:31 +0200, Andy Shevchenko wrote:
> > > On Fri, Nov 3, 2017 at 9:03 PM, Stefan Brüns
> > > 
> > > <stefan.bruens@rwth-aachen.de> wrote:
> > > > Currently, intel-vbtn only supports the most relevant buttons,
> > > > although
> > > > there are several more events, mostly in use by convertibles.
> > > > 
> > > > This series adds support for three more events. One of these
> > > > events
> > > > is a switch (SW_*) event, which is currently not working when
> > > > using
> > > > sparse keymaps. The first patch fixes this combination.
> > > > 
> > > > The second patch adds support for the SW_TABLET_MODE switch,
> > > > which
> > > > is used by current convertibles.
> > > > 
> > > > The third patch adds support for the KEY_ROTATE_DISPLAY. On the
> > > > Dell
> > > > XPS 12 (9Q33), rotation lock is implemented as a button to
> > > > toggle
> > > > between locked and unlocked state. In locked state, the
> > > > accelerometer
> > > > should be ignored, while in unlocked the screen contents should
> > > > autorotate based on the tablet orientation. The same
> > > > functionality
> > > > is likely implemented as a switch (SW_ROTATE_LOCK event) on
> > > > different
> > > > hardware.
> > > > 
> > > > The fourth patch adds support for the "Windows logo" button/key
> > > > found on
> > > > the XPS 12 display (i.e. in tablet mode, it is the only key
> > > > reachable).
> > > > The Lenovo Helix 2 has an equivalent touch button. The event
> > > > currently
> > > > uses KEY_MENU, although a distinct key code may be a better
> > > > choice.
> > > 
> > > All, except first, are applied to my review and testing queue,
> > > thanks!
> > > 
> > > > Stefan Brüns (4):
> > > >   Input: sparse-keymap - send sync event for KE_SW/KW_VSW
> > > >   platform/x86: intel-vbtn: support SW_TABLET_MODE
> > > >   platform/x86: intel-vbtn: support KEY_ROTATE_DISPLAY
> > > >   platform/x86: intel-vbtn: support panel front button
> > 
> > KEY_MENU is the key for the contextual menu. You need to use
> > KEY_LEFTMETA. See 791738be57473fddaf393dcedcef31b577231aaa which
> > does
> > this for soc_button_array.
> 
> IMHO LEFTMETA is a bad idea for several reasons:
> 
> - LEFTMETA aka Windows aka RightGUI key is used as a modifier/flag
> (see e.g. 
> USB HID HUT, Keyboard Page 0x07), while on a tablet, it is the only
> regular 
> button (save special funtions like power, volume).

It isn't. There are a number of functions that can be triggered using
the Windows key + other buttons on the device, such as:

 ⊞ Win+PrtScr or ⊞ Win+Volume up instantly saves a screenshot to the
"Screenshots" folder in "Pictures" library. All screenshots are saved
as PNG files.

(from https://en.wikipedia.org/wiki/Windows_key#Windows_8)

In Windows, the button behaves the exact same way as the Windows key on
a keyboard would.

> - on a regular keyboard, I expect the LEFTMETA key to be
> handled/usable as a 
> modifier key. I would not expect it to be used as a shortcut key.
> 
> So if KEY_MENU is not acceptable, a new keycode IMHO is a much better
> option.
> 
> Kind regards,
> 
> Stefan
> 

Re: [PATCH 0/4] platform/x86: intel-vbtn: Add support for several more switches/buttons

[Andy Shevchenko]

On Mon, Nov 6, 2017 at 2:41 PM, Bastien Nocera <hadess@hadess.net> wrote:
> On Sun, 2017-11-05 at 14:31 +0200, Andy Shevchenko wrote:
>> On Fri, Nov 3, 2017 at 9:03 PM, Stefan Brüns
>> <stefan.bruens@rwth-aachen.de> wrote:

> Please revert this patch, it should send out Win+O instead (or it needs
> another keycode added).

Based on the fact of discussion itself the patches didn't make our
published tree anyway, no need to revert.

-- 
With Best Regards,
Andy Shevchenko