On Tue, Aug 8, 2023 at 10:36 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Hi,
>
> On 8/7/23 17:19, Hans de Goede wrote:
> > 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")
>
> So today I have started with continueing the investigation looking
> at laptop models where we used to not override because of:
>
> if (boot_cpu_has(X86_FEATURE_ZEN))
> return false;
>
> And where removing this and thus using the override has led to a
> regression.
>
> Looking at the acpidump-s from the following bugs:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=2229165
> https://bugzilla.redhat.com/show_bug.cgi?id=2229317
> https://bugzilla.kernel.org/show_bug.cgi?id=217726
>
> All of these use the following settings for the kbd in the DSDT:
>
> IRQ (Edge, ActiveLow, Shared, )
> {1}
>
> So we know these are at least 3 models with "Edge, ActiveLow, Shared" IRQ 1 settings which must not use the override. But the existing quirks before a9c4a912b7dc ("ACPI: resource: Remove "Zen" specific match and quirks") contain:
>
> { tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
> { maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
>
> IOW models with "Edge, ActiveLow, Shared" IRQ 1 settings which OTOH must use the override, so as I was already afraid there is no easy for these DSDT IRQ 1 settings skip the override solution :|
>
> So I have 2 plans to move forward with this:
>
> Plan 1. Short them for 6.5 and backporting to 6.4.y (and other stable series):
>
> 1. Revert a9c4a912b7dc
> 2. Limit the acpi_dev_irq_override() check to only ever skip the IRQ override
> (return false) for GSI 1.
> 3. Add a check if there is a INT_SRC_OVR MADT entry for GSI 1 and in that case
> use the override even on ZEN, fixing:
>
> 217394 - IRQ override skipping breaks the Aya Neo Air Plus 6800U keyboard buttons
> https://bugzilla.kernel.org/show_bug.cgi?id=217394
> 217406 - very slow keyboard typing without IRQ override with new AMD Ryzen CPUs
> https://bugzilla.kernel.org/show_bug.cgi?id=217406
> 217336 - keyboard not working Asus TUF FA617NS
> https://bugzilla.kernel.org/show_bug.cgi?id=217336
>
> Which are known AMD ZEN based laptops which do need the override for IRQ 1.
>
> This short term plan is not ideal, but it does fix all currently known issues / models and does so in a way which will hopefully not cause regressions on any other models.
>
>
> Plan 2. Long term, see if I can come up with a way to read back the actual trigger type set in the IOAPIC for IRQ 1 at boot (in drivers/acpi/resource.c) and use that.
Sounds reasonable to me.
It looks like using the IRQ 1 configuration left by the BIOS as is
would be the best choice unless that is not viable for some reason.
