All of lore.kernel.org
 help / color / mirror / Atom feed
* platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices
@ 2020-09-01 21:55 Samuel Čavoj
  2020-09-02 11:52 ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Samuel Čavoj @ 2020-09-01 21:55 UTC (permalink / raw)
  To: Hans de Goede, Andy Shevchenko, Corentin Chary
  Cc: acpi4asus-user, platform-driver-x86, linux-kernel

Hello!

A bug was introduced with the following commit[1]:

    b0dbd97de: platform/x86: asus-wmi: Add support for SW_TABLET_MODE

The SW_TABLET_MODE switch seems to be always 1 on some devices,
including my UX360CA and a UX390UAK[2].

This can be seen in the output of evtest:

    # evtest /dev/input/by-path/platform-asus-nb-wmi-event
    Input driver version is 1.0.1
    Input device ID: bus 0x19 vendor 0x0 product 0x0 version 0x0
    Input device name: "Asus WMI hotkeys"
    Supported events:
      (...)
      Event type 5 (EV_SW)
        Event code 1 (SW_TABLET_MODE) state 1

And directly results in libinput disabling the trackpad and keyboard via
its tablet-mode mechanism, rendering X.org and Wayland unusable (not even
switching to VT works without sysrq+r):

    # libinput debug-events
    (...)
    -event8   DEVICE_ADDED     Asus WMI hotkeys     seat0 default group10 cap:kS
     event8   SWITCH_TOGGLE    +0.000s	switch tablet-mode state 1
    (...)

I have been using the following workaround to get my input working
again:

    # cat /usr/share/libinput/50-system-asus.quirks
    (...)
    [Asus WMI hotkeys]
    MatchName=*Asus WMI hotkeys*
    ModelTabletModeSwitchUnreliable=1

Another option would be to rmmod asus_nb_wmi and blacklist it for now.

I am not sure what the solution would be as I am not acquainted with the
WMI module. However, I can provide some information about my hardware:

The UX360CA fully disables the keyboard in hardware(firmware?) when the
lid is flipped beyond 180 degrees (tablet mode). The trackpad is not
disabled. A KEY_PROG2 event is generated by the same "Asus WMI hotkeys"
input device at this moment, it however does not carry the actual state
-- a 1 is sent and a 0 follows immediately[3]. The same KEY_PROG2
sequence is generated when the lid is returned back to laptop position.
The SW_TABLET_MODE switch does not change state at all during this.
Thank you.

Have a nice day,
Samuel

[1]: https://patchwork.kernel.org/patch/11539215/
[2]: https://bugzilla.kernel.org/show_bug.cgi?id=209011
[3]: https://lore.kernel.org/patchwork/patch/973647/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices
  2020-09-01 21:55 platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices Samuel Čavoj
@ 2020-09-02 11:52 ` Hans de Goede
       [not found]   ` <20200902125220.25x52dl2vupejg5f@fastboi.localdomain>
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2020-09-02 11:52 UTC (permalink / raw)
  To: Samuel Čavoj, Andy Shevchenko, Corentin Chary
  Cc: acpi4asus-user, platform-driver-x86, linux-kernel

Hi,

