ibm-acpi-devel — ibm-acpi Linux driver development

Re: [ibm-acpi-devel] [PATCH v2 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Hans de G. <hde...@re...>]

Hi,

On 4/18/24 2:24 PM, Mark Pearson wrote:
> Hi Hans,
> 
> On Thu, Apr 18, 2024, at 7:34 AM, Hans de Goede wrote:
>> Hi Mark,
>>
>> On 4/18/24 1:57 AM, Mark Pearson wrote:
>>> Hi Hans,
>>>
>>> On Wed, Apr 17, 2024, at 4:06 PM, Hans de Goede wrote:
>>>> Hi Mark,
>>>>
>>>> On 4/17/24 9:39 PM, Hans de Goede wrote:
>>>>> Hi Mark,
>>>>>
>>>>> Thank you for the new version of this series, overall this looks good,
>>>>> one small remark below.
>>>>>
>>>>> On 4/17/24 7:31 PM, Mark Pearson wrote:
>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>>>>> This handles the doubletap event and sends the KEY_PROG1 event to
>>>>>> userspace.
>>>>>>
>>>>>> Signed-off-by: Mark Pearson <mpe...@sq...>
>>>>>> Signed-off-by: Vishnu Sankar <vis...@gm...>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>>  - Use KEY_PROG1 instead of KEY_DOUBLETAP as input maintainer doesn't
>>>>>>    want new un-specific key codes added.
>>>>>>  - Add doubletap to hotkey scan code table and use existing hotkey
>>>>>>    functionality.
>>>>>>  - Tested using evtest, and then gnome settings to configure a custom shortcut
>>>>>>    to launch an application.
>>>>>>
>>>>>>  drivers/platform/x86/thinkpad_acpi.c | 18 ++++++++++++++++++
>>>>>>  1 file changed, 18 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>>>>>> index 3b48d893280f..6d04d45e8d45 100644
>>>>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>>>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>>>>> @@ -232,6 +232,9 @@ enum tpacpi_hkey_event_t {
>>>>>>  
>>>>>>  	/* Misc */
>>>>>>  	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
>>>>>> +
>>>>>> +	/* Misc2 */
>>>>>> +	TP_HKEY_EV_TRACK_DOUBLETAP      = 0x8036, /* trackpoint doubletap */
>>>>>>  };
>>>>>>  
>>>>>>  /****************************************************************************
>>>>>> @@ -1786,6 +1789,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>>>>>>  	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
>>>>>>  	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
>>>>>>  	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
>>>>>
>>>>> I understand why you've done this but I think this needs a comment,
>>>>> something like:
>>>>>
>>>>>         /*
>>>>>          * For TP_HKEY_EV_TRACK_DOUBLETAP, unlike the codes above which map to:
>>>>>          * (hkey_event - 0x1300) + TP_ACPI_HOTKEYSCAN_EXTENDED_START, this is
>>>>>          * hardcoded for TP_HKEY_EV_TRACK_DOUBLETAP handling. Therefor this must
>>>>>          * always be the last entry (after any 0x1300-0x13ff entries).
>>>>>          */
>>>>> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
>>>>
>>>> Ugh, actually this will not work becuuse we want hotkeyscancodes to be stable
>>>> because these are userspace API since they can be remapped using hwdb so we
>>>> cannot have the hotkeyscancode changing when new 0x1300-0x13ff range entries
>>>> get added.
>>>>
>>>> So we need to either grow the table a lot and reserve a whole bunch of space
>>>> for future 0x13xx - 0x13ff codes or maybe something like this:
>>>>
>>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
>>>> b/drivers/platform/x86/thinkpad_acpi.c
>>>> index 771aaa7ae4cf..af3279889ecc 100644
>>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>>> @@ -1742,7 +1742,12 @@ enum {	/* hot key scan codes (derived from ACPI 
>>>> DSDT) */
>>>>  	TP_ACPI_HOTKEYSCAN_VOLUMEDOWN,
>>>>  	TP_ACPI_HOTKEYSCAN_MUTE,
>>>>  	TP_ACPI_HOTKEYSCAN_THINKPAD,
>>>> -	TP_ACPI_HOTKEYSCAN_UNK1,
>>>> +	/*
>>>> +	 * Note this gets send both on 0x1019 and on 
>>>> TP_HKEY_EV_TRACK_DOUBLETAP
>>>> +	 * hotkey-events. 0x1019 events have never been seen on any actual hw
>>>> +	 * and a scancode is needed for the special 0x8036 doubletap 
>>>> hotkey-event.
>>>> +	 */
>>>> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
>>>>  	TP_ACPI_HOTKEYSCAN_UNK2,
>>>>  	TP_ACPI_HOTKEYSCAN_UNK3,
>>>>  	TP_ACPI_HOTKEYSCAN_UNK4,
>>>>
>>>> or just hardcode KEY_PROG1 like your previous patch does, but I'm not
>>>> a fan of that because of loosing hwdb remapping functionality for this
>>>> "key" then.
>>>>
>>>> Note I'm open to other suggestions.
>>>>
>>> Oh...I hadn't thought of that impact. That's not great :(
>>>
>>> I have an idea, but want to prototype it to see if it works out or not. Will update once I've had a chance to play with it.
>>
>> Thinking more about this I just realized that the input subsystem
>> already has a mechanism for dealing with scancode ranges with
>> (big) holes in them in the form of linux/input/sparse-keymap.h .
>>
>> I think that what needs to be done is convert the existing code
>> to use sparse-keymap, keeping the mapping of the "MHKP"
>> returned hkey codes to internal TP_ACPI_HOTKEYSCAN_* values
>> for currently supported "MHKP" hkey codes for compatibility
>> and then for new codes just directly map them in the sparse map
>> aka the struct key_entry table. After converting the existing code
>> to use sparse-keymap, then for the new events we would simply add:
>>
>>
>> 	{ KE_KEY, 0x131d, { KEY_VENDOR} }, /* Fn + N, system debug info */
>> 	{ KE_KEY, 0x8036, { KEY_PROG1 } }, /* Trackpoint doubletap */
>>
>> entries to the table without needing to define intermediate
>> TP_ACPI_HOTKEYSCAN_* values for these.
>>
> 
> Ah! I didn't know about sparse-keymap but it looks similar to what I was thinking and played with a bit last night. Agreed using existing infrastructure is better.
> 
> Only things I'd flag is that:
>  - It did look like it would be useful to identify keys that the driver handles (there aren't many but a few). Maybe one of the other key types can help handle that?
>  - There are also some keys that use the tpacpi_input_send_key_masked that might need some special consideration.
> 
>> I already have somewhat of a design for this in my head and I really
>> believe this is the way forward as it uses existing kernel infra
>> and it will avoid hitting this problem again when some other new
>> "MHKP" hkey codes show up.
>>
>> I plan to start working on implementing conversion of the existing
>> code to use sparse-keymap, which should result in a nice cleanup
>> after lunch and I hope to have something for you to test no later
>> then next Tuesday.
>>
> 
> That would be amazing - do let me know if there is anything I can help with. Agreed this will help clean up a bunch of the keycode handling :)

I noticed a small problem while working on this. The hwdb shipped with
systemd has:

# thinkpad_acpi driver
evdev:name:ThinkPad Extra Buttons:dmi:bvn*:bvr*:bd*:svnIBM*:pn*:*
 KEYBOARD_KEY_01=battery                                # Fn+F2
 KEYBOARD_KEY_02=screenlock                             # Fn+F3
 KEYBOARD_KEY_03=sleep                                  # Fn+F4
 KEYBOARD_KEY_04=wlan                                   # Fn+F5
 KEYBOARD_KEY_06=switchvideomode                        # Fn+F7
 KEYBOARD_KEY_07=zoom                                   # Fn+F8 screen expand
 KEYBOARD_KEY_08=f24                                    # Fn+F9 undock
 KEYBOARD_KEY_0b=suspend                                # Fn+F12
 KEYBOARD_KEY_0f=brightnessup                           # Fn+Home
 KEYBOARD_KEY_10=brightnessdown                         # Fn+End
 KEYBOARD_KEY_11=kbdillumtoggle                         # Fn+PgUp - ThinkLight
 KEYBOARD_KEY_13=zoom                                   # Fn+Space
 KEYBOARD_KEY_14=volumeup
 KEYBOARD_KEY_15=volumedown
 KEYBOARD_KEY_16=mute
 KEYBOARD_KEY_17=prog1                                  # ThinkPad/ThinkVantage button (high k

Notice the last line, this last line maps the old thinkpad /
thinkvantage key: https://www.thinkwiki.org/wiki/ThinkPad_Button
which is define by the kernel as KEY_VENDOR to KEY_PROG1 to
use a keycode below 240 for X11 compatiblity which does not
handle higher keycodes.

This means that in practice at least on older models
KEY_PROG1 is already taken and the thinkpad / thinkvantage key
does the same (open lenovo help center / sysinfo) as
what the new Fn + N key combo does. So it does makes sense
to map Fn + N to KEY_VENDOR so those align but given the existing
remapping of the thinkpad / thinkvantage key to PROG1 I think
it would be better to not use PROG1 for the doubletap.

I guess we can just use PROG2 instead to avoid the overlap
with the remapped old ThinkPad / ThinkVantage buttons
(which are more like Fn + N then doubletap).

Regards,

Hans

Re: [ibm-acpi-devel] [PATCH v2 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Mark P. <mpe...@sq...>]

Hi Hans,

On Thu, Apr 18, 2024, at 7:34 AM, Hans de Goede wrote:
> Hi Mark,
>
> On 4/18/24 1:57 AM, Mark Pearson wrote:
>> Hi Hans,
>> 
>> On Wed, Apr 17, 2024, at 4:06 PM, Hans de Goede wrote:
>>> Hi Mark,
>>>
>>> On 4/17/24 9:39 PM, Hans de Goede wrote:
>>>> Hi Mark,
>>>>
>>>> Thank you for the new version of this series, overall this looks good,
>>>> one small remark below.
>>>>
>>>> On 4/17/24 7:31 PM, Mark Pearson wrote:
>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>>>> This handles the doubletap event and sends the KEY_PROG1 event to
>>>>> userspace.
>>>>>
>>>>> Signed-off-by: Mark Pearson <mpe...@sq...>
>>>>> Signed-off-by: Vishnu Sankar <vis...@gm...>
>>>>> ---
>>>>> Changes in v2:
>>>>>  - Use KEY_PROG1 instead of KEY_DOUBLETAP as input maintainer doesn't
>>>>>    want new un-specific key codes added.
>>>>>  - Add doubletap to hotkey scan code table and use existing hotkey
>>>>>    functionality.
>>>>>  - Tested using evtest, and then gnome settings to configure a custom shortcut
>>>>>    to launch an application.
>>>>>
>>>>>  drivers/platform/x86/thinkpad_acpi.c | 18 ++++++++++++++++++
>>>>>  1 file changed, 18 insertions(+)
>>>>>
>>>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>>>>> index 3b48d893280f..6d04d45e8d45 100644
>>>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>>>> @@ -232,6 +232,9 @@ enum tpacpi_hkey_event_t {
>>>>>  
>>>>>  	/* Misc */
>>>>>  	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
>>>>> +
>>>>> +	/* Misc2 */
>>>>> +	TP_HKEY_EV_TRACK_DOUBLETAP      = 0x8036, /* trackpoint doubletap */
>>>>>  };
>>>>>  
>>>>>  /****************************************************************************
>>>>> @@ -1786,6 +1789,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>>>>>  	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
>>>>>  	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
>>>>>  	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
>>>>
>>>> I understand why you've done this but I think this needs a comment,
>>>> something like:
>>>>
>>>>         /*
>>>>          * For TP_HKEY_EV_TRACK_DOUBLETAP, unlike the codes above which map to:
>>>>          * (hkey_event - 0x1300) + TP_ACPI_HOTKEYSCAN_EXTENDED_START, this is
>>>>          * hardcoded for TP_HKEY_EV_TRACK_DOUBLETAP handling. Therefor this must
>>>>          * always be the last entry (after any 0x1300-0x13ff entries).
>>>>          */
>>>> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
>>>
>>> Ugh, actually this will not work becuuse we want hotkeyscancodes to be stable
>>> because these are userspace API since they can be remapped using hwdb so we
>>> cannot have the hotkeyscancode changing when new 0x1300-0x13ff range entries
>>> get added.
>>>
>>> So we need to either grow the table a lot and reserve a whole bunch of space
>>> for future 0x13xx - 0x13ff codes or maybe something like this:
>>>
>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
>>> b/drivers/platform/x86/thinkpad_acpi.c
>>> index 771aaa7ae4cf..af3279889ecc 100644
>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>> @@ -1742,7 +1742,12 @@ enum {	/* hot key scan codes (derived from ACPI 
>>> DSDT) */
>>>  	TP_ACPI_HOTKEYSCAN_VOLUMEDOWN,
>>>  	TP_ACPI_HOTKEYSCAN_MUTE,
>>>  	TP_ACPI_HOTKEYSCAN_THINKPAD,
>>> -	TP_ACPI_HOTKEYSCAN_UNK1,
>>> +	/*
>>> +	 * Note this gets send both on 0x1019 and on 
>>> TP_HKEY_EV_TRACK_DOUBLETAP
>>> +	 * hotkey-events. 0x1019 events have never been seen on any actual hw
>>> +	 * and a scancode is needed for the special 0x8036 doubletap 
>>> hotkey-event.
>>> +	 */
>>> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
>>>  	TP_ACPI_HOTKEYSCAN_UNK2,
>>>  	TP_ACPI_HOTKEYSCAN_UNK3,
>>>  	TP_ACPI_HOTKEYSCAN_UNK4,
>>>
>>> or just hardcode KEY_PROG1 like your previous patch does, but I'm not
>>> a fan of that because of loosing hwdb remapping functionality for this
>>> "key" then.
>>>
>>> Note I'm open to other suggestions.
>>>
>> Oh...I hadn't thought of that impact. That's not great :(
>> 
>> I have an idea, but want to prototype it to see if it works out or not. Will update once I've had a chance to play with it.
>
> Thinking more about this I just realized that the input subsystem
> already has a mechanism for dealing with scancode ranges with
> (big) holes in them in the form of linux/input/sparse-keymap.h .
>
> I think that what needs to be done is convert the existing code
> to use sparse-keymap, keeping the mapping of the "MHKP"
> returned hkey codes to internal TP_ACPI_HOTKEYSCAN_* values
> for currently supported "MHKP" hkey codes for compatibility
> and then for new codes just directly map them in the sparse map
> aka the struct key_entry table. After converting the existing code
> to use sparse-keymap, then for the new events we would simply add:
>
>
> 	{ KE_KEY, 0x131d, { KEY_VENDOR} }, /* Fn + N, system debug info */
> 	{ KE_KEY, 0x8036, { KEY_PROG1 } }, /* Trackpoint doubletap */
>
> entries to the table without needing to define intermediate
> TP_ACPI_HOTKEYSCAN_* values for these.
>

Ah! I didn't know about sparse-keymap but it looks similar to what I was thinking and played with a bit last night. Agreed using existing infrastructure is better.

Only things I'd flag is that:
 - It did look like it would be useful to identify keys that the driver handles (there aren't many but a few). Maybe one of the other key types can help handle that?
 - There are also some keys that use the tpacpi_input_send_key_masked that might need some special consideration.

> I already have somewhat of a design for this in my head and I really
> believe this is the way forward as it uses existing kernel infra
> and it will avoid hitting this problem again when some other new
> "MHKP" hkey codes show up.
>
> I plan to start working on implementing conversion of the existing
> code to use sparse-keymap, which should result in a nice cleanup
> after lunch and I hope to have something for you to test no later
> then next Tuesday.
>

That would be amazing - do let me know if there is anything I can help with. Agreed this will help clean up a bunch of the keycode handling :)

Mark

Re: [ibm-acpi-devel] [PATCH v2 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Hans de G. <hde...@re...>]

Hi Mark,

On 4/18/24 1:57 AM, Mark Pearson wrote:
> Hi Hans,
> 
> On Wed, Apr 17, 2024, at 4:06 PM, Hans de Goede wrote:
>> Hi Mark,
>>
>> On 4/17/24 9:39 PM, Hans de Goede wrote:
>>> Hi Mark,
>>>
>>> Thank you for the new version of this series, overall this looks good,
>>> one small remark below.
>>>
>>> On 4/17/24 7:31 PM, Mark Pearson wrote:
>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>>> This handles the doubletap event and sends the KEY_PROG1 event to
>>>> userspace.
>>>>
>>>> Signed-off-by: Mark Pearson <mpe...@sq...>
>>>> Signed-off-by: Vishnu Sankar <vis...@gm...>
>>>> ---
>>>> Changes in v2:
>>>>  - Use KEY_PROG1 instead of KEY_DOUBLETAP as input maintainer doesn't
>>>>    want new un-specific key codes added.
>>>>  - Add doubletap to hotkey scan code table and use existing hotkey
>>>>    functionality.
>>>>  - Tested using evtest, and then gnome settings to configure a custom shortcut
>>>>    to launch an application.
>>>>
>>>>  drivers/platform/x86/thinkpad_acpi.c | 18 ++++++++++++++++++
>>>>  1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>>>> index 3b48d893280f..6d04d45e8d45 100644
>>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>>> @@ -232,6 +232,9 @@ enum tpacpi_hkey_event_t {
>>>>  
>>>>  	/* Misc */
>>>>  	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
>>>> +
>>>> +	/* Misc2 */
>>>> +	TP_HKEY_EV_TRACK_DOUBLETAP      = 0x8036, /* trackpoint doubletap */
>>>>  };
>>>>  
>>>>  /****************************************************************************
>>>> @@ -1786,6 +1789,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>>>>  	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
>>>>  	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
>>>>  	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
>>>
>>> I understand why you've done this but I think this needs a comment,
>>> something like:
>>>
>>>         /*
>>>          * For TP_HKEY_EV_TRACK_DOUBLETAP, unlike the codes above which map to:
>>>          * (hkey_event - 0x1300) + TP_ACPI_HOTKEYSCAN_EXTENDED_START, this is
>>>          * hardcoded for TP_HKEY_EV_TRACK_DOUBLETAP handling. Therefor this must
>>>          * always be the last entry (after any 0x1300-0x13ff entries).
>>>          */
>>> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
>>
>> Ugh, actually this will not work becuuse we want hotkeyscancodes to be stable
>> because these are userspace API since they can be remapped using hwdb so we
>> cannot have the hotkeyscancode changing when new 0x1300-0x13ff range entries
>> get added.
>>
>> So we need to either grow the table a lot and reserve a whole bunch of space
>> for future 0x13xx - 0x13ff codes or maybe something like this:
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
>> b/drivers/platform/x86/thinkpad_acpi.c
>> index 771aaa7ae4cf..af3279889ecc 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -1742,7 +1742,12 @@ enum {	/* hot key scan codes (derived from ACPI 
>> DSDT) */
>>  	TP_ACPI_HOTKEYSCAN_VOLUMEDOWN,
>>  	TP_ACPI_HOTKEYSCAN_MUTE,
>>  	TP_ACPI_HOTKEYSCAN_THINKPAD,
>> -	TP_ACPI_HOTKEYSCAN_UNK1,
>> +	/*
>> +	 * Note this gets send both on 0x1019 and on 
>> TP_HKEY_EV_TRACK_DOUBLETAP
>> +	 * hotkey-events. 0x1019 events have never been seen on any actual hw
>> +	 * and a scancode is needed for the special 0x8036 doubletap 
>> hotkey-event.
>> +	 */
>> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
>>  	TP_ACPI_HOTKEYSCAN_UNK2,
>>  	TP_ACPI_HOTKEYSCAN_UNK3,
>>  	TP_ACPI_HOTKEYSCAN_UNK4,
>>
>> or just hardcode KEY_PROG1 like your previous patch does, but I'm not
>> a fan of that because of loosing hwdb remapping functionality for this
>> "key" then.
>>
>> Note I'm open to other suggestions.
>>
> Oh...I hadn't thought of that impact. That's not great :(
> 
> I have an idea, but want to prototype it to see if it works out or not. Will update once I've had a chance to play with it.

Thinking more about this I just realized that the input subsystem
already has a mechanism for dealing with scancode ranges with
(big) holes in them in the form of linux/input/sparse-keymap.h .

I think that what needs to be done is convert the existing code
to use sparse-keymap, keeping the mapping of the "MHKP"
returned hkey codes to internal TP_ACPI_HOTKEYSCAN_* values
for currently supported "MHKP" hkey codes for compatibility
and then for new codes just directly map them in the sparse map
aka the struct key_entry table. After converting the existing code
to use sparse-keymap, then for the new events we would simply add:


	{ KE_KEY, 0x131d, { KEY_VENDOR} }, /* Fn + N, system debug info */
	{ KE_KEY, 0x8036, { KEY_PROG1 } }, /* Trackpoint doubletap */

entries to the table without needing to define intermediate
TP_ACPI_HOTKEYSCAN_* values for these.

I already have somewhat of a design for this in my head and I really
believe this is the way forward as it uses existing kernel infra
and it will avoid hitting this problem again when some other new
"MHKP" hkey codes show up.

I plan to start working on implementing conversion of the existing
code to use sparse-keymap, which should result in a nice cleanup
after lunch and I hope to have something for you to test no later
then next Tuesday.

Regards,

Hans

Re: [ibm-acpi-devel] [PATCH v2 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Mark P. <mpe...@sq...>]

Hi Hans,

On Wed, Apr 17, 2024, at 4:06 PM, Hans de Goede wrote:
> Hi Mark,
>
> On 4/17/24 9:39 PM, Hans de Goede wrote:
>> Hi Mark,
>> 
>> Thank you for the new version of this series, overall this looks good,
>> one small remark below.
>> 
>> On 4/17/24 7:31 PM, Mark Pearson wrote:
>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>> This handles the doubletap event and sends the KEY_PROG1 event to
>>> userspace.
>>>
>>> Signed-off-by: Mark Pearson <mpe...@sq...>
>>> Signed-off-by: Vishnu Sankar <vis...@gm...>
>>> ---
>>> Changes in v2:
>>>  - Use KEY_PROG1 instead of KEY_DOUBLETAP as input maintainer doesn't
>>>    want new un-specific key codes added.
>>>  - Add doubletap to hotkey scan code table and use existing hotkey
>>>    functionality.
>>>  - Tested using evtest, and then gnome settings to configure a custom shortcut
>>>    to launch an application.
>>>
>>>  drivers/platform/x86/thinkpad_acpi.c | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>>> index 3b48d893280f..6d04d45e8d45 100644
>>> --- a/drivers/platform/x86/thinkpad_acpi.c
>>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>>> @@ -232,6 +232,9 @@ enum tpacpi_hkey_event_t {
>>>  
>>>  	/* Misc */
>>>  	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
>>> +
>>> +	/* Misc2 */
>>> +	TP_HKEY_EV_TRACK_DOUBLETAP      = 0x8036, /* trackpoint doubletap */
>>>  };
>>>  
>>>  /****************************************************************************
>>> @@ -1786,6 +1789,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>>>  	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
>>>  	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
>>>  	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
>> 
>> I understand why you've done this but I think this needs a comment,
>> something like:
>> 
>>         /*
>>          * For TP_HKEY_EV_TRACK_DOUBLETAP, unlike the codes above which map to:
>>          * (hkey_event - 0x1300) + TP_ACPI_HOTKEYSCAN_EXTENDED_START, this is
>>          * hardcoded for TP_HKEY_EV_TRACK_DOUBLETAP handling. Therefor this must
>>          * always be the last entry (after any 0x1300-0x13ff entries).
>>          */
>> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
>
> Ugh, actually this will not work becuuse we want hotkeyscancodes to be stable
> because these are userspace API since they can be remapped using hwdb so we
> cannot have the hotkeyscancode changing when new 0x1300-0x13ff range entries
> get added.
>
> So we need to either grow the table a lot and reserve a whole bunch of space
> for future 0x13xx - 0x13ff codes or maybe something like this:
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 771aaa7ae4cf..af3279889ecc 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -1742,7 +1742,12 @@ enum {	/* hot key scan codes (derived from ACPI 
> DSDT) */
>  	TP_ACPI_HOTKEYSCAN_VOLUMEDOWN,
>  	TP_ACPI_HOTKEYSCAN_MUTE,
>  	TP_ACPI_HOTKEYSCAN_THINKPAD,
> -	TP_ACPI_HOTKEYSCAN_UNK1,
> +	/*
> +	 * Note this gets send both on 0x1019 and on 
> TP_HKEY_EV_TRACK_DOUBLETAP
> +	 * hotkey-events. 0x1019 events have never been seen on any actual hw
> +	 * and a scancode is needed for the special 0x8036 doubletap 
> hotkey-event.
> +	 */
> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
>  	TP_ACPI_HOTKEYSCAN_UNK2,
>  	TP_ACPI_HOTKEYSCAN_UNK3,
>  	TP_ACPI_HOTKEYSCAN_UNK4,
>
> or just hardcode KEY_PROG1 like your previous patch does, but I'm not
> a fan of that because of loosing hwdb remapping functionality for this
> "key" then.
>
> Note I'm open to other suggestions.
>
Oh...I hadn't thought of that impact. That's not great :(

I have an idea, but want to prototype it to see if it works out or not. Will update once I've had a chance to play with it.

Mark

Re: [ibm-acpi-devel] [PATCH v2 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Hans de G. <hde...@re...>]

Hi Mark,

On 4/17/24 9:39 PM, Hans de Goede wrote:
> Hi Mark,
> 
> Thank you for the new version of this series, overall this looks good,
> one small remark below.
> 
> On 4/17/24 7:31 PM, Mark Pearson wrote:
>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>> This handles the doubletap event and sends the KEY_PROG1 event to
>> userspace.
>>
>> Signed-off-by: Mark Pearson <mpe...@sq...>
>> Signed-off-by: Vishnu Sankar <vis...@gm...>
>> ---
>> Changes in v2:
>>  - Use KEY_PROG1 instead of KEY_DOUBLETAP as input maintainer doesn't
>>    want new un-specific key codes added.
>>  - Add doubletap to hotkey scan code table and use existing hotkey
>>    functionality.
>>  - Tested using evtest, and then gnome settings to configure a custom shortcut
>>    to launch an application.
>>
>>  drivers/platform/x86/thinkpad_acpi.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index 3b48d893280f..6d04d45e8d45 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -232,6 +232,9 @@ enum tpacpi_hkey_event_t {
>>  
>>  	/* Misc */
>>  	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
>> +
>> +	/* Misc2 */
>> +	TP_HKEY_EV_TRACK_DOUBLETAP      = 0x8036, /* trackpoint doubletap */
>>  };
>>  
>>  /****************************************************************************
>> @@ -1786,6 +1789,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>>  	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
>>  	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
>>  	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
> 
> I understand why you've done this but I think this needs a comment,
> something like:
> 
>         /*
>          * For TP_HKEY_EV_TRACK_DOUBLETAP, unlike the codes above which map to:
>          * (hkey_event - 0x1300) + TP_ACPI_HOTKEYSCAN_EXTENDED_START, this is
>          * hardcoded for TP_HKEY_EV_TRACK_DOUBLETAP handling. Therefor this must
>          * always be the last entry (after any 0x1300-0x13ff entries).
>          */
> +	TP_ACPI_HOTKEYSCAN_DOUBLETAP,

Ugh, actually this will not work becuuse we want hotkeyscancodes to be stable
because these are userspace API since they can be remapped using hwdb so we
cannot have the hotkeyscancode changing when new 0x1300-0x13ff range entries
get added.

So we need to either grow the table a lot and reserve a whole bunch of space
for future 0x13xx - 0x13ff codes or maybe something like this:

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 771aaa7ae4cf..af3279889ecc 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -1742,7 +1742,12 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
 	TP_ACPI_HOTKEYSCAN_VOLUMEDOWN,
 	TP_ACPI_HOTKEYSCAN_MUTE,
 	TP_ACPI_HOTKEYSCAN_THINKPAD,
-	TP_ACPI_HOTKEYSCAN_UNK1,
+	/*
+	 * Note this gets send both on 0x1019 and on TP_HKEY_EV_TRACK_DOUBLETAP
+	 * hotkey-events. 0x1019 events have never been seen on any actual hw
+	 * and a scancode is needed for the special 0x8036 doubletap hotkey-event.
+	 */
+	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
 	TP_ACPI_HOTKEYSCAN_UNK2,
 	TP_ACPI_HOTKEYSCAN_UNK3,
 	TP_ACPI_HOTKEYSCAN_UNK4,

or just hardcode KEY_PROG1 like your previous patch does, but I'm not
a fan of that because of loosing hwdb remapping functionality for this
"key" then.

Note I'm open to other suggestions.

Regards,

Hans

Re: [ibm-acpi-devel] [PATCH v2 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Hans de G. <hde...@re...>]

Hi Mark,

Thank you for the new version of this series, overall this looks good,
one small remark below.

On 4/17/24 7:31 PM, Mark Pearson wrote:
> Lenovo trackpoints are adding the ability to generate a doubletap event.
> This handles the doubletap event and sends the KEY_PROG1 event to
> userspace.
> 
> Signed-off-by: Mark Pearson <mpe...@sq...>
> Signed-off-by: Vishnu Sankar <vis...@gm...>
> ---
> Changes in v2:
>  - Use KEY_PROG1 instead of KEY_DOUBLETAP as input maintainer doesn't
>    want new un-specific key codes added.
>  - Add doubletap to hotkey scan code table and use existing hotkey
>    functionality.
>  - Tested using evtest, and then gnome settings to configure a custom shortcut
>    to launch an application.
> 
>  drivers/platform/x86/thinkpad_acpi.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 3b48d893280f..6d04d45e8d45 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -232,6 +232,9 @@ enum tpacpi_hkey_event_t {
>  
>  	/* Misc */
>  	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
> +
> +	/* Misc2 */
> +	TP_HKEY_EV_TRACK_DOUBLETAP      = 0x8036, /* trackpoint doubletap */
>  };
>  
>  /****************************************************************************
> @@ -1786,6 +1789,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>  	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
>  	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
>  	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,

I understand why you've done this but I think this needs a comment,
something like:

        /*
         * For TP_HKEY_EV_TRACK_DOUBLETAP, unlike the codes above which map to:
         * (hkey_event - 0x1300) + TP_ACPI_HOTKEYSCAN_EXTENDED_START, this is
         * hardcoded for TP_HKEY_EV_TRACK_DOUBLETAP handling. Therefor this must
         * always be the last entry (after any 0x1300-0x13ff entries).
         */
+	TP_ACPI_HOTKEYSCAN_DOUBLETAP,

I see you got adding the new 0x13xx related hkeyscancodes right in the next
patch in this series but I think such a comment as above will be helpful
for future patches.

If you agree with adding this comment I can add this while merging, no need
to send a new version just for this.

Regards,

Hans





>  	/* Hotkey keymap size */
>  	TPACPI_HOTKEY_MAP_LEN
> @@ -3336,6 +3340,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
>  		KEY_NOTIFICATION_CENTER,	/* Notification Center */
>  		KEY_PICKUP_PHONE,		/* Answer incoming call */
>  		KEY_HANGUP_PHONE,		/* Decline incoming call */
> +		KEY_PROG1,                      /* Trackpoint doubletap */
>  		},
>  	};
>  
> @@ -3996,6 +4001,15 @@ static bool hotkey_notify_6xxx(const u32 hkey,
>  	return true;
>  }
>  
> +static bool hotkey_notify_8xxx(const u32 hkey)
> +{
> +	if (hkey == TP_HKEY_EV_TRACK_DOUBLETAP) {
> +		tpacpi_input_send_key(TP_ACPI_HOTKEYSCAN_DOUBLETAP);
> +		return true;
> +	}
> +	return false;
> +}
> +
>  static void hotkey_notify(struct ibm_struct *ibm, u32 event)
>  {
>  	u32 hkey;
> @@ -4079,6 +4093,10 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
>  				known_ev = true;
>  			}
>  			break;
> +		case 8:
> +			/* 0x8000-0x8FFF: misc2 */
> +			known_ev = hotkey_notify_8xxx(hkey);
> +			break;
>  		}
>  		if (!known_ev) {
>  			pr_notice("unhandled HKEY event 0x%04x\n", hkey);

[ibm-acpi-devel] [PATCH v2 4/4] platform/x86: thinkpad_acpi: Support hotkey to disable trackpoint doubletap

[Mark P. <mpe...@sq...>]

The hotkey combination FN+G can be used to disable the trackpoint
doubletap feature on Windows.
Add matching functionality for Linux.

Signed-off-by: Mark Pearson <mpe...@sq...>
Signed-off-by: Vishnu Sankar <vis...@gm...>
---
Changes in v2: 
 - Improved comments on keycodes in init function

 drivers/platform/x86/thinkpad_acpi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index a2f21e958d39..370b9285156c 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -167,6 +167,7 @@ enum tpacpi_hkey_event_t {
 	TP_HKEY_EV_VOL_MUTE		= 0x1017, /* Mixer output mute */
 	TP_HKEY_EV_PRIVACYGUARD_TOGGLE	= 0x130f, /* Toggle priv.guard on/off */
 	TP_HKEY_EV_AMT_TOGGLE		= 0x131a, /* Toggle AMT on/off */
+	TP_HKEY_EV_DOUBLETAP_TOGGLE	= 0x131c, /* Toggle trackpoint doubletap on/off */
 	TP_HKEY_EV_PROFILE_TOGGLE	= 0x131f, /* Toggle platform profile */
 
 	/* Reasons for waking up from S3/S4 */
@@ -356,6 +357,7 @@ static struct {
 	u32 hotkey_poll_active:1;
 	u32 has_adaptive_kbd:1;
 	u32 kbd_lang:1;
+	u32 trackpoint_doubletap:1;
 	struct quirk_entry *quirks;
 } tp_features;
 
@@ -3346,7 +3348,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 		KEY_HANGUP_PHONE,		/* Decline incoming call */
 		KEY_UNKNOWN,			/* AMT_TOGGLE handled in driver, 0x31a */
 		KEY_UNKNOWN,			/* Camera Shutter Switch, 0X31b */
-		KEY_UNKNOWN,			/* DOUBLETAP_TOGGLE, 0x31c */
+		KEY_UNKNOWN,			/* DOUBLETAP_TOGGLE handled in driver, 0x31c */
 		KEY_VENDOR,                     /* System debug info, 0x31D */
 		KEY_PROG1,                      /* Trackpoint doubletap */
 		},
@@ -3606,6 +3608,9 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 
 	hotkey_poll_setup_safe(true);
 
+	/* Enable doubletap by default */
+	tp_features.trackpoint_doubletap = 1;
+
 	return 0;
 }
 
@@ -3747,6 +3752,7 @@ static bool hotkey_notify_extended_hotkey(const u32 hkey)
 	case TP_HKEY_EV_PRIVACYGUARD_TOGGLE:
 	case TP_HKEY_EV_AMT_TOGGLE:
 	case TP_HKEY_EV_PROFILE_TOGGLE:
+	case TP_HKEY_EV_DOUBLETAP_TOGGLE:
 		tpacpi_driver_event(hkey);
 		return true;
 	}
@@ -4011,7 +4017,7 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 
 static bool hotkey_notify_8xxx(const u32 hkey)
 {
-	if (hkey == TP_HKEY_EV_TRACK_DOUBLETAP) {
+	if (hkey == TP_HKEY_EV_TRACK_DOUBLETAP && tp_features.trackpoint_doubletap) {
 		tpacpi_input_send_key(TP_ACPI_HOTKEYSCAN_DOUBLETAP);
 		return true;
 	}
@@ -11229,6 +11235,8 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
 		/* Notify user space the profile changed */
 		platform_profile_notify();
 	}
+	if (hkey_event == TP_HKEY_EV_DOUBLETAP_TOGGLE)
+		tp_features.trackpoint_doubletap = !tp_features.trackpoint_doubletap;
 }
 
 static void hotkey_driver_event(const unsigned int scancode)
-- 
2.44.0

[ibm-acpi-devel] [PATCH v2 3/4] platform/x86: thinkpad_acpi: Support for system debug info hotkey

[Mark P. <mpe...@sq...>]

New Lenovo platforms are adding the FN+N key to generate system debug
details that support can use for collecting important details on any
customer cases for Windows.
Add the infrastructure so we can do the same on Linux by generating a
SYS_VENDOR keycode to userspace.

Signed-off-by: Mark Pearson <mpe...@sq...>
Signed-off-by: Nitin Joshi <nj...@le...>
---
Changes in v2: 
 - Improved comments on keycodes in init function.
 - Filled in missing gaps

 drivers/platform/x86/thinkpad_acpi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 6d04d45e8d45..a2f21e958d39 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -1789,6 +1789,10 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
 	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
 	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
 	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
+	TP_ACPI_HOTKEYSCAN_AMT_TOGGLE,
+	TP_ACPI_HOTKEYSCAN_CAMERA_SHUTTER,
+	TP_ACPI_HOTKEYSCAN_DOUBLETAP_TOGGLE,
+	TP_ACPI_HOTKEYSCAN_SYS_DEBUG_INFO,
 	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
 
 	/* Hotkey keymap size */
@@ -3340,6 +3344,10 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 		KEY_NOTIFICATION_CENTER,	/* Notification Center */
 		KEY_PICKUP_PHONE,		/* Answer incoming call */
 		KEY_HANGUP_PHONE,		/* Decline incoming call */
+		KEY_UNKNOWN,			/* AMT_TOGGLE handled in driver, 0x31a */
+		KEY_UNKNOWN,			/* Camera Shutter Switch, 0X31b */
+		KEY_UNKNOWN,			/* DOUBLETAP_TOGGLE, 0x31c */
+		KEY_VENDOR,                     /* System debug info, 0x31D */
 		KEY_PROG1,                      /* Trackpoint doubletap */
 		},
 	};
-- 
2.44.0

[ibm-acpi-devel] [PATCH v2 1/4] platform/x86: thinkpad_acpi: simplify known_ev handling

[Mark P. <mpe...@sq...>]

Modify how known_ev event is handled in preparation for adding new keycode
range.

Signed-off-by: Mark Pearson <mpe...@sq...>
---
Changes in v2: 
 - New addition to series based on recommendations from review.
 - Note previous input patch was dropped so in numbering gets replaced by this.

 drivers/platform/x86/thinkpad_acpi.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 82429e59999d..3b48d893280f 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4026,6 +4026,7 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 
 		send_acpi_ev = true;
 		ignore_acpi_ev = false;
+		known_ev = false;
 
 		switch (hkey >> 12) {
 		case 1:
@@ -4051,8 +4052,6 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 				/* FIXME: kick libata if SATA link offline */
 				known_ev = true;
 				break;
-			default:
-				known_ev = false;
 			}
 			break;
 		case 4:
@@ -4078,11 +4077,8 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 				tpacpi_send_radiosw_update();
 				send_acpi_ev = 0;
 				known_ev = true;
-				break;
 			}
-			fallthrough;	/* to default */
-		default:
-			known_ev = false;
+			break;
 		}
 		if (!known_ev) {
 			pr_notice("unhandled HKEY event 0x%04x\n", hkey);
-- 
2.44.0

[ibm-acpi-devel] [PATCH v2 2/4] platform/x86: thinkpad_acpi: Support for trackpoint doubletap

[Mark P. <mpe...@sq...>]

Lenovo trackpoints are adding the ability to generate a doubletap event.
This handles the doubletap event and sends the KEY_PROG1 event to
userspace.

Signed-off-by: Mark Pearson <mpe...@sq...>
Signed-off-by: Vishnu Sankar <vis...@gm...>
---
Changes in v2:
 - Use KEY_PROG1 instead of KEY_DOUBLETAP as input maintainer doesn't
   want new un-specific key codes added.
 - Add doubletap to hotkey scan code table and use existing hotkey
   functionality.
 - Tested using evtest, and then gnome settings to configure a custom shortcut
   to launch an application.

 drivers/platform/x86/thinkpad_acpi.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 3b48d893280f..6d04d45e8d45 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -232,6 +232,9 @@ enum tpacpi_hkey_event_t {
 
 	/* Misc */
 	TP_HKEY_EV_RFKILL_CHANGED	= 0x7000, /* rfkill switch changed */
+
+	/* Misc2 */
+	TP_HKEY_EV_TRACK_DOUBLETAP      = 0x8036, /* trackpoint doubletap */
 };
 
 /****************************************************************************
@@ -1786,6 +1789,7 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
 	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
 	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
 	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
+	TP_ACPI_HOTKEYSCAN_DOUBLETAP,
 
 	/* Hotkey keymap size */
 	TPACPI_HOTKEY_MAP_LEN
@@ -3336,6 +3340,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 		KEY_NOTIFICATION_CENTER,	/* Notification Center */
 		KEY_PICKUP_PHONE,		/* Answer incoming call */
 		KEY_HANGUP_PHONE,		/* Decline incoming call */
+		KEY_PROG1,                      /* Trackpoint doubletap */
 		},
 	};
 
