> -----Original Message----- > From: Hans de Goede <hdegoede@xxxxxxxxxx> > Sent: Wednesday, September 30, 2020 10:37 > To: Limonciello, Mario; Barnabás Pőcze; Andy Shevchenko > Cc: platform-driver-x86@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Takashi > Iwai > Subject: Re: Keyboard regression by intel-vbtn > > > [EXTERNAL EMAIL] > > Hi, > > On 9/30/20 5:12 PM, Limonciello, Mario wrote: > >> -----Original Message----- > >> From: Hans de Goede <hdegoede@xxxxxxxxxx> > >> Sent: Wednesday, September 30, 2020 8:28 > >> To: Limonciello, Mario; Barnabás Pőcze; Andy Shevchenko > >> Cc: platform-driver-x86@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > Takashi > >> Iwai > >> Subject: Re: Keyboard regression by intel-vbtn > >> > >> > >> [EXTERNAL EMAIL] > >> > >> Hi, > >> > >> On 9/29/20 10:47 PM, Limonciello, Mario wrote: > >>>> > >>>> I requested on the Ubuntu bug for someone to provide these. > >>>> > >>> > >>> Joe Barnett was kind enough to share two ACPI dumps to compare. > >>> Not affected: > >>> > >> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1822394/+attachment/54153 > >> 18/+files/1.2.0.acpidump > >>> > >>> Affected: > >>> > >> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1822394/+attachment/54154 > >> 05/+files/1.13.0.acpidump > >> > >> Thank you, I took a look at these (before completing my allow-list fix), > >> but there is not really much which stands out. The only related thing which > >> stands out is that the 1.13.0 dsdt.dsl has this new bit: > >> > >> > >> Case (0x08) > >> { > >> Return (^^PCI0.LPCB.H_EC.VGBI.VGBS ()) > >> } > >> > >> Inside the _DSM of the HIDD / INT33D5 device. > >> > >> Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method > >> { > >> If ((Arg0 == ToUUID ("eeec56b3-4442-408f-a792- > >> 4edd4d758054"))) > >> > >> > >> What is interesting here is that the PCI0.LPCB.H_EC.VGBI.VGBS object/method > >> does not actually exist the correct path is: > >> > >> ^^PCI0.LPCB.ECDV.VGBI.VGBS > >> > >> So this does suggest that something around the VGBS handling changed > >> (and since it points to a non existing ACPI object, possibly broke) > >> in the newer BIOS version. But what exactly is going on on this XPS 2-in-1 > >> cannot really be derived from the acpidumps. > >> > >> Regards, > >> > >> Hans > > > > Looking through some publicly found content I think I might have figured out > what > > bight be the missing link. > > > > https://software.intel.com/sites/default/files/detecting-slate-clamshell- > mode-and-screen-orientation-in-convertible-pc-1.pdf > > > > You can see that the device with CID PNP0C60 is supposed to indicate the > presence > > of a convertible hinge. We don't currently have anything that matches that > _CID or _HID > > in intel-vbtn. > > > > In the DSDT dump you can see that the status method for the INT33D3 device > returns > > 0x0F on 2-in-1.s > > > > Device (CIND) > > { > > Name (_HID, "INT33D3" /* Intel GPIO Buttons */) // _HID: > Hardware ID > > Name (_CID, "PNP0C60" /* Display Sensor Device */) // _CID: > Compatible ID > > Method (_STA, 0, Serialized) // _STA: Status > > { > > If ((OSYS >= 0x07DC)) > > { > > Return (0x0F) > > } > > > > Return (Zero) > > } > > } > > > > On a non 2-in-1 device I don't see this present. So I think we should have > intel-vbtn > > look for that INT33D3/PNP0C60 device to decide whether to offer the switch. > > > > Similarly as mentioned in that document I think that we should not be > showing the > > docking switch only when INT33D4/PNP0C70 is present and returns 0xF. > > That is a good find, thank you for digging into this and finding this. > > But it seems we have a typical case of the microsoft/intel example DSDT code > being > blindly copied everywhere here too: > > The chassis-type check was originally introduced by you in: > commit de9647efeaa9 ("platform/x86: intel-vbtn: Only activate tablet mode > switch on 2-in-1's") > > Some laptops such as the XPS 9360 support the intel-vbtn INT33D6 > interface but don't initialize the bit that intel-vbtn uses to > represent switching tablet mode. > > By running this only on real 2-in-1's it shouldn't cause false > positives. > > Fixes: 30323fb6d5 ("Support tablet mode switch") > > I have a XPS 9360 (which is not 2-in-1) acpidump and that has: > > Device (CIND) > { > Name (_HID, "INT33D3" /* Intel GPIO Buttons */) // _HID: > Hardware ID > Name (_CID, "PNP0C60" /* Display Sensor Device */) // _CID: > Compatible ID > Method (_STA, 0, Serialized) // _STA: Status > { > If ((OSYS >= 0x07DC)) > { > Return (0x0F) > } > > Return (Zero) > } > } > > And on an asus e200ha (also not a 2-in-1), where we were seeing > similar problems, but then using asus custom WMI interface for > getting SW_TABLET_MODE I see: > > Device (CIND) > { > Name (_HID, "INT33D3" /* Intel GPIO Buttons */) // _HID: > Hardware ID > Name (_CID, "PNP0C60" /* Display Sensor Device */) // _CID: > Compatible ID > Method (_STA, 0, Serialized) // _STA: Status > { > Return (0x0F) > } > } > > I have quite a few DSDTs (mostly byt/cht tablets or 2-in-1s though) and > 65 of them define a "PNP0C60" device and only 1 unconditionally > returns 0 from the _STA method for this device. Most others have > an OSYS check. Some also check for some other, presumably BIOS > configured variable, but by far most always return 0x0F, or do > so after an OSYS check which will be true in our case. > > So I'm afraid that almost all devices which have the intel-vbtn (INT33D6) > ACPI device will also have a PNP0C60 device with the exact same > _STA method as found on the XPS 9360 and that this thus is not > helpful. > > Regards, > > Hans Well dang, that's unfortunate. I guess for now the allowlist is the best solution then. Thanks,