On 9/1/20 11:55 PM, Samuel Čavoj wrote:
> Hello!
> 
> A bug was introduced with the following commit[1]:
> 
>      b0dbd97de: platform/x86: asus-wmi: Add support for SW_TABLET_MODE
> 
> The SW_TABLET_MODE switch seems to be always 1 on some devices,
> including my UX360CA and a UX390UAK[2].
> 
> This can be seen in the output of evtest:
> 
>      # evtest /dev/input/by-path/platform-asus-nb-wmi-event
>      Input driver version is 1.0.1
>      Input device ID: bus 0x19 vendor 0x0 product 0x0 version 0x0
>      Input device name: "Asus WMI hotkeys"
>      Supported events:
>        (...)
>        Event type 5 (EV_SW)
>          Event code 1 (SW_TABLET_MODE) state 1
> 
> And directly results in libinput disabling the trackpad and keyboard via
> its tablet-mode mechanism, rendering X.org and Wayland unusable (not even
> switching to VT works without sysrq+r):
> 
>      # libinput debug-events
>      (...)
>      -event8   DEVICE_ADDED     Asus WMI hotkeys     seat0 default group10 cap:kS
>       event8   SWITCH_TOGGLE    +0.000s	switch tablet-mode state 1
>      (...)
> 
> I have been using the following workaround to get my input working
> again:
> 
>      # cat /usr/share/libinput/50-system-asus.quirks
>      (...)
>      [Asus WMI hotkeys]
>      MatchName=*Asus WMI hotkeys*
>      ModelTabletModeSwitchUnreliable=1
> 
> Another option would be to rmmod asus_nb_wmi and blacklist it for now.
> 
> I am not sure what the solution would be as I am not acquainted with the
> WMI module. However, I can provide some information about my hardware:
> 
> The UX360CA fully disables the keyboard in hardware(firmware?) when the
> lid is flipped beyond 180 degrees (tablet mode). The trackpad is not
> disabled. A KEY_PROG2 event is generated by the same "Asus WMI hotkeys"
> input device at this moment, it however does not carry the actual state
> -- a 1 is sent and a 0 follows immediately[3]. The same KEY_PROG2
> sequence is generated when the lid is returned back to laptop position.
> The SW_TABLET_MODE switch does not change state at all during this.
> Thank you.

Thank you for your detailed bug report.

I have only tested the new TABLET_MODE support on Bay Trail and
Cherry Trail based devices. So one possible solution would be to
limit the support based on cpu-id.

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

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 ?

Regards,

Hans


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices
       [not found]       ` <320c0b71-af94-c673-21c8-c32a0fdb4d4e@redhat.com>
@ 2020-09-04 17:17         ` Samuel Čavoj
  2020-09-10 17:44           ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Samuel Čavoj @ 2020-09-04 17:17 UTC (permalink / raw)
  To: Hans de Goede, Andy Shevchenko, Corentin Chary
  Cc: acpi4asus-user, platform-driver-x86, linux-kernel

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. 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.

Please don't hesitate to ask for any additional testing or information
required from my side.

Regards,
Sam

P.S.: I'm adding back the lists and other people I addressed initially.

