Index | Thread | Search

From:
James Hastings <moosetek4@gmail.com>
Subject:
Re: amdgpio(4): add wakeup support
To:
Mark Kettenis <mark.kettenis@xs4all.nl>
Cc:
tech@openbsd.org
Date:
Wed, 4 Jun 2025 05:53:35 -0400

Download raw body.

Thread
On Tue, Jun 3, 2025 at 3:41 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: James Hastings <moosetek4@gmail.com>
> > Date: Mon, 2 Jun 2025 18:37:39 -0400 (EDT)
>
> Hi James,
>
> > Diff below does two things:
> >
> > (1) Teach acpi(4) about gpio wakeup resources.
> >
> > (2) Add gpio wakeup support to amdgpio(4).
> >
> > At suspend time the driver saves the pin configuration as before but now
> > disables all non-wakeup interrupts.
> >
> > ok?
>
> I don't think this is enough to actually wake the machine up as the
> acpi code will send the machine straight back to sleep as you don't
> set sc_wakeup in the acpi(4) softc.
>
> Also, my understanding is that the LR_GPIO_WAKE flag marks a GPIO as
> wakeup *capable*.  But we don't necessarily want to configure all
> wakeup capable GPIOs to actually wake up the system.  That will just
> result in spurious wakeups.
>
> I sent out the diff below that handles the upper layers and passes
> down IPL_WAKEUP to the GPIO drivers.  But some of your bits make me
> suspect that I'm missing some stuff in my code.
>
> I haven't had any test reports for my diff yet, but I'd like to find
> if it works or not.  And if it doesn't I'd like to roll in some of the
> bits from your diff to see if they help.  Is that ok with you?
>

That is ok with me.
I tested your diff by itself and the machine did not wake.

Further testing and a printf in amdgpio_pin_intr() combined with
ddb.suspend=1 confirmed gpio interrupts are working.
Pressing the power button triggers interrupt on pin 0 with
config reg 0x1005f800 each press.
Hard shutdown still required.

