Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede <hdegoede@redhat.com>]

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

Hi Rafael,

On 7/29/24 2:20 PM, Rafael J. Wysocki wrote:
> Hi Hans,
> 
> On Mon, Jul 29, 2024 at 1:29 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 7/29/24 1:15 PM, Hans de Goede wrote:
>>> Hi Rafael,
>>>
>>> There are 2 bug reports:
>>>
>>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
>>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
>>>
>>> 2. EC related ACPI errors and bad performance:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
>>>
>>> Both of which started with 6.9.7 which has the 2 commits related to "EC: Install
>>> address space handler at the namespace root" from 6.10 backported:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=2b2b0ac1533d790690d6d28899f01a2924d54d4d
>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=9750135f2f326879889ed60ccd68b96572dfa6ee
>>>
>>> i have build a test 6.9.9 kernel with these 2 reverted and 1. is confirmed to be fixed
>>> by reverting these 2 commits. Although the user does report an IRQ storm on the ACPI IRQ
>>> (IRQ 9) related to thunderbolt after this.
>>>
>>> I have not yet got confirmation that the second bug is also resolved by the commits.
>>
>> ... resolved by *reverting* the commits.
>>
>>> Either way it looks like we need to dig into this and figure out what is causing
>>> these EC related regressions.
> 
> Right, so I looked at the dmesg output in 2. and saw that the EC
> errors were reported right after enabling the EC for the first time in
> acpi_ec_dsdt_probe().
> 
> Because acpi_ec_dsdt_probe() passes true as the last argument to
> acpi_ec_setup(), it will evaluate _REG everywhere at this point, but
> previously it only evaluated _REG in the EC scope.
> 
> In the ECDT case, the _REG evaluation is deferred until the EC has
> been found in the namespace, so maybe that's the right time to
> evaluate EC opregions _REG in general.
> 
> So one thing to try may be to pass "false" to acpi_ec_setup() in
> acpi_ec_dsdt_probe().

That is a good idea. I have written the attached patch for this and done
a Fedora kernel test build with that patch added for the reporter of:

https://bugzilla.redhat.com/show_bug.cgi?id=2298938

To test.

Regards,

Hans


[-- Attachment #2: 0001-ACPI-EC-Delay-running-_REG-handlers-until-the-EC-acp.patch --]
[-- Type: text/x-patch, Size: 2905 bytes --]

From ecaed52d8eaa6a302ed8c216598a58ca45f3c148 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 1 Aug 2024 11:27:54 +0200
Subject: [PATCH] ACPI: EC: Delay running _REG handlers until the EC
 acpi_device is enumerated

Since commit 60fa6ae6e6d0 ("ACPI: EC: Install address space handler at
the namespace root") on systems without an ECDT any EC _REG handlers are
now run as soon as acpi_ec_dsdt_probe() runs, where as before only
the _REG handler under the EC acpi_device was run.

This is causing EC related ACPI errors and thermalzone issues on Acer
Aspire ES1-572 laptops:

jul 19 17:33:41 kernel: ACPI: EC: EC started
jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
jul 19 17:33:41 kernel: ACPI: Interpreter enabled

For ECDT declared ECs the calling of _REG is delayed till the acpi_device
for the EC gets enumerated (till acpi_ec_add() is called). Make the path
for ECs which are discovered early on through the DSDT consistent with this
and all also delay the calling of _REG till acpi_device enumeration time
there.

This fixes the mentioned ACPI EC errors and thermalzone issues.

Fixes: 60fa6ae6e6d0 ("ACPI: EC: Install address space handler at the namespace root")
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2298938
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/ec.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 299ec653388c..8929f9215b70 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1788,13 +1788,12 @@ void __init acpi_ec_dsdt_probe(void)
 	}
 
 	/*
-	 * When the DSDT EC is available, always re-configure boot EC to
-	 * have _REG evaluated. _REG can only be evaluated after the
-	 * namespace initialization.
+	 * Delay _REG evaluation until the EC is found by regular ACPI device
+	 * hierarchy parsing and acpi_ec_add() is called for the EC.
 	 * At this point, the GPE is not fully initialized, so do not to
 	 * handle the events.
 	 */
-	ret = acpi_ec_setup(ec, NULL, true);
+	ret = acpi_ec_setup(ec, NULL, false);
 	if (ret) {
 		acpi_ec_free(ec);
 		return;
-- 
2.45.2