@@ -3996,6 +4001,15 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 	return true;
 }
 
+static bool hotkey_notify_8xxx(const u32 hkey)
+{
+	if (hkey == TP_HKEY_EV_TRACK_DOUBLETAP) {
+		tpacpi_input_send_key(TP_ACPI_HOTKEYSCAN_DOUBLETAP);
+		return true;
+	}
+	return false;
+}
+
 static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 {
 	u32 hkey;
@@ -4079,6 +4093,10 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 				known_ev = true;
 			}
 			break;
+		case 8:
+			/* 0x8000-0x8FFF: misc2 */
+			known_ev = hotkey_notify_8xxx(hkey);
+			break;
 		}
 		if (!known_ev) {
 			pr_notice("unhandled HKEY event 0x%04x\n", hkey);
-- 
2.44.0

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de G. <hde...@re...>]

Hi,

On 4/16/24 2:48 PM, Mark Pearson wrote:
> Hi Hans
> 
> On Tue, Apr 16, 2024, at 4:33 AM, Hans de Goede wrote:
>> Hi Mark,
>>
>> On 4/16/24 1:57 AM, Mark Pearson wrote:
>>> Hi Dmitry,
>>>
>>> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>>>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>>>> Hi
>>>>>
>>>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>>>
>>>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>>>
>>>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>>>> is now).
>>>>>>>>
>>>>>>>> For standalone application, recalling your statement that on Win you
>>>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>>>> KEY_CONFIG for it.
>>>>>>>
>>>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>>>
>>>>>> Not KEY_CONTROLPANEL?
>>>>>>
>>>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>>>> acceptable behavior for the users to have them behave the same?
>>>>>>
>>>>> Catching up with the thread, thanks for all the comments.
>>>>>
>>>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>>>> specific functionality, and that was the closest fit, if a new keycode
>>>>> was not preferred.
>>>>
>>>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>>>
>>>>>
>>>>> For the doubletap (which is a unique input event - not related to the
>>>>> pointer) I would like to keep it as a new unique event. 
>>>>>
>>>>> I think it's most likely use would be for control panel, but I don't
>>>>> think it should be limited to that. I can see it being useful if users
>>>>> are able to reconfigure it to launch something different (browser or
>>>>> music player maybe?), hence it would be best if it did not conflict
>>>>> with an existing keycode function. I also can't confirm it doesn't
>>>>> clash on existing or future systems - it's possible.
>>>>
>>>> So here is the problem. Keycodes in linux input are not mere
>>>> placeholders for something that will be decided later how it is to be
>>>> used, they are supposed to communicate intent and userspace ideally does
>>>> not need to have any additional knowledge about where the event is
>>>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>>>> and the system should lock the screen. It should not be aware that one
>>>> device was a generic USB external keyboard while another had an internal
>>>> sensor that recognized hovering palm making swiping motion to the right
>>>> because a vendor decided to make it. Otherwise you have millions of
>>>> input devices all generating unique codes and you need userspace to
>>>> decide how to interpret data coming from each device individually.
>>>>
>>>> If you truly do not have a defined use case for it you have a couple
>>>> options:
>>>>
>>>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>>>>   arbitrary keycode, let user or distro assign desired keycode to it
>>>>
>>>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>>>>   hand of the user to define a shortcut in their DE to make it useful
>>>>
>>>>>
>>>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>>>> joystick might be useful to have, if the HW supports it?
>>>>
>>>> What would it do exactly? Once we have this answer we can define key or
>>>> button code (although I do agree that game controller buttons are
>>>> different from "normal" keys because they map to the geometry of the
>>>> controller which in turn defines their commonly understood function).
>>>>
>>>> But in any case you would not reuse the same keycode for something that
>>>> is supposed to invoke a configuration utility and also to let's say
>>>> drop a flash grenade in a game.
>>>>
>>>
>>> Understood.
>>>
>>> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
>>>
>>> I will drop this patch from the series and proceed using existing defined codes only.
>>>
>>> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.
>>
>> Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
>> me and for the doubletap any one of 
>> KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
>> or some other suitable KEY_foo define works for me.
>>
> I think this should be a configurable input, by design. So my preference (if not allowed a new keycode, which I personally think is the better option) is for PROG1.
> 
> I discussed with Peter last night and it looks likely OK on their side. I do plan on doing some testing first, so it might take a few days to get the next set of patches out.

