> 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).
>
> ok?
>
> -ml
>
>
> diff a82721d2c9ea32a8f6043a3e06b2a7f8280ef68b /export/bin/src/OpenBSD/g14
> blob - 1d0cd5fcede71f0495a271a9d06fc9c0ecb16412
> file + sys/dev/acpi/amdgpio.c
> --- sys/dev/acpi/amdgpio.c
> +++ sys/dev/acpi/amdgpio.c
> @@ -62,13 +62,17 @@ struct amdgpio_softc {
> struct amdgpio_intrhand *sc_pin_ih;
>
> struct acpi_gpio sc_gpio;
> +
> + uint32_t *sc_pincfg;
> };
>
> 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 = {
> @@ -98,6 +102,24 @@ amdgpio_match(struct device *parent, void *match, void
> return acpi_matchhids(aaa, amdgpio_hids, cf->cf_driver->cd_name);
> }
>
> +int
> +amdgpio_activate(struct device *self, int act)
> +{
> + struct amdgpio_softc *sc = (struct amdgpio_softc *)self;
> + int rv = 0, i;
> +
> + switch (act) {
> + case DVACT_WAKEUP:
> + for (i = 0; i < sc->sc_npins; i++) {
> + if (sc->sc_pincfg[i])
> + bus_space_write_4(sc->sc_memt, sc->sc_memh,
> + i * 4, sc->sc_pincfg[i]);
> + }
> + }
> +
> + return (rv);
> +}
> +
> void
> amdgpio_attach(struct device *parent, struct device *self, void *aux)
> {
> @@ -152,6 +174,8 @@ amdgpio_attach(struct device *parent, struct device *s
> sc->sc_node->gpio = &sc->sc_gpio;
>
> printf(", %d pins\n", sc->sc_npins);
> + sc->sc_pincfg = malloc(sc->sc_npins * sizeof(uint32_t), M_DEVBUF,
> + M_WAITOK | M_ZERO);
>
> acpi_register_gpio(sc->sc_acpi, sc->sc_node);
> return;
> @@ -210,6 +234,8 @@ amdgpio_intr_establish(void *cookie, int pin, int flag
> reg |= AMDGPIO_CONF_ACTBOTH;
> reg |= (AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
> bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
> +
> + sc->sc_pincfg[pin] = reg;
> }
>
> int
>
>
