> Date: Sun, 20 Dec 2020 16:23:25 -0600
> From: Scott Cheloha <[email protected]>
>
> 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.
