Bug 209989

Summary: ECDT EC has wrong GPE No. and overrides DSDT EC - HP Gaming Pavilion - 15-cx0095tx
Product: ACPI Reporter: Shao Fu, Chen (leo881003)
Component: ECAssignee: Zhang Rui (rui.zhang)
Status: CLOSED CODE_FIX    
Severity: normal CC: rui.zhang
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 5.9.2 Subsystem:
Regression: No Bisected commit-id:
Attachments: The patch I currently use
The decompiled ECDT
Part of the DSDT to provide the EC information
The dmidecode of the Laptop
quirk patch to use ECDT GPE for this HP laptop
dmesg with "dyndbg=module acpi +p"
Full acpi dump
dmesg under my patch
dmesg of Arch Linux official kernel
debug patch to avoid duplicate DSDT EC with ECDT EC if GPEs are different
debug patch: quirk list for trusting DSDT GPE
dmesg after applying #297281
dmesg after applying #297283
Patch target for upstream

Description Shao Fu, Chen 2020-11-01 06:24:54 UTC
Created attachment 293345 [details]
The patch I currently use

My laptop is HP Gaming Pavilion - 15-cx0095tx.

This laptop has the same EC command and data address on ec & boot_ec, however, the GPE number is different. On boot, it uses 0x17 as GPE, but after that, the GPE is 0x14.

Currently, there is a quirk fix for ASUS laptop in acpi_ec_add(), but I think it should not be there.  
(I think this is caused by: 69b957c26b32c3407d1b8cc0d2390b271728db8a)

---

Detailed Information:
- Kernel version: 5.9.2
- Distribution: Arch Linux
- Command line: BOOT_IMAGE=/boot/vmlinuz-linux root=UUID=72ecfe23-1521-442e-a2aa-abcb1dff4180 rw resume=UUID=0872bf8b-b786-428a-9d9e-5aeb967e91eb log_buf_len=64M acpi.debug_layer=0x2fffffff acpi.debug_level=0x2 wmi.debug_event=1 "dyndbg=module acpi +p"
Comment 1 Shao Fu, Chen 2020-11-01 06:27:36 UTC
Created attachment 293347 [details]
The decompiled ECDT
Comment 2 Shao Fu, Chen 2020-11-01 06:30:05 UTC
Created attachment 293349 [details]
Part of the DSDT to provide the EC information
Comment 3 Shao Fu, Chen 2020-11-01 06:41:28 UTC
After apply the patch, my AC adapter events and WMI related hotkeys are back. Since the patch is a dirty fix, it is suppose to break other model computers.
Comment 4 Zhang Rui 2020-11-02 03:47:54 UTC
Let's see what Rafael thinks about this.
Comment 5 Zhang Rui 2021-05-28 05:50:25 UTC
please attach the dmidecode for this laptop.
Comment 6 Shao Fu, Chen 2021-05-30 04:14:13 UTC
Created attachment 297041 [details]
The dmidecode of the Laptop
Comment 7 Zhang Rui 2021-05-31 07:18:37 UTC
Created attachment 297065 [details]
quirk patch to use ECDT GPE for this HP laptop

Please apply this patch on top of latest upstream Linux kernel and check if it works as expected.
Comment 8 Shao Fu, Chen 2021-06-05 09:10:15 UTC
I applied the patch on 5.12.9. However, the patch doesn't work.

Should I expect "Detected system needing ignore DSDT GPE setting." appears in dmesg?
Comment 9 Zhang Rui 2021-06-07 05:55:35 UTC
(In reply to Shao Fu, Chen from comment #8)
> I applied the patch on 5.12.9. However, the patch doesn't work.
> 
> Should I expect "Detected system needing ignore DSDT GPE setting." appears
> in dmesg?

you should only after enabling the EC dynamic debug.

I don't see why it fails.
please attach the output of "dmidecode -s system-manufacturer" and "dmidecode -s system-product-name".

Please make a double check that you're running with the customized kernel, say, you can add a printk in the ec driver probe function, and make sure your kernel can print the new message you added.
Comment 10 Shao Fu, Chen 2021-06-09 05:14:19 UTC
I found the "Detected system needing ignore DSDT GPE setting." message after I add back the `log_buf_len` parameter.

However, the actual issue is not resolved.

It still use the 0x17 as GPE (in ECDT) after boot. It should use 0x14 as GPE (in DSDT) after the boot process. In this case, ignoring DSDT GPE setting cannot be used.
Comment 11 Zhang Rui 2021-06-09 07:45:19 UTC
without your patch,
in acpi_ec_ecdt_probe()
boot_ec->gpe is 0x17
boot_ec->handle is ACPI_ROOT_OBJECT
first_ec is boot_ec

kernel default behavior
acpi_init() (subsys_initcall)
   -> acpi_bus_init()
         -> acpi_ec_ecdt_probe()
               : boot_ec->gpe = 0x17
               : boot_ec->hndle = ACPI_ROOT_OBJECT
      -> acpi_ec_dsdt_probe()
            : nop
   -> acpi_ec_init()
         -> acpi_ec_add()
               -> ec_parse_device
                     : alloc ec struct for EC0 (DSDT EC)
                     : dsdt_ec->gpe = 0x14
                        : with my patch: dsdt_ec->GPE = 0x17
                     : dsdt_ec->handle = EC0
               : boot_ec->handle = EC0
               : dsdt_ec = boot_ec
      -> acpi_ec_ecdt_start()
               : nop

with your patch:
acpi_init() (subsys_initcall)
   -> acpi_bus_init()
         -> acpi_ec_ecdt_probe()
               : boot_ec->gpe = 0x17
               : boot_ec->hndle = ACPI_ROOT_OBJECT
      -> acpi_ec_dsdt_probe()
            : nop
   -> acpi_ec_init()
         -> acpi_ec_add()
               -> ec_parse_device
                     : alloc ec struct for EC0 (DSDT EC)
                     : dsdt_ec->gpe = 0x14
                        : with my patch: dsdt_ec->GPE = 0x17
                     : dsdt_ec->handle = EC0

      -> acpi_ec_ecdt_start()
              : boot_ec->handle = \_SB.PCI0.LPCB.H_EC
              : register a fake ECDT EC object

So the difference is not just about GPE, it actually means we should have a separate device object for ECDT EC, even if the command/data register are duplicated, which suggests the DSDT EC may not be sufficient, and we rely on ECDT EC to work even after DSDT EC is probed.


please attach the full acpidump output using "acpidump > acpidump.out".
please attach the full dmesg output after boot, with ec dynamic debug enabled.
Comment 12 Shao Fu, Chen 2021-06-09 07:55:20 UTC
Created attachment 297255 [details]
dmesg with "dyndbg=module acpi +p"
Comment 13 Shao Fu, Chen 2021-06-09 07:55:56 UTC
Created attachment 297257 [details]
Full acpi dump
Comment 14 Zhang Rui 2021-06-09 07:58:07 UTC
(In reply to Shao Fu, Chen from comment #12)
> Created attachment 297255 [details]
> dmesg with "dyndbg=module acpi +p"

is this done with or without your patch?
Please attach the dmesg output for both cases.
Comment 15 Shao Fu, Chen 2021-06-09 08:14:46 UTC
(In reply to Zhang Rui from comment #14)
> (In reply to Shao Fu, Chen from comment #12)
> > Created attachment 297255 [details]
> > dmesg with "dyndbg=module acpi +p"
> 
> is this done with or without your patch?
> Please attach the dmesg output for both cases.

This is done under the patch provided by you.

I will attach another one after I recompile the kernel and reboot.
Comment 16 Shao Fu, Chen 2021-06-09 09:37:14 UTC
Created attachment 297261 [details]
dmesg under my patch
Comment 17 Zhang Rui 2021-06-09 13:08:47 UTC
and also the dmesg with default kernel, which does not contain our patches.
Comment 18 Shao Fu, Chen 2021-06-10 07:23:08 UTC
Created attachment 297279 [details]
dmesg of Arch Linux official kernel
Comment 19 Zhang Rui 2021-06-10 08:11:00 UTC
Created attachment 297281 [details]
debug patch to avoid duplicate DSDT EC with ECDT EC if GPEs are different

I see.
So the root cause is that, DSDT EC and ECDT EC share the same port address but different GPEs. And it is the DSDT EC gpe that really works.

Please confirm if the debug patch, on top of vanilla Linux, works or not, if yes, I will raise this and try to push an upstream solution.
Comment 20 Zhang Rui 2021-06-10 08:11:57 UTC
Maybe we need a new quirk list for this issue.
Comment 21 Zhang Rui 2021-06-10 08:18:20 UTC
Created attachment 297283 [details]
debug patch: quirk list for trusting DSDT GPE

Please also confirm if this patch fixes the problem or not, on top of vanilla kernel.
Comment 22 Shao Fu, Chen 2021-06-10 16:16:03 UTC
Created attachment 297297 [details]
dmesg after applying #297281

I have tried the "debug patch to avoid duplicate DSDT EC with ECDT EC if GPEs are different". The patch can correctly fix this bug, the ACPI hotkeys and power notification are working.

I will try the other patch ASAP.
Comment 23 Shao Fu, Chen 2021-06-17 05:45:31 UTC
Created attachment 297425 [details]
dmesg after applying #297283

This quirk patch is also works as expected.
Comment 24 Zhang Rui 2021-06-17 06:15:54 UTC
Created attachment 297427 [details]
Patch target for upstream

Great. Thanks for the test.
I changed the code and changlog to come up a version that is targeted for upstream Linux kernel.
In theory this does not contain any functional change.
can you please try this on top of the vanilla kernel and see if it fixes the problem for you?
If yes, I will submit this patch for usptream.
Comment 25 Shao Fu, Chen 2021-06-18 10:44:05 UTC
I applied the patch on 5.12.10 and it works very well now.

Thanks for your help on this issue.
Comment 26 Zhang Rui 2021-06-24 12:55:33 UTC
Patch has been queued for next merge window.
Bug closed. Thanks for reporting, Shaofu!