diff mbox series

[v2] ACPI: resource: Remove "Zen" specific match and quirks

Message ID 20230518183920.93472-1-mario.limonciello@amd.com
State New
Headers show
Series [v2] ACPI: resource: Remove "Zen" specific match and quirks | expand

Commit Message

Mario Limonciello May 18, 2023, 6:39 p.m. UTC
commit 9946e39fe8d0 ("ACPI: resource: skip IRQ override on
AMD Zen platforms") attempted to overhaul the override logic so it
didn't apply on X86 AMD Zen systems.  This was intentional so that
systems would prefer DSDT values instead of default MADT value for
IRQ 1 on Ryzen 6000 systems which use ActiveLow for IRQ1.

This turned out to be a bad assumption because several vendors seem
to add Interrupt Source Override but don't fix the DSDT. A pile of
quirks was collecting that proved this wasn't sustaintable.

Adjust the logic so that only IRQ1 is overridden in Ryzen 6000 case.

This effectively reverts the following commits:
commit 17bb7046e7ce ("ACPI: resource: Do IRQ override on all TongFang
GMxRGxx")
commit f3cb9b740869 ("ACPI: resource: do IRQ override on Lenovo 14ALC7")
commit bfcdf58380b1 ("ACPI: resource: do IRQ override on LENOVO IdeaPad")
commit 7592b79ba4a9 ("ACPI: resource: do IRQ override on XMG Core 15")

Cc: ofenfisch@googlemail.com
Cc: wse@tuxedocomputers.com
Cc: adam.niederer@gmail.com
Cc: adrian@freund.io
Cc: jirislaby@kernel.org
Tested-by: Renjith.Pananchikkal@amd.com
Tested-by: anson.tsao@amd.com
Tested-by: Richard.Gong@amd.com
Tested-by: Chuanhong Guo <gch981213@gmail.com>
Reported-by: evilsnoo@proton.me
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
Reported-by: ruinairas1992@gmail.com
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217406
Fixes: 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen platforms")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Rebase on 71a485624c4c ("ACPI: resource: Add IRQ override quirk for LG UltraPC 17U70P")
 * Pick up tag
---
 drivers/acpi/resource.c | 154 +++++++++++++++++-----------------------
 1 file changed, 65 insertions(+), 89 deletions(-)


base-commit: c554eee18c9a440bd2dd5a363b0f79325717f0bf

Comments

Rafael J. Wysocki May 22, 2023, 4:42 p.m. UTC | #1
On Thu, May 18, 2023 at 8:39 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> commit 9946e39fe8d0 ("ACPI: resource: skip IRQ override on
> AMD Zen platforms") attempted to overhaul the override logic so it
> didn't apply on X86 AMD Zen systems.  This was intentional so that
> systems would prefer DSDT values instead of default MADT value for
> IRQ 1 on Ryzen 6000 systems which use ActiveLow for IRQ1.
>
> This turned out to be a bad assumption because several vendors seem
> to add Interrupt Source Override but don't fix the DSDT. A pile of
> quirks was collecting that proved this wasn't sustaintable.
>
> Adjust the logic so that only IRQ1 is overridden in Ryzen 6000 case.
>
> This effectively reverts the following commits:
> commit 17bb7046e7ce ("ACPI: resource: Do IRQ override on all TongFang
> GMxRGxx")
> commit f3cb9b740869 ("ACPI: resource: do IRQ override on Lenovo 14ALC7")
> commit bfcdf58380b1 ("ACPI: resource: do IRQ override on LENOVO IdeaPad")
> commit 7592b79ba4a9 ("ACPI: resource: do IRQ override on XMG Core 15")

Should it also remove the quirk added by commit 71a485624c4c ("ACPI:
resource: Add IRQ override quirk for LG UltraPC 17U70P")?

> Cc: ofenfisch@googlemail.com
> Cc: wse@tuxedocomputers.com
> Cc: adam.niederer@gmail.com
> Cc: adrian@freund.io
> Cc: jirislaby@kernel.org
> Tested-by: Renjith.Pananchikkal@amd.com
> Tested-by: anson.tsao@amd.com
> Tested-by: Richard.Gong@amd.com
> Tested-by: Chuanhong Guo <gch981213@gmail.com>
> Reported-by: evilsnoo@proton.me
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
> Reported-by: ruinairas1992@gmail.com
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217406
> Fixes: 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen platforms")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>  * Rebase on 71a485624c4c ("ACPI: resource: Add IRQ override quirk for LG UltraPC 17U70P")
>  * Pick up tag
> ---
>  drivers/acpi/resource.c | 154 +++++++++++++++++-----------------------
>  1 file changed, 65 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 0800a9d77558..c6ac87e01e1c 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -470,52 +470,6 @@ static const struct dmi_system_id asus_laptop[] = {
>         { }
>  };
>
> -static const struct dmi_system_id lenovo_laptop[] = {
> -       {
> -               .ident = "LENOVO IdeaPad Flex 5 14ALC7",
> -               .matches = {
> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "82R9"),
> -               },
> -       },
> -       {
> -               .ident = "LENOVO IdeaPad Flex 5 16ALC7",
> -               .matches = {
> -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
> -               },
> -       },
> -       { }
> -};
> -
> -static const struct dmi_system_id tongfang_gm_rg[] = {
> -       {
> -               .ident = "TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris 15 Gen4 AMD",
> -               .matches = {
> -                       DMI_MATCH(DMI_BOARD_NAME, "GMxRGxx"),
> -               },
> -       },
> -       { }
> -};
> -
> -static const struct dmi_system_id maingear_laptop[] = {
> -       {
> -               .ident = "MAINGEAR Vector Pro 2 15",
> -               .matches = {
> -                       DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-15A3070T"),
> -               }
> -       },
> -       {
> -               .ident = "MAINGEAR Vector Pro 2 17",
> -               .matches = {
> -                       DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-17A3070T"),
> -               },
> -       },
> -       { }
> -};
> -
>  static const struct dmi_system_id lg_laptop[] = {
>         {
>                 .ident = "LG Electronics 17U70P",
> @@ -527,7 +481,7 @@ static const struct dmi_system_id lg_laptop[] = {
>         { }
>  };
>
> -struct irq_override_cmp {
> +struct irq_override_dmi_cmp {
>         const struct dmi_system_id *system;
>         unsigned char irq;
>         unsigned char triggering;
> @@ -536,50 +490,86 @@ struct irq_override_cmp {
>         bool override;
>  };
>
> -static const struct irq_override_cmp override_table[] = {
> +struct irq_override_acpi_cmp {
> +       const char *id;
> +       unsigned char irq;
> +       unsigned char triggering;
> +       unsigned char polarity;
> +};
> +
> +static const struct irq_override_dmi_cmp dmi_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_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> -       { lenovo_laptop, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> -       { tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
> -       { maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
>         { lg_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
>  };
>
> -static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> -                                 u8 shareable)
> +/*
> + * Ryzen 6000 requires ActiveLow for keyboard, but a number of machines
> + * seem to get it wrong in DSDT or don't have an Interrupt Source
> + * Override.
> + */
> +static const struct irq_override_acpi_cmp acpi_override_table[] = {
> +       { "AMDI0007", 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW },
> +};
> +
> +static void acpi_dev_irq_override(u32 gsi, u8 *triggering, u8 *polarity,
> +                                 u8 *shareable)
>  {
> -       int i;
> +       int i, p, t;
> +       int check_override = true;
>
> -       for (i = 0; i < ARRAY_SIZE(override_table); i++) {
> -               const struct irq_override_cmp *entry = &override_table[i];
> +       for (i = 0; i < ARRAY_SIZE(dmi_override_table); i++) {
> +               const struct irq_override_dmi_cmp *entry = &dmi_override_table[i];
>
>                 if (dmi_check_system(entry->system) &&
>                     entry->irq == gsi &&
> -                   entry->triggering == triggering &&
> -                   entry->polarity == polarity &&
> -                   entry->shareable == shareable)
> -                       return entry->override;
> +                   entry->triggering == *triggering &&
> +                   entry->polarity == *polarity &&
> +                   entry->shareable == *shareable)
> +                       check_override = entry->override;
>         }
>
> -#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
> +       if (!check_override)
> +               return;
>
> -       return true;
> +       if (!acpi_get_override_irq(gsi, &t, &p)) {
> +               u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> +               u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> +
> +               if (*triggering != trig || *polarity != pol) {
> +                       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;
> +               }
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(acpi_override_table); i++) {
> +               const struct irq_override_acpi_cmp *entry = &acpi_override_table[i];
> +
> +               if (acpi_dev_found(entry->id) && gsi == entry->irq &&
> +                  (*polarity != entry->polarity || *triggering != entry->triggering)) {
> +                       pr_warn("ACPI: IRQ %d override to %s%s, %s%s due to %s\n",
> +                               gsi,
> +                               entry->triggering ? "level" : "edge",
> +                               entry->triggering == *triggering ? "" : "(!)",
> +                               entry->polarity ? "low" : "high",
> +                               entry->polarity == *polarity ? "" : "(!)",
> +                               entry->id);
> +                       *polarity = entry->polarity;
> +                       *triggering = entry->triggering;
> +               }
> +       }
>  }
>
>  static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>                                      u8 triggering, u8 polarity, u8 shareable,
>                                      u8 wake_capable, bool check_override)
>  {
> -       int irq, p, t;
> +       int irq;
>
>         if (!valid_IRQ(gsi)) {
>                 irqresource_disabled(res, gsi);
> @@ -592,26 +582,12 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>          * 2. BIOS uses IO-APIC mode Interrupt Source Override
>          *
>          * We do this only if we are dealing with IRQ() or IRQNoFlags()
> -        * resource (the legacy ISA resources). With modern ACPI 5 devices
> +        * resource (the legacy ISA resources). With ACPI devices
>          * using extended IRQ descriptors we take the IRQ configuration
>          * from _CRS directly.
>          */
> -       if (check_override &&
> -           acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
> -           !acpi_get_override_irq(gsi, &t, &p)) {
> -               u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> -               u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> -
> -               if (triggering != trig || polarity != pol) {
> -                       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;
> -               }
> -       }
> +       if (check_override)
> +               acpi_dev_irq_override(gsi, &triggering, &polarity, &shareable);
>
>         res->flags = acpi_dev_irq_flags(triggering, polarity, shareable, wake_capable);
>         irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>
> base-commit: c554eee18c9a440bd2dd5a363b0f79325717f0bf
> --
> 2.34.1
>
Mario Limonciello May 22, 2023, 4:44 p.m. UTC | #2
[AMD Official Use Only - General]

> -----Original Message-----
> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Monday, May 22, 2023 11:42 AM
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: rafael@kernel.org; gch981213@gmail.com; linux-acpi@vger.kernel.org;
> linux-kernel@vger.kernel.org; regressions@leemhuis.info;
> ofenfisch@googlemail.com; wse@tuxedocomputers.com;
> adam.niederer@gmail.com; adrian@freund.io; jirislaby@kernel.org;
> Pananchikkal, Renjith <Renjith.Pananchikkal@amd.com>; Tsao, Anson
> <anson.tsao@amd.com>; Gong, Richard <Richard.Gong@amd.com>;
> evilsnoo@proton.me; ruinairas1992@gmail.com
> Subject: Re: [PATCH v2] ACPI: resource: Remove "Zen" specific match and
> quirks
>
> On Thu, May 18, 2023 at 8:39 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> >
> > commit 9946e39fe8d0 ("ACPI: resource: skip IRQ override on
> > AMD Zen platforms") attempted to overhaul the override logic so it
> > didn't apply on X86 AMD Zen systems.  This was intentional so that
> > systems would prefer DSDT values instead of default MADT value for
> > IRQ 1 on Ryzen 6000 systems which use ActiveLow for IRQ1.
> >
> > This turned out to be a bad assumption because several vendors seem
> > to add Interrupt Source Override but don't fix the DSDT. A pile of
> > quirks was collecting that proved this wasn't sustaintable.
> >
> > Adjust the logic so that only IRQ1 is overridden in Ryzen 6000 case.
> >
> > This effectively reverts the following commits:
> > commit 17bb7046e7ce ("ACPI: resource: Do IRQ override on all TongFang
> > GMxRGxx")
> > commit f3cb9b740869 ("ACPI: resource: do IRQ override on Lenovo
> 14ALC7")
> > commit bfcdf58380b1 ("ACPI: resource: do IRQ override on LENOVO
> IdeaPad")
> > commit 7592b79ba4a9 ("ACPI: resource: do IRQ override on XMG Core 15")
>
> Should it also remove the quirk added by commit 71a485624c4c ("ACPI:
> resource: Add IRQ override quirk for LG UltraPC 17U70P")?

I wondered the same thing, but a quick search showed that
"LG UltraPC 17U70P" is

" LG Ultra PC 17” Lightweight & High Performance Laptop Intel® 11th Gen Core® i7, NVIDIA® GeForce® GTX™ 1650Ti Graphics, Windows 11 Home, 16GB RAM, 512GB SSD, Silver"

So I don't think so.
>
> > Cc: ofenfisch@googlemail.com
> > Cc: wse@tuxedocomputers.com
> > Cc: adam.niederer@gmail.com
> > Cc: adrian@freund.io
> > Cc: jirislaby@kernel.org
> > Tested-by: Renjith.Pananchikkal@amd.com
> > Tested-by: anson.tsao@amd.com
> > Tested-by: Richard.Gong@amd.com
> > Tested-by: Chuanhong Guo <gch981213@gmail.com>
> > Reported-by: evilsnoo@proton.me
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
> > Reported-by: ruinairas1992@gmail.com
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217406
> > Fixes: 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen
> platforms")
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> > v1->v2:
> >  * Rebase on 71a485624c4c ("ACPI: resource: Add IRQ override quirk for LG
> UltraPC 17U70P")
> >  * Pick up tag
> > ---
> >  drivers/acpi/resource.c | 154 +++++++++++++++++-----------------------
> >  1 file changed, 65 insertions(+), 89 deletions(-)
> >
> > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > index 0800a9d77558..c6ac87e01e1c 100644
> > --- a/drivers/acpi/resource.c
> > +++ b/drivers/acpi/resource.c
> > @@ -470,52 +470,6 @@ static const struct dmi_system_id asus_laptop[] = {
> >         { }
> >  };
> >
> > -static const struct dmi_system_id lenovo_laptop[] = {
> > -       {
> > -               .ident = "LENOVO IdeaPad Flex 5 14ALC7",
> > -               .matches = {
> > -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> > -                       DMI_MATCH(DMI_PRODUCT_NAME, "82R9"),
> > -               },
> > -       },
> > -       {
> > -               .ident = "LENOVO IdeaPad Flex 5 16ALC7",
> > -               .matches = {
> > -                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> > -                       DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
> > -               },
> > -       },
> > -       { }
> > -};
> > -
> > -static const struct dmi_system_id tongfang_gm_rg[] = {
> > -       {
> > -               .ident = "TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris
> 15 Gen4 AMD",
> > -               .matches = {
> > -                       DMI_MATCH(DMI_BOARD_NAME, "GMxRGxx"),
> > -               },
> > -       },
> > -       { }
> > -};
> > -
> > -static const struct dmi_system_id maingear_laptop[] = {
> > -       {
> > -               .ident = "MAINGEAR Vector Pro 2 15",
> > -               .matches = {
> > -                       DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> > -                       DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-15A3070T"),
> > -               }
> > -       },
> > -       {
> > -               .ident = "MAINGEAR Vector Pro 2 17",
> > -               .matches = {
> > -                       DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> > -                       DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-17A3070T"),
> > -               },
> > -       },
> > -       { }
> > -};
> > -
> >  static const struct dmi_system_id lg_laptop[] = {
> >         {
> >                 .ident = "LG Electronics 17U70P",
> > @@ -527,7 +481,7 @@ static const struct dmi_system_id lg_laptop[] = {
> >         { }
> >  };
> >
> > -struct irq_override_cmp {
> > +struct irq_override_dmi_cmp {
> >         const struct dmi_system_id *system;
> >         unsigned char irq;
> >         unsigned char triggering;
> > @@ -536,50 +490,86 @@ struct irq_override_cmp {
> >         bool override;
> >  };
> >
> > -static const struct irq_override_cmp override_table[] = {
> > +struct irq_override_acpi_cmp {
> > +       const char *id;
> > +       unsigned char irq;
> > +       unsigned char triggering;
> > +       unsigned char polarity;
> > +};
> > +
> > +static const struct irq_override_dmi_cmp dmi_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_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true
> },
> > -       { lenovo_laptop, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true
> },
> > -       { tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1,
> true },
> > -       { maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1,
> true },
> >         { lg_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
> >  };
> >
> > -static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> > -                                 u8 shareable)
> > +/*
> > + * Ryzen 6000 requires ActiveLow for keyboard, but a number of machines
> > + * seem to get it wrong in DSDT or don't have an Interrupt Source
> > + * Override.
> > + */
> > +static const struct irq_override_acpi_cmp acpi_override_table[] = {
> > +       { "AMDI0007", 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW },
> > +};
> > +
> > +static void acpi_dev_irq_override(u32 gsi, u8 *triggering, u8 *polarity,
> > +                                 u8 *shareable)
> >  {
> > -       int i;
> > +       int i, p, t;
> > +       int check_override = true;
> >
> > -       for (i = 0; i < ARRAY_SIZE(override_table); i++) {
> > -               const struct irq_override_cmp *entry = &override_table[i];
> > +       for (i = 0; i < ARRAY_SIZE(dmi_override_table); i++) {
> > +               const struct irq_override_dmi_cmp *entry =
> &dmi_override_table[i];
> >
> >                 if (dmi_check_system(entry->system) &&
> >                     entry->irq == gsi &&
> > -                   entry->triggering == triggering &&
> > -                   entry->polarity == polarity &&
> > -                   entry->shareable == shareable)
> > -                       return entry->override;
> > +                   entry->triggering == *triggering &&
> > +                   entry->polarity == *polarity &&
> > +                   entry->shareable == *shareable)
> > +                       check_override = entry->override;
> >         }
> >
> > -#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
> > +       if (!check_override)
> > +               return;
> >
> > -       return true;
> > +       if (!acpi_get_override_irq(gsi, &t, &p)) {
> > +               u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> > +               u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> > +
> > +               if (*triggering != trig || *polarity != pol) {
> > +                       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;
> > +               }
> > +       }
> > +
> > +       for (i = 0; i < ARRAY_SIZE(acpi_override_table); i++) {
> > +               const struct irq_override_acpi_cmp *entry =
> &acpi_override_table[i];
> > +
> > +               if (acpi_dev_found(entry->id) && gsi == entry->irq &&
> > +                  (*polarity != entry->polarity || *triggering != entry->triggering)) {
> > +                       pr_warn("ACPI: IRQ %d override to %s%s, %s%s due to %s\n",
> > +                               gsi,
> > +                               entry->triggering ? "level" : "edge",
> > +                               entry->triggering == *triggering ? "" : "(!)",
> > +                               entry->polarity ? "low" : "high",
> > +                               entry->polarity == *polarity ? "" : "(!)",
> > +                               entry->id);
> > +                       *polarity = entry->polarity;
> > +                       *triggering = entry->triggering;
> > +               }
> > +       }
> >  }
> >
> >  static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> >                                      u8 triggering, u8 polarity, u8 shareable,
> >                                      u8 wake_capable, bool check_override)
> >  {
> > -       int irq, p, t;
> > +       int irq;
> >
> >         if (!valid_IRQ(gsi)) {
> >                 irqresource_disabled(res, gsi);
> > @@ -592,26 +582,12 @@ static void acpi_dev_get_irqresource(struct
> resource *res, u32 gsi,
> >          * 2. BIOS uses IO-APIC mode Interrupt Source Override
> >          *
> >          * We do this only if we are dealing with IRQ() or IRQNoFlags()
> > -        * resource (the legacy ISA resources). With modern ACPI 5 devices
> > +        * resource (the legacy ISA resources). With ACPI devices
> >          * using extended IRQ descriptors we take the IRQ configuration
> >          * from _CRS directly.
> >          */
> > -       if (check_override &&
> > -           acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
> > -           !acpi_get_override_irq(gsi, &t, &p)) {
> > -               u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> > -               u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> > -
> > -               if (triggering != trig || polarity != pol) {
> > -                       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;
> > -               }
> > -       }
> > +       if (check_override)
> > +               acpi_dev_irq_override(gsi, &triggering, &polarity, &shareable);
> >
> >         res->flags = acpi_dev_irq_flags(triggering, polarity, shareable,
> wake_capable);
> >         irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> >
> > base-commit: c554eee18c9a440bd2dd5a363b0f79325717f0bf
> > --
> > 2.34.1
> >
Werner Sembach May 23, 2023, 5:52 p.m. UTC | #3
Hi,

Am 18.05.23 um 20:39 schrieb Mario Limonciello:
> commit 9946e39fe8d0 ("ACPI: resource: skip IRQ override on
> AMD Zen platforms") attempted to overhaul the override logic so it
> didn't apply on X86 AMD Zen systems.  This was intentional so that
> systems would prefer DSDT values instead of default MADT value for
> IRQ 1 on Ryzen 6000 systems which use ActiveLow for IRQ1.
>
> This turned out to be a bad assumption because several vendors seem
> to add Interrupt Source Override but don't fix the DSDT. A pile of
> quirks was collecting that proved this wasn't sustaintable.
>
> Adjust the logic so that only IRQ1 is overridden in Ryzen 6000 case.
>
> This effectively reverts the following commits:
> commit 17bb7046e7ce ("ACPI: resource: Do IRQ override on all TongFang
> GMxRGxx")
> commit f3cb9b740869 ("ACPI: resource: do IRQ override on Lenovo 14ALC7")
> commit bfcdf58380b1 ("ACPI: resource: do IRQ override on LENOVO IdeaPad")
> commit 7592b79ba4a9 ("ACPI: resource: do IRQ override on XMG Core 15")

The TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris 15 Gen4 AMD breaks again with this patch (applied to 6.4-rc3). Am I missing an additional patch that is required?

>
> Cc: ofenfisch@googlemail.com
> Cc: wse@tuxedocomputers.com
> Cc: adam.niederer@gmail.com
> Cc: adrian@freund.io
> Cc: jirislaby@kernel.org
> Tested-by: Renjith.Pananchikkal@amd.com
> Tested-by: anson.tsao@amd.com
> Tested-by: Richard.Gong@amd.com
> Tested-by: Chuanhong Guo <gch981213@gmail.com>
> Reported-by: evilsnoo@proton.me
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
> Reported-by: ruinairas1992@gmail.com
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217406
> Fixes: 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen platforms")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>   * Rebase on 71a485624c4c ("ACPI: resource: Add IRQ override quirk for LG UltraPC 17U70P")
>   * Pick up tag
> ---
>   drivers/acpi/resource.c | 154 +++++++++++++++++-----------------------
>   1 file changed, 65 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 0800a9d77558..c6ac87e01e1c 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -470,52 +470,6 @@ static const struct dmi_system_id asus_laptop[] = {
>   	{ }
>   };
>   
> -static const struct dmi_system_id lenovo_laptop[] = {
> -	{
> -		.ident = "LENOVO IdeaPad Flex 5 14ALC7",
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "82R9"),
> -		},
> -	},
> -	{
> -		.ident = "LENOVO IdeaPad Flex 5 16ALC7",
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
> -		},
> -	},
> -	{ }
> -};
> -
> -static const struct dmi_system_id tongfang_gm_rg[] = {
> -	{
> -		.ident = "TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris 15 Gen4 AMD",
> -		.matches = {
> -			DMI_MATCH(DMI_BOARD_NAME, "GMxRGxx"),
> -		},
> -	},
> -	{ }
> -};
> -
> -static const struct dmi_system_id maingear_laptop[] = {
> -	{
> -		.ident = "MAINGEAR Vector Pro 2 15",
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-15A3070T"),
> -		}
> -	},
> -	{
> -		.ident = "MAINGEAR Vector Pro 2 17",
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-17A3070T"),
> -		},
> -	},
> -	{ }
> -};
> -
>   static const struct dmi_system_id lg_laptop[] = {
>   	{
>   		.ident = "LG Electronics 17U70P",
> @@ -527,7 +481,7 @@ static const struct dmi_system_id lg_laptop[] = {
>   	{ }
>   };
>   
> -struct irq_override_cmp {
> +struct irq_override_dmi_cmp {
>   	const struct dmi_system_id *system;
>   	unsigned char irq;
>   	unsigned char triggering;
> @@ -536,50 +490,86 @@ struct irq_override_cmp {
>   	bool override;
>   };
>   
> -static const struct irq_override_cmp override_table[] = {
> +struct irq_override_acpi_cmp {
> +	const char *id;
> +	unsigned char irq;
> +	unsigned char triggering;
> +	unsigned char polarity;
> +};
> +
> +static const struct irq_override_dmi_cmp dmi_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_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> -	{ lenovo_laptop, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> -	{ tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
> -	{ maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
>   	{ lg_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
>   };
>   
> -static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> -				  u8 shareable)
> +/*
> + * Ryzen 6000 requires ActiveLow for keyboard, but a number of machines
> + * seem to get it wrong in DSDT or don't have an Interrupt Source
> + * Override.
> + */
> +static const struct irq_override_acpi_cmp acpi_override_table[] = {
> +	{ "AMDI0007", 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW },
> +};
> +
> +static void acpi_dev_irq_override(u32 gsi, u8 *triggering, u8 *polarity,
> +				  u8 *shareable)
>   {
> -	int i;
> +	int i, p, t;
> +	int check_override = true;
>   
> -	for (i = 0; i < ARRAY_SIZE(override_table); i++) {
> -		const struct irq_override_cmp *entry = &override_table[i];
> +	for (i = 0; i < ARRAY_SIZE(dmi_override_table); i++) {
> +		const struct irq_override_dmi_cmp *entry = &dmi_override_table[i];
>   
>   		if (dmi_check_system(entry->system) &&
>   		    entry->irq == gsi &&
> -		    entry->triggering == triggering &&
> -		    entry->polarity == polarity &&
> -		    entry->shareable == shareable)
> -			return entry->override;
> +		    entry->triggering == *triggering &&
> +		    entry->polarity == *polarity &&
> +		    entry->shareable == *shareable)
> +			check_override = entry->override;
>   	}
>   
> -#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
> +	if (!check_override)
> +		return;
>   
> -	return true;
> +	if (!acpi_get_override_irq(gsi, &t, &p)) {
> +		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> +		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> +
> +		if (*triggering != trig || *polarity != pol) {
> +			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;
> +		}
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(acpi_override_table); i++) {
> +		const struct irq_override_acpi_cmp *entry = &acpi_override_table[i];
> +
> +		if (acpi_dev_found(entry->id) && gsi == entry->irq &&
> +		   (*polarity != entry->polarity || *triggering != entry->triggering)) {
> +			pr_warn("ACPI: IRQ %d override to %s%s, %s%s due to %s\n",
> +				gsi,
> +				entry->triggering ? "level" : "edge",
> +				entry->triggering == *triggering ? "" : "(!)",
> +				entry->polarity ? "low" : "high",
> +				entry->polarity == *polarity ? "" : "(!)",
> +				entry->id);
> +			*polarity = entry->polarity;
> +			*triggering = entry->triggering;
> +		}
> +	}
>   }
>   
>   static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>   				     u8 triggering, u8 polarity, u8 shareable,
>   				     u8 wake_capable, bool check_override)
>   {
> -	int irq, p, t;
> +	int irq;
>   
>   	if (!valid_IRQ(gsi)) {
>   		irqresource_disabled(res, gsi);
> @@ -592,26 +582,12 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>   	 * 2. BIOS uses IO-APIC mode Interrupt Source Override
>   	 *
>   	 * We do this only if we are dealing with IRQ() or IRQNoFlags()
> -	 * resource (the legacy ISA resources). With modern ACPI 5 devices
> +	 * resource (the legacy ISA resources). With ACPI devices
>   	 * using extended IRQ descriptors we take the IRQ configuration
>   	 * from _CRS directly.
>   	 */
> -	if (check_override &&
> -	    acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
> -	    !acpi_get_override_irq(gsi, &t, &p)) {
> -		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> -		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> -
> -		if (triggering != trig || polarity != pol) {
> -			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;
> -		}
> -	}
> +	if (check_override)
> +		acpi_dev_irq_override(gsi, &triggering, &polarity, &shareable);
>   
>   	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable, wake_capable);
>   	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>
> base-commit: c554eee18c9a440bd2dd5a363b0f79325717f0bf
Mario Limonciello May 23, 2023, 5:56 p.m. UTC | #4
[AMD Official Use Only - General]

> -----Original Message-----
> From: Werner Sembach <wse@tuxedocomputers.com>
> Sent: Tuesday, May 23, 2023 12:53 PM
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; rafael@kernel.org
> Cc: gch981213@gmail.com; linux-acpi@vger.kernel.org; linux-
> kernel@vger.kernel.org; regressions@leemhuis.info;
> ofenfisch@googlemail.com; adam.niederer@gmail.com; adrian@freund.io;
> jirislaby@kernel.org; Pananchikkal, Renjith <Renjith.Pananchikkal@amd.com>;
> Tsao, Anson <anson.tsao@amd.com>; Gong, Richard
> <Richard.Gong@amd.com>; evilsnoo@proton.me; ruinairas1992@gmail.com
> Subject: Re: [PATCH v2] ACPI: resource: Remove "Zen" specific match and
> quirks
>
> Hi,
>
> Am 18.05.23 um 20:39 schrieb Mario Limonciello:
> > commit 9946e39fe8d0 ("ACPI: resource: skip IRQ override on
> > AMD Zen platforms") attempted to overhaul the override logic so it
> > didn't apply on X86 AMD Zen systems.  This was intentional so that
> > systems would prefer DSDT values instead of default MADT value for
> > IRQ 1 on Ryzen 6000 systems which use ActiveLow for IRQ1.
> >
> > This turned out to be a bad assumption because several vendors seem
> > to add Interrupt Source Override but don't fix the DSDT. A pile of
> > quirks was collecting that proved this wasn't sustaintable.
> >
> > Adjust the logic so that only IRQ1 is overridden in Ryzen 6000 case.
> >
> > This effectively reverts the following commits:
> > commit 17bb7046e7ce ("ACPI: resource: Do IRQ override on all TongFang
> > GMxRGxx")
> > commit f3cb9b740869 ("ACPI: resource: do IRQ override on Lenovo
> 14ALC7")
> > commit bfcdf58380b1 ("ACPI: resource: do IRQ override on LENOVO
> IdeaPad")
> > commit 7592b79ba4a9 ("ACPI: resource: do IRQ override on XMG Core 15")
>
> The TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris 15 Gen4 AMD
> breaks again with this patch (applied to 6.4-rc3). Am I missing an additional
> patch that is required?

You're not missing anything extra, but let's gather some detail about your system
and figure out why.

What CPU is in it?
Also would you mind sending me an acpidump?  You can send directly to me off-list
if you want.

>
> >
> > Cc: ofenfisch@googlemail.com
> > Cc: wse@tuxedocomputers.com
> > Cc: adam.niederer@gmail.com
> > Cc: adrian@freund.io
> > Cc: jirislaby@kernel.org
> > Tested-by: Renjith.Pananchikkal@amd.com
> > Tested-by: anson.tsao@amd.com
> > Tested-by: Richard.Gong@amd.com
> > Tested-by: Chuanhong Guo <gch981213@gmail.com>
> > Reported-by: evilsnoo@proton.me
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
> > Reported-by: ruinairas1992@gmail.com
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217406
> > Fixes: 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen
> platforms")
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> > v1->v2:
> >   * Rebase on 71a485624c4c ("ACPI: resource: Add IRQ override quirk for LG
> UltraPC 17U70P")
> >   * Pick up tag
> > ---
> >   drivers/acpi/resource.c | 154 +++++++++++++++++-----------------------
> >   1 file changed, 65 insertions(+), 89 deletions(-)
> >
> > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > index 0800a9d77558..c6ac87e01e1c 100644
> > --- a/drivers/acpi/resource.c
> > +++ b/drivers/acpi/resource.c
> > @@ -470,52 +470,6 @@ static const struct dmi_system_id asus_laptop[] = {
> >     { }
> >   };
> >
> > -static const struct dmi_system_id lenovo_laptop[] = {
> > -   {
> > -           .ident = "LENOVO IdeaPad Flex 5 14ALC7",
> > -           .matches = {
> > -                   DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> > -                   DMI_MATCH(DMI_PRODUCT_NAME, "82R9"),
> > -           },
> > -   },
> > -   {
> > -           .ident = "LENOVO IdeaPad Flex 5 16ALC7",
> > -           .matches = {
> > -                   DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> > -                   DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
> > -           },
> > -   },
> > -   { }
> > -};
> > -
> > -static const struct dmi_system_id tongfang_gm_rg[] = {
> > -   {
> > -           .ident = "TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO
> Stellaris 15 Gen4 AMD",
> > -           .matches = {
> > -                   DMI_MATCH(DMI_BOARD_NAME, "GMxRGxx"),
> > -           },
> > -   },
> > -   { }
> > -};
> > -
> > -static const struct dmi_system_id maingear_laptop[] = {
> > -   {
> > -           .ident = "MAINGEAR Vector Pro 2 15",
> > -           .matches = {
> > -                   DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics
> Inc"),
> > -                   DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-
> 15A3070T"),
> > -           }
> > -   },
> > -   {
> > -           .ident = "MAINGEAR Vector Pro 2 17",
> > -           .matches = {
> > -                   DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics
> Inc"),
> > -                   DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-
> 17A3070T"),
> > -           },
> > -   },
> > -   { }
> > -};
> > -
> >   static const struct dmi_system_id lg_laptop[] = {
> >     {
> >             .ident = "LG Electronics 17U70P",
> > @@ -527,7 +481,7 @@ static const struct dmi_system_id lg_laptop[] = {
> >     { }
> >   };
> >
> > -struct irq_override_cmp {
> > +struct irq_override_dmi_cmp {
> >     const struct dmi_system_id *system;
> >     unsigned char irq;
> >     unsigned char triggering;
> > @@ -536,50 +490,86 @@ struct irq_override_cmp {
> >     bool override;
> >   };
> >
> > -static const struct irq_override_cmp override_table[] = {
> > +struct irq_override_acpi_cmp {
> > +   const char *id;
> > +   unsigned char irq;
> > +   unsigned char triggering;
> > +   unsigned char polarity;
> > +};
> > +
> > +static const struct irq_override_dmi_cmp dmi_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_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0,
> true },
> > -   { lenovo_laptop, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0,
> true },
> > -   { tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1,
> true },
> > -   { maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1,
> true },
> >     { lg_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
> >   };
> >
> > -static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> > -                             u8 shareable)
> > +/*
> > + * Ryzen 6000 requires ActiveLow for keyboard, but a number of machines
> > + * seem to get it wrong in DSDT or don't have an Interrupt Source
> > + * Override.
> > + */
> > +static const struct irq_override_acpi_cmp acpi_override_table[] = {
> > +   { "AMDI0007", 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW },
> > +};
> > +
> > +static void acpi_dev_irq_override(u32 gsi, u8 *triggering, u8 *polarity,
> > +                             u8 *shareable)
> >   {
> > -   int i;
> > +   int i, p, t;
> > +   int check_override = true;
> >
> > -   for (i = 0; i < ARRAY_SIZE(override_table); i++) {
> > -           const struct irq_override_cmp *entry = &override_table[i];
> > +   for (i = 0; i < ARRAY_SIZE(dmi_override_table); i++) {
> > +           const struct irq_override_dmi_cmp *entry =
> &dmi_override_table[i];
> >
> >             if (dmi_check_system(entry->system) &&
> >                 entry->irq == gsi &&
> > -               entry->triggering == triggering &&
> > -               entry->polarity == polarity &&
> > -               entry->shareable == shareable)
> > -                   return entry->override;
> > +               entry->triggering == *triggering &&
> > +               entry->polarity == *polarity &&
> > +               entry->shareable == *shareable)
> > +                   check_override = entry->override;
> >     }
> >
> > -#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
> > +   if (!check_override)
> > +           return;
> >
> > -   return true;
> > +   if (!acpi_get_override_irq(gsi, &t, &p)) {
> > +           u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> > +           u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> > +
> > +           if (*triggering != trig || *polarity != pol) {
> > +                   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;
> > +           }
> > +   }
> > +
> > +   for (i = 0; i < ARRAY_SIZE(acpi_override_table); i++) {
> > +           const struct irq_override_acpi_cmp *entry =
> &acpi_override_table[i];
> > +
> > +           if (acpi_dev_found(entry->id) && gsi == entry->irq &&
> > +              (*polarity != entry->polarity || *triggering != entry-
> >triggering)) {
> > +                   pr_warn("ACPI: IRQ %d override to %s%s, %s%s due
> to %s\n",
> > +                           gsi,
> > +                           entry->triggering ? "level" : "edge",
> > +                           entry->triggering == *triggering ? "" : "(!)",
> > +                           entry->polarity ? "low" : "high",
> > +                           entry->polarity == *polarity ? "" : "(!)",
> > +                           entry->id);
> > +                   *polarity = entry->polarity;
> > +                   *triggering = entry->triggering;
> > +           }
> > +   }
> >   }
> >
> >   static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> >                                  u8 triggering, u8 polarity, u8 shareable,
> >                                  u8 wake_capable, bool check_override)
> >   {
> > -   int irq, p, t;
> > +   int irq;
> >
> >     if (!valid_IRQ(gsi)) {
> >             irqresource_disabled(res, gsi);
> > @@ -592,26 +582,12 @@ static void acpi_dev_get_irqresource(struct
> resource *res, u32 gsi,
> >      * 2. BIOS uses IO-APIC mode Interrupt Source Override
> >      *
> >      * We do this only if we are dealing with IRQ() or IRQNoFlags()
> > -    * resource (the legacy ISA resources). With modern ACPI 5 devices
> > +    * resource (the legacy ISA resources). With ACPI devices
> >      * using extended IRQ descriptors we take the IRQ configuration
> >      * from _CRS directly.
> >      */
> > -   if (check_override &&
> > -       acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
> > -       !acpi_get_override_irq(gsi, &t, &p)) {
> > -           u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> > -           u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> > -
> > -           if (triggering != trig || polarity != pol) {
> > -                   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;
> > -           }
> > -   }
> > +   if (check_override)
> > +           acpi_dev_irq_override(gsi, &triggering, &polarity, &shareable);
> >
> >     res->flags = acpi_dev_irq_flags(triggering, polarity, shareable,
> wake_capable);
> >     irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> >
> > base-commit: c554eee18c9a440bd2dd5a363b0f79325717f0bf
Adam May 23, 2023, 9:20 p.m. UTC | #5
Hey, I have tested 6.4-rc3 and this breaks the integrated trackpad on my 
MAINGEAR Vector Pro 2 15" 6900H model (MG-VCP2-15A3070T), but the 
keyboard still works (the trackpad worked and keyboard did not before I 
submitted my ACPI override). I'll send over an acpidump directly as well.

On 5/18/23 14:39, Mario Limonciello wrote:
> commit 9946e39fe8d0 ("ACPI: resource: skip IRQ override on
> AMD Zen platforms") attempted to overhaul the override logic so it
> didn't apply on X86 AMD Zen systems.  This was intentional so that
> systems would prefer DSDT values instead of default MADT value for
> IRQ 1 on Ryzen 6000 systems which use ActiveLow for IRQ1.
>
> This turned out to be a bad assumption because several vendors seem
> to add Interrupt Source Override but don't fix the DSDT. A pile of
> quirks was collecting that proved this wasn't sustaintable.
>
> Adjust the logic so that only IRQ1 is overridden in Ryzen 6000 case.
>
> This effectively reverts the following commits:
> commit 17bb7046e7ce ("ACPI: resource: Do IRQ override on all TongFang
> GMxRGxx")
> commit f3cb9b740869 ("ACPI: resource: do IRQ override on Lenovo 14ALC7")
> commit bfcdf58380b1 ("ACPI: resource: do IRQ override on LENOVO IdeaPad")
> commit 7592b79ba4a9 ("ACPI: resource: do IRQ override on XMG Core 15")
>
> Cc: ofenfisch@googlemail.com
> Cc: wse@tuxedocomputers.com
> Cc: adam.niederer@gmail.com
> Cc: adrian@freund.io
> Cc: jirislaby@kernel.org
> Tested-by: Renjith.Pananchikkal@amd.com
> Tested-by: anson.tsao@amd.com
> Tested-by: Richard.Gong@amd.com
> Tested-by: Chuanhong Guo <gch981213@gmail.com>
> Reported-by: evilsnoo@proton.me
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
> Reported-by: ruinairas1992@gmail.com
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217406
> Fixes: 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen platforms")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>   * Rebase on 71a485624c4c ("ACPI: resource: Add IRQ override quirk for LG UltraPC 17U70P")
>   * Pick up tag
> ---
>   drivers/acpi/resource.c | 154 +++++++++++++++++-----------------------
>   1 file changed, 65 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 0800a9d77558..c6ac87e01e1c 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -470,52 +470,6 @@ static const struct dmi_system_id asus_laptop[] = {
>   	{ }
>   };
>   
> -static const struct dmi_system_id lenovo_laptop[] = {
> -	{
> -		.ident = "LENOVO IdeaPad Flex 5 14ALC7",
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "82R9"),
> -		},
> -	},
> -	{
> -		.ident = "LENOVO IdeaPad Flex 5 16ALC7",
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
> -		},
> -	},
> -	{ }
> -};
> -
> -static const struct dmi_system_id tongfang_gm_rg[] = {
> -	{
> -		.ident = "TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris 15 Gen4 AMD",
> -		.matches = {
> -			DMI_MATCH(DMI_BOARD_NAME, "GMxRGxx"),
> -		},
> -	},
> -	{ }
> -};
> -
> -static const struct dmi_system_id maingear_laptop[] = {
> -	{
> -		.ident = "MAINGEAR Vector Pro 2 15",
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-15A3070T"),
> -		}
> -	},
> -	{
> -		.ident = "MAINGEAR Vector Pro 2 17",
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> -			DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-17A3070T"),
> -		},
> -	},
> -	{ }
> -};
> -
>   static const struct dmi_system_id lg_laptop[] = {
>   	{
>   		.ident = "LG Electronics 17U70P",
> @@ -527,7 +481,7 @@ static const struct dmi_system_id lg_laptop[] = {
>   	{ }
>   };
>   
> -struct irq_override_cmp {
> +struct irq_override_dmi_cmp {
>   	const struct dmi_system_id *system;
>   	unsigned char irq;
>   	unsigned char triggering;
> @@ -536,50 +490,86 @@ struct irq_override_cmp {
>   	bool override;
>   };
>   
> -static const struct irq_override_cmp override_table[] = {
> +struct irq_override_acpi_cmp {
> +	const char *id;
> +	unsigned char irq;
> +	unsigned char triggering;
> +	unsigned char polarity;
> +};
> +
> +static const struct irq_override_dmi_cmp dmi_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_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> -	{ lenovo_laptop, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> -	{ tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
> -	{ maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
>   	{ lg_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
>   };
>   
> -static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> -				  u8 shareable)
> +/*
> + * Ryzen 6000 requires ActiveLow for keyboard, but a number of machines
> + * seem to get it wrong in DSDT or don't have an Interrupt Source
> + * Override.
> + */
> +static const struct irq_override_acpi_cmp acpi_override_table[] = {
> +	{ "AMDI0007", 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW },
> +};
> +
> +static void acpi_dev_irq_override(u32 gsi, u8 *triggering, u8 *polarity,
> +				  u8 *shareable)
>   {
> -	int i;
> +	int i, p, t;
> +	int check_override = true;
>   
> -	for (i = 0; i < ARRAY_SIZE(override_table); i++) {
> -		const struct irq_override_cmp *entry = &override_table[i];
> +	for (i = 0; i < ARRAY_SIZE(dmi_override_table); i++) {
> +		const struct irq_override_dmi_cmp *entry = &dmi_override_table[i];
>   
>   		if (dmi_check_system(entry->system) &&
>   		    entry->irq == gsi &&
> -		    entry->triggering == triggering &&
> -		    entry->polarity == polarity &&
> -		    entry->shareable == shareable)
> -			return entry->override;
> +		    entry->triggering == *triggering &&
> +		    entry->polarity == *polarity &&
> +		    entry->shareable == *shareable)
> +			check_override = entry->override;
>   	}
>   
> -#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
> +	if (!check_override)
> +		return;
>   
> -	return true;
> +	if (!acpi_get_override_irq(gsi, &t, &p)) {
> +		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> +		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> +
> +		if (*triggering != trig || *polarity != pol) {
> +			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;
> +		}
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(acpi_override_table); i++) {
> +		const struct irq_override_acpi_cmp *entry = &acpi_override_table[i];
> +
> +		if (acpi_dev_found(entry->id) && gsi == entry->irq &&
> +		   (*polarity != entry->polarity || *triggering != entry->triggering)) {
> +			pr_warn("ACPI: IRQ %d override to %s%s, %s%s due to %s\n",
> +				gsi,
> +				entry->triggering ? "level" : "edge",
> +				entry->triggering == *triggering ? "" : "(!)",
> +				entry->polarity ? "low" : "high",
> +				entry->polarity == *polarity ? "" : "(!)",
> +				entry->id);
> +			*polarity = entry->polarity;
> +			*triggering = entry->triggering;
> +		}
> +	}
>   }
>   
>   static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>   				     u8 triggering, u8 polarity, u8 shareable,
>   				     u8 wake_capable, bool check_override)
>   {
> -	int irq, p, t;
> +	int irq;
>   
>   	if (!valid_IRQ(gsi)) {
>   		irqresource_disabled(res, gsi);
> @@ -592,26 +582,12 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>   	 * 2. BIOS uses IO-APIC mode Interrupt Source Override
>   	 *
>   	 * We do this only if we are dealing with IRQ() or IRQNoFlags()
> -	 * resource (the legacy ISA resources). With modern ACPI 5 devices
> +	 * resource (the legacy ISA resources). With ACPI devices
>   	 * using extended IRQ descriptors we take the IRQ configuration
>   	 * from _CRS directly.
>   	 */
> -	if (check_override &&
> -	    acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
> -	    !acpi_get_override_irq(gsi, &t, &p)) {
> -		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> -		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> -
> -		if (triggering != trig || polarity != pol) {
> -			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;
> -		}
> -	}
> +	if (check_override)
> +		acpi_dev_irq_override(gsi, &triggering, &polarity, &shareable);
>   
>   	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable, wake_capable);
>   	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>
> base-commit: c554eee18c9a440bd2dd5a363b0f79325717f0bf
Werner Sembach May 24, 2023, 4:24 p.m. UTC | #6
Am 23.05.23 um 19:56 schrieb Limonciello, Mario:
> [AMD Official Use Only - General]
>
>> -----Original Message-----
>> From: Werner Sembach <wse@tuxedocomputers.com>
>> Sent: Tuesday, May 23, 2023 12:53 PM
>> To: Limonciello, Mario <Mario.Limonciello@amd.com>; rafael@kernel.org
>> Cc: gch981213@gmail.com; linux-acpi@vger.kernel.org; linux-
>> kernel@vger.kernel.org; regressions@leemhuis.info;
>> ofenfisch@googlemail.com; adam.niederer@gmail.com; adrian@freund.io;
>> jirislaby@kernel.org; Pananchikkal, Renjith <Renjith.Pananchikkal@amd.com>;
>> Tsao, Anson <anson.tsao@amd.com>; Gong, Richard
>> <Richard.Gong@amd.com>; evilsnoo@proton.me; ruinairas1992@gmail.com
>> Subject: Re: [PATCH v2] ACPI: resource: Remove "Zen" specific match and
>> quirks
>>
>> Hi,
>>
>> Am 18.05.23 um 20:39 schrieb Mario Limonciello:
>>> commit 9946e39fe8d0 ("ACPI: resource: skip IRQ override on
>>> AMD Zen platforms") attempted to overhaul the override logic so it
>>> didn't apply on X86 AMD Zen systems.  This was intentional so that
>>> systems would prefer DSDT values instead of default MADT value for
>>> IRQ 1 on Ryzen 6000 systems which use ActiveLow for IRQ1.
>>>
>>> This turned out to be a bad assumption because several vendors seem
>>> to add Interrupt Source Override but don't fix the DSDT. A pile of
>>> quirks was collecting that proved this wasn't sustaintable.
>>>
>>> Adjust the logic so that only IRQ1 is overridden in Ryzen 6000 case.
>>>
>>> This effectively reverts the following commits:
>>> commit 17bb7046e7ce ("ACPI: resource: Do IRQ override on all TongFang
>>> GMxRGxx")
>>> commit f3cb9b740869 ("ACPI: resource: do IRQ override on Lenovo
>> 14ALC7")
>>> commit bfcdf58380b1 ("ACPI: resource: do IRQ override on LENOVO
>> IdeaPad")
>>> commit 7592b79ba4a9 ("ACPI: resource: do IRQ override on XMG Core 15")
>> The TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris 15 Gen4 AMD
>> breaks again with this patch (applied to 6.4-rc3). Am I missing an additional
>> patch that is required?
> You're not missing anything extra, but let's gather some detail about your system
> and figure out why.
>
> What CPU is in it?
AMD Ryzen 9 6900HX
> Also would you mind sending me an acpidump?  You can send directly to me off-list
> if you want.

Doing that to not load the list with big attachments.

But for reference: here it was discussed too: 
https://bugzilla.kernel.org/show_bug.cgi?id=216698

Note that TF did eventually provide a fixed BIOS, but most likely not all 
vendors ship that BIOS and/or all users are aware that this fix exists. So I 
purposefully used the older unfixed BIOS to test the kernel.

>
>>> Cc: ofenfisch@googlemail.com
>>> Cc: wse@tuxedocomputers.com
>>> Cc: adam.niederer@gmail.com
>>> Cc: adrian@freund.io
>>> Cc: jirislaby@kernel.org
>>> Tested-by: Renjith.Pananchikkal@amd.com
>>> Tested-by: anson.tsao@amd.com
>>> Tested-by: Richard.Gong@amd.com
>>> Tested-by: Chuanhong Guo <gch981213@gmail.com>
>>> Reported-by: evilsnoo@proton.me
>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
>>> Reported-by: ruinairas1992@gmail.com
>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217406
>>> Fixes: 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen
>> platforms")
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>> v1->v2:
>>>    * Rebase on 71a485624c4c ("ACPI: resource: Add IRQ override quirk for LG
>> UltraPC 17U70P")
>>>    * Pick up tag
>>> ---
>>>    drivers/acpi/resource.c | 154 +++++++++++++++++-----------------------
>>>    1 file changed, 65 insertions(+), 89 deletions(-)
>>>
>>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>>> index 0800a9d77558..c6ac87e01e1c 100644
>>> --- a/drivers/acpi/resource.c
>>> +++ b/drivers/acpi/resource.c
>>> @@ -470,52 +470,6 @@ static const struct dmi_system_id asus_laptop[] = {
>>>      { }
>>>    };
>>>
>>> -static const struct dmi_system_id lenovo_laptop[] = {
>>> -   {
>>> -           .ident = "LENOVO IdeaPad Flex 5 14ALC7",
>>> -           .matches = {
>>> -                   DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>>> -                   DMI_MATCH(DMI_PRODUCT_NAME, "82R9"),
>>> -           },
>>> -   },
>>> -   {
>>> -           .ident = "LENOVO IdeaPad Flex 5 16ALC7",
>>> -           .matches = {
>>> -                   DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>>> -                   DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
>>> -           },
>>> -   },
>>> -   { }
>>> -};
>>> -
>>> -static const struct dmi_system_id tongfang_gm_rg[] = {
>>> -   {
>>> -           .ident = "TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO
>> Stellaris 15 Gen4 AMD",
>>> -           .matches = {
>>> -                   DMI_MATCH(DMI_BOARD_NAME, "GMxRGxx"),
>>> -           },
>>> -   },
>>> -   { }
>>> -};
>>> -
>>> -static const struct dmi_system_id maingear_laptop[] = {
>>> -   {
>>> -           .ident = "MAINGEAR Vector Pro 2 15",
>>> -           .matches = {
>>> -                   DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics
>> Inc"),
>>> -                   DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-
>> 15A3070T"),
>>> -           }
>>> -   },
>>> -   {
>>> -           .ident = "MAINGEAR Vector Pro 2 17",
>>> -           .matches = {
>>> -                   DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics
>> Inc"),
>>> -                   DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-
>> 17A3070T"),
>>> -           },
>>> -   },
>>> -   { }
>>> -};
>>> -
>>>    static const struct dmi_system_id lg_laptop[] = {
>>>      {
>>>              .ident = "LG Electronics 17U70P",
>>> @@ -527,7 +481,7 @@ static const struct dmi_system_id lg_laptop[] = {
>>>      { }
>>>    };
>>>
>>> -struct irq_override_cmp {
>>> +struct irq_override_dmi_cmp {
>>>      const struct dmi_system_id *system;
>>>      unsigned char irq;
>>>      unsigned char triggering;
>>> @@ -536,50 +490,86 @@ struct irq_override_cmp {
>>>      bool override;
>>>    };
>>>
>>> -static const struct irq_override_cmp override_table[] = {
>>> +struct irq_override_acpi_cmp {
>>> +   const char *id;
>>> +   unsigned char irq;
>>> +   unsigned char triggering;
>>> +   unsigned char polarity;
>>> +};
>>> +
>>> +static const struct irq_override_dmi_cmp dmi_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_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0,
>> true },
>>> -   { lenovo_laptop, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0,
>> true },
>>> -   { tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1,
>> true },
>>> -   { maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1,
>> true },
>>>      { lg_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
>>>    };
>>>
>>> -static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
>>> -                             u8 shareable)
>>> +/*
>>> + * Ryzen 6000 requires ActiveLow for keyboard, but a number of machines
>>> + * seem to get it wrong in DSDT or don't have an Interrupt Source
>>> + * Override.
>>> + */
>>> +static const struct irq_override_acpi_cmp acpi_override_table[] = {
>>> +   { "AMDI0007", 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW },
>>> +};
>>> +
>>> +static void acpi_dev_irq_override(u32 gsi, u8 *triggering, u8 *polarity,
>>> +                             u8 *shareable)
>>>    {
>>> -   int i;
>>> +   int i, p, t;
>>> +   int check_override = true;
>>>
>>> -   for (i = 0; i < ARRAY_SIZE(override_table); i++) {
>>> -           const struct irq_override_cmp *entry = &override_table[i];
>>> +   for (i = 0; i < ARRAY_SIZE(dmi_override_table); i++) {
>>> +           const struct irq_override_dmi_cmp *entry =
>> &dmi_override_table[i];
>>>              if (dmi_check_system(entry->system) &&
>>>                  entry->irq == gsi &&
>>> -               entry->triggering == triggering &&
>>> -               entry->polarity == polarity &&
>>> -               entry->shareable == shareable)
>>> -                   return entry->override;
>>> +               entry->triggering == *triggering &&
>>> +               entry->polarity == *polarity &&
>>> +               entry->shareable == *shareable)
>>> +                   check_override = entry->override;
>>>      }
>>>
>>> -#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
>>> +   if (!check_override)
>>> +           return;
>>>
>>> -   return true;
>>> +   if (!acpi_get_override_irq(gsi, &t, &p)) {
>>> +           u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>>> +           u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
>>> +
>>> +           if (*triggering != trig || *polarity != pol) {
>>> +                   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;
>>> +           }
>>> +   }
>>> +
>>> +   for (i = 0; i < ARRAY_SIZE(acpi_override_table); i++) {
>>> +           const struct irq_override_acpi_cmp *entry =
>> &acpi_override_table[i];
>>> +
>>> +           if (acpi_dev_found(entry->id) && gsi == entry->irq &&
>>> +              (*polarity != entry->polarity || *triggering != entry-
>>> triggering)) {
>>> +                   pr_warn("ACPI: IRQ %d override to %s%s, %s%s due
>> to %s\n",
>>> +                           gsi,
>>> +                           entry->triggering ? "level" : "edge",
>>> +                           entry->triggering == *triggering ? "" : "(!)",
>>> +                           entry->polarity ? "low" : "high",
>>> +                           entry->polarity == *polarity ? "" : "(!)",
>>> +                           entry->id);
>>> +                   *polarity = entry->polarity;
>>> +                   *triggering = entry->triggering;
>>> +           }
>>> +   }
>>>    }
>>>
>>>    static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>>>                                   u8 triggering, u8 polarity, u8 shareable,
>>>                                   u8 wake_capable, bool check_override)
>>>    {
>>> -   int irq, p, t;
>>> +   int irq;
>>>
>>>      if (!valid_IRQ(gsi)) {
>>>              irqresource_disabled(res, gsi);
>>> @@ -592,26 +582,12 @@ static void acpi_dev_get_irqresource(struct
>> resource *res, u32 gsi,
>>>       * 2. BIOS uses IO-APIC mode Interrupt Source Override
>>>       *
>>>       * We do this only if we are dealing with IRQ() or IRQNoFlags()
>>> -    * resource (the legacy ISA resources). With modern ACPI 5 devices
>>> +    * resource (the legacy ISA resources). With ACPI devices
>>>       * using extended IRQ descriptors we take the IRQ configuration
>>>       * from _CRS directly.
>>>       */
>>> -   if (check_override &&
>>> -       acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
>>> -       !acpi_get_override_irq(gsi, &t, &p)) {
>>> -           u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>>> -           u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
>>> -
>>> -           if (triggering != trig || polarity != pol) {
>>> -                   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;
>>> -           }
>>> -   }
>>> +   if (check_override)
>>> +           acpi_dev_irq_override(gsi, &triggering, &polarity, &shareable);
>>>
>>>      res->flags = acpi_dev_irq_flags(triggering, polarity, shareable,
>> wake_capable);
>>>      irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>>>
>>> base-commit: c554eee18c9a440bd2dd5a363b0f79325717f0bf
diff mbox series

Patch

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 0800a9d77558..c6ac87e01e1c 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -470,52 +470,6 @@  static const struct dmi_system_id asus_laptop[] = {
 	{ }
 };
 
-static const struct dmi_system_id lenovo_laptop[] = {
-	{
-		.ident = "LENOVO IdeaPad Flex 5 14ALC7",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "82R9"),
-		},
-	},
-	{
-		.ident = "LENOVO IdeaPad Flex 5 16ALC7",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
-		},
-	},
-	{ }
-};
-
-static const struct dmi_system_id tongfang_gm_rg[] = {
-	{
-		.ident = "TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris 15 Gen4 AMD",
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "GMxRGxx"),
-		},
-	},
-	{ }
-};
-
-static const struct dmi_system_id maingear_laptop[] = {
-	{
-		.ident = "MAINGEAR Vector Pro 2 15",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-15A3070T"),
-		}
-	},
-	{
-		.ident = "MAINGEAR Vector Pro 2 17",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-17A3070T"),
-		},
-	},
-	{ }
-};
-
 static const struct dmi_system_id lg_laptop[] = {
 	{
 		.ident = "LG Electronics 17U70P",
@@ -527,7 +481,7 @@  static const struct dmi_system_id lg_laptop[] = {
 	{ }
 };
 
-struct irq_override_cmp {
+struct irq_override_dmi_cmp {
 	const struct dmi_system_id *system;
 	unsigned char irq;
 	unsigned char triggering;
@@ -536,50 +490,86 @@  struct irq_override_cmp {
 	bool override;
 };
 
-static const struct irq_override_cmp override_table[] = {
+struct irq_override_acpi_cmp {
+	const char *id;
+	unsigned char irq;
+	unsigned char triggering;
+	unsigned char polarity;
+};
+
+static const struct irq_override_dmi_cmp dmi_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_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
-	{ lenovo_laptop, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
-	{ tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
-	{ maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
 	{ lg_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
 };
 
-static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
-				  u8 shareable)
+/*
+ * Ryzen 6000 requires ActiveLow for keyboard, but a number of machines
+ * seem to get it wrong in DSDT or don't have an Interrupt Source
+ * Override.
+ */
+static const struct irq_override_acpi_cmp acpi_override_table[] = {
+	{ "AMDI0007", 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW },
+};
+
+static void acpi_dev_irq_override(u32 gsi, u8 *triggering, u8 *polarity,
+				  u8 *shareable)
 {
-	int i;
+	int i, p, t;
+	int check_override = true;
 
-	for (i = 0; i < ARRAY_SIZE(override_table); i++) {
-		const struct irq_override_cmp *entry = &override_table[i];
+	for (i = 0; i < ARRAY_SIZE(dmi_override_table); i++) {
+		const struct irq_override_dmi_cmp *entry = &dmi_override_table[i];
 
 		if (dmi_check_system(entry->system) &&
 		    entry->irq == gsi &&
-		    entry->triggering == triggering &&
-		    entry->polarity == polarity &&
-		    entry->shareable == shareable)
-			return entry->override;
+		    entry->triggering == *triggering &&
+		    entry->polarity == *polarity &&
+		    entry->shareable == *shareable)
+			check_override = entry->override;
 	}
 
-#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
+	if (!check_override)
+		return;
 
-	return true;
+	if (!acpi_get_override_irq(gsi, &t, &p)) {
+		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
+		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
+
+		if (*triggering != trig || *polarity != pol) {
+			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;
+		}
+	}
+
+	for (i = 0; i < ARRAY_SIZE(acpi_override_table); i++) {
+		const struct irq_override_acpi_cmp *entry = &acpi_override_table[i];
+
+		if (acpi_dev_found(entry->id) && gsi == entry->irq &&
+		   (*polarity != entry->polarity || *triggering != entry->triggering)) {
+			pr_warn("ACPI: IRQ %d override to %s%s, %s%s due to %s\n",
+				gsi,
+				entry->triggering ? "level" : "edge",
+				entry->triggering == *triggering ? "" : "(!)",
+				entry->polarity ? "low" : "high",
+				entry->polarity == *polarity ? "" : "(!)",
+				entry->id);
+			*polarity = entry->polarity;
+			*triggering = entry->triggering;
+		}
+	}
 }
 
 static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 				     u8 triggering, u8 polarity, u8 shareable,
 				     u8 wake_capable, bool check_override)
 {
-	int irq, p, t;
+	int irq;
 
 	if (!valid_IRQ(gsi)) {
 		irqresource_disabled(res, gsi);
@@ -592,26 +582,12 @@  static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 	 * 2. BIOS uses IO-APIC mode Interrupt Source Override
 	 *
 	 * We do this only if we are dealing with IRQ() or IRQNoFlags()
-	 * resource (the legacy ISA resources). With modern ACPI 5 devices
+	 * resource (the legacy ISA resources). With ACPI devices
 	 * using extended IRQ descriptors we take the IRQ configuration
 	 * from _CRS directly.
 	 */
-	if (check_override &&
-	    acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
-	    !acpi_get_override_irq(gsi, &t, &p)) {
-		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
-		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
-
-		if (triggering != trig || polarity != pol) {
-			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;
-		}
-	}
+	if (check_override)
+		acpi_dev_irq_override(gsi, &triggering, &polarity, &shareable);
 
 	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable, wake_capable);
 	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);