[1]: https://lore.kernel.org/patchwork/patch/973647/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices
  2020-09-04 17:17         ` Samuel Čavoj
@ 2020-09-10 17:44           ` Hans de Goede
  2020-09-10 22:31             ` Samuel Čavoj
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2020-09-10 17:44 UTC (permalink / raw)
  To: Samuel Čavoj, Andy Shevchenko, Corentin Chary
  Cc: acpi4asus-user, platform-driver-x86, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3908 bytes --]

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!

> 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.

I've attached the patch, please give it a try.

Regards,

Hans

p.s.

I did successfully receive your acpidump, thanks.

[-- Attachment #2: 0001-platform-x86-asus-wmi-Fix-SW_TABLET_MODE-always-repo.patch --]
[-- Type: text/x-patch, Size: 4342 bytes --]

From 5f5dc9a48b5b71f44b25727cf241e2533dc8061b Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 10 Sep 2020 18:06:37 +0200
Subject: [PATCH] platform/x86: asus-wmi: Fix SW_TABLET_MODE always reporting 1
 on the Asus UX360CA
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On the Asus UX360CA the ASUS_WMI_DEVID_KBD_DOCK devstate always reports 0,
which we translate to SW_TABLET_MODE=1. Which causes libinput to disable
the keyboard and touchpad even if the 360 degree hinges style 2-in-1 is
in laptop mode.

Samual found out that this model has another WMI "object" with an devid of
0x00060062 which correctly reports laptop vs tablet-mode on the
Asus UX360CA.

All the models on which the SW_TABLET_MODE code was previously tested do
not have this new devid 0x00060062 object.

This commit adds support for the new devid 0x00060062 object and prefers it
over the older 0x00120063 object when present, fixing SW_TABLET_MODE always
being reported as 1 on these models.

Reported-by: Samuel Čavoj <samuel@cavoj.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/asus-wmi.c            | 32 ++++++++++++++++++----
 include/linux/platform_data/x86/asus-wmi.h |  1 +
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 8f4acdc06b13..c8689da0323b 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -63,6 +63,7 @@ MODULE_LICENSE("GPL");
 #define NOTIFY_KBD_BRTTOGGLE		0xc7
 #define NOTIFY_KBD_FBM			0x99
 #define NOTIFY_KBD_TTP			0xae
+#define NOTIFY_FLIP_TABLET_MODE_CHANGE	0xfa
 
 #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
 
@@ -173,6 +174,7 @@ struct asus_wmi {
 	int spec;
 	int sfun;
 	bool wmi_event_queue;
+	bool use_flip_tablet_mode;
 
 	struct input_dev *inputdev;
 	struct backlight_device *backlight_device;
@@ -365,12 +367,22 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
 	if (err)
 		goto err_free_dev;
 
-	result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_DOCK);
+	result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_FLIP_TABLET_MODE);
 	if (result >= 0) {
+		asus->use_flip_tablet_mode = true;
 		input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
-		input_report_switch(asus->inputdev, SW_TABLET_MODE, !result);
-	} else if (result != -ENODEV) {
-		pr_err("Error checking for keyboard-dock: %d\n", result);
+		input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
+	} else {
+		if (result != -ENODEV)
+			pr_err("Error checking for flip-tablet-mode: %d\n", result);
+
+		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_DOCK);
+		if (result >= 0) {
+			input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
+			input_report_switch(asus->inputdev, SW_TABLET_MODE, !result);
+		} else if (result != -ENODEV) {
+			pr_err("Error checking for keyboard-dock: %d\n", result);
+		}
 	}
 
 	err = input_register_device(asus->inputdev);
@@ -2114,7 +2126,7 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 
-	if (code == NOTIFY_KBD_DOCK_CHANGE) {
+	if (!asus->use_flip_tablet_mode && code == NOTIFY_KBD_DOCK_CHANGE) {
 		result = asus_wmi_get_devstate_simple(asus,
 						      ASUS_WMI_DEVID_KBD_DOCK);
 		if (result >= 0) {
@@ -2125,6 +2137,16 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 
+	if (asus->use_flip_tablet_mode && code == NOTIFY_FLIP_TABLET_MODE_CHANGE) {
+		result = asus_wmi_get_devstate_simple(asus,
+						      ASUS_WMI_DEVID_FLIP_TABLET_MODE);
+		if (result >= 0) {
+			input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
+			input_sync(asus->inputdev);
+		}
+		return;
+	}
+
 	if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
 		fan_boost_mode_switch_next(asus);
 		return;
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 897b8332a39f..1897b4683562 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -62,6 +62,7 @@
 
 /* Misc */
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
+#define ASUS_WMI_DEVID_FLIP_TABLET_MODE	0x00060062
 
 /* Storage */
 #define ASUS_WMI_DEVID_CARDREADER	0x00080013
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices
  2020-09-10 17:44           ` Hans de Goede
@ 2020-09-10 22:31             ` Samuel Čavoj
  2020-09-11 13:24               ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Samuel Čavoj @ 2020-09-10 22:31 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Andy Shevchenko, Corentin Chary, acpi4asus-user,
	platform-driver-x86, linux-kernel

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!

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.

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. Thank you for kindly your work.

Regards,
Samuel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices
  2020-09-10 22:31             ` Samuel Čavoj
@ 2020-09-11 13:24               ` Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2020-09-11 13:24 UTC (permalink / raw)
  To: Samuel Čavoj
  Cc: Andy Shevchenko, Corentin Chary, acpi4asus-user,
	platform-driver-x86, linux-kernel

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@cavoj.net>

to:

Reported-and-tested-by: Samuel Čavoj <samuel@cavoj.net>

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-11 15:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 21:55 platform/x86: asus-wmi: SW_TABLET_MODE is always 1 on some devices Samuel Čavoj
2020-09-02 11:52 ` Hans de Goede
     [not found]   ` <20200902125220.25x52dl2vupejg5f@fastboi.localdomain>
     [not found]     ` <20200904094546.jes44d2kn5mtn2zu@fastboi.localdomain>
     [not found]       ` <320c0b71-af94-c673-21c8-c32a0fdb4d4e@redhat.com>
2020-09-04 17:17         ` Samuel Čavoj
2020-09-10 17:44           ` Hans de Goede
2020-09-10 22:31             ` Samuel Čavoj
2020-09-11 13:24               ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.