Ok, PROG1 works for me.

Regards,

Hans

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark P. <mpe...@sq...>]

Hi Hans

On Tue, Apr 16, 2024, at 4:33 AM, Hans de Goede wrote:
> Hi Mark,
>
> On 4/16/24 1:57 AM, Mark Pearson wrote:
>> Hi Dmitry,
>> 
>> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>>> Hi
>>>>
>>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>>
>>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>>
>>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>>> is now).
>>>>>>>
>>>>>>> For standalone application, recalling your statement that on Win you
>>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>>> KEY_CONFIG for it.
>>>>>>
>>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>>
>>>>> Not KEY_CONTROLPANEL?
>>>>>
>>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>>> acceptable behavior for the users to have them behave the same?
>>>>>
>>>> Catching up with the thread, thanks for all the comments.
>>>>
>>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>>> specific functionality, and that was the closest fit, if a new keycode
>>>> was not preferred.
>>>
>>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>>
>>>>
>>>> For the doubletap (which is a unique input event - not related to the
>>>> pointer) I would like to keep it as a new unique event. 
>>>>
>>>> I think it's most likely use would be for control panel, but I don't
>>>> think it should be limited to that. I can see it being useful if users
>>>> are able to reconfigure it to launch something different (browser or
>>>> music player maybe?), hence it would be best if it did not conflict
>>>> with an existing keycode function. I also can't confirm it doesn't
>>>> clash on existing or future systems - it's possible.
>>>
>>> So here is the problem. Keycodes in linux input are not mere
>>> placeholders for something that will be decided later how it is to be
>>> used, they are supposed to communicate intent and userspace ideally does
>>> not need to have any additional knowledge about where the event is
>>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>>> and the system should lock the screen. It should not be aware that one
>>> device was a generic USB external keyboard while another had an internal
>>> sensor that recognized hovering palm making swiping motion to the right
>>> because a vendor decided to make it. Otherwise you have millions of
>>> input devices all generating unique codes and you need userspace to
>>> decide how to interpret data coming from each device individually.
>>>
>>> If you truly do not have a defined use case for it you have a couple
>>> options:
>>>
>>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>>>   arbitrary keycode, let user or distro assign desired keycode to it
>>>
>>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>>>   hand of the user to define a shortcut in their DE to make it useful
>>>
>>>>
>>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>>> joystick might be useful to have, if the HW supports it?
>>>
>>> What would it do exactly? Once we have this answer we can define key or
>>> button code (although I do agree that game controller buttons are
>>> different from "normal" keys because they map to the geometry of the
>>> controller which in turn defines their commonly understood function).
>>>
>>> But in any case you would not reuse the same keycode for something that
>>> is supposed to invoke a configuration utility and also to let's say
>>> drop a flash grenade in a game.
>>>
>> 
>> Understood.
>> 
>> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
>> 
>> I will drop this patch from the series and proceed using existing defined codes only.
>> 
>> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.
>
> Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
> me and for the doubletap any one of 
> KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
> or some other suitable KEY_foo define works for me.
>
I think this should be a configurable input, by design. So my preference (if not allowed a new keycode, which I personally think is the better option) is for PROG1.

