[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1921a791-10ad-90c4-5656-71bca1dfded5@tuxedocomputers.com>
Date: Wed, 24 May 2023 18:24:13 +0200
From: Werner Sembach <wse@...edocomputers.com>
To: "Limonciello, Mario" <Mario.Limonciello@....com>,
"rafael@...nel.org" <rafael@...nel.org>
Cc: "gch981213@...il.com" <gch981213@...il.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"regressions@...mhuis.info" <regressions@...mhuis.info>,
"ofenfisch@...glemail.com" <ofenfisch@...glemail.com>,
"adam.niederer@...il.com" <adam.niederer@...il.com>,
"adrian@...und.io" <adrian@...und.io>,
"jirislaby@...nel.org" <jirislaby@...nel.org>,
"Pananchikkal, Renjith" <Renjith.Pananchikkal@....com>,
"Tsao, Anson" <anson.tsao@....com>,
"Gong, Richard" <Richard.Gong@....com>,
"evilsnoo@...ton.me" <evilsnoo@...ton.me>,
"ruinairas1992@...il.com" <ruinairas1992@...il.com>
Subject: Re: [PATCH v2] ACPI: resource: Remove "Zen" specific match and quirks
Am 23.05.23 um 19:56 schrieb Limonciello, Mario:
> [AMD Official Use Only - General]
>
>> -----Original Message-----
>> From: Werner Sembach <wse@...edocomputers.com>
>> Sent: Tuesday, May 23, 2023 12:53 PM
>> To: Limonciello, Mario <Mario.Limonciello@....com>; rafael@...nel.org
>> Cc: gch981213@...il.com; linux-acpi@...r.kernel.org; linux-
>> kernel@...r.kernel.org; regressions@...mhuis.info;
>> ofenfisch@...glemail.com; adam.niederer@...il.com; adrian@...und.io;
>> jirislaby@...nel.org; Pananchikkal, Renjith <Renjith.Pananchikkal@....com>;
>> Tsao, Anson <anson.tsao@....com>; Gong, Richard
>> <Richard.Gong@....com>; evilsnoo@...ton.me; ruinairas1992@...il.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@...glemail.com
>>> Cc: wse@...edocomputers.com
>>> Cc: adam.niederer@...il.com
>>> Cc: adrian@...und.io
>>> Cc: jirislaby@...nel.org
>>> Tested-by: Renjith.Pananchikkal@....com
>>> Tested-by: anson.tsao@....com
>>> Tested-by: Richard.Gong@....com
>>> Tested-by: Chuanhong Guo <gch981213@...il.com>
>>> Reported-by: evilsnoo@...ton.me
>>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
>>> Reported-by: ruinairas1992@...il.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@....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
Powered by blists - more mailing lists