lkml.org 
[lkml]   [2022]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v6] ACPI: skip IRQ override on AMD Zen platforms
From
On 28. 09. 22, 8:20, Jiri Slaby wrote:
> Hi,
>
> On 12. 07. 22, 4:00, Chuanhong Guo wrote:
>> IRQ override isn't needed on modern AMD Zen systems.
>> There's an active low keyboard IRQ on AMD Ryzen 6000 and it will stay
>> this way on newer platforms. This IRQ override breaks keyboards for
>> almost all Ryzen 6000 laptops currently on the market.
>>
>> Skip this IRQ override for all AMD Zen platforms because this IRQ
>> override is supposed to be a workaround for buggy ACPI DSDT and we can't
>> have a long list of all future AMD CPUs/Laptops in the kernel code.
>> If a device with buggy ACPI DSDT shows up, a separated list containing
>> just them should be created.
>
> This breaks pads on IdeaPad 5 Flex:
> https://bugzilla.suse.com/show_bug.cgi?id=1203794
>
> > [    1.058135] hid-generic 0020:1022:0001.0001: hidraw0: SENSOR HUB
> HID v0.00 Device [hid-amdsfh 1022:0001] on pcie_mp2_amd
> > [    2.038937] i2c_designware AMDI0010:00: controller timed out
> > [    2.146627] i2c_designware AMDI0010:03: controller timed out
> > [    6.166859] i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
> > [    8.279604] i2c_designware AMDI0010:03: controller timed out
> > [   12.310897] i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
> > [   14.429372] i2c_designware AMDI0010:03: controller timed out
> > [   18.462629] i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
> > [   20.579183] i2c_designware AMDI0010:03: controller timed out
> > [   24.598703] i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
> > [   25.629071] i2c_hid_acpi i2c-MSFT0001:00: can't add hid device: -61
> > [   25.629430] i2c_hid_acpi: probe of i2c-MSFT0001:00 failed with
> error -61
>
> The diff of good and bad dmesgs:
> -ACPI: IRQ 10 override to edge, high
> -ACPI: IRQ 6 override to edge, high
>
> The diff of /proc/interrupts:
>      6: ...  IR-IO-APIC    [-6-fasteoi-]    {+6-edge+}      AMDI0010:03
>     10: ...  IR-IO-APIC   [-10-fasteoi-]   {+10-edge+}      AMDI0010:00
>
> And:
>   i2c_designware: /devices/platform/AMDI0010:00
>   i2c_designware: /devices/platform/AMDI0010:03
>
>
> So the if needs to be fine-tuned, apparently. Maybe introduce some list
> as suggested in the commit log. Based on the below?

Something like the attached. It's:
1) untested yet
2) contains more debug messaging
3) contains both cases for ACPI_ACTIVE_* as I don't know the original
polarity

I don't know how widely this is spread -- maybe it would be worth a
commandline parameter so that people can work around this until this is
fixed by a DMI entry permanently?