I discussed with Peter last night and it looks likely OK on their side. I do plan on doing some testing first, so it might take a few days to get the next set of patches out.

Mark

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de G. <hde...@re...>]

Hi,

On 4/15/24 9:58 PM, Dmitry Torokhov wrote:
> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>
>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>
>>> Please see my response to Peter's letter. I think it very much depends
>>> on how it will be used (associated with the pointer or standalone as it
>>> is now).
>>>
>>> For standalone application, recalling your statement that on Win you
>>> have this gesture invoke configuration utility I would argue for
>>> KEY_CONFIG for it.
>>
>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>> the GNOME/KDE control center/panel and I believe that at least GNOME
>> comes with a default binding to map KEY_CONFIG to the control-center.
> 
> Not KEY_CONTROLPANEL?

No when this was added most distros were still using X11 not Wayland so
it was preferable to use KEY_foo symbols with codes below 240.

> Are there devices that use both Fn+# and the doubleclick? Would it be an
> acceptable behavior for the users to have them behave the same?

That is a good question, at least my current ThinkPad does not have
the Fn + F## combo for the control center anymore. So I guess we could
indeed re-use KEY_CONFIG for the double-tap.

Regards,

Hans

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de G. <hde...@re...>]

Hi Mark,

On 4/16/24 1:57 AM, Mark Pearson wrote:
> Hi Dmitry,
> 
> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>> Hi
>>>
>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>
>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>
>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>> is now).
>>>>>>
>>>>>> For standalone application, recalling your statement that on Win you
>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>> KEY_CONFIG for it.
>>>>>
>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>
>>>> Not KEY_CONTROLPANEL?
>>>>
>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>> acceptable behavior for the users to have them behave the same?
>>>>
>>> Catching up with the thread, thanks for all the comments.
>>>
>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>> specific functionality, and that was the closest fit, if a new keycode
>>> was not preferred.
>>
>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>
>>>
>>> For the doubletap (which is a unique input event - not related to the
>>> pointer) I would like to keep it as a new unique event. 
>>>
>>> I think it's most likely use would be for control panel, but I don't
>>> think it should be limited to that. I can see it being useful if users
>>> are able to reconfigure it to launch something different (browser or
>>> music player maybe?), hence it would be best if it did not conflict
>>> with an existing keycode function. I also can't confirm it doesn't
>>> clash on existing or future systems - it's possible.
>>
>> So here is the problem. Keycodes in linux input are not mere
>> placeholders for something that will be decided later how it is to be
>> used, they are supposed to communicate intent and userspace ideally does
>> not need to have any additional knowledge about where the event is
>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>> and the system should lock the screen. It should not be aware that one
>> device was a generic USB external keyboard while another had an internal
>> sensor that recognized hovering palm making swiping motion to the right
>> because a vendor decided to make it. Otherwise you have millions of
>> input devices all generating unique codes and you need userspace to
>> decide how to interpret data coming from each device individually.
>>
>> If you truly do not have a defined use case for it you have a couple
>> options:
>>
>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>>   arbitrary keycode, let user or distro assign desired keycode to it
>>
>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>>   hand of the user to define a shortcut in their DE to make it useful
>>
>>>
>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>> joystick might be useful to have, if the HW supports it?
>>
>> What would it do exactly? Once we have this answer we can define key or
>> button code (although I do agree that game controller buttons are
>> different from "normal" keys because they map to the geometry of the
>> controller which in turn defines their commonly understood function).
>>
>> But in any case you would not reuse the same keycode for something that
>> is supposed to invoke a configuration utility and also to let's say
>> drop a flash grenade in a game.
>>
> 
> Understood.
> 
> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
> 
> I will drop this patch from the series and proceed using existing defined codes only.
> 
> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.

Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
me and for the doubletap any one of KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
or some other suitable KEY_foo define works for me.

Regards,

Hans

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark P. <mpe...@sq...>]

Hi Dmitry,

On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>> Hi
>> 
>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>> > On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>> >> Hi,
>> >> 
>> >> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>> >> > On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>> >> >>
>> >> >> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>> >> > 
>> >> > Please see my response to Peter's letter. I think it very much depends
>> >> > on how it will be used (associated with the pointer or standalone as it
>> >> > is now).
>> >> > 
>> >> > For standalone application, recalling your statement that on Win you
>> >> > have this gesture invoke configuration utility I would argue for
>> >> > KEY_CONFIG for it.
>> >> 
>> >> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>> >> the GNOME/KDE control center/panel and I believe that at least GNOME
>> >> comes with a default binding to map KEY_CONFIG to the control-center.
>> >
>> > Not KEY_CONTROLPANEL?
>> >
>> > Are there devices that use both Fn+# and the doubleclick? Would it be an
>> > acceptable behavior for the users to have them behave the same?
>> >
>> Catching up with the thread, thanks for all the comments.
>> 
>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>> specific functionality, and that was the closest fit, if a new keycode
>> was not preferred.
>
> Fn+N -> KEY_VENDOR mapping sounds good to me.
>
>> 
>> For the doubletap (which is a unique input event - not related to the
>> pointer) I would like to keep it as a new unique event. 
>> 
>> I think it's most likely use would be for control panel, but I don't
>> think it should be limited to that. I can see it being useful if users
>> are able to reconfigure it to launch something different (browser or
>> music player maybe?), hence it would be best if it did not conflict
>> with an existing keycode function. I also can't confirm it doesn't
>> clash on existing or future systems - it's possible.
>
> So here is the problem. Keycodes in linux input are not mere
> placeholders for something that will be decided later how it is to be
> used, they are supposed to communicate intent and userspace ideally does
> not need to have any additional knowledge about where the event is
> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
> and the system should lock the screen. It should not be aware that one
> device was a generic USB external keyboard while another had an internal
> sensor that recognized hovering palm making swiping motion to the right
> because a vendor decided to make it. Otherwise you have millions of
> input devices all generating unique codes and you need userspace to
> decide how to interpret data coming from each device individually.
>
> If you truly do not have a defined use case for it you have a couple
> options:
>
> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>   arbitrary keycode, let user or distro assign desired keycode to it
>
> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>   hand of the user to define a shortcut in their DE to make it useful
>
>> 
>> FWIW - I wouldn't be surprised with some of the new gaming systems
>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>> joystick might be useful to have, if the HW supports it?
>
> What would it do exactly? Once we have this answer we can define key or
> button code (although I do agree that game controller buttons are
> different from "normal" keys because they map to the geometry of the
> controller which in turn defines their commonly understood function).
>
> But in any case you would not reuse the same keycode for something that
> is supposed to invoke a configuration utility and also to let's say
> drop a flash grenade in a game.
>

