On Wed, Apr 20, 2022 at 11:39:00AM +0200, Mark Kettenis wrote: > > Date: Tue, 19 Apr 2022 22:02:00 -0700 > > From: Mike Larkin <mlar...@nested.page> > > > > On at least the Asus ROG Zephyrus 14 (2020), the trackpad fails to generate > > any interrupts after resume. I tracked this down to amdgpio(4) not > > generating > > interrupts after resume, and started looking at missing soft state. > > > > This diff preserves the interrupt pin configurations and restores them after > > resume. This makes the device function properly post-zzz and post-ZZZ. > > I think it might make sense to structure this a bit more like > pchgpio(4). There we only restore the configuration for pins that are > "in use" by OpenBSD. > > > Note: amdgpio_read_pin does not return the value that was previously written > > during amdgpio_intr_establish (it always just returns 0x1 if the pin is > > in use), so I'm just saving the actual value we write during > > amdgpio_intr_establish and restoring that during resume. > > Well, using amdgpio_read_pin() for the purpose of saving the pin > configuration doesn't make sense. That function returns the pin input > state. > > What you need to do is to read the register using bus_space_read_4() > and restore that value. Again take a look at pchgpio(4). > > > Note 2: In xxx_activate() functions, we usually call > > config_activate_children > > but since amdgpio doesn't have any children, I left that out. > > I think that's fine. But you should do the save/restore in > DVACT_SUSPEND/DVACT_RESUME. You want to restore the state as early as > possible such that you don't get spurious interrupts when the BIOS > leaves GPIO pins misconfigured. Again, look at pchgpio(4). >
I reworked this diff and made it look just like pchgpio. But it's a little simpler than pchgpio since there is less to save/restore. ok? -ml Index: amdgpio.c =================================================================== RCS file: /cvs/src/sys/dev/acpi/amdgpio.c,v retrieving revision 1.7 diff -u -p -a -u -r1.7 amdgpio.c --- amdgpio.c 6 Apr 2022 18:59:27 -0000 1.7 +++ amdgpio.c 26 Jun 2022 13:53:19 -0000 @@ -48,6 +48,11 @@ struct amdgpio_intrhand { void *ih_arg; }; +struct amdgpio_pincfg { + /* Modeled after pchgpio but we only have one value to save/restore */ + uint32_t pin_cfg; +}; + struct amdgpio_softc { struct device sc_dev; struct acpi_softc *sc_acpi; @@ -59,6 +64,7 @@ struct amdgpio_softc { void *sc_ih; int sc_npins; + struct amdgpio_pincfg *sc_pin_cfg; struct amdgpio_intrhand *sc_pin_ih; struct acpi_gpio sc_gpio; @@ -66,9 +72,11 @@ struct amdgpio_softc { int amdgpio_match(struct device *, void *, void *); void amdgpio_attach(struct device *, struct device *, void *); +int amdgpio_activate(struct device *, int); const struct cfattach amdgpio_ca = { - sizeof(struct amdgpio_softc), amdgpio_match, amdgpio_attach + sizeof(struct amdgpio_softc), amdgpio_match, amdgpio_attach, + NULL, amdgpio_activate }; struct cfdriver amdgpio_cd = { @@ -86,6 +94,10 @@ void amdgpio_write_pin(void *, int, int) void amdgpio_intr_establish(void *, int, int, int (*)(void *), void *); int amdgpio_pin_intr(struct amdgpio_softc *, int); int amdgpio_intr(void *); +void amdgpio_save_pin(struct amdgpio_softc *, int pin); +void amdgpio_save(struct amdgpio_softc *); +void amdgpio_restore_pin(struct amdgpio_softc *, int pin); +void amdgpio_restore(struct amdgpio_softc *); int amdgpio_match(struct device *parent, void *match, void *aux) @@ -135,6 +147,8 @@ amdgpio_attach(struct device *parent, st return; } + sc->sc_pin_cfg = mallocarray(sc->sc_npins, sizeof(*sc->sc_pin_cfg), + M_DEVBUF, M_WAITOK); sc->sc_pin_ih = mallocarray(sc->sc_npins, sizeof(*sc->sc_pin_ih), M_DEVBUF, M_WAITOK | M_ZERO); @@ -159,6 +173,58 @@ amdgpio_attach(struct device *parent, st unmap: free(sc->sc_pin_ih, M_DEVBUF, sc->sc_npins * sizeof(*sc->sc_pin_ih)); bus_space_unmap(sc->sc_memt, sc->sc_memh, aaa->aaa_size[0]); +} + +int +amdgpio_activate(struct device *self, int act) +{ + struct amdgpio_softc *sc = (struct amdgpio_softc *)self; + + switch (act) { + case DVACT_SUSPEND: + amdgpio_save(sc); + break; + case DVACT_RESUME: + amdgpio_restore(sc); + break; + } + + return 0; +} + +void +amdgpio_save_pin(struct amdgpio_softc *sc, int pin) +{ + sc->sc_pin_cfg[pin].pin_cfg = bus_space_read_4(sc->sc_memt, sc->sc_memh, + pin * 4); +} + +void +amdgpio_save(struct amdgpio_softc *sc) +{ + int pin; + + for (pin = 0 ; pin < sc->sc_npins; pin++) + amdgpio_save_pin(sc, pin); +} + +void +amdgpio_restore_pin(struct amdgpio_softc *sc, int pin) +{ + if (!sc->sc_pin_ih[pin].ih_func) + return; + + bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, + sc->sc_pin_cfg[pin].pin_cfg); +} + +void +amdgpio_restore(struct amdgpio_softc *sc) +{ + int pin; + + for (pin = 0; pin < sc->sc_npins; pin++) + amdgpio_restore_pin(sc, pin); } int