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?
Index: acpiac.c
===================================================================
RCS file: /d/cvs/src/sys/dev/acpi/acpiac.c,v
retrieving revision 1.31
diff -u -p -p -u -r1.31 acpiac.c
--- acpiac.c 1 Jul 2018 19:40:49 -0000 1.31
+++ acpiac.c 25 Jan 2020 18:33: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