Understood.

I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.

I will drop this patch from the series and proceed using existing defined codes only.

Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.

Mark

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry T. <dmi...@gm...>]

On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
> Hi
> 
> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
> > On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
> >> Hi,
> >> 
> >> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
> >> > On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
> >> >>
> >> >> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
> >> > 
> >> > Please see my response to Peter's letter. I think it very much depends
> >> > on how it will be used (associated with the pointer or standalone as it
> >> > is now).
> >> > 
> >> > For standalone application, recalling your statement that on Win you
> >> > have this gesture invoke configuration utility I would argue for
> >> > KEY_CONFIG for it.
> >> 
> >> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
> >> the GNOME/KDE control center/panel and I believe that at least GNOME
> >> comes with a default binding to map KEY_CONFIG to the control-center.
> >
> > Not KEY_CONTROLPANEL?
> >
> > Are there devices that use both Fn+# and the doubleclick? Would it be an
> > acceptable behavior for the users to have them behave the same?
> >
> Catching up with the thread, thanks for all the comments.
> 
> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
> KEY_VENDOR there. My conclusion was that this is targeting vendor
> specific functionality, and that was the closest fit, if a new keycode
> was not preferred.

Fn+N -> KEY_VENDOR mapping sounds good to me.

> 
> For the doubletap (which is a unique input event - not related to the
> pointer) I would like to keep it as a new unique event. 
> 
> I think it's most likely use would be for control panel, but I don't
> think it should be limited to that. I can see it being useful if users
> are able to reconfigure it to launch something different (browser or
> music player maybe?), hence it would be best if it did not conflict
> with an existing keycode function. I also can't confirm it doesn't
> clash on existing or future systems - it's possible.

So here is the problem. Keycodes in linux input are not mere
placeholders for something that will be decided later how it is to be
used, they are supposed to communicate intent and userspace ideally does
not need to have any additional knowledge about where the event is
coming from. A keyboard either internal or external sends KEY_SCREENLOCK
and the system should lock the screen. It should not be aware that one
device was a generic USB external keyboard while another had an internal
sensor that recognized hovering palm making swiping motion to the right
because a vendor decided to make it. Otherwise you have millions of
input devices all generating unique codes and you need userspace to
decide how to interpret data coming from each device individually.

If you truly do not have a defined use case for it you have a couple
options:

- assign it KEY_RESERVED, ensure your driver allows remapping to an
  arbitrary keycode, let user or distro assign desired keycode to it

- assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
  hand of the user to define a shortcut in their DE to make it useful

> 
> FWIW - I wouldn't be surprised with some of the new gaming systems
> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
> joystick might be useful to have, if the HW supports it?

What would it do exactly? Once we have this answer we can define key or
button code (although I do agree that game controller buttons are
different from "normal" keys because they map to the geometry of the
controller which in turn defines their commonly understood function).

But in any case you would not reuse the same keycode for something that
is supposed to invoke a configuration utility and also to let's say
drop a flash grenade in a game.

Thanks.

-- 
Dmitry

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Mark P. <mpe...@sq...>]

Hi

On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>> Hi,
>> 
>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>> > On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>> >>
>> >> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>> > 
>> > Please see my response to Peter's letter. I think it very much depends
>> > on how it will be used (associated with the pointer or standalone as it
>> > is now).
>> > 
>> > For standalone application, recalling your statement that on Win you
>> > have this gesture invoke configuration utility I would argue for
>> > KEY_CONFIG for it.
>> 
>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>> the GNOME/KDE control center/panel and I believe that at least GNOME
>> comes with a default binding to map KEY_CONFIG to the control-center.
>
> Not KEY_CONTROLPANEL?
>
> Are there devices that use both Fn+# and the doubleclick? Would it be an
> acceptable behavior for the users to have them behave the same?
>
Catching up with the thread, thanks for all the comments.

For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use KEY_VENDOR there. My conclusion was that this is targeting vendor specific functionality, and that was the closest fit, if a new keycode was not preferred.

For the doubletap (which is a unique input event - not related to the pointer) I would like to keep it as a new unique event. 

I think it's most likely use would be for control panel, but I don't think it should be limited to that. I can see it being useful if users are able to reconfigure it to launch something different (browser or music player maybe?), hence it would be best if it did not conflict with an existing keycode function. I also can't confirm it doesn't clash on existing or future systems - it's possible.

FWIW - I wouldn't be surprised with some of the new gaming systems we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a joystick might be useful to have, if the HW supports it?

Mark

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry T. <dmi...@gm...>]

On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
> > On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
> >>
> >> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
> > 
> > Please see my response to Peter's letter. I think it very much depends
> > on how it will be used (associated with the pointer or standalone as it
> > is now).
> > 
> > For standalone application, recalling your statement that on Win you
> > have this gesture invoke configuration utility I would argue for
> > KEY_CONFIG for it.
> 
> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
> the GNOME/KDE control center/panel and I believe that at least GNOME
> comes with a default binding to map KEY_CONFIG to the control-center.

Not KEY_CONTROLPANEL?

