Re: platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices

[Hans de Goede]

Hi,

On 9/11/20 12:31 AM, Samuel Čavoj wrote:
> Hello!
>
> On 10.09.2020 19:44, Hans de Goede wrote:
> > Hi,
> >
> > On 9/4/20 7:17 PM, Samuel Čavoj wrote:
> > > Hi,
> > >
> > > On 04.09.2020 12:06, Hans de Goede wrote:
> > > > Hi,
> > > >
> > > > On 9/4/20 11:45 AM, Samuel Čavoj wrote:
> > > > > Hello!
> > > > >
> > > > > On 02.09.2020 14:52, Samuel Čavoj wrote:
> > > > > > Hello,
> > > > > >
> > > > > > On 02.09.2020 13:52, Hans de Goede wrote:
> > > > > > > But I would rather try to figure out a better way. Can you
> > > > > > > create an acpidump, by as root running:
> > > > > > >
> > > > > > > acpidump -o acpidump.asus-UX360CA
> > > > > >
> > > > > > The file is attached gzipped.
> > > > > >
> > > > > > > And then send me a direct (so without including the list)
> > > > > > > email with the generated acpidump.asus-UX360CA file attached please?
> > > > > > >
> > > > > > > Also, if necessary are you capable of building your own
> > > > > > > kernel with a (test)patch applied ?
> > > > > >
> > > > > > Yes, that is no problem at all.
> > > > > > Thank you for your quick response.
> > > > > >
> > > > > > Regards,
> > > > > > Samuel
> > > > >
> > > > > I don't mean to waste your time, it's just that my trust in mail systems
> > > > > has been steadily decreasing. I would just like to make sure you have
> > > > > received my previous email with the acpidump.
> > > > >
> > > > > In case not, here[1] it is available over https, if the message got
> > > > > dropped because of the attachment.
> > > >
> > > > I got your mail, but I've been burried under a ton of work,
> > > > so it may take a couple of days at least before I can take
> > > > a closer look at this.
> > >
> > > That's quite alright.
> > >
> > > I decided I would try and see if I can be of any use, so I looked around
> > > in the WMI implementation in the DSDT and found the following in the
> > > DSTS method:
> > >
> > > [...]
> > > 37486     If ((IIA0 == 0x00120063))
> > > 37487     {
> > > 37488         Local0 = ^^PCI0.LPCB.EC0.DKPS ()
> > > 37489         If ((Local0 == One))
> > > 37490         {
> > > 37491             Return (0x00010001)
> > > 37492         }
> > > 37493         Else
> > > 37494         {
> > > 37495             Return (0x00010000)
> > > 37496         }
> > > 37497     }
> > > [...]
> > >
> > > This is the If statement responsible for the ASUS_WMI_DEVID_KBD_DOCK
> > > device, and it always seems to return 0x00010000 on my machine. I
> > > followed it up the call chain but in the end it just read some bit from
> > > some register of the EC.
> > >
> > > Then I noticed the If statement right above it, which corresponds to
> > > dev_id 0x00060062:
> > >
> > > [...]
> > > 37472     If ((IIA0 == 0x00060062))
> > > 37473     {
> > > 37474         If (^^PCI0.LPCB.EC0.RPIN (0x15))
> > > 37475         {
> > > 37476             Local0 = 0x00010001
> > > 37477         }
> > > 37478         Else
> > > 37479         {
> > > 37480             Local0 = 0x00010000
> > > 37481         }
> > > 37482
> > > 37483         Return (Local0)
> > > 37484     }
> > > [...]
> > >
> > > By a stroke of luck, it turns out it's the correct one! I patched the
> > > driver to query the state on every event and print it out, and it is
> > > exactly what we are looking for.
> > >
> > > The state is 0 if the device is in normal, laptop state and changes to 1
> > > if flipped over 180 degrees. I patched the module so that the
> > > SW_TABLET_MODE switch was set according to it, and everything seems to
> > > be behaving as it should.
> >
> > Good work on figuring this out!
>
> I'm glad to have learned something new. ACPI was mostly a magic black
> box for me up to now.
>
> > > This is, of course, not a full solution, as we
> > > still somehow need to decide whether to use the KDB_DOCK device or this
> > > one. I don't know what to do about that. Ideally find some flag in the
> > > ACPI which says which one we should use?
> > >
> > > The event code which is fired when the lid switch state changes, as we
> > > already know from the sparse keymap[1], is 0xfa. When the laptop is
> > > suspended in laptop mode, flipped to tablet mode in its sleep and
> > > awoken, the event is fired. It is, however, not fired when doing it the
> > > other way around, so we should probably check the state on resume as
> > > well.
> >
> > Ok, I've written a patch to try and use the 0x00060062 WMI object/devid
> > first and only if that is not there use the 0x00120063 one which the
> > Bay Trail and Cherry Trail devices use.
>
> Yeah, that's the solution I had in mind as well and should hopefully be
> fine. Until ASUS ships a device with yet another weird firmware quirk,
> anyway.
>
> > I've attached the patch, please give it a try.
>
> I've tested the patch on the laptop applied on top of 5.8.8 and it works
> as it should!

Great, thank you for testing.

> The patch itself looks good to me, but I have one tiny nitpick: A typo
> in my name on line 15. I feel bad for even mentioning that though.

No need to feel bad, I know the feeling, many people mangle
my last name. So I always try to get this right.

So I will fix the mis-spelling of your name and change the:

Reported-by: Samuel Čavoj <samuel@xxxxxxxxx>

to:

Reported-and-tested-by: Samuel Čavoj <samuel@xxxxxxxxx>

And then submit this upstream.

> I'm glad we are able to resolve the issue so quickly. I was going to say
> that the report on bugzilla should be addressed also, but I see you have
> already done that.

Yes, I just hope that the fix works for the model in the
bugzilla too. I expect it will, but you never know.

> Thank you for kindly your work.

You're welcome, thank you for helping me quickly address this
regression.

Regards,

Hans