Re: Suspend/Resume on modern Intel laptop platforms

2021-09-20 Thread Thomas Frohwein
On Thu, Sep 16, 2021 at 12:11:24AM +0200, Mark Kettenis wrote:
> > Date: Wed, 15 Sep 2021 17:29:39 +0200 (CEST)
> > From: Mark Kettenis 
> > 
> > The diff below is a preliminary diff to fix a suspend/resume issue on
> > recent Thinkpads.  This needs testing on a wider range of laptops to
> > make sure it doesn't break things.  The diff also puts some
> > information in dmesg that will help me improve things in the future.
> > 
> > So, if you have a laptop where pchgpio(4) attaches *and* supports S3
> > suspen/resume, please apply this diff, do a suspend/resume cycle and
> > send me a dmesg collected after that suspend/resume cycle.
> 
> This diff is now in snapshots, so instead of applying the diff and
> build your own kernel, you can just upgrade to the latest snapshot.

My ASUS Expertbook B9400 has pchgpio(4) attach, so here a dmesg after
a suspend and resume. I don't notice any changes - resuming after a few
minutes of sleep (zzz) or stand-by (apm -S) works, but if I leave the
laptop alone for longer, it doesn't wake anymore. None of opening lid,
pressing keys, or pressing power button brings it back then. That was
the same before and after the update to snapshot from 16 Sep.

(I have machdep.pwraction=0 in /etc/sysctl.conf, but doubt that would
be related...)

OpenBSD 7.0 (GENERIC.MP) #219: Thu Sep 16 00:17:22 MDT 2021
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 16825262080 (16045MB)
avail mem = 16299315200 (15544MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.3 @ 0x5a4b2000 (95 entries)
bios0: vendor ASUSTeK COMPUTER INC. version "B9400CEA.304" date 06/02/2021
bios0: ASUSTeK COMPUTER INC. ASUS EXPERTBOOK B9400CEA_B9450CEA
acpi0 at bios0: ACPI 6.2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP MCFG SSDT FIDT ECDT MSDM SSDT SSDT SSDT SSDT HPET APIC 
SSDT NHLT LPIT SSDT DBGP DBG2 SSDT DMAR SSDT SSDT BGRT TPM2 PTDT WSMT FPDT
acpi0: wakeup devices PEGP(S4) PEGP(S4) PEGP(S4) PEG0(S4) PEGP(S4) GLAN(S4) 
XHCI(S3) XDCI(S4) HDAS(S4) CNVW(S4) TXHC(S3) TDM0(S4) TRP0(S3) PXSX(S4) 
TRP1(S3) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xc000, bus 0-255
acpiec0 at acpi0
acpihpet0 at acpi0: 1920 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 4689.19 MHz, 06-8c-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 256KB 64b/line disabled L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 38MHz
cpu0: mwait min=64, max=64, C-substates=0.2.0.1.2.1.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 4689.19 MHz, 06-8c-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu1: 256KB 64b/line disabled L2 cache
cpu1: disabling user TSC (skew=-911753242)
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 4090.57 MHz, 06-8c-01
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu2: 256KB 64b/line disabled L2 cache
cpu2: disabling user TSC (skew=-911753252)
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 

Re: Suspend/Resume on modern Intel laptop platforms

2021-09-15 Thread Mark Kettenis
> Date: Wed, 15 Sep 2021 17:29:39 +0200 (CEST)
> From: Mark Kettenis 
> 
> The diff below is a preliminary diff to fix a suspend/resume issue on
> recent Thinkpads.  This needs testing on a wider range of laptops to
> make sure it doesn't break things.  The diff also puts some
> information in dmesg that will help me improve things in the future.
> 
> So, if you have a laptop where pchgpio(4) attaches *and* supports S3
> suspen/resume, please apply this diff, do a suspend/resume cycle and
> send me a dmesg collected after that suspend/resume cycle.

This diff is now in snapshots, so instead of applying the diff and
build your own kernel, you can just upgrade to the latest snapshot.

Thanks,

Mark

> Index: dev/acpi/pchgpio.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/pchgpio.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 pchgpio.c
> --- dev/acpi/pchgpio.c30 Aug 2021 18:40:19 -  1.5
> +++ dev/acpi/pchgpio.c15 Sep 2021 15:16:52 -
> @@ -28,12 +28,13 @@
>  
>  #define PCHGPIO_MAXCOM   4
>  
> -#define PCHGPIO_CONF_TXSTATE 0x0001
> -#define PCHGPIO_CONF_RXSTATE 0x0002
> -#define PCHGPIO_CONF_RXINV   0x0080
> -#define PCHGPIO_CONF_RXEV_EDGE   0x0200
> -#define PCHGPIO_CONF_RXEV_ZERO   0x0400
> -#define PCHGPIO_CONF_RXEV_MASK   0x0600
> +#define PCHGPIO_CONF_TXSTATE 0x0001
> +#define PCHGPIO_CONF_RXSTATE 0x0002
> +#define PCHGPIO_CONF_RXINV   0x0080
> +#define PCHGPIO_CONF_RXEV_EDGE   0x0200
> +#define PCHGPIO_CONF_RXEV_ZERO   0x0400
> +#define PCHGPIO_CONF_RXEV_MASK   0x0600
> +#define PCHGPIO_CONF_PADRSTCFG_MASK  0xc000
>  
>  #define PCHGPIO_PADBAR   0x00c
>  
> @@ -59,6 +60,11 @@ struct pchgpio_match {
>   const struct pchgpio_device *device;
>  };
>  
> +struct pchgpio_pincfg {
> + uint32_tpad_cfg_dw0;
> + uint32_tpad_cfg_dw1;
> +};
> +
>  struct pchgpio_intrhand {
>   int (*ih_func)(void *);
>   void *ih_arg;
> @@ -80,6 +86,7 @@ struct pchgpio_softc {
>   int sc_padsize;
>  
>   int sc_npins;
> + struct pchgpio_pincfg *sc_pin_cfg;
>   struct pchgpio_intrhand *sc_pin_ih;
>  
>   struct acpi_gpio sc_gpio;
> @@ -87,9 +94,11 @@ struct pchgpio_softc {
>  
>  int  pchgpio_match(struct device *, void *, void *);
>  void pchgpio_attach(struct device *, struct device *, void *);
> +int  pchgpio_activate(struct device *, int);
>  
>  struct cfattach pchgpio_ca = {
> - sizeof(struct pchgpio_softc), pchgpio_match, pchgpio_attach
> + sizeof(struct pchgpio_softc), pchgpio_match, pchgpio_attach,
> + NULL, pchgpio_activate
>  };
>  
>  struct cfdriver pchgpio_cd = {
> @@ -170,6 +179,8 @@ int   pchgpio_read_pin(void *, int);
>  void pchgpio_write_pin(void *, int, int);
>  void pchgpio_intr_establish(void *, int, int, int (*)(void *), void *);
>  int  pchgpio_intr(void *);
> +void pchgpio_save(struct pchgpio_softc *);
> +void pchgpio_restore(struct pchgpio_softc *);
>  
>  int
>  pchgpio_match(struct device *parent, void *match, void *aux)
> @@ -240,6 +251,8 @@ pchgpio_attach(struct device *parent, st
>  
>   sc->sc_padsize = sc->sc_device->pad_size;
>   sc->sc_npins = sc->sc_device->npins;
> + 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);
>  
> @@ -263,11 +276,48 @@ pchgpio_attach(struct device *parent, st
>  
>  unmap:
>   free(sc->sc_pin_ih, M_DEVBUF, sc->sc_npins * sizeof(*sc->sc_pin_ih));
> + free(sc->sc_pin_cfg, M_DEVBUF, sc->sc_npins * sizeof(*sc->sc_pin_cfg));
>   for (i = 0; i < sc->sc_naddr; i++)
>   bus_space_unmap(sc->sc_memt[i], sc->sc_memh[i],
>   aaa->aaa_size[i]);
>  }
>  
> +int
> +pchgpio_activate(struct device *self, int act)
> +{
> + struct pchgpio_softc *sc = (struct pchgpio_softc *)self;
> +//   int i, j;
> +
> + switch (act) {
> + case DVACT_SUSPEND:
> + printf("%s: suspend\n", sc->sc_dev.dv_xname);
> +#if 0
> + for (i = 0; i < 4; i++) {
> + for (j = 0; j < 0xc00; j += 4) {
> + printf("%04x: 0x%08x\n", j,
> + bus_space_read_4(sc->sc_memt[i], 
> sc->sc_memh[i], j));
> + }
> + }
> +#endif
> + pchgpio_save(sc);
> + break;
> + case DVACT_RESUME:
> + printf("%s: resume\n", sc->sc_dev.dv_xname);
> +#if 0
> + for (i = 0; i < 4; i++) {
> + for (j = 0; j < 0xc00; j += 4) {
> + printf("%04x: 0x%08x\n", j,
> + bus_space_read_4(sc->sc_memt[i], 
> sc->sc_memh[i], j));
> + }
> + }
> 

Suspend/Resume on modern Intel laptop platforms

2021-09-15 Thread Mark Kettenis
The diff below is a preliminary diff to fix a suspend/resume issue on
recent Thinkpads.  This needs testing on a wider range of laptops to
make sure it doesn't break things.  The diff also puts some
information in dmesg that will help me improve things in the future.

So, if you have a laptop where pchgpio(4) attaches *and* supports S3
suspen/resume, please apply this diff, do a suspend/resume cycle and
send me a dmesg collected after that suspend/resume cycle.

Thanks,

Mark


Index: dev/acpi/pchgpio.c
===
RCS file: /cvs/src/sys/dev/acpi/pchgpio.c,v
retrieving revision 1.5
diff -u -p -r1.5 pchgpio.c
--- dev/acpi/pchgpio.c  30 Aug 2021 18:40:19 -  1.5
+++ dev/acpi/pchgpio.c  15 Sep 2021 15:16:52 -
@@ -28,12 +28,13 @@
 
 #define PCHGPIO_MAXCOM 4
 
-#define PCHGPIO_CONF_TXSTATE   0x0001
-#define PCHGPIO_CONF_RXSTATE   0x0002
-#define PCHGPIO_CONF_RXINV 0x0080
-#define PCHGPIO_CONF_RXEV_EDGE 0x0200
-#define PCHGPIO_CONF_RXEV_ZERO 0x0400
-#define PCHGPIO_CONF_RXEV_MASK 0x0600
+#define PCHGPIO_CONF_TXSTATE   0x0001
+#define PCHGPIO_CONF_RXSTATE   0x0002
+#define PCHGPIO_CONF_RXINV 0x0080
+#define PCHGPIO_CONF_RXEV_EDGE 0x0200
+#define PCHGPIO_CONF_RXEV_ZERO 0x0400
+#define PCHGPIO_CONF_RXEV_MASK 0x0600
+#define PCHGPIO_CONF_PADRSTCFG_MASK0xc000
 
 #define PCHGPIO_PADBAR 0x00c
 
@@ -59,6 +60,11 @@ struct pchgpio_match {
const struct pchgpio_device *device;
 };
 
+struct pchgpio_pincfg {
+   uint32_tpad_cfg_dw0;
+   uint32_tpad_cfg_dw1;
+};
+
 struct pchgpio_intrhand {
int (*ih_func)(void *);
void *ih_arg;
@@ -80,6 +86,7 @@ struct pchgpio_softc {
int sc_padsize;
 
int sc_npins;
+   struct pchgpio_pincfg *sc_pin_cfg;
struct pchgpio_intrhand *sc_pin_ih;
 
struct acpi_gpio sc_gpio;
@@ -87,9 +94,11 @@ struct pchgpio_softc {
 
 intpchgpio_match(struct device *, void *, void *);
 void   pchgpio_attach(struct device *, struct device *, void *);
+intpchgpio_activate(struct device *, int);
 
 struct cfattach pchgpio_ca = {
-   sizeof(struct pchgpio_softc), pchgpio_match, pchgpio_attach
+   sizeof(struct pchgpio_softc), pchgpio_match, pchgpio_attach,
+   NULL, pchgpio_activate
 };
 
 struct cfdriver pchgpio_cd = {
@@ -170,6 +179,8 @@ int pchgpio_read_pin(void *, int);
 void   pchgpio_write_pin(void *, int, int);
 void   pchgpio_intr_establish(void *, int, int, int (*)(void *), void *);
 intpchgpio_intr(void *);
+void   pchgpio_save(struct pchgpio_softc *);
+void   pchgpio_restore(struct pchgpio_softc *);
 
 int
 pchgpio_match(struct device *parent, void *match, void *aux)
@@ -240,6 +251,8 @@ pchgpio_attach(struct device *parent, st
 
sc->sc_padsize = sc->sc_device->pad_size;
sc->sc_npins = sc->sc_device->npins;
+   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);
 
@@ -263,11 +276,48 @@ pchgpio_attach(struct device *parent, st
 
 unmap:
free(sc->sc_pin_ih, M_DEVBUF, sc->sc_npins * sizeof(*sc->sc_pin_ih));
+   free(sc->sc_pin_cfg, M_DEVBUF, sc->sc_npins * sizeof(*sc->sc_pin_cfg));
for (i = 0; i < sc->sc_naddr; i++)
bus_space_unmap(sc->sc_memt[i], sc->sc_memh[i],
aaa->aaa_size[i]);
 }
 
+int
+pchgpio_activate(struct device *self, int act)
+{
+   struct pchgpio_softc *sc = (struct pchgpio_softc *)self;
+// int i, j;
+
+   switch (act) {
+   case DVACT_SUSPEND:
+   printf("%s: suspend\n", sc->sc_dev.dv_xname);
+#if 0
+   for (i = 0; i < 4; i++) {
+   for (j = 0; j < 0xc00; j += 4) {
+   printf("%04x: 0x%08x\n", j,
+   bus_space_read_4(sc->sc_memt[i], 
sc->sc_memh[i], j));
+   }
+   }
+#endif
+   pchgpio_save(sc);
+   break;
+   case DVACT_RESUME:
+   printf("%s: resume\n", sc->sc_dev.dv_xname);
+#if 0
+   for (i = 0; i < 4; i++) {
+   for (j = 0; j < 0xc00; j += 4) {
+   printf("%04x: 0x%08x\n", j,
+   bus_space_read_4(sc->sc_memt[i], 
sc->sc_memh[i], j));
+   }
+   }
+#endif
+   pchgpio_restore(sc);
+   break;
+   }
+
+   return 0;
+}
+
 const struct pchgpio_group *
 pchgpio_find_group(struct pchgpio_softc *sc, int pin)
 {
@@ -403,4 +453,74 @@ pchgpio_intr(void *arg)
}
 
return handled;
+}
+
+void
+pchgpio_save(struct pchgpio_softc *sc)
+{
+   int gpiobase, group, bit, pin, pad;
+   uint16_t base, limit;
+