Message ID | 20200930131905.48924-1-hdegoede@redhat.com (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Andy Shevchenko |
Headers | show |
Series | platform/x86: intel-vbtn: Switch to an allow-list for SW_TABLET_MODE reporting | expand |
On Wed, Sep 30, 2020 at 4:19 PM Hans de Goede <hdegoede@redhat.com> 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@protonmail.com> ...seems like a broken name to me. I'll try to fix this. > Cc: Takashi Iwai <tiwai@suse.de> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > 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 >
> [...] > 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@protonmail.com > > ...seems like a broken name to me. I'll try to fix this. > [...] Yes, it is. :-( Maybe I shouldn't have used accented characters, sorry. I thought UTF-8 was reasonably well-supported in 2020. Regards, Barnabás Pőcze
On Fri, Oct 2, 2020 at 6:01 PM Barnabás Pőcze <pobrn@protonmail.com> wrote: > > 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@protonmail.com > > > > ...seems like a broken name to me. I'll try to fix this. > Yes, it is. :-( > Maybe I shouldn't have used accented characters, sorry. I thought UTF-8 > was reasonably well-supported in 2020. Sorry for that. I agree that UTF-8 must be supported well. I think Hans can check what happened and act accordingly.
Hi, On 10/2/20 5:07 PM, Andy Shevchenko wrote: > On Fri, Oct 2, 2020 at 6:01 PM Barnabás Pőcze <pobrn@protonmail.com> wrote: >>> 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@protonmail.com >>> >>> ...seems like a broken name to me. I'll try to fix this. > >> Yes, it is. :-( >> Maybe I shouldn't have used accented characters, sorry. I thought UTF-8 >> was reasonably well-supported in 2020. > > Sorry for that. I agree that UTF-8 must be supported well. I think > Hans can check what happened and act accordingly. Ugh, no idea what happened there, it is already broken in my local git tree. IIRC just copy and pasted it out of thunderbird. I just did that again and this time it is fine ... Maybe I picked the wrong email as source and it got mangled by some email system on the way. Anyways I've just send out a v2 with this fixed. Regards, Hans
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 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 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@protonmail.com> Cc: Takashi Iwai <tiwai@suse.de> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/intel-vbtn.c | 52 +++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 9 deletions(-)