> DMI says:
>   System Info: #14
>     Manufacturer: "LENOVO"
>     Product: "82RA"
>     Version: "IdeaPad Flex 5 16ALC7"
>     Serial: "PW02359K"
>     UUID: 6b2d54d9-cd80-11ec-83eb-e00af665fbac
>     Wake-up: 0x06 (Power Switch)
>   Board Info: #15
>     Manufacturer: "LENOVO"
>     Product: "LNVNB161216"
>     Version: "SDK0T76463 WIN"
>     Serial: "PW02359K"
>     Asset Tag: "No Asset Tag"
>     Type: 0x0a (Motherboard)
>     Features: 0x09
>       Hosting Board
>       Replaceable
>     Location: "Chassis Location Unknown"
>   Chassis Info: #16
>     Manufacturer: "LENOVO"
>     Version: "IdeaPad Flex 5 16ALC7"
>     Serial: "PW02359K"
>     Asset Tag: "No Asset Tag"
>     Type: 0x1f (Other)
>     Bootup State: 0x03 (Safe)
>     Power Supply State: 0x03 (Safe)
>     Thermal State: 0x01 (Other)
>     Security Status: 0x01 (Other)
>
>> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
>> ---
>> Change sice v5: reworked
>>
>>   drivers/acpi/resource.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index c2d494784425..510cdec375c4 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -416,6 +416,16 @@ static bool acpi_dev_irq_override(u32 gsi, u8
>> triggering, u8 polarity,
>>   {
>>       int i;
>> +#ifdef CONFIG_X86
>> +    /*
>> +     * IRQ override isn't needed on modern AMD Zen systems and
>> +     * this override breaks active low IRQs on AMD Ryzen 6000 and
>> +     * newer systems. Skip it.
>> +     */
>> +    if (boot_cpu_has(X86_FEATURE_ZEN))
>> +        return false;
>> +#endif
>> +
>>       for (i = 0; i < ARRAY_SIZE(skip_override_table); i++) {
>>           const struct irq_override_cmp *entry = &skip_override_table[i];
>
> thanks,

--
js
From 485dd1229b9d2f322879d85731af4a7952b9cc4b Mon Sep 17 00:00:00 2001
From: Jiri Slaby <jslaby@suse.cz>
Date: Wed, 28 Sep 2022 09:43:05 +0200
Subject: [PATCH] ACPI: resource: do IRQ override on LENOVO IdeaPad

LENOVO IdeaPad Flex 5 is ryzen-5 based and the commit below removed irq
overriding for those. This broke touchscreen and trackpad:
i2c_designware AMDI0010:00: controller timed out
i2c_designware AMDI0010:03: controller timed out
i2c_hid_acpi i2c-MSFT0001:00: failed to reset device: -61
i2c_designware AMDI0010:03: controller timed out
...
i2c_hid_acpi i2c-MSFT0001:00: can't add hid device: -61
i2c_hid_acpi: probe of i2c-MSFT0001:00 failed with error -61

White list this specific model in the override_table.

For this to work, the ZEN test needs to be put below the table walk.

Fixes: 37c81d9f1d1b (ACPI: resource: skip IRQ override on AMD Zen platforms)
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
drivers/acpi/resource.c | 51 ++++++++++++++++++++++++++++-------------
1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index e40b2153911d..ab25e93cf22a 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -417,17 +417,33 @@ static const struct dmi_system_id asus_laptop[] = {
{ }
};

+static const struct dmi_system_id lenovo_laptop[] = {
+ {
+ .ident = "LENOVO IdeaPad Flex 5 16ALC7",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
+ },
+ },
+ { }
+};
+
struct irq_override_cmp {
const struct dmi_system_id *system;
unsigned char irq;
unsigned char triggering;
unsigned char polarity;
unsigned char shareable;
+ bool override;
};

-static const struct irq_override_cmp skip_override_table[] = {
- { medion_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0 },
- { asus_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0 },
+static const struct irq_override_cmp override_table[] = {
+ { medion_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
+ { asus_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
+ { lenovo_laptop, 6, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
+ { lenovo_laptop, 10, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
+ { lenovo_laptop, 6, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_HIGH, 0, true },
+ { lenovo_laptop, 10, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_HIGH, 0, true },
};

static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
@@ -435,6 +451,17 @@ static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
{
int i;

+ for (i = 0; i < ARRAY_SIZE(override_table); i++) {
+ const struct irq_override_cmp *entry = &override_table[i];
+
+ if (dmi_check_system(entry->system) &&
+ entry->irq == gsi &&
+ entry->triggering == triggering &&
+ entry->polarity == polarity &&
+ entry->shareable == shareable)
+ return entry->override;
+ }
+
#ifdef CONFIG_X86
/*
* IRQ override isn't needed on modern AMD Zen systems and
@@ -445,17 +472,6 @@ static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
return false;
#endif

- for (i = 0; i < ARRAY_SIZE(skip_override_table); i++) {
- const struct irq_override_cmp *entry = &skip_override_table[i];
-
- if (dmi_check_system(entry->system) &&
- entry->irq == gsi &&
- entry->triggering == triggering &&
- entry->polarity == polarity &&
- entry->shareable == shareable)
- return false;
- }
-
return true;
}

@@ -487,8 +503,11 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;

if (triggering != trig || polarity != pol) {
- pr_warn("ACPI: IRQ %d override to %s, %s\n", gsi,
- t ? "level" : "edge", p ? "low" : "high");
+ pr_warn("ACPI: IRQ %d override to %s%s, %s%s\n", gsi,
+ t ? "level" : "edge",
+ trig == triggering ? "" : "(!)",
+ p ? "low" : "high",
+ pol == polarity ? "" : "(!)");
triggering = trig;
polarity = pol;
}
--
2.37.3
\
 
 \ /
  Last update: 2022-09-28 10:32    [W:0.103 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site