On Thu, May 09, 2024 at 06:29:42PM +0200, Mark Kettenis wrote:
> > Date: Sun, 21 Apr 2024 17:04:21 +0000
> > From: James Cook <falsifian@falsifian.org>
> >
> > This patch is adapted from one deraadt sent me in February.
> >
> > Without this patch, if I wake my laptop by pressing the power button,
> > the laptop shuts down immediately after waking. The patch seems to
> > solve the problem.
> >
> > I spent some time trying to be clever and re-enable the power button
> > right after resuming was done, without using gettime(), but I don't
> > understand how ACPI events work and could not push the re-enable-ing
> > late enough. So I ended up sticking with this approach.
> >
> > Originally reported at
> > https://marc.info/?l=openbsd-bugs&m=161038121430416&w=2 , but other
> > bugs I mentioned on that thread have already disappeared.
> >
> > --
> > James
> >
> >
> > diff refs/heads/master 16b4fd4f9a94db0f64c04f33b222b076e5808b3c
> > commit - b809db8c59b9d17241cabc19d997f1eceddcff41
> > commit + 16b4fd4f9a94db0f64c04f33b222b076e5808b3c
> > blob - 70b67bb8fa7fb283c773098abb0d31dc01c323d1
> > blob + da1703340257c980fe491db6323ef439dca4a9ed
> > --- sys/dev/acpi/acpi.c
> > +++ sys/dev/acpi/acpi.c
> > @@ -2014,6 +2014,17 @@ acpi_gpe_task(void *arg0, int gpe)
> >  	}
> >  }
> >
> > +/* Ignore the power button for a short time after a resume */
> > +int
> > +acpi_buttonvalid(void)
> > +{
> > +	extern time_t waketime;
> > +
> > +	if (gettime() < waketime + 10)
> > +		return 0;
> > +	return 1;
> > +}
> > +
> >  void
> >  acpi_pbtn_task(void *arg0, int dummy)
> >  {
> > @@ -2035,7 +2046,8 @@ acpi_pbtn_task(void *arg0, int dummy)
> >  	case 0:
> >  		break;
> >  	case 1:
> > -		acpi_addtask(sc, acpi_powerdown_task, sc, 0);
> > +		if (acpi_buttonvalid())
> > +			acpi_addtask(sc, acpi_powerdown_task, sc, 0);
> >  		break;
> >  #ifndef SMALL_KERNEL
> >  	case 2:
> > blob - 373f05e1377fad67f7435b3ba238fdc6fae4fec4
> > blob + cbced59a97fbb257f3290e769c1eb8ba837553d8
> > --- sys/dev/acpi/acpibtn.c
> > +++ sys/dev/acpi/acpibtn.c
> > @@ -286,8 +286,9 @@ sleep:
> >  			case 0:
> >  				break;
> >  			case 1:
> > -				acpi_addtask(sc->sc_acpi, acpi_powerdown_task,
> > -				    sc->sc_acpi, 0);
> > +				if (acpi_buttonvalid())
> > +					acpi_addtask(sc->sc_acpi, acpi_powerdown_task,
> > +					    sc->sc_acpi, 0);
> >  				break;
> >  #ifndef SMALL_KERNEL
> >  			case 2:
> > blob - 20521653bc8ff99e82a0f7ccb4e498b6a92bf0c7
> > blob + 2bddceaa6689a1250b10b256a8c24657cd15d2bb
> > --- sys/dev/acpi/acpivar.h
> > +++ sys/dev/acpi/acpivar.h
> > @@ -371,6 +371,7 @@ void	acpi_write_pmreg(struct acpi_softc *, int, int, i
> >
> >  void	acpi_poll(void *);
> >  void	acpi_sleep(int, char *);
> > +int	acpi_buttonvalid(void);
> >
> >  int	acpi_matchcls(struct acpi_attach_args *, int, int, int);
> >  int	acpi_matchhids(struct acpi_attach_args *, const char *[], const char *);
> > blob - 1b4f406291812c59af2d4fc8e3910e618c063f40
> > blob + ab1fffb6099d40fcf2db9c5cb090729c20d40cd5
> > --- sys/kern/subr_suspend.c
> > +++ sys/kern/subr_suspend.c
> > @@ -45,6 +45,8 @@ device_register_wakeup(struct device *dev)
> >  	wakeup_devices++;
> >  }
> >
> > +time_t waketime;
> > +
> >  int
> >  sleep_state(void *v, int sleepmode)
> >  {
> > @@ -180,6 +182,8 @@ fail_suspend:
> >  	clockintr_cpu_init(NULL);
> >  	clockintr_trigger();
> >
> > +	waketime = gettime();
> > +
> >  	sleep_resume(v);
> >  	resume_randomness(rndbuf, rndbuflen);
> >  #ifdef MULTIPROCESSOR
> >
>
> A ran into a similar issue while working on non-S3 suspend.  I
> initially used a timeout, but using a timestamp is simpler.  But I
> think this should use getuptime() instead of gettime().  I also don't
> think we need this for non-ACPI code, so I reworked it a bit to keep
> the changes restricted to the ACPI code.
>
> Does this still work for you?
>

if this works for OP, it looks fine to me. ok mlarkin if/when you're
ready.

>
> Index: dev/acpi/acpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> retrieving revision 1.426
> diff -u -p -r1.426 acpi.c
> --- dev/acpi/acpi.c	8 Jan 2024 19:52:29 -0000	1.426
> +++ dev/acpi/acpi.c	9 May 2024 16:21:31 -0000
> @@ -1965,6 +1965,12 @@ acpi_sleep_task(void *arg0, int sleepmod
>
>  #endif /* SMALL_KERNEL */
>
> +int
> +acpi_resuming(struct acpi_softc *sc)
> +{
> +	return (getuptime() < sc->sc_resume_time + 10);
> +}
> +
>  void
>  acpi_reset(void)
>  {
> @@ -2030,6 +2036,10 @@ acpi_pbtn_task(void *arg0, int dummy)
>  	acpi_write_pmreg(sc, ACPIREG_PM1_EN,  0,
>  	    en | ACPI_PM1_PWRBTN_EN);
>  	splx(s);
> +
> +	/* Ignore button events if we're resuming. */
> +	if (acpi_resuming(sc))
> +		return;
>
>  	switch (pwr_action) {
>  	case 0:
> Index: dev/acpi/acpi_x86.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpi_x86.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 acpi_x86.c
> --- dev/acpi/acpi_x86.c	6 Jul 2023 06:58:07 -0000	1.17
> +++ dev/acpi/acpi_x86.c	9 May 2024 16:21:31 -0000
> @@ -114,6 +114,8 @@ sleep_resume(void *v)
>  {
>  	struct acpi_softc *sc = v;
>
> +	sc->sc_resume_time = getuptime();
> +
>  	acpibtn_disable_psw();		/* disable _LID for wakeup */
>
>  	/* 3rd resume AML step: _TTS(runstate) */
> Index: dev/acpi/acpibtn.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpibtn.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 acpibtn.c
> --- dev/acpi/acpibtn.c	29 Jun 2023 20:58:08 -0000	1.51
> +++ dev/acpi/acpibtn.c	9 May 2024 16:21:31 -0000
> @@ -229,6 +229,10 @@ acpibtn_notify(struct aml_node *node, in
>  	dnprintf(10, "acpibtn_notify: %.2x %s\n", notify_type,
>  	    sc->sc_devnode->name);
>
> +	/* Ignore button events if we're resuming. */
> +	if (acpi_resuming(sc->sc_acpi))
> +		return (0);
> +
>  	switch (sc->sc_btn_type) {
>  	case ACPIBTN_LID:
>  #ifndef SMALL_KERNEL
> Index: dev/acpi/acpivar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
> retrieving revision 1.125
> diff -u -p -r1.125 acpivar.h
> --- dev/acpi/acpivar.h	29 Nov 2023 03:41:31 -0000	1.125
> +++ dev/acpi/acpivar.h	9 May 2024 16:21:31 -0000
> @@ -269,6 +269,7 @@ struct acpi_softc {
>  	struct aml_node		*sc_sst;
>  	struct aml_node		*sc_wak;
>  	int			sc_state;
> +	time_t			sc_resume_time;
>  	struct acpiec_softc	*sc_ec;		/* XXX assume single EC */
>
>  	struct acpi_ac_head	sc_ac;
> @@ -341,6 +342,7 @@ void	 acpi_sleep_pm(struct acpi_softc *,
>  void	 acpi_resume_pm(struct acpi_softc *, int);
>  void	 acpi_resume_cpu(struct acpi_softc *, int);
>  void	 acpi_sleep_walk(struct acpi_softc *, int);
> +int	 acpi_resuming(struct acpi_softc *);
>
>  #define ACPI_IOREAD 0
>  #define ACPI_IOWRITE 1
>