Re: [PATCH] platform/x86: intel-vbtn: Switch to an allow-list for SW_TABLET_MODE reporting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 30, 2020 at 4:19 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> 2 recent commits:
> cfae58ed681c ("platform/x86: intel-vbtn: Only blacklist SW_TABLET_MODE
> on the 9 / "Laptop" chasis-type")
> 1fac39fd0316 ("platform/x86: intel-vbtn: Also handle tablet-mode switch on
> "Detachable" and "Portable" chassis-types")
>
> Enabled reporting of SW_TABLET_MODE on more devices since the vbtn ACPI
> interface is used by the firmware on some of those devices to report this.
>
> Testing has shown that unconditionally enabling SW_TABLET_MODE reporting
> on all devices with a chassis type of 8 ("Portable") or 10 ("Notebook")
> which support the VGBS method is a very bad idea.
>
> Many of these devices are normal laptops (non 2-in-1) models with a VGBS
> which always returns 0, which we translate to SW_TABLET_MODE=1. This in
> turn causes userspace (libinput) to suppress events from the builtin
> keyboard and touchpad, making the laptop essentially unusable.
>
> Since the problem of wrongly reporting SW_TABLET_MODE=1 in combination
> with libinput, leads to a non-usable system. Where as OTOH many people will
> not even notice when SW_TABLET_MODE is not being reported, this commit
> changes intel_vbtn_has_switches() to use a DMI based allow-list.
>
> The new DMI based allow-list matches on the 31 ("Convertible") and
> 32 ("Detachable") chassis-types, as these clearly are 2-in-1s and
> so far if they support the intel-vbtn ACPI interface they all have
> properly working SW_TABLET_MODE reporting.
>
> Besides these 2 generic matches, it also contains model specific matches
> for 2-in-1 models which use a different chassis-type and which are known
> to have properly working SW_TABLET_MODE reporting.
>
> This has been tested on the following 2-in-1 devices:
>
> Dell Venue 11 Pro 7130 vPro
> HP Pavilion X2 10-p002nd
> HP Stream x360 Convertible PC 11
> Medion E1239T

I have reverted previous attempts and applied this one, but...

> Fixes: cfae58ed681c ("platform/x86: intel-vbtn: Only blacklist SW_TABLET_MODE on the 9 / "Laptop" chasis-type")
> BugLink: https://forum.manjaro.org/t/keyboard-and-touchpad-only-work-on-kernel-5-6/22668
> BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1175599

> Cc: Barnab1s PY1cze <pobrn@xxxxxxxxxxxxxx>

...seems like a broken name to me. I'll try to fix this.

> Cc: Takashi Iwai <tiwai@xxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  drivers/platform/x86/intel-vbtn.c | 52 +++++++++++++++++++++++++------
>  1 file changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
> index e85d8e58320c..f5901b0b07cd 100644
> --- a/drivers/platform/x86/intel-vbtn.c
> +++ b/drivers/platform/x86/intel-vbtn.c
> @@ -167,20 +167,54 @@ static bool intel_vbtn_has_buttons(acpi_handle handle)
>         return ACPI_SUCCESS(status);
>  }
>
> +/*
> + * There are several laptops (non 2-in-1) models out there which support VGBS,
> + * but simply always return 0, which we translate to SW_TABLET_MODE=1. This in
> + * turn causes userspace (libinput) to suppress events from the builtin
> + * keyboard and touchpad, making the laptop essentially unusable.
> + *
> + * Since the problem of wrongly reporting SW_TABLET_MODE=1 in combination
> + * with libinput, leads to a non-usable system. Where as OTOH many people will
> + * not even notice when SW_TABLET_MODE is not being reported, a DMI based allow
> + * list is used here. This list mainly matches on the chassis-type of 2-in-1s.
> + *
> + * There are also some 2-in-1s which use the intel-vbtn ACPI interface to report
> + * SW_TABLET_MODE with a chassis-type of 8 ("Portable") or 10 ("Notebook"),
> + * these are matched on a per model basis, since many normal laptops with a
> + * possible broken VGBS ACPI-method also use these chassis-types.
> + */
> +static const struct dmi_system_id dmi_switches_allow_list[] = {
> +       {
> +               .matches = {
> +                       DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "31" /* Convertible */),
> +               },
> +       },
> +       {
> +               .matches = {
> +                       DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "32" /* Detachable */),
> +               },
> +       },
> +       {
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Venue 11 Pro 7130"),
> +               },
> +       },
> +       {
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "HP Stream x360 Convertible PC 11"),
> +               },
> +       },
> +       {} /* Array terminator */
> +};
> +
>  static bool intel_vbtn_has_switches(acpi_handle handle)
>  {
> -       const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
>         unsigned long long vgbs;
>         acpi_status status;
>
> -       /*
> -        * Some normal laptops have a VGBS method despite being non-convertible
> -        * and their VGBS method always returns 0, causing detect_tablet_mode()
> -        * to report SW_TABLET_MODE=1 to userspace, which causes issues.
> -        * These laptops have a DMI chassis_type of 9 ("Laptop"), do not report
> -        * switches on any devices with a DMI chassis_type of 9.
> -        */
> -       if (chassis_type && strcmp(chassis_type, "9") == 0)
> +       if (!dmi_check_system(dmi_switches_allow_list))
>                 return false;
>
>         status = acpi_evaluate_integer(handle, "VGBS", NULL, &vgbs);
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux