2022-10-04 10:50:47

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH 1/2] 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)
Link: https://bugzilla.suse.com/show_bug.cgi?id=1203794
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Len Brown <[email protected]>
Cc: [email protected]
Cc: Chuanhong Guo <[email protected]>
Cc: Tighe Donnelly <[email protected]>
Cc: Mario Limonciello <[email protected]>
Cc: Fridrich Strba <[email protected]>
Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
---
drivers/acpi/resource.c | 42 +++++++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 514d89656dde..8d13e94bb921 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -424,17 +424,31 @@ static const struct dmi_system_id asus_laptop[] = {
{ }
};

+static const struct dmi_system_id lenovo_82ra[] = {
+ {
+ .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_82ra, 6, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
+ { lenovo_82ra, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
};

static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
@@ -442,6 +456,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
@@ -452,17 +477,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;
}

--
2.37.3


2022-10-04 21:05:27

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH 1/2] ACPI: resource: do IRQ override on LENOVO IdeaPad

[Public]



> -----Original Message-----
> From: Jiri Slaby (SUSE) <[email protected]>
> Sent: Tuesday, October 4, 2022 05:34
> To: [email protected]
> Cc: [email protected]; Jiri Slaby (SUSE) <[email protected]>; Rafael
> J. Wysocki <[email protected]>; Len Brown <[email protected]>; linux-
> [email protected]; Chuanhong Guo <[email protected]>; Tighe
> Donnelly <[email protected]>; Limonciello, Mario
> <[email protected]>; Fridrich Strba <[email protected]>
> Subject: [PATCH 1/2] 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.

Unfortunately this is the second case that popped up very recently.
Another one is listed here:
https://bugzilla.kernel.org/show_bug.cgi?id=216552

I don't think we have a good solution to cover the intersection of these bugs. The
existing heuristic to look at legacy syntax and the IOAPIC doesn't work properly
on all the Lenovo and ASUS Ryzen 6000 systems, but it does on these other two.

We're going to be adding more to this table either way. I /suspect/ the better solution
is to revert 37c81d9f1d1b and add to the table all those that are broken.

If there is an agreement I'll try to gather a list of the ones I knew about and we can
build that table.

To make that list not grow very large, we also need to get the IBVs to move to the modern
extended IRQ descriptor syntax for future systems though. I can certainly have that
conversation with the people who talk to them, but it's not going to be effective for
a generation or two if they agree.

>
> Fixes: 37c81d9f1d1b (ACPI: resource: skip IRQ override on AMD Zen platforms)
> Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
> suse.com%2Fshow_bug.cgi%3Fid%3D1203794&amp;data=05%7C01%7Cmario.li
> monciello%40amd.com%7Cdcaa6ca41a97434ebb7308daa5f3ebe1%7C3dd8961f
> e4884e608e11a82d994e183d%7C0%7C0%7C638004764302852606%7CUnknow
> n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=S2udkzf131LnGABs9huj
> %2Fw%2BwyHp9PT0bPaUyaH4rIDY%3D&amp;reserved=0
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: [email protected]
> Cc: Chuanhong Guo <[email protected]>
> Cc: Tighe Donnelly <[email protected]>
> Cc: Mario Limonciello <[email protected]>
> Cc: Fridrich Strba <[email protected]>
> Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
> ---
> drivers/acpi/resource.c | 42 +++++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 514d89656dde..8d13e94bb921 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -424,17 +424,31 @@ static const struct dmi_system_id asus_laptop[] = {
> { }
> };
>
> +static const struct dmi_system_id lenovo_82ra[] = {
> + {
> + .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_82ra, 6, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> + { lenovo_82ra, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true
> },
> };
>
> static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> @@ -442,6 +456,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
> @@ -452,17 +477,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;
> }
>
> --
> 2.37.3

2022-10-05 04:44:10

by Chuanhong Guo

[permalink] [raw]
Subject: Re: [PATCH 1/2] ACPI: resource: do IRQ override on LENOVO IdeaPad

Hi!

On Wed, Oct 5, 2022 at 5:02 AM Limonciello, Mario
<[email protected]> wrote:
> [...]
> >
> > White-list this specific model in the override_table.
> >
> > For this to work, the ZEN test needs to be put below the table walk.
>
> Unfortunately this is the second case that popped up very recently.
> Another one is listed here:
> https://bugzilla.kernel.org/show_bug.cgi?id=216552

