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

From: "Stefan Brüns" <stefan.bruens@rwth-aachen.de>
To: Darren Hart <dvhart@infradead.org>
Cc: <platform-driver-x86@vger.kernel.org>,
	<linux-input@vger.kernel.org>,
	AceLan Kao <acelan.kao@canonical.com>,
	Andy Shevchenko <andy@infradead.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/5] platform/x86: intel-vbtn: Support separate press/release events
Date: Fri, 10 Nov 2017 03:44:08 +0100	[thread overview]
Message-ID: <6160107.5aeZWuaZvU@pebbles> (raw)
In-Reply-To: <20171110021137.GE9783@fury>

[-- Attachment #1: Type: text/plain, Size: 3108 bytes --]

On Friday, November 10, 2017 3:11:37 AM CET Darren Hart wrote:
> 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?

- 0xcc/0xcd SW_TABLET mode seems to be consistent and widespread

- 0xc4-0xc7 KEY_VOLUME_{UP,DOWN} seems to be consistent, although not used 
very often (Lenovo Helix 2, XPS 12, other Dell XPS), others probably use WMI

- 0xc2/0xc3 KEY_LEFTMETA is used on Helix 2 and XPS 12, but only the XPS 
issues 0xc2 on press and 0xc3 on release, the Helix 2 issues both codes on 
release and none on press.

The statements above are verified on the XPS 12, the other findings are from 
the Internet, search for "unknown event index" + "intel-vbtn".

As far as I can see, there are no implementation which do not issue the events 
in pairs, so currently the quirks table would be empty.

Kind regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]