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