Now I'm really curious how Windows is able to handle all these vendor crap...

> I don't think we have a good solution to cover the intersection of these bugs. The
> existing heuristic to look at legacy syntax and the IOAPIC doesn't work properly
> on all the Lenovo and ASUS Ryzen 6000 systems, but it does on these other two.

These legacy IRQ declarations are obsolete, but they aren't really wrong.
Meanwhile the two devices popped up until now both got IRQ declarations which
don't match the actual device configuration.

> We're going to be adding more to this table either way. I /suspect/ the better solution
> is to revert 37c81d9f1d1b and add to the table all those that are broken.

I think we should have a list of only the wrong IRQ declaration and
apply the fix
just for them, instead of applying the fix to all devices and skip it
for selected
devices the fix breaks.

--
Regards,
Chuanhong Guo

2022-10-05 17:45:46

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH 1/2] ACPI: resource: do IRQ override on LENOVO IdeaPad

[Public]



> -----Original Message-----
> From: Chuanhong Guo <[email protected]>
> Sent: Tuesday, October 4, 2022 23:02
> To: Limonciello, Mario <[email protected]>
> Cc: Jiri Slaby (SUSE) <[email protected]>; [email protected]; linux-
> [email protected]; Rafael J. Wysocki <[email protected]>; Len Brown
> <[email protected]>; [email protected]; Tighe Donnelly
> <[email protected]>; Fridrich Strba <[email protected]>
> Subject: Re: [PATCH 1/2] ACPI: resource: do IRQ override on LENOVO
> IdeaPad
>
> Hi!
>
> On Wed, Oct 5, 2022 at 5:02 AM Limonciello, Mario
> <[email protected]> wrote:
> > [...]
> > >
> > > White-list this specific model in the override_table.
> > >
> > > For this to work, the ZEN test needs to be put below the table walk.
> >
> > Unfortunately this is the second case that popped up very recently.
> > Another one is listed here:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D216552&amp;data=05%7C01%7CM
> ario.Limonciello%40amd.com%7C27a32c2395ed4d2a85e208daa68666bb%7C3
> dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638005393451041667%7C
> Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
> TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=iLj95fy44%
> 2BT2KCahzTD8HP2bl2dD6gXVOcVnHylPWJc%3D&amp;reserved=0
>
> Now I'm really curious how Windows is able to handle all these vendor crap...
>
> > I don't think we have a good solution to cover the intersection of these
> bugs. The
> > existing heuristic to look at legacy syntax and the IOAPIC doesn't work
> properly
> > on all the Lenovo and ASUS Ryzen 6000 systems, but it does on these other
> two.
>
> These legacy IRQ declarations are obsolete, but they aren't really wrong.
> Meanwhile the two devices popped up until now both got IRQ declarations
> which
> don't match the actual device configuration.

You're right; both of these are technically BIOS DSDT bugs if you had assumed that this
workaround wasn't in place.

>
> > We're going to be adding more to this table either way. I /suspect/ the
> better solution
> > is to revert 37c81d9f1d1b and add to the table all those that are broken.
>
> I think we should have a list of only the wrong IRQ declaration and
> apply the fix
> just for them, instead of applying the fix to all devices and skip it
> for selected
> devices the fix breaks.

OK. In that case Jiri I think your patch series makes sense.

>
> --
> Regards,
> Chuanhong Guo

2022-10-13 19:26:52

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 1/2] ACPI: resource: do IRQ override on LENOVO IdeaPad

On Tue, Oct 4, 2022 at 12:33 PM Jiri Slaby (SUSE) <[email protected]> wrote:
>
> 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)
> Link: https://bugzilla.suse.com/show_bug.cgi?id=1203794
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: [email protected]
> Cc: Chuanhong Guo <[email protected]>
> Cc: Tighe Donnelly <[email protected]>
> Cc: Mario Limonciello <[email protected]>
> Cc: Fridrich Strba <[email protected]>
> Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
> ---
> drivers/acpi/resource.c | 42 +++++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 514d89656dde..8d13e94bb921 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -424,17 +424,31 @@ static const struct dmi_system_id asus_laptop[] = {
> { }
> };
>
> +static const struct dmi_system_id lenovo_82ra[] = {
> + {
> + .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_82ra, 6, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> + { lenovo_82ra, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> };
>
> static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> @@ -442,6 +456,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
> @@ -452,17 +477,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;
> }
>
> --

Applied along with the [2/2] as 6.1-rc material, thanks!