| Message ID | 20240421154520.37089-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | platform/x86: thinkpad_acpi: Refactor hotkey handling and add support for some new hotkeys | expand |
Thanks Hans! On Sun, Apr 21, 2024, at 11:44 AM, Hans de Goede wrote: > Hi All, > > My reply in the "[PATCH v2 1/4] platform/x86: thinkpad_acpi: > simplify known_ev handling" handling where I indicated that I would work > on converting the thinkpad_acpi hotkey handling to use sparse-keymaps > underestimated the work this required quite a bit. > > The hotkey code is quite old and crufty and full of special cases to > support many generations of ThinkPads, so I ended up doing some > significant refactoring and cleanup before doing the actual conversion > to sparse-keymaps. > > I have been vary careful to not introduce any changes wrt support for > the original ThinkPad models / hotkeys which use the hotkey_*_mask > related code. > > I've also done my best to avoid any *significant* functional change but > there are still some functional changes, which should all not impact > userspace compatibility: > > 1. Adaptive keyboard special keys will now also send EV_MSC events with > the scancode, just like all the other hotkeys. > > 2. Rely on the input core for KEY_RESERVED suppression. This means that > keys marked as KEY_RESERVED will still send EV_MSC evdev events when > pressed (which helps users with mapping them to non reserved KEY_FOO > values if desired). > > 3. Align the keycodes for volume up/down/mute and brightness up/down with > those set by userspace through udev upstream's hwdb. Note these are all > for keys which are suppressed by hotkey_reserved_mask by default. > So this is only a functional change for users who override the default > hotkey-mask *and* who do not have udev's default hwdb installed. > > 4. Suppress ACPI netlink event generation for unknown 0x1xxx hkey events to > avoid userspace starting to rely on the netlink events for new hotkeys > before these have been added to the keymap, only to have the netlink > events get disabled by the adding of the new hotkeys to the keymap. > > This should not cause a behavior change for existing models since all > currently known 0x1xxx events have a mapping. > > Here is a quick breakdown of the patches in this series: > > Patch 1 - 2: Fix a small locking issue on rmmod the only problem here > really is a lockdep warning, so I plan to route these fixes through > for-next together with the rest to keep things simple. > > Patch 3 - 14: Do a bunch of cleanups and refactoring > > Patch 15: Implements functional change no 4. I really kinda want to just > completely disable ACPI netlink event generation when also sending evdev > events instead of reporting these twice. But for the 0x11xx / 0x13xx > hkey events the kernel has send ACPI netlink events for years now. So > this disables ACPI netlink events for any new hotkeys going forward. > > Patch 16 - 18: Refactor / cleanup reserved key handling > > Patch 19: Actually move to sparse-keymaps > > Patch 20: Update the keymap for 2 adaptive kbd Fn row keys > > Patch 21 - 24: Mark's original patches adding support for the new Fn + N > key combo and for trackpoint doubletap slightly reworked to use > the new sparse-keymap. > > Mark if you can make some time to review patches 1-20 that would be great. > Definitely will do. I'll do some testing on platforms here as much as I can. If there's anything in particular that you think is worth taking extra care over let me know (with a caveat that my team doesn't have all the platforms, and for anything older than 5 years it can be hard to get hold of them) Thanks for your work on this. Mark > Regards, > > Hans > > > Hans de Goede (20): > platform/x86: thinkpad_acpi: Take hotkey_mutex during hotkey_exit() > platform/x86: thinkpad_acpi: Provide hotkey_poll_stop_sync() dummy > platform/x86: thinkpad_acpi: Drop setting send_/ignore_acpi_ev > defaults twice > platform/x86: thinkpad_acpi: Drop ignore_acpi_ev > platform/x86: thinkpad_acpi: Use tpacpi_input_send_key() in adaptive > kbd code > platform/x86: thinkpad_acpi: Do hkey to scancode translation later > platform/x86: thinkpad_acpi: Make tpacpi_driver_event() return if it > handled the event > platform/x86: thinkpad_acpi: Move adaptive kbd event handling to > tpacpi_driver_event() > platform/x86: thinkpad_acpi: Move special original hotkeys handling > out of switch-case > platform/x86: thinkpad_acpi: Move hotkey_user_mask check to > tpacpi_input_send_key() > platform/x86: thinkpad_acpi: Always call tpacpi_driver_event() for > hotkeys > platform/x86: thinkpad_acpi: Drop tpacpi_input_send_key_masked() and > hotkey_driver_event() > platform/x86: thinkpad_acpi: Move hkey > scancode mapping to > tpacpi_input_send_key() > platform/x86: thinkpad_acpi: Move tpacpi_driver_event() call to > tpacpi_input_send_key() > platform/x86: thinkpad_acpi: Do not send ACPI netlink events for > unknown hotkeys > platform/x86: thinkpad_acpi: Change hotkey_reserved_mask > initialization > platform/x86: thinkpad_acpi: Use correct keycodes for volume and > brightness keys > platform/x86: thinkpad_acpi: Drop KEY_RESERVED special handling > platform/x86: thinkpad_acpi: Switch to using sparse-keymap helpers > platform/x86: thinkpad_acpi: Add mappings for adaptive kbd > clipping-tool and cloud keys > > Mark Pearson (4): > platform/x86: thinkpad_acpi: Simplify known_ev handling > platform/x86: thinkpad_acpi: Support for trackpoint doubletap > platform/x86: thinkpad_acpi: Support for system debug info hotkey > platform/x86: thinkpad_acpi: Support hotkey to disable trackpoint > doubletap > > drivers/platform/x86/thinkpad_acpi.c | 849 +++++++++++---------------- > 1 file changed, 348 insertions(+), 501 deletions(-) > > -- > 2.44.0
On Sun, Apr 21, 2024, at 1:17 PM, Mark Pearson wrote: > Thanks Hans! > > On Sun, Apr 21, 2024, at 11:44 AM, Hans de Goede wrote: >> Hi All, >> >> My reply in the "[PATCH v2 1/4] platform/x86: thinkpad_acpi: >> simplify known_ev handling" handling where I indicated that I would work >> on converting the thinkpad_acpi hotkey handling to use sparse-keymaps >> underestimated the work this required quite a bit. >> >> The hotkey code is quite old and crufty and full of special cases to >> support many generations of ThinkPads, so I ended up doing some >> significant refactoring and cleanup before doing the actual conversion >> to sparse-keymaps. >> >> I have been vary careful to not introduce any changes wrt support for >> the original ThinkPad models / hotkeys which use the hotkey_*_mask >> related code. >> >> I've also done my best to avoid any *significant* functional change but >> there are still some functional changes, which should all not impact >> userspace compatibility: >> >> 1. Adaptive keyboard special keys will now also send EV_MSC events with >> the scancode, just like all the other hotkeys. >> >> 2. Rely on the input core for KEY_RESERVED suppression. This means that >> keys marked as KEY_RESERVED will still send EV_MSC evdev events when >> pressed (which helps users with mapping them to non reserved KEY_FOO >> values if desired). >> >> 3. Align the keycodes for volume up/down/mute and brightness up/down with >> those set by userspace through udev upstream's hwdb. Note these are all >> for keys which are suppressed by hotkey_reserved_mask by default. >> So this is only a functional change for users who override the default >> hotkey-mask *and* who do not have udev's default hwdb installed. >> >> 4. Suppress ACPI netlink event generation for unknown 0x1xxx hkey events to >> avoid userspace starting to rely on the netlink events for new hotkeys >> before these have been added to the keymap, only to have the netlink >> events get disabled by the adding of the new hotkeys to the keymap. >> >> This should not cause a behavior change for existing models since all >> currently known 0x1xxx events have a mapping. >> >> Here is a quick breakdown of the patches in this series: >> >> Patch 1 - 2: Fix a small locking issue on rmmod the only problem here >> really is a lockdep warning, so I plan to route these fixes through >> for-next together with the rest to keep things simple. >> >> Patch 3 - 14: Do a bunch of cleanups and refactoring >> >> Patch 15: Implements functional change no 4. I really kinda want to just >> completely disable ACPI netlink event generation when also sending evdev >> events instead of reporting these twice. But for the 0x11xx / 0x13xx >> hkey events the kernel has send ACPI netlink events for years now. So >> this disables ACPI netlink events for any new hotkeys going forward. >> >> Patch 16 - 18: Refactor / cleanup reserved key handling >> >> Patch 19: Actually move to sparse-keymaps >> >> Patch 20: Update the keymap for 2 adaptive kbd Fn row keys >> >> Patch 21 - 24: Mark's original patches adding support for the new Fn + N >> key combo and for trackpoint doubletap slightly reworked to use >> the new sparse-keymap. >> >> Mark if you can make some time to review patches 1-20 that would be great. >> > Definitely will do. > > I'll do some testing on platforms here as much as I can. If there's > anything in particular that you think is worth taking extra care over > let me know (with a caveat that my team doesn't have all the platforms, > and for anything older than 5 years it can be hard to get hold of them) > > Thanks for your work on this. > > Mark > I've tested the series on a couple of platforms (Z13 G2 and X1 Carbon G12) and so far all looking good. - tried all hotkey combinations that are supported that I can think of and they work - including brightness control, volume control, platform profile toggle, airplane, snipping tool and privacy screen. - trackpoint double tap is working well on the Z13 G2 (set up custom key setting in gnome-settings and launched browser on a doubletap) - FN+N is working well on the X1 Carbon G12 (tested with evtest to confirm vendor key generated) So for the series: Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca> I have read through the patches, but not in enough depth for it to count as a review yet. Need a bit more time for that. Mark
Hi All, My reply in the "[PATCH v2 1/4] platform/x86: thinkpad_acpi: simplify known_ev handling" handling where I indicated that I would work on converting the thinkpad_acpi hotkey handling to use sparse-keymaps underestimated the work this required quite a bit. The hotkey code is quite old and crufty and full of special cases to support many generations of ThinkPads, so I ended up doing some significant refactoring and cleanup before doing the actual conversion to sparse-keymaps. I have been vary careful to not introduce any changes wrt support for the original ThinkPad models / hotkeys which use the hotkey_*_mask related code. I've also done my best to avoid any *significant* functional change but there are still some functional changes, which should all not impact userspace compatibility: 1. Adaptive keyboard special keys will now also send EV_MSC events with the scancode, just like all the other hotkeys. 2. Rely on the input core for KEY_RESERVED suppression. This means that keys marked as KEY_RESERVED will still send EV_MSC evdev events when pressed (which helps users with mapping them to non reserved KEY_FOO values if desired). 3. Align the keycodes for volume up/down/mute and brightness up/down with those set by userspace through udev upstream's hwdb. Note these are all for keys which are suppressed by hotkey_reserved_mask by default. So this is only a functional change for users who override the default hotkey-mask *and* who do not have udev's default hwdb installed. 4. Suppress ACPI netlink event generation for unknown 0x1xxx hkey events to avoid userspace starting to rely on the netlink events for new hotkeys before these have been added to the keymap, only to have the netlink events get disabled by the adding of the new hotkeys to the keymap. This should not cause a behavior change for existing models since all currently known 0x1xxx events have a mapping. Here is a quick breakdown of the patches in this series: Patch 1 - 2: Fix a small locking issue on rmmod the only problem here really is a lockdep warning, so I plan to route these fixes through for-next together with the rest to keep things simple. Patch 3 - 14: Do a bunch of cleanups and refactoring Patch 15: Implements functional change no 4. I really kinda want to just completely disable ACPI netlink event generation when also sending evdev events instead of reporting these twice. But for the 0x11xx / 0x13xx hkey events the kernel has send ACPI netlink events for years now. So this disables ACPI netlink events for any new hotkeys going forward. Patch 16 - 18: Refactor / cleanup reserved key handling Patch 19: Actually move to sparse-keymaps Patch 20: Update the keymap for 2 adaptive kbd Fn row keys Patch 21 - 24: Mark's original patches adding support for the new Fn + N key combo and for trackpoint doubletap slightly reworked to use the new sparse-keymap. Mark if you can make some time to review patches 1-20 that would be great. Regards, Hans Hans de Goede (20): platform/x86: thinkpad_acpi: Take hotkey_mutex during hotkey_exit() platform/x86: thinkpad_acpi: Provide hotkey_poll_stop_sync() dummy platform/x86: thinkpad_acpi: Drop setting send_/ignore_acpi_ev defaults twice platform/x86: thinkpad_acpi: Drop ignore_acpi_ev platform/x86: thinkpad_acpi: Use tpacpi_input_send_key() in adaptive kbd code platform/x86: thinkpad_acpi: Do hkey to scancode translation later platform/x86: thinkpad_acpi: Make tpacpi_driver_event() return if it handled the event platform/x86: thinkpad_acpi: Move adaptive kbd event handling to tpacpi_driver_event() platform/x86: thinkpad_acpi: Move special original hotkeys handling out of switch-case platform/x86: thinkpad_acpi: Move hotkey_user_mask check to tpacpi_input_send_key() platform/x86: thinkpad_acpi: Always call tpacpi_driver_event() for hotkeys platform/x86: thinkpad_acpi: Drop tpacpi_input_send_key_masked() and hotkey_driver_event() platform/x86: thinkpad_acpi: Move hkey > scancode mapping to tpacpi_input_send_key() platform/x86: thinkpad_acpi: Move tpacpi_driver_event() call to tpacpi_input_send_key() platform/x86: thinkpad_acpi: Do not send ACPI netlink events for unknown hotkeys platform/x86: thinkpad_acpi: Change hotkey_reserved_mask initialization platform/x86: thinkpad_acpi: Use correct keycodes for volume and brightness keys platform/x86: thinkpad_acpi: Drop KEY_RESERVED special handling platform/x86: thinkpad_acpi: Switch to using sparse-keymap helpers platform/x86: thinkpad_acpi: Add mappings for adaptive kbd clipping-tool and cloud keys Mark Pearson (4): platform/x86: thinkpad_acpi: Simplify known_ev handling platform/x86: thinkpad_acpi: Support for trackpoint doubletap platform/x86: thinkpad_acpi: Support for system debug info hotkey platform/x86: thinkpad_acpi: Support hotkey to disable trackpoint doubletap drivers/platform/x86/thinkpad_acpi.c | 849 +++++++++++---------------- 1 file changed, 348 insertions(+), 501 deletions(-)