Re: [PATCH] platform/x86: thinkpad-acpi: Add X1 Fold keyboard attachment detection

["Mark Pearson" <mpearson-lenovo@squebb.ca>]

Hi Pit

On Mon, Mar 16, 2026, at 5:24 PM, Pit Henrich wrote:
> Heya,
>
>>  - We usually do the DMI_MATCH on the BOARD_NAME for quirks. I was checking psref.lenovo.com and there seem to be two distinct models for this platform, which is a bit confusing. I don't have a way to confirm that your changes will work for the other model so wondering if it's safer to do the quirks for 21ES/21ET or 20RL/20RK depending on what you have in hand? (I might be overthinking this too)
>
> I based this on the code in drivers/platform/x86/intel/hid.c which
> also does a DMI_MATCH against DMI_PRODUCT_FAMILY.
> Just for the records: I have an ES21 model, and the code in
> drivers/platform/x86/intel/hid.c works for me too (correctly enables
> the 5-button array).
>
>> If I get details back on the EC spec and confirmation it's correct I'll let you know :)
>
> Awesome :)
>
> Greetings
> Pit
>
> On Sat, 14 Mar 2026 at 19:24, Mark Pearson <mpearson-lenovo@squebb.ca> wrote:
>>
>> Hi Pit,
>>
>> On Sat, Mar 14, 2026, at 10:22 AM, Pit Henrich wrote:
>> > ThinkPad X1 Fold 16 Gen 1 firmware reports whether the keyboard is magnetically
>> > attached (on the screen) via EC register 0xc1 bit 7, but thinkpad-acpi does
>> > not expose this to userspace.
>> >
>> > Add a read-only keyboard_attached_on_screen sysfs attribute, gated by
>> > a DMI match. The state is read directly from the EC.
>> >
>> > Cache the state and emit a sysfs notification on
>> > TP_HKEY_EV_TABLET_CHANGED (0x60c0) when it changes. Initialize the cache during
>> > hotkey setup and refresh it before the resume notification to keep the state
>> > consistent across suspend and resume.
>> >
>> > Signed-off-by: Pit Henrich <pithenrich2d@gmail.com>
>> > ---
>> >  drivers/platform/x86/lenovo/thinkpad_acpi.c | 92 ++++++++++++++++++++-
>> >  1 file changed, 90 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> > b/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> > index 8982d92dfd97..5b255062ff51 100644
>> > --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> > +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> > @@ -218,8 +218,9 @@ enum tpacpi_hkey_event_t {
>> >       TP_HKEY_EV_LID_OPEN             = 0x5002, /* laptop lid opened */
>> >       TP_HKEY_EV_TABLET_TABLET        = 0x5009, /* tablet swivel up */
>> >       TP_HKEY_EV_TABLET_NOTEBOOK      = 0x500a, /* tablet swivel down */
>> > -     TP_HKEY_EV_TABLET_CHANGED       = 0x60c0, /* X1 Yoga (2016):
>> > -                                                * enter/leave tablet mode
>> > +     TP_HKEY_EV_TABLET_CHANGED       = 0x60c0, /* posture change event:
>> > +                                                * X1 Yoga (2016): enter/leave tablet mode
>> > +                                                * X1 Fold 16 Gen 1: keyboard attachment state changed
>> >                                                  */
>> >       TP_HKEY_EV_PEN_INSERTED         = 0x500b, /* tablet pen inserted */
>> >       TP_HKEY_EV_PEN_REMOVED          = 0x500c, /* tablet pen removed */
>> > @@ -375,6 +376,7 @@ static struct {
>> >       u32 has_adaptive_kbd:1;
>> >       u32 kbd_lang:1;
>> >       u32 trackpoint_doubletap:1;
>> > +     u32 has_keyboard_attached_on_screen:1;
>> >       struct quirk_entry *quirks;
>> >  } tp_features;
>> >
>> > @@ -2928,6 +2930,70 @@ static void hotkey_tablet_mode_notify_change(void)
>> >                            "hotkey_tablet_mode");
>> >  }
>> >
>> > +/*
>> > + * On the ThinkPad X1 Fold 16 Gen 1, EC register 0xc1 reports the
>> > keyboard
>> > + * attachment state in bit 7.
>> > + */
>> > +#define TPACPI_X1_FOLD_KBD_EC_STATUS 0xc1
>> > +#define TPACPI_X1_FOLD_KBD_ATTACHED  BIT(7)
>> > +
>> > +static bool keyboard_attached_on_screen;
>> > +static bool keyboard_attached_on_screen_initialized;
>> > +
>> > +static int x1_fold_keyboard_attached_on_screen_get(bool *attached)
>> > +{
>> > +     u8 status;
>> > +
>> > +     if (!tp_features.has_keyboard_attached_on_screen)
>> > +             return -ENODEV;
>> > +
>> > +     if (!acpi_ec_read(TPACPI_X1_FOLD_KBD_EC_STATUS, &status))
>> > +             return -EIO;
>> > +
>> > +     *attached = status & TPACPI_X1_FOLD_KBD_ATTACHED;
>> > +     return 0;
>> > +}
>> > +
>> > +static ssize_t keyboard_attached_on_screen_show(struct device *dev,
>> > +                                             struct device_attribute *attr,
>> > +                                             char *buf)
>> > +{
>> > +     bool attached;
>> > +     int res;
>> > +
>> > +     res = x1_fold_keyboard_attached_on_screen_get(&attached);
>> > +     if (res)
>> > +             return res;
>> > +
>> > +     return sysfs_emit(buf, "%d\n", attached);
>> > +}
>> > +
>> > +static DEVICE_ATTR_RO(keyboard_attached_on_screen);
>> > +
>> > +static void keyboard_attached_on_screen_notify_change(void)
>> > +{
>> > +     if (tp_features.has_keyboard_attached_on_screen)
>> > +             sysfs_notify(&tpacpi_pdev->dev.kobj, NULL,
>> > +                          "keyboard_attached_on_screen");
>> > +}
>> > +
>> > +static bool keyboard_attached_on_screen_update(void)
>> > +{
>> > +     bool attached;
>> > +
>> > +     if (x1_fold_keyboard_attached_on_screen_get(&attached))
>> > +             return false;
>> > +
>> > +     if (keyboard_attached_on_screen_initialized &&
>> > +         keyboard_attached_on_screen == attached)
>> > +             return false;
>> > +
>> > +     keyboard_attached_on_screen = attached;
>> > +     keyboard_attached_on_screen_initialized = true;
>> > +
>> > +     return true;
>> > +}
>> > +
>> >  /* sysfs wakeup reason (pollable)
>> > -------------------------------------- */
>> >  static ssize_t hotkey_wakeup_reason_show(struct device *dev,
>> >                          struct device_attribute *attr,
>> > @@ -3032,6 +3098,7 @@ static struct attribute *hotkey_attributes[] = {
>> >       &dev_attr_hotkey_adaptive_all_mask.attr,
>> >       &dev_attr_hotkey_recommended_mask.attr,
>> >       &dev_attr_hotkey_tablet_mode.attr,
>> > +     &dev_attr_keyboard_attached_on_screen.attr,
>> >       &dev_attr_hotkey_radio_sw.attr,
>> >  #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
>> >       &dev_attr_hotkey_source_mask.attr,
>> > @@ -3046,6 +3113,9 @@ static umode_t hotkey_attr_is_visible(struct
>> > kobject *kobj,
>> >       if (attr == &dev_attr_hotkey_tablet_mode.attr) {
>> >               if (!tp_features.hotkey_tablet)
>> >                       return 0;
>> > +     } else if (attr == &dev_attr_keyboard_attached_on_screen.attr) {
>> > +             if (!tp_features.has_keyboard_attached_on_screen)
>> > +                     return 0;
>> >       } else if (attr == &dev_attr_hotkey_radio_sw.attr) {
>> >               if (!tp_features.hotkey_wlsw)
>> >                       return 0;
>> > @@ -3462,6 +3532,7 @@ static int __init hotkey_init(struct
>> > ibm_init_struct *iibm)
>> >       }
>> >
>> >       tabletsw_state = hotkey_init_tablet_mode();
>> > +     keyboard_attached_on_screen_update();
>> >
>> >       /* Set up key map */
>> >       keymap_id = tpacpi_check_quirks(tpacpi_keymap_qtable,
>> > @@ -3842,6 +3913,8 @@ static bool hotkey_notify_6xxx(const u32 hkey,
>> > bool *send_acpi_ev)
>> >       case TP_HKEY_EV_TABLET_CHANGED:
>> >               tpacpi_input_send_tabletsw();
>> >               hotkey_tablet_mode_notify_change();
>> > +             if (keyboard_attached_on_screen_update())
>> > +                     keyboard_attached_on_screen_notify_change();
>> >               *send_acpi_ev = false;
>> >               return true;
>> >
>> > @@ -3998,6 +4071,8 @@ static void hotkey_resume(void)
>> >       tpacpi_send_radiosw_update();
>> >       tpacpi_input_send_tabletsw();
>> >       hotkey_tablet_mode_notify_change();
>> > +     keyboard_attached_on_screen_update();
>> > +     keyboard_attached_on_screen_notify_change();
>> >       hotkey_wakeup_reason_notify_change();
>> >       hotkey_wakeup_hotunplug_complete_notify_change();
>> >       hotkey_poll_setup_safe(false);
>> > @@ -4296,6 +4371,17 @@ static const struct dmi_system_id fwbug_list[]
>> > __initconst = {
>> >       {}
>> >  };
>> >
>> > +static const struct dmi_system_id keyboard_attached_on_screen_list[]
>> > __initconst = {
>> > +     {
>> > +             .ident = "ThinkPad X1 Fold 16 Gen 1",
>> > +             .matches = {
>> > +                     DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> > +                     DMI_MATCH(DMI_PRODUCT_FAMILY, "ThinkPad X1 Fold 16 Gen 1"),
>> > +             },
>> > +     },
>> > +     {}
>> > +};
>> > +
>> >  static const struct pci_device_id fwbug_cards_ids[] __initconst = {
>> >       { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x24F3) },
>> >       { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x24FD) },
>> > @@ -12230,6 +12316,8 @@ static int __init
>> > thinkpad_acpi_module_init(void)
>> >       dmi_id = dmi_first_match(fwbug_list);
>> >       if (dmi_id)
>> >               tp_features.quirks = dmi_id->driver_data;
>> > +     tp_features.has_keyboard_attached_on_screen =
>> > +             dmi_check_system(keyboard_attached_on_screen_list);
>> >
>> >       /* Device initialization */
>> >       tpacpi_pdev = platform_device_register_simple(TPACPI_DRVR_NAME,
>> > PLATFORM_DEVID_NONE,
>> > --
>> > 2.43.0
>>
>> I'm checking to see if we have the EC spec for this platform (it's not in our Linux certification program).
>>
>> The code looks fine but some notes:
>>  - We usually do the DMI_MATCH on the BOARD_NAME for quirks. I was checking psref.lenovo.com and there seem to be two distinct models for this platform, which is a bit confusing. I don't have a way to confirm that your changes will work for the other model so wondering if it's safer to do the quirks for 21ES/21ET or 20RL/20RK depending on what you have in hand? (I might be overthinking this too)
>>  - I would have called it detachable_keyboard instead of keyboard_attached_on_screen. See what other reviewers think.
>>
>> If I get details back on the EC spec and confirmation it's correct I'll let you know :)
>>

Sorry this took a bit - the engineer we needed to talk to was on PTO.

I got some internal details on the implementation and whilst there isn't a spec I can share, we did determine that Windows driver is using some ACPI methods instead: GDST (get device state), GDSZ (get device display occlusion) and NSMC (Notify BIOS of screen mode change).

I don't have any details on how those ACPI methods break down I'm afraid - but just wanted you to be aware that  Windows is doing things differently.

Mark