> > Index: dev/acpi/dsdt.h
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.h,v
> > retrieving revision 1.82
> > diff -u -p -r1.82 dsdt.h
> > --- dev/acpi/dsdt.h   13 May 2024 01:15:50 -0000      1.82
> > +++ dev/acpi/dsdt.h   2 Jun 2025 22:04:02 -0000
> > @@ -243,6 +243,9 @@ union acpi_resource {
> >               uint16_t        flags;
> >               uint16_t        tflags;
> >  #define LR_GPIO_SHR          (3L << 3)
> > +#define  LR_GPIO_EXCLUSIVE   (0L << 3)
> > +#define  LR_GPIO_SHARED              (1L << 3)
> > +#define  LR_GPIO_WAKE                (2L << 3)
> >  #define LR_GPIO_POLARITY     (3L << 1)
> >  #define  LR_GPIO_ACTHI               (0L << 1)
> >  #define  LR_GPIO_ACTLO               (1L << 1)
> > Index: dev/acpi/amdgpio.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/acpi/amdgpio.c,v
> > retrieving revision 1.10
> > diff -u -p -r1.10 amdgpio.c
> > --- dev/acpi/amdgpio.c        20 Oct 2022 20:40:57 -0000      1.10
> > +++ dev/acpi/amdgpio.c        2 Jun 2025 22:04:02 -0000
> > @@ -32,10 +32,15 @@
> >  #define AMDGPIO_CONF_MASK            0x00000600
> >  #define AMDGPIO_CONF_INT_EN          0x00000800
> >  #define AMDGPIO_CONF_INT_MASK                0x00001000
> > +#define AMDGPIO_CONF_WAKE_S0         0x00002000
> > +#define AMDGPIO_CONF_WAKE_S3         0x00004000
> > +#define AMDGPIO_CONF_WAKE_S4         0x00008000
> > +#define AMDGPIO_CONF_WAKE_MASK               0x0000e000
> >  #define AMDGPIO_CONF_RXSTATE         0x00010000
> >  #define AMDGPIO_CONF_TXSTATE         0x00400000
> >  #define AMDGPIO_CONF_TXSTATE_EN              0x00800000
> >  #define AMDGPIO_CONF_INT_STS         0x10000000
> > +#define AMDGPIO_CONF_WAKE_STS                0x20000000
> >  #define AMDGPIO_IRQ_MASTER_EOI               0x20000000
> >  #define AMDGPIO_IRQ_BITS             46
> >  #define AMDGPIO_IRQ_PINS             4
> > @@ -155,7 +160,7 @@ amdgpio_attach(struct device *parent, st
> >           M_DEVBUF, M_WAITOK | M_ZERO);
> >
> >       sc->sc_ih = acpi_intr_establish(aaa->aaa_irq[0], aaa->aaa_irq_flags[0],
> > -         IPL_BIO, amdgpio_intr, sc, sc->sc_dev.dv_xname);
> > +         IPL_BIO | IPL_WAKEUP, amdgpio_intr, sc, sc->sc_dev.dv_xname);
> >       if (sc->sc_ih == NULL) {
> >               printf(": can't establish interrupt\n");
> >               goto unmap;
> > @@ -201,6 +206,10 @@ amdgpio_save_pin(struct amdgpio_softc *s
> >  {
> >       sc->sc_pin_cfg[pin].pin_cfg = bus_space_read_4(sc->sc_memt, sc->sc_memh,
> >           pin * 4);
> > +
> > +     if (sc->sc_pin_ih[pin].ih_func &&
> > +         (sc->sc_pin_cfg[pin].pin_cfg & AMDGPIO_CONF_WAKE_MASK) == 0)
> > +             amdgpio_intr_disable(sc, pin);
> >  }
> >
> >  void
> > @@ -271,13 +280,15 @@ amdgpio_intr_establish(void *cookie, int
> >
> >       reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
> >       reg &= ~(AMDGPIO_CONF_MASK | AMDGPIO_CONF_LEVEL |
> > -         AMDGPIO_CONF_TXSTATE_EN);
> > +         AMDGPIO_CONF_TXSTATE_EN | AMDGPIO_CONF_WAKE_MASK);
> >       if ((flags & LR_GPIO_MODE) == 0)
> >               reg |= AMDGPIO_CONF_LEVEL;
> >       if ((flags & LR_GPIO_POLARITY) == LR_GPIO_ACTLO)
> >               reg |= AMDGPIO_CONF_ACTLO;
> >       if ((flags & LR_GPIO_POLARITY) == LR_GPIO_ACTBOTH)
> >               reg |= AMDGPIO_CONF_ACTBOTH;
> > +     if ((flags & LR_GPIO_WAKE) == LR_GPIO_WAKE)
> > +             reg |= AMDGPIO_CONF_WAKE_S0;
> >       reg |= (AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
> >       bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
> >  }
> >
> >
>
> Index: arch/amd64/amd64/intr.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/intr.c,v
> diff -u -p -r1.62 intr.c
> --- arch/amd64/amd64/intr.c     23 Apr 2025 15:08:05 -0000      1.62
> +++ arch/amd64/amd64/intr.c     31 May 2025 20:15:47 -0000
> @@ -712,6 +712,13 @@ intr_barrier(void *cookie)
>         sched_barrier(ih->ih_cpu);
>  }
>
> +void
> +intr_set_wakeup(void *cookie)
> +{
> +       struct intrhand *ih = cookie;
> +       ih->ih_flags |= IPL_WAKEUP;
> +}
> +
>  #ifdef SUSPEND
>
>  void
> Index: arch/amd64/include/intr.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/include/intr.h,v
> diff -u -p -r1.35 intr.h
> --- arch/amd64/include/intr.h   23 Apr 2025 15:08:05 -0000      1.35
> +++ arch/amd64/include/intr.h   31 May 2025 20:15:47 -0000
> @@ -210,6 +210,7 @@ int intr_handler(struct intrframe *, str
>  void cpu_intr_init(struct cpu_info *);
>  void intr_printconfig(void);
>  void intr_barrier(void *);
> +void intr_set_wakeup(void *);
>  void intr_enable_wakeup(void);
>  void intr_disable_wakeup(void);
>
> Index: dev/acpi/acpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> diff -u -p -r1.447 acpi.c
> --- dev/acpi/acpi.c     28 May 2025 09:53:53 -0000      1.447
> +++ dev/acpi/acpi.c     31 May 2025 20:15:47 -0000
> @@ -945,7 +945,8 @@ acpi_gpio_parse_events(int crsidx, union
>                         ev->tflags = crs->lr_gpio.tflags;
>                         ev->pin = pin;
>                         gpio->intr_establish(gpio->cookie, pin,
> -                           crs->lr_gpio.tflags, acpi_gpio_event, ev);
> +                           crs->lr_gpio.tflags, IPL_BIO | IPL_WAKEUP,
> +                           acpi_gpio_event, ev);
>                 }
>                 break;
>         default:
> Index: dev/acpi/amdgpio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/amdgpio.c,v
> diff -u -p -r1.10 amdgpio.c
> --- dev/acpi/amdgpio.c  20 Oct 2022 20:40:57 -0000      1.10
> +++ dev/acpi/amdgpio.c  31 May 2025 20:15:47 -0000
> @@ -46,6 +46,7 @@
>  struct amdgpio_intrhand {
>         int (*ih_func)(void *);
>         void *ih_arg;
> +       int ih_wakeup;
>  };
>
>  struct amdgpio_pincfg {
> @@ -91,7 +92,7 @@ const char *amdgpio_hids[] = {
>
>  int    amdgpio_read_pin(void *, int);
>  void   amdgpio_write_pin(void *, int, int);
> -void   amdgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void   amdgpio_intr_establish(void *, int, int, int, int (*)(void *), void *);
>  void   amdgpio_intr_enable(void *, int);
>  void   amdgpio_intr_disable(void *, int);
>  int    amdgpio_pin_intr(struct amdgpio_softc *, int);
> @@ -258,7 +259,7 @@ amdgpio_write_pin(void *cookie, int pin,
>  }
>
>  void
> -amdgpio_intr_establish(void *cookie, int pin, int flags,
> +amdgpio_intr_establish(void *cookie, int pin, int flags, int level,
>      int (*func)(void *), void *arg)
>  {
>         struct amdgpio_softc *sc = cookie;
> @@ -268,6 +269,7 @@ amdgpio_intr_establish(void *cookie, int
>
>         sc->sc_pin_ih[pin].ih_func = func;
>         sc->sc_pin_ih[pin].ih_arg = arg;
> +       sc->sc_pin_ih[pin].ih_wakeup = level & IPL_WAKEUP;
>
>         reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
>         reg &= ~(AMDGPIO_CONF_MASK | AMDGPIO_CONF_LEVEL |
> @@ -280,6 +282,9 @@ amdgpio_intr_establish(void *cookie, int
>                 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);
> +
> +       if (sc->sc_pin_ih[pin].ih_wakeup)
> +               intr_set_wakeup(sc->sc_ih);
>  }
>
>  void
> Index: dev/acpi/amltypes.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/amltypes.h,v
> diff -u -p -r1.50 amltypes.h
> --- dev/acpi/amltypes.h 2 Jun 2024 11:08:41 -0000       1.50
> +++ dev/acpi/amltypes.h 31 May 2025 20:15:47 -0000
> @@ -369,7 +369,8 @@ struct acpi_gpio {
>         void    *cookie;
>         int     (*read_pin)(void *, int);
>         void    (*write_pin)(void *, int, int);
> -       void    (*intr_establish)(void *, int, int, int (*)(void *), void *);
> +       void    (*intr_establish)(void *, int, int, int,
> +                   int (*)(void *), void *);
>         void    (*intr_enable)(void *, int);
>         void    (*intr_disable)(void *, int);
>  };
> Index: dev/acpi/aplgpio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/aplgpio.c,v
> diff -u -p -r1.6 aplgpio.c
> --- dev/acpi/aplgpio.c  20 Oct 2022 20:40:57 -0000      1.6
> +++ dev/acpi/aplgpio.c  31 May 2025 20:15:47 -0000
> @@ -75,7 +75,7 @@ const char *aplgpio_hids[] = {
>
>  int    aplgpio_read_pin(void *, int);
>  void   aplgpio_write_pin(void *, int, int);
> -void   aplgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void   aplgpio_intr_establish(void *, int, int, int, int (*)(void *), void *);
>  void   aplgpio_intr_enable(void *, int);
>  void   aplgpio_intr_disable(void *, int);
>  int    aplgpio_intr(void *);
> @@ -205,7 +205,7 @@ aplgpio_write_pin(void *cookie, int pin,
>  }
>
>  void
> -aplgpio_intr_establish(void *cookie, int pin, int flags,
> +aplgpio_intr_establish(void *cookie, int pin, int flags, int level,
>      int (*func)(void *), void *arg)
>  {
>         struct aplgpio_softc *sc = cookie;
> Index: dev/acpi/bytgpio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/bytgpio.c,v
> diff -u -p -r1.18 bytgpio.c
> --- dev/acpi/bytgpio.c  20 Oct 2022 20:40:57 -0000      1.18
> +++ dev/acpi/bytgpio.c  31 May 2025 20:15:47 -0000
> @@ -102,7 +102,7 @@ const int byt_sus_pins[] = {
>
>  int    bytgpio_read_pin(void *, int);
>  void   bytgpio_write_pin(void *, int, int);
> -void   bytgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void   bytgpio_intr_establish(void *, int, int, int, int (*)(void *), void *);
>  void   bytgpio_intr_enable(void *, int);
>  void   bytgpio_intr_disable(void *, int);
>  int    bytgpio_intr(void *);
> @@ -234,7 +234,7 @@ bytgpio_write_pin(void *cookie, int pin,
>  }
>
>  void
> -bytgpio_intr_establish(void *cookie, int pin, int flags,
> +bytgpio_intr_establish(void *cookie, int pin, int flags, int level,
>      int (*func)(void *), void *arg)
>  {
>         struct bytgpio_softc *sc = cookie;
> Index: dev/acpi/chvgpio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/chvgpio.c,v
> diff -u -p -r1.13 chvgpio.c
> --- dev/acpi/chvgpio.c  20 Oct 2022 20:40:57 -0000      1.13
> +++ dev/acpi/chvgpio.c  31 May 2025 20:15:47 -0000
> @@ -145,7 +145,7 @@ const int chv_southeast_pins[] = {
>  int    chvgpio_check_pin(struct chvgpio_softc *, int);
>  int    chvgpio_read_pin(void *, int);
>  void   chvgpio_write_pin(void *, int, int);
> -void   chvgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void   chvgpio_intr_establish(void *, int, int, int, int (*)(void *), void *);
>  void   chvgpio_intr_enable(void *, int);
>  void   chvgpio_intr_disable(void *, int);
>  int    chvgpio_intr(void *);
> @@ -291,7 +291,7 @@ chvgpio_write_pin(void *cookie, int pin,
>  }
>
>  void
> -chvgpio_intr_establish(void *cookie, int pin, int flags,
> +chvgpio_intr_establish(void *cookie, int pin, int flags, int level,
>      int (*func)(void *), void *arg)
>  {
>         struct chvgpio_softc *sc = cookie;
> Index: dev/acpi/dwgpio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/dwgpio.c,v
> diff -u -p -r1.8 dwgpio.c
> --- dev/acpi/dwgpio.c   13 May 2024 01:15:50 -0000      1.8
> +++ dev/acpi/dwgpio.c   31 May 2025 20:15:47 -0000
> @@ -90,7 +90,7 @@ const char *dwgpio_hids[] = {
>  int    dwgpio_found_hid(struct aml_node *, void *);
>  int    dwgpio_read_pin(void *, int);
>  void   dwgpio_write_pin(void *, int, int);
> -void   dwgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void   dwgpio_intr_establish(void *, int, int, int, int (*)(void *), void *);
>  int    dwgpio_intr(void *);
>
>  int
> @@ -228,7 +228,7 @@ dwgpio_write_pin(void *cookie, int pin,
>  }
>
>  void
> -dwgpio_intr_establish(void *cookie, int pin, int flags,
> +dwgpio_intr_establish(void *cookie, int pin, int flags, int level,
>      int (*func)(void *), void *arg)
>  {
>         struct dwgpio_softc *sc = cookie;
> Index: dev/acpi/dwiic_acpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/dwiic_acpi.c,v
> diff -u -p -r1.22 dwiic_acpi.c
> --- dev/acpi/dwiic_acpi.c       8 Jul 2023 02:43:02 -0000       1.22
> +++ dev/acpi/dwiic_acpi.c       31 May 2025 20:15:47 -0000
> @@ -387,7 +387,7 @@ dwiic_i2c_intr_establish(void *cookie, v
>
>                 struct acpi_gpio *gpio = crs->gpio_int_node->gpio;
>                 gpio->intr_establish(gpio->cookie, crs->gpio_int_pin,
> -                                    crs->gpio_int_flags, func, arg);
> +                   crs->gpio_int_flags, level, func, arg);
>                 return ih;
>         }
>
> Index: dev/acpi/glkgpio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/glkgpio.c,v
> diff -u -p -r1.7 glkgpio.c
> --- dev/acpi/glkgpio.c  13 May 2024 01:15:50 -0000      1.7
> +++ dev/acpi/glkgpio.c  31 May 2025 20:15:47 -0000
> @@ -75,7 +75,7 @@ const char *glkgpio_hids[] = {
>
>  int    glkgpio_read_pin(void *, int);
>  void   glkgpio_write_pin(void *, int, int);
> -void   glkgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void   glkgpio_intr_establish(void *, int, int, int, int (*)(void *), void *);
>  void   glkgpio_intr_enable(void *, int);
>  void   glkgpio_intr_disable(void *, int);
>  int    glkgpio_intr(void *);
> @@ -205,7 +205,7 @@ glkgpio_write_pin(void *cookie, int pin,
>  }
>
>  void
> -glkgpio_intr_establish(void *cookie, int pin, int flags,
> +glkgpio_intr_establish(void *cookie, int pin, int flags, int level,
>      int (*func)(void *), void *arg)
>  {
>         struct glkgpio_softc *sc = cookie;
> Index: dev/acpi/pchgpio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/pchgpio.c,v
> diff -u -p -r1.16 pchgpio.c
> --- dev/acpi/pchgpio.c  18 Aug 2024 11:10:10 -0000      1.16
> +++ dev/acpi/pchgpio.c  31 May 2025 20:15:47 -0000
> @@ -463,7 +463,7 @@ struct pchgpio_match pchgpio_devices[] =
>
>  int    pchgpio_read_pin(void *, int);
>  void   pchgpio_write_pin(void *, int, int);
> -void   pchgpio_intr_establish(void *, int, int, int (*)(void *), void *);
> +void   pchgpio_intr_establish(void *, int, int, int, int (*)(void *), void *);
>  void   pchgpio_intr_enable(void *, int);
>  void   pchgpio_intr_disable(void *, int);
>  int    pchgpio_intr(void *);
> @@ -640,7 +640,7 @@ pchgpio_write_pin(void *cookie, int pin,
>  }
>
>  void
> -pchgpio_intr_establish(void *cookie, int pin, int flags,
> +pchgpio_intr_establish(void *cookie, int pin, int flags, int level,
>      int (*func)(void *), void *arg)
>  {
>         struct pchgpio_softc *sc = cookie;
> Index: dev/acpi/pckbc_acpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/pckbc_acpi.c,v
> diff -u -p -r1.4 pckbc_acpi.c
> --- dev/acpi/pckbc_acpi.c       28 May 2025 07:04:27 -0000      1.4
> +++ dev/acpi/pckbc_acpi.c       31 May 2025 20:15:47 -0000
> @@ -290,8 +290,8 @@ pckbc_acpi_attach_kbd(struct device *par
>                         if (pasc->sc_nints == nitems(pasc->sc_ih))
>                                 break;
>                         pasc->sc_ih[pasc->sc_nints] = acpi_intr_establish(
> -                           aaa->aaa_irq[irq], aaa->aaa_irq_flags[irq], IPL_TTY,
> -                           pckbcintr, pasc, self->dv_xname);
> +                           aaa->aaa_irq[irq], aaa->aaa_irq_flags[irq],
> +                           IPL_TTY, pckbcintr, pasc, self->dv_xname);
>                         if (pasc->sc_ih[pasc->sc_nints] == NULL) {
>                                 printf("%s: can't establish interrupt %d\n",
>                                     self->dv_xname, aaa->aaa_irq[irq]);
> @@ -392,8 +392,8 @@ pckbc_acpi_attach_mouse(struct device *p
>                         if (pasc->sc_nints == nitems(pasc->sc_ih))
>                                 break;
>                         pasc->sc_ih[pasc->sc_nints] = acpi_intr_establish(
> -                           aaa->aaa_irq[irq], aaa->aaa_irq_flags[irq], IPL_TTY,
> -                           pckbcintr, pasc, self->dv_xname);
> +                           aaa->aaa_irq[irq], aaa->aaa_irq_flags[irq],
> +                           IPL_TTY, pckbcintr, pasc, self->dv_xname);
>                         if (pasc->sc_ih[pasc->sc_nints] == NULL) {
>                                 printf("%s: can't establish interrupt %d\n",
>                                     self->dv_xname, aaa->aaa_irq[irq]);
> @@ -433,7 +433,7 @@ pckbc_acpi_register_gpio_intrs(struct de
>                 }
>                 gpio->intr_establish(gpio->cookie,
>                     sc->sc_gpioint[irq].pin, sc->sc_gpioint[irq].flags,
> -                   pckbc_acpi_gpio_intr_wrapper, sc);
> +                   IPL_TTY, pckbc_acpi_gpio_intr_wrapper, sc);
>         }
>  }
>
> Index: dev/acpi/sdhc_acpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/sdhc_acpi.c,v
> diff -u -p -r1.23 sdhc_acpi.c
> --- dev/acpi/sdhc_acpi.c        9 Oct 2024 00:38:25 -0000       1.23
> +++ dev/acpi/sdhc_acpi.c        31 May 2025 20:15:47 -0000
> @@ -131,7 +131,8 @@ sdhc_acpi_attach(struct device *parent,
>                 struct acpi_gpio *gpio = sc->sc_gpio_int_node->gpio;
>
>                 gpio->intr_establish(gpio->cookie, sc->sc_gpio_int_pin,
> -                   sc->sc_gpio_int_flags, sdhc_acpi_card_detect_intr, sc);
> +                   sc->sc_gpio_int_flags, IPL_BIO,
> +                   sdhc_acpi_card_detect_intr, sc);
>         }
>
>         sdhc_acpi_power_on(sc, sc->sc_node);
>