> Date: Sun, 20 Dec 2020 16:23:25 -0600 > From: Scott Cheloha <scottchel...@gmail.com> > > Short version: > > Please test if this patch breaks suspend or hibernate on your > machine. Reply with the results of your test and your dmesg. > > Both are still working on my Lenovo Thinkpad X1 Carbon Gen 6. > > Long version: > > This patch converts the remaining tick sleeps in dev/acpi to > nanosecond sleeps. > > In acpiec_wait() we have a tsleep(9) for up to 1 tick. There is no > discernable unit in this code and no timeout so as with all the other > 1 tick sleeps I think tsleep_nsec(9) for 1ms will work fine. It may > oversleep 1ms but because there is no timeout it doesn't really > matter. > > In acpi_event_wait() we have a timeout in milliseconds. Currently we > sleep for up to 1 tick and deduct time from the timeout when the sleep > returns EWOULDBLOCK. Of note here is that this code is broken on > kernels where hz > 1000: we will never time out. > > I have changed the code to rwsleep_nsec(9) for at least 1ms. The > timeout is in milliseconds so this seems logical to me. > > The caveat is that on default HZ=100 kernels we will oversleep and it > will take much longer for us to time out in this loop. We can fix > this by measuring the sleep and deducting the elapsed time from the > total timeout. Or, if it doesn't really matter, we can just oversleep > and not worry about it. > > Preferences? > > Assuming we get no bad tests back, OK? > > Index: acpiec.c > =================================================================== > RCS file: /cvs/src/sys/dev/acpi/acpiec.c,v > retrieving revision 1.62 > diff -u -p -r1.62 acpiec.c > --- acpiec.c 26 Aug 2020 03:29:06 -0000 1.62 > +++ acpiec.c 20 Dec 2020 22:19:14 -0000 > @@ -105,8 +105,10 @@ acpiec_wait(struct acpiec_softc *sc, uin > sc->sc_gotsci = 1; > if (cold || (stat & EC_STAT_BURST)) > delay(1); > - else > - tsleep(&acpiecnowait, PWAIT, "acpiec", 1); > + else { > + tsleep_nsec(&acpiecnowait, PWAIT, "acpiec", > + MSEC_TO_NSEC(1)); > + } > } > > dnprintf(40, "%s: EC wait_ns, stat: %b\n", DEVNAME(sc), (int)stat,
This ties in with the discussion we had earlier about sleeping without a wakeup channel. Whatever solution is agreed upon there should probably be applied here. > Index: dsdt.c > =================================================================== > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v > retrieving revision 1.257 > diff -u -p -r1.257 dsdt.c > --- dsdt.c 17 Dec 2020 17:57:19 -0000 1.257 > +++ dsdt.c 20 Dec 2020 22:19:15 -0000 > @@ -2907,10 +2907,10 @@ acpi_event_wait(struct aml_scope *scope, > if (acpi_dotask(acpi_softc)) > continue; > if (!cold) { > - if (rwsleep(evt, &acpi_softc->sc_lck, PWAIT, > - "acpievt", 1) == EWOULDBLOCK) { > + if (rwsleep_nsec(evt, &acpi_softc->sc_lck, PWAIT, > + "acpievt", MSEC_TO_NSEC(1)) == EWOULDBLOCK) { > if (timeout < AML_NO_TIMEOUT) > - timeout -= (1000 / hz); > + timeout--; > } > } else { > delay(1000); This is problematic. Since MSEC_TO_NSEC(1) will actually sleep for up to 20 ms, this will sleep for much longer than timeout milliseconds. I'd keep this as-is such that we can revisit this when we have a tickless kernel. But if you really want to get rid of the rwsleep() call now, maybe using MSEC_TO_NSEC(10) in the loop with timeout -= 10 would probably be the way to go.