Re: [PATCH v2 2/5] platform/x86: intel-vbtn: Support separate press/release events

From: Darren Hart
Date: Thu Nov 09 2017 - 21:11:45 EST


On Fri, Nov 10, 2017 at 02:58:36AM +0100, Stefan Brüns wrote:
> On Friday, November 10, 2017 2:34:17 AM CET Darren Hart wrote:
> > On Thu, Nov 09, 2017 at 11:44:33PM +0100, Stefan Brüns wrote:
> > > Currently all key events use autorelease, but this forbids use as a
> > > modifier key.
> > >
> > > As all event codes come in even/odd pairs, we can lookup the key type
> > > (KE_KEY/KE_IGNORE) for the key up event corresponding to the currently
> > > handled key down event. If the key up is ignored, we keep setting the
> > > autorelease flag for the key down.
> >
> > What is the use-case for using these buttons as modifiers? I'm picturing one
> > of these devices in tablet mode, with a physical Windows button. What other
> > action does a user want to modify by holding the Windows button down? Or is
> > there another scenario we're trying to support here?
>
> Windows/KEY_LEFTMETA can be used as a modifier key, e.g. in combination with
> the Volume Up/Down keys. On Windows, the default for Win + VolumeUp creates a
> screenshot.
>
> You can also use this in combination with an onscreen keyboard. Pressing the
> hardware button with the hand holding the tablet and typing with the other
> hand on the OSK is probably easier than hitting both keys on the OSK.

This all makes sense - good context for the commit message. If no other changes
end up being needed, I'm happy to paste it in at merge. If changes are required,
please add it in v3.

>
> Additionally, the Volume Up/Down currently do not autorepeat, as the key is
> autoreleased on the press event. The XPS 12 does issue distinct press/release
> events, so this could be done properly. The same apparently holds for some
> other convertibles, see the links in Patch 1/5.

It sounds like this is spotty across intel-vbtn implementations? Some devices
emit release, others do not? How would you teach the driver about the
differences? Assume autorelease and change the sparse keymap entry for DMI
matched platforms with release? Doable... sounds unpleasant to maintain and
update. Do we support this fully in userspace already?

--
Darren Hart
VMware Open Source Technology Center