Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b97242cf-33cf-66b8-4816-ba1f90df41a4@gmail.com>
Date:   Thu, 1 Jun 2023 19:01:15 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.2
Subject: Re: [PATCH] ACPI: resource: Remove "Zen" specific match and quirks
Content-Language: en-US
To:     Mario Limonciello <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, wse@tuxedocomputers.com,
        adam.niederer@gmail.com, adrian@freund.io, jirislaby@kernel.org,
        Renjith.Pananchikkal@amd.com, anson.tsao@amd.com,
        Richard.Gong@amd.com, evilsnoo@proton.me
References: <20230515213822.1277-1-mario.limonciello@amd.com>
From:   Matthew Anderson <ruinairas1992@gmail.com>
In-Reply-To: <20230515213822.1277-1-mario.limonciello@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

I tested Patch v3 and it still works on the Aya Neo Air Plus.

Tested-by: Matthew Anderson<ruinairas1992@gmail.com>

On 5/15/23 4:38 PM, 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: gch981213@gmail.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
> 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>
> ---
>   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 e8492b3a393a..828adb4be721 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -470,53 +470,7 @@ 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"),
> -		},
> -	},
> -	{ }
> -};
> -
> -struct irq_override_cmp {
> +struct irq_override_dmi_cmp {
>   	const struct dmi_system_id *system;
>   	unsigned char irq;
>   	unsigned char triggering;
> @@ -525,49 +479,85 @@ 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 },
>   };
>   
> -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);
> @@ -580,26 +570,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);