Are there devices that use both Fn+# and the doubleclick? Would it be an
acceptable behavior for the users to have them behave the same?

Thanks.

-- 
Dmitry

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry T. <dmi...@gm...>]

On Mon, Apr 15, 2024 at 09:47:10PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 4/15/24 9:35 PM, Dmitry Torokhov wrote:
> > On Thu, Apr 11, 2024 at 02:30:35PM +0200, Hans de Goede wrote:
> >> Hi Dmitry,
> >>
> >> On 4/11/24 2:02 AM, Dmitry Torokhov wrote:
> >>> On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
> >>>> Hi Dmitry
> >>>>
> >>>> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
> >>>>> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> >>>>>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> >>>>>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
> >>>>>>>> Hi Mark,
> >>>>>>>>
> >>>>>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> >>>>>>>>> Add support for new input events on Lenovo laptops that need exporting to
> >>>>>>>>> user space.
> >>>>>>>>>
> >>>>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
> >>>>>>>>> Add a new keycode to allow this to be used by userspace.
> >>>>>>>>
> >>>>>>>> What is the intended meaning of this keycode? How does it differ from
> >>>>>>>> the driver sending BTN_LEFT press/release twice?
> >>>>>>>>>
> >>>>>>>>> Lenovo support is using FN+N with Windows to collect needed details for
> >>>>>>>>> support cases. Add a keycode so that we'll be able to provide similar
> >>>>>>>>> support on Linux.
> >>>>>>>>
> >>>>>>>> Is there a userspace consumer for this?
> >>>>>>>
> >>>>>>> Funnily enough XKB has had a keysym for this for decades but it's not
> >>>>>>> hooked up anywhere due to the way it's pointer keys accessibility
> >>>>>>> feature was implemented. Theory is that most of userspace just needs
> >>>>>>> to patch the various pieces together for the new evdev code + keysym,
> >>>>>>> it's not really any different to handling a volume key (except this
> >>>>>>> one needs to be assignable).
> >>>>>>
> >>>>>> What is the keysym? If we can make them relatable to each other that
> >>>>>> would be good. Or maybe we could find a matching usage from HID usage
> >>>>>> tables...
> >>>>>
> >>>>> I was looking through the existing codes and I see:
> >>>>>
> >>>>> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
> >>>>>
> >>>>> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> >>>>> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> >>>>> specific debug info collection application (which I honestly doubt will
> >>>>> materialize).
> >>>>>
> >>>>
> >>>> That's a somewhat disappointing note on your doubts, is that based on
> >>>> anything? Just wondering what we've done to deserve that criticism.
> >>>
> >>> Sorry, this was not meant as a criticism really, but you mentioned
> >>> yourself that there isn't anything in the works yet, you just have some
> >>> plans.
> >>>
> >>> For such a project to succeed Lenovo needs to invest into selling
> >>> devices with Linux as a primary operating system, and it has to be
> >>> consumer segment (or small business, because for corporate they
> >>> typically roll their own support channels). The case of retrofitting
> >>> Linux onto a that device originally came with Windows OS rarely gets
> >>> much if any response from the normal support channels.
> >>>
> >>> Is this something that is actually happening?
> >>
> >> Yes, Lenovo is actually offering Fedora as an OS choice when
> >> ordering ThinkPads directly from their website in many countries
> >> including when ordering as a consumer.
> > 
> > Ah, very nice, I was not aware of this.
> > 
> >>
> >> And unlike other vendor's Linux preloads which often use a kernel
> >> with downstream laptop specific changes these laptops are running
> >> unmodified Fedora kernels, which themselves are almost pristine
> >> upstream kernels.
> >>
> >> Lenovo (Mark) has been really good the last couple of years in
> >> making sure that their hw just works with mainline kernels without
> >> any downstream vendor specific patches.
> >>
> >>>> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
> >>>> personally don't think KEY_CONFIG matches well), but I would be
> >>>> worried about clashing with existing functionality.
> >>
> >> Using KEY_INFO / KEY_VENDOR works for me too. So maybe we should
> >> just go with one of those 2 ?
> > 
> > It looks like Mark's preference is KEY_VENDOR, so let's go with it?
> 
> Ack KEY_VENDOR sounds good to me for the doubletap on the trackpoint event.
> 
> What about the new Fn + N keycombo which also generates a WMI
> event which we want to translate to a key code to launch a
> (to be written) debug-info collecting app for when the customer
> calls Lenovo support.
> 
> Mark suggested a new KEY_SYS_DEBUG_INFO for that. So do we use:
> 
> #define KEY_INFO                0x166   /* AL OEM Features/Tips/Tutorial */
> 
> for this, or do we define a new keycode ?
> 
> Mark would using KEY_INFO for this work for you.
> 
> Dmitry any opinion on this ?

No, my understanding is that Mark was OK with using KEY_VENDOR for Fn+N
combination that is supposed to start the utility that would collect
the debug info.

For double click there is still the discussion whether to have
KEY_DOUBLECLICK (which I think will need to be tied to the pointer
device somehow), or something else, like KEY_CONFIG or a new keycode if
we continue keeping it separate from the pointer operations and match
Windows behavior which invokes Lenovo configuration utility.

Thanks.

-- 
Dmitry

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de G. <hde...@re...>]

Hi,

On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>
>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
> 
> Please see my response to Peter's letter. I think it very much depends
> on how it will be used (associated with the pointer or standalone as it
> is now).
> 
> For standalone application, recalling your statement that on Win you
> have this gesture invoke configuration utility I would argue for
> KEY_CONFIG for it.

KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
the GNOME/KDE control center/panel and I believe that at least GNOME
comes with a default binding to map KEY_CONFIG to the control-center.

So IMHO re-using KEY_CONFIG for the doubletap trackpoint thing is not
a good idea. But as mentioned elsewhere in the thread everyone seems
to be ok with using KEY_VENDOR for this ?

Regards,

Hans

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de G. <hde...@re...>]

Hi,

On 4/15/24 9:35 PM, Dmitry Torokhov wrote:
> On Thu, Apr 11, 2024 at 02:30:35PM +0200, Hans de Goede wrote:
>> Hi Dmitry,
>>
>> On 4/11/24 2:02 AM, Dmitry Torokhov wrote:
>>> On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
>>>> Hi Dmitry
>>>>
>>>> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
>>>>> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
>>>>>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
>>>>>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
>>>>>>>> Hi Mark,
>>>>>>>>
>>>>>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>>>>>>>>> Add support for new input events on Lenovo laptops that need exporting to
>>>>>>>>> user space.
>>>>>>>>>
>>>>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>>>>>>>> Add a new keycode to allow this to be used by userspace.
>>>>>>>>
>>>>>>>> What is the intended meaning of this keycode? How does it differ from
>>>>>>>> the driver sending BTN_LEFT press/release twice?
>>>>>>>>>
>>>>>>>>> Lenovo support is using FN+N with Windows to collect needed details for
>>>>>>>>> support cases. Add a keycode so that we'll be able to provide similar
>>>>>>>>> support on Linux.
>>>>>>>>
>>>>>>>> Is there a userspace consumer for this?
>>>>>>>
>>>>>>> Funnily enough XKB has had a keysym for this for decades but it's not
>>>>>>> hooked up anywhere due to the way it's pointer keys accessibility
>>>>>>> feature was implemented. Theory is that most of userspace just needs
>>>>>>> to patch the various pieces together for the new evdev code + keysym,
>>>>>>> it's not really any different to handling a volume key (except this
>>>>>>> one needs to be assignable).
>>>>>>
>>>>>> What is the keysym? If we can make them relatable to each other that
>>>>>> would be good. Or maybe we could find a matching usage from HID usage
>>>>>> tables...
>>>>>
>>>>> I was looking through the existing codes and I see:
>>>>>
>>>>> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
>>>>>
>>>>> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
>>>>> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
>>>>> specific debug info collection application (which I honestly doubt will
>>>>> materialize).
>>>>>
>>>>
>>>> That's a somewhat disappointing note on your doubts, is that based on
>>>> anything? Just wondering what we've done to deserve that criticism.
>>>
>>> Sorry, this was not meant as a criticism really, but you mentioned
>>> yourself that there isn't anything in the works yet, you just have some
>>> plans.
>>>
>>> For such a project to succeed Lenovo needs to invest into selling
>>> devices with Linux as a primary operating system, and it has to be
>>> consumer segment (or small business, because for corporate they
>>> typically roll their own support channels). The case of retrofitting
>>> Linux onto a that device originally came with Windows OS rarely gets
>>> much if any response from the normal support channels.
>>>
>>> Is this something that is actually happening?
>>
>> Yes, Lenovo is actually offering Fedora as an OS choice when
>> ordering ThinkPads directly from their website in many countries
>> including when ordering as a consumer.
> 
> Ah, very nice, I was not aware of this.
> 
>>
>> And unlike other vendor's Linux preloads which often use a kernel
>> with downstream laptop specific changes these laptops are running
>> unmodified Fedora kernels, which themselves are almost pristine
>> upstream kernels.
>>
>> Lenovo (Mark) has been really good the last couple of years in
>> making sure that their hw just works with mainline kernels without
>> any downstream vendor specific patches.
>>
>>>> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
>>>> personally don't think KEY_CONFIG matches well), but I would be
>>>> worried about clashing with existing functionality.
>>
>> Using KEY_INFO / KEY_VENDOR works for me too. So maybe we should
>> just go with one of those 2 ?
> 
> It looks like Mark's preference is KEY_VENDOR, so let's go with it?

Ack KEY_VENDOR sounds good to me for the doubletap on the trackpoint event.

What about the new Fn + N keycombo which also generates a WMI
event which we want to translate to a key code to launch a
(to be written) debug-info collecting app for when the customer
calls Lenovo support.

Mark suggested a new KEY_SYS_DEBUG_INFO for that. So do we use:

#define KEY_INFO                0x166   /* AL OEM Features/Tips/Tutorial */

for this, or do we define a new keycode ?

Mark would using KEY_INFO for this work for you.

Dmitry any opinion on this ?

Regards,

Hans

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry T. <dmi...@gm...>]

