Hi All, On 8/6/23 17:14, Hans de Goede wrote: > Commit a9c4a912b7dc ("ACPI: resource: Remove "Zen" specific match and > quirks") is causing keyboard problems for quite a log of AMD based > laptop users, leading to many bug reports. > > Revert this change for now, until we can come up with > a better fix for the PS/2 IRQ trigger-type/polarity problems > on some x86 laptops. > > Fixes: a9c4a912b7dc ("ACPI: resource: Remove "Zen" specific match and quirks") > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2229165 > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2229317 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217726 > Cc: Mario Limonciello <mario.limonciello@xxxxxxx> > Cc: Linux regressions mailing list <regressions@xxxxxxxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> I've spend most of today analysing the situation / this problem : 213031 - MEDION notebook internal keyboard not recognized / not working correctly https://bugzilla.kernel.org/show_bug.cgi?id=213031 This is the bug that started it all, the issue here was overriding a level low DSDT entry: IRQ (Level, ActiveLow, Exclusive, ) {1} With an edge high entry from the MADT, note that edge high is the default mp_irqs[idx].irqflags value for legacy/ISA IRQs. The dmesg for the Ice Lake Medion M15T this bug is about shows no INT_SRC_OVR entry for IRQ 1 in the MADT, it does show INT_SRC_OVR entries for IRQ 0 and 9. At first a fix was attempted to not use the MADT override unless the DSDT entry was edge high. But that caused regressions, so a switch to a DMI based approach was used instead. Noteworthy is that some of the regressions benefitted from a MADT override to high edge for IRQ 3 and 4 (UART IRQs) even though there are no INT_SRC_OVR messages in the dmesg of the machine with the regression. *** fast forward to today *** The DMI quirk based approach seems to have worked well for the Ice Lake era problems from approx. 3 years ago. But on AMD Zen based systems the situation seems to be more complex. Not using the MADT override is a problem for quite a few models. But using the MADT override is a problem on quite a few other models ... Looking at the status quo for v6.4 where MADT overriding by default is not used, 3 bugs have been filed where the override is actually necessary (note dmesg snippets with patched kernel to enable MADT override): 217394 - IRQ override skipping breaks the Aya Neo Air Plus 6800U keyboard buttons https://bugzilla.kernel.org/show_bug.cgi?id=217394 Aya Neo Air Plus - AMD Ryzen 7 6800U [ 0.003333] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) [ 0.003333] ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 1 low edge) [ 0.003333] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) [ 0.410670] ACPI: IRQ 1 override to edge, low(!) 217406 - very slow keyboard typing without IRQ override with new AMD Ryzen CPUs https://bugzilla.kernel.org/show_bug.cgi?id=217406 HP Pavilion Aero 13 - AMD Ryzen 7735U [ 0.026135] ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 1 low edge) [ 0.026136] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) [ 0.026137] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) [ 0.361640] ACPI: IRQ 1 override to edge, low(!) 217336 - keyboard not working Asus TUF FA617NS https://bugzilla.kernel.org/show_bug.cgi?id=217336 Asus TUF FA617NS - AMD Ryzen 7 7735HS Noteworthy DSTD keyboard resource: IRQNoFlags () {1} ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 1 low edge) ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) ACPI: IRQ 1 override to edge, low(!) So for all 3 do use MADT override on Zen bugs we have an INT_SRC_OVR dmesg entry for IRQ 1. Unfortunately the "MAINGEAR Vector Pro 2 17" / "MG-VCP2-17A3070T" for which a quirk was added in commit 9946e39fe8d0 to force the override even though it it Zen based breaks this pattern: [ 0.073733] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) [ 0.073734] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) [ 0.341347] ACPI: IRQ 1 override to edge, high(!) Still the presence of an INT_SRC_OVR for a specific legacy IRQ seems to be a strong indicator that MADT overriding should be used in that case and can be used to at least reduce the amount of DMI quirks. Another interesting data point is that all the devices for which DMI quirks are present for which MADT overriding should not be used for IRQ 1 all have a DSDT entry with the IRQ configured as level low and exclusive. I think that the best thing to do might be to go back to the original approach from: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?id=0ec4e55e9f571f08970ed115ec0addc691eda613 and then limit this to IRQ1. Also maybe inverting the check to: static bool irq_is_legacy(struct acpi_resource_irq *irq) { return !(irq->triggering == ACPI_LEVEL_SENSITIVE && irq->polarity == ACPI_ACTIVE_LOW && irq->shareable == ACPI_EXCLUSIVE); } But I need to check if this will work for all the new Zen models for which we got bug reports after the recent dropping of 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen platforms") Regards, Hans > --- > drivers/acpi/resource.c | 60 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c > index 1dd8d5aebf67..0800a9d77558 100644 > --- a/drivers/acpi/resource.c > +++ b/drivers/acpi/resource.c > @@ -470,6 +470,52 @@ 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", > @@ -493,6 +539,10 @@ struct irq_override_cmp { > static const struct irq_override_cmp override_table[] = { > { medion_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false }, > { asus_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false }, > + { lenovo_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 }, > }; > > @@ -512,6 +562,16 @@ static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity, > return 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 > + > return true; > } >