diff mbox series

[1/2] ACPI: resource: do IRQ override on LENOVO IdeaPad

Message ID 20221004103341.12646-1-jirislaby@kernel.org
State Accepted
Commit bfcdf58380b1d9be564a78a9370da722ed1a9965
Headers show
Series [1/2] ACPI: resource: do IRQ override on LENOVO IdeaPad | expand

Commit Message

Jiri Slaby (SUSE) Oct. 4, 2022, 10:33 a.m. UTC
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" <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
Cc: Chuanhong Guo <gch981213@gmail.com>
Cc: Tighe Donnelly <tighe.donnelly@protonmail.com>
Cc: Mario Limonciello <mario.limonciello@amd.com>
Cc: Fridrich Strba <fstrba@suse.com>
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/acpi/resource.c | 42 +++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

Comments

Mario Limonciello Oct. 4, 2022, 9:02 p.m. UTC | #1
[Public]



> -----Original Message-----
> From: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Sent: Tuesday, October 4, 2022 05:34
> To: rafael.j.wysocki@intel.com
> Cc: linux-kernel@vger.kernel.org; Jiri Slaby (SUSE) <jirislaby@kernel.org>; Rafael
> J. Wysocki <rafael@kernel.org>; Len Brown <lenb@kernel.org>; linux-
> acpi@vger.kernel.org; Chuanhong Guo <gch981213@gmail.com>; Tighe
> Donnelly <tighe.donnelly@protonmail.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Fridrich Strba <fstrba@suse.com>
> 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" <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> Cc: Chuanhong Guo <gch981213@gmail.com>
> Cc: Tighe Donnelly <tighe.donnelly@protonmail.com>
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Cc: Fridrich Strba <fstrba@suse.com>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  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
Chuanhong Guo Oct. 5, 2022, 4:02 a.m. UTC | #2
Hi!

On Wed, Oct 5, 2022 at 5:02 AM Limonciello, Mario
<Mario.Limonciello@amd.com> 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.
Mario Limonciello Oct. 5, 2022, 4:34 p.m. UTC | #3
[Public]



> -----Original Message-----
> From: Chuanhong Guo <gch981213@gmail.com>
> Sent: Tuesday, October 4, 2022 23:02
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Jiri Slaby (SUSE) <jirislaby@kernel.org>; rafael.j.wysocki@intel.com; linux-
> kernel@vger.kernel.org; Rafael J. Wysocki <rafael@kernel.org>; Len Brown
> <lenb@kernel.org>; linux-acpi@vger.kernel.org; Tighe Donnelly
> <tighe.donnelly@protonmail.com>; Fridrich Strba <fstrba@suse.com>
> 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
> <Mario.Limonciello@amd.com> 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
Rafael J. Wysocki Oct. 13, 2022, 6:51 p.m. UTC | #4
On Tue, Oct 4, 2022 at 12:33 PM Jiri Slaby (SUSE) <jirislaby@kernel.org> 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" <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> Cc: Chuanhong Guo <gch981213@gmail.com>
> Cc: Tighe Donnelly <tighe.donnelly@protonmail.com>
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Cc: Fridrich Strba <fstrba@suse.com>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  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!
diff mbox series

Patch

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;
 }