On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
> 
> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.

Please see my response to Peter's letter. I think it very much depends
on how it will be used (associated with the pointer or standalone as it
is now).

For standalone application, recalling your statement that on Win you
have this gesture invoke configuration utility I would argue for
KEY_CONFIG for it.

Thanks.

-- 
Dmitry

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry T. <dmi...@gm...>]

On Thu, Apr 11, 2024 at 02:30:35PM +0200, Hans de Goede wrote:
> Hi Dmitry,
> 
> On 4/11/24 2:02 AM, Dmitry Torokhov wrote:
> > On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
> >> Hi Dmitry
> >>
> >> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
> >>> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> >>>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> >>>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
> >>>>>> Hi Mark,
> >>>>>>
> >>>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> >>>>>>> Add support for new input events on Lenovo laptops that need exporting to
> >>>>>>> user space.
> >>>>>>>
> >>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
> >>>>>>> Add a new keycode to allow this to be used by userspace.
> >>>>>>
> >>>>>> What is the intended meaning of this keycode? How does it differ from
> >>>>>> the driver sending BTN_LEFT press/release twice?
> >>>>>>>
> >>>>>>> Lenovo support is using FN+N with Windows to collect needed details for
> >>>>>>> support cases. Add a keycode so that we'll be able to provide similar
> >>>>>>> support on Linux.
> >>>>>>
> >>>>>> Is there a userspace consumer for this?
> >>>>>
> >>>>> Funnily enough XKB has had a keysym for this for decades but it's not
> >>>>> hooked up anywhere due to the way it's pointer keys accessibility
> >>>>> feature was implemented. Theory is that most of userspace just needs
> >>>>> to patch the various pieces together for the new evdev code + keysym,
> >>>>> it's not really any different to handling a volume key (except this
> >>>>> one needs to be assignable).
> >>>>
> >>>> What is the keysym? If we can make them relatable to each other that
> >>>> would be good. Or maybe we could find a matching usage from HID usage
> >>>> tables...
> >>>
> >>> I was looking through the existing codes and I see:
> >>>
> >>> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
> >>>
> >>> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> >>> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> >>> specific debug info collection application (which I honestly doubt will
> >>> materialize).
> >>>
> >>
> >> That's a somewhat disappointing note on your doubts, is that based on
> >> anything? Just wondering what we've done to deserve that criticism.
> > 
> > Sorry, this was not meant as a criticism really, but you mentioned
> > yourself that there isn't anything in the works yet, you just have some
> > plans.
> > 
> > For such a project to succeed Lenovo needs to invest into selling
> > devices with Linux as a primary operating system, and it has to be
> > consumer segment (or small business, because for corporate they
> > typically roll their own support channels). The case of retrofitting
> > Linux onto a that device originally came with Windows OS rarely gets
> > much if any response from the normal support channels.
> > 
> > Is this something that is actually happening?
> 
> Yes, Lenovo is actually offering Fedora as an OS choice when
> ordering ThinkPads directly from their website in many countries
> including when ordering as a consumer.

Ah, very nice, I was not aware of this.

> 
> And unlike other vendor's Linux preloads which often use a kernel
> with downstream laptop specific changes these laptops are running
> unmodified Fedora kernels, which themselves are almost pristine
> upstream kernels.
> 
> Lenovo (Mark) has been really good the last couple of years in
> making sure that their hw just works with mainline kernels without
> any downstream vendor specific patches.
> 
> >> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
> >> personally don't think KEY_CONFIG matches well), but I would be
> >> worried about clashing with existing functionality.
> 
> Using KEY_INFO / KEY_VENDOR works for me too. So maybe we should
> just go with one of those 2 ?

It looks like Mark's preference is KEY_VENDOR, so let's go with it?

Thanks.

-- 
Dmitry

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Dmitry T. <dmi...@gm...>]

On Wed, Apr 10, 2024 at 02:32:56PM +1000, Peter Hutterer wrote:
> On 10/04/2024 11:20, Dmitry Torokhov wrote:
> > On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
> > > On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
> > > > On 09/04/2024 09:31, Dmitry Torokhov wrote:
> > > > > Hi Mark,
> > > > > 
> > > > > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
> > > > > > Add support for new input events on Lenovo laptops that need exporting to
> > > > > > user space.
> > > > > > 
> > > > > > Lenovo trackpoints are adding the ability to generate a doubletap event.
> > > > > > Add a new keycode to allow this to be used by userspace.
> > > > > 
> > > > > What is the intended meaning of this keycode? How does it differ from
> > > > > the driver sending BTN_LEFT press/release twice?
> > > > > > 
> > > > > > Lenovo support is using FN+N with Windows to collect needed details for
> > > > > > support cases. Add a keycode so that we'll be able to provide similar
> > > > > > support on Linux.
> > > > > 
> > > > > Is there a userspace consumer for this?
> > > > 
> > > > Funnily enough XKB has had a keysym for this for decades but it's not
> > > > hooked up anywhere due to the way it's pointer keys accessibility
> > > > feature was implemented. Theory is that most of userspace just needs
> > > > to patch the various pieces together for the new evdev code + keysym,
> > > > it's not really any different to handling a volume key (except this
> > > > one needs to be assignable).
> > > 
> > > What is the keysym? If we can make them relatable to each other that
> > > would be good. Or maybe we could find a matching usage from HID usage
> > > tables...
> 
> There's a set of  XK_Pointer_ keysyms defined in X11/keysym.h,
> including XK_Pointer_DblClick1 and XK_Pointer_DblClickDefault.
> Unfortunately they're not hooked up to anything atm, see this draft
> MR:
> https://gitlab.freedesktop.org/xkeyboard-config/xkeyboard-config/-/merge_requests/659
> Because they're not hooked up anywhere they'll also need to be hooked
> into the client space, same as the various XF86FooBar symbols we've
> added over the years.
> 
> If we were to add KEY_DOUBLECLICK we can patch xkeyboard-config to
> bind that to the existing XK_Pointer_DblClickDefault symbol (it would
> get XF86DoubleClick assigned by the current automated scripts), so in
> theory that key would work like any other key with that symbol
> assigned.
> 
> > I was looking through the existing codes and I see:
> > 
> > #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
> > 
> > We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
> > thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
> > specific debug info collection application (which I honestly doubt will
> > materialize).
> 
> fwiw, I suggested KEY_DOUBLECLICK because that is the action the user
> takes. Whether that starts a particular application is mostly a
> question of configuration, defaulting to something that emulates a
> double-click seems prudent though. And if someone wants to remap that
> to the compose key that'd be trivial too then.

I think whether to create and use KEY_DOUBLECLICK is very much depends
if we associate this with the pointer somehow, or if we keep it as a
completely separate action.

If we continue with KEY_DOUBLECLICK then we need to try and define what
exactly it means to the applications. Actually same goes if we want
another new keycode.

As far as easy remapping, I think one can map this to KEY_RESERVED and
then remap to whatever they want, you do not need to have a new keycode
for that.

Thanks.

-- 
Dmitry

Re: [ibm-acpi-devel] [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

[Hans de G. <hde...@re...>]

Hi Dmitry,

On 4/11/24 2:02 AM, Dmitry Torokhov wrote:
> On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
>> Hi Dmitry
>>
>> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
>>> On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
>>>> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
>>>>> On 09/04/2024 09:31, Dmitry Torokhov wrote:
>>>>>> Hi Mark,
>>>>>>
>>>>>> On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>>>>>>> Add support for new input events on Lenovo laptops that need exporting to
>>>>>>> user space.
>>>>>>>
>>>>>>> Lenovo trackpoints are adding the ability to generate a doubletap event.
>>>>>>> Add a new keycode to allow this to be used by userspace.
>>>>>>
>>>>>> What is the intended meaning of this keycode? How does it differ from
>>>>>> the driver sending BTN_LEFT press/release twice?
>>>>>>>
>>>>>>> Lenovo support is using FN+N with Windows to collect needed details for
>>>>>>> support cases. Add a keycode so that we'll be able to provide similar
>>>>>>> support on Linux.
>>>>>>
>>>>>> Is there a userspace consumer for this?
>>>>>
>>>>> Funnily enough XKB has had a keysym for this for decades but it's not
>>>>> hooked up anywhere due to the way it's pointer keys accessibility
>>>>> feature was implemented. Theory is that most of userspace just needs
>>>>> to patch the various pieces together for the new evdev code + keysym,
>>>>> it's not really any different to handling a volume key (except this
>>>>> one needs to be assignable).
>>>>
>>>> What is the keysym? If we can make them relatable to each other that
>>>> would be good. Or maybe we could find a matching usage from HID usage
>>>> tables...
>>>
>>> I was looking through the existing codes and I see:
>>>
>>> #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
>>>
>>> We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
>>> thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
>>> specific debug info collection application (which I honestly doubt will
>>> materialize).
>>>
>>
>> That's a somewhat disappointing note on your doubts, is that based on
>> anything? Just wondering what we've done to deserve that criticism.
> 
> Sorry, this was not meant as a criticism really, but you mentioned
> yourself that there isn't anything in the works yet, you just have some
> plans.
> 
> For such a project to succeed Lenovo needs to invest into selling
> devices with Linux as a primary operating system, and it has to be
> consumer segment (or small business, because for corporate they
> typically roll their own support channels). The case of retrofitting
> Linux onto a that device originally came with Windows OS rarely gets
> much if any response from the normal support channels.
> 
> Is this something that is actually happening?

Yes, Lenovo is actually offering Fedora as an OS choice when
ordering ThinkPads directly from their website in many countries
including when ordering as a consumer.

And unlike other vendor's Linux preloads which often use a kernel
with downstream laptop specific changes these laptops are running
unmodified Fedora kernels, which themselves are almost pristine
upstream kernels.

Lenovo (Mark) has been really good the last couple of years in
making sure that their hw just works with mainline kernels without
any downstream vendor specific patches.

>> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
>> personally don't think KEY_CONFIG matches well), but I would be
>> worried about clashing with existing functionality.

Using KEY_INFO / KEY_VENDOR works for me too. So maybe we should
just go with one of those 2 ?

Regards,

Hans