On Sat, Jan 25 2020, Jeremie Courreges-Anglas <[email protected]> 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
