On Sat, Jan 25 2020, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote: > So I have this diff for apmd -z/-Z that uses APM_POWER_CHANGE events to > trigger autosuspend. It works fine except for one glitch: if I plug the > AC cable and then resume, apmd will receive another APM_POWER_CHANGE > event and read the power info only to find that the AC is *unplugged*. > So apmd happily suspends my system again. If I resume once more, the AC > status is correctly detected and apmd(8) doesn't suspend the system again. > > What happens is that acpibat_notify runs because of ACPIDEV_POLL, it > grabs battery data and sends an APM_POWER_CHANGE event to apmd. At this > point apmd may see out of date data from acpiac(4). > > acpi_sleep_task later forces a refresh of (in order) acpiac(4), > acpibat(4) and acpisbs(4), but in my use case it's too late, the window > with stale acpiac(4) data is too long. > > The diff below refreshes AC status with notifications at DVACT_WAKEUP > time. Is is correct that DVACT_WAKEUP handlers will always run before > the ACPI task queue is processed? "No" seems unlikely but I'd rather > ask... > > I hear we prefer running ACPI code in its dedicated thread but there are > other drivers in dev/acpi that run ACPI code in DVACT_WAKEUP, so I'm > assuming it is fine. :) > > There are probably many ways to tackle this problem, from having > userland ignore stale data after a resume to tweaking acpibat(4) to > stop/restart/skip the ACPIDEV_POLL loop, but all of them look > hackish/undoable so far. > > Thoughts? oks?
Still looking for feedback regarding the approach used, and also for test reports. Diff included again for convenience, Index: acpiac.c =================================================================== RCS file: /d/cvs/src/sys/dev/acpi/acpiac.c,v retrieving revision 1.31 diff -u -p -r1.31 acpiac.c --- acpiac.c 1 Jul 2018 19:40:49 -0000 1.31 +++ acpiac.c 21 Jan 2020 01:43:15 -0000 @@ -33,13 +33,18 @@ int acpiac_match(struct device *, void *, void *); void acpiac_attach(struct device *, struct device *, void *); +int acpiac_activate(struct device *, int); int acpiac_notify(struct aml_node *, int, void *); void acpiac_refresh(void *); int acpiac_getsta(struct acpiac_softc *); struct cfattach acpiac_ca = { - sizeof(struct acpiac_softc), acpiac_match, acpiac_attach + sizeof(struct acpiac_softc), + acpiac_match, + acpiac_attach, + NULL, + acpiac_activate, }; struct cfdriver acpiac_cd = { @@ -92,6 +97,21 @@ acpiac_attach(struct device *parent, str acpiac_notify, sc, ACPIDEV_NOPOLL); } +int +acpiac_activate(struct device *self, int act) +{ + struct acpiac_softc *sc = (struct acpiac_softc *)self; + + switch (act) { + case DVACT_WAKEUP: + acpiac_refresh(sc); + dnprintf(10, "A/C status: %d\n", sc->sc_ac_stat); + break; + } + + return (0); +} + void acpiac_refresh(void *arg) { @@ -99,7 +119,6 @@ acpiac_refresh(void *arg) acpiac_getsta(sc); sc->sc_sens[0].value = sc->sc_ac_stat; - acpi_record_event(sc->sc_acpi, APM_POWER_CHANGE); } int @@ -140,6 +159,7 @@ acpiac_notify(struct aml_node *node, int /* FALLTHROUGH */ case 0x80: acpiac_refresh(sc); + acpi_record_event(sc->sc_acpi, APM_POWER_CHANGE); dnprintf(10, "A/C status: %d\n", sc->sc_ac_stat); break; } -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE