kestrel: microsecond delays that hold on Windows

[agustinglez]

The problem

On Windows a Kestrel bring-up takes about a minute instead of a few seconds. Both delay_us helpers (HalKestrel.cpp, KestrelFw.cpp) are std::this_thread::sleep_for(microseconds(us)), and on Windows no sleep ends before the next scheduler tick: 15.6 ms by default, 1 ms at best with timeBeginPeriod(1). A 50 us settle becomes 15.6 ms, a 300x overshoot.

The bring-up waits a few microseconds thousands of times (the 48.5 s difference below is ~3,100 ticks). The MAC init and firmware download poll in short steps, and every delay the vendored halbb/halrf code makes (_os_delay_us -> kestrel_halbb_bridge::delay_us -> HalKestrel::halbb_delay) lands in the same helper, so the BB/RF table load and the RF calibrations pay the tick on every step.

The change

• New src/PreciseDelay.h: devourer::precise_delay_us(us).
  • On _WIN32, the wait spins on steady_clock (QPC-backed), yielding each pass, and only sleeps (1 ms at a time) while more time is left than one sleep can take plus a 0.5 ms margin. That bound is learned, not assumed, because the tick is process-global and any library can change it: it starts at one default tick (16 ms), follows shorter sleeps down slowly once timeBeginPeriod(1) is in effect, and jumps back up as soon as a sleep runs long. A wait shorter than the current tick (the 5 ms EDCCA settle under the default tick) therefore never sleeps, and no wait ends before it should.
  • Everywhere else it is the same sleep_for as before. Linux and macOS builds are unchanged.
• Both Kestrel delay_us helpers call it. KestrelFw.cpp drops the <chrono>/<thread> includes it no longer uses.

Scope is Kestrel only, because that is what I measured. Jaguar1/2 and HalmacCfgParam.cpp have the same sleep_for(microseconds(..)) pattern and would likely benefit too. The 1 ms polls in HalKestrel.cpp are left alone: they are poll intervals, not settle times.

Measured

kestrelprobe phy --pid 0x0108 (power -> EFUSE -> FWDL -> MAC -> PHY), TP-Link Archer TX20U Nano (RTL8852BU), Windows 11, MSVC 19.43 Release, WinUSB, same adapter and USB port for every run, two runs each:

build	bring-up
origin/master 74c6b37	54.0 s, 53.9 s
this branch	5.5 s, 5.4 s

Every run ends with kestrel.id, kestrel.power and kestrel.phy all ok:true. The probe does not call timeBeginPeriod, so master sees the default 15.6 ms tick, as any application linking devourer would. The learned-bound revision (second commit) measured 5.46 s and 5.45 s on the same setup.

Per-call accuracy, standalone bench (40 calls each, 10 for 20 ms+), old = first commit's fixed 2 ms spin reserve, new = this revision:

tick	request	old p50 / max	new p50 / max
default (15.6 ms)	3 ms	15.5 / 16.1 ms	3.0 / 3.0 ms
default	5 ms	15.5 / 16.0 ms	5.0 / 5.0 ms
default	20 ms	31.5 / 33.4 ms	20.0 / 20.2 ms
default	100 ms	108.9 / 109.9 ms	100.0 / 100.0 ms
1 ms	5 ms	5.0 / 5.0 ms	5.0 / 5.0 ms
1 ms	100 ms	100.0 / 100.0 ms	100.0 / 100.0 ms

The adversarial case: when the tick goes back from 1 ms to 15.6 ms in the middle of a run (another process ends its timeBeginPeriod), the first wait long enough to sleep can still overshoot by up to one tick (seen once: a 3 ms request took 15.5 ms) before the bound catches up. No approach that doesn't query the effective resolution avoids that, and NtQueryTimerResolution reports the global value, not what Windows 11 actually grants a background process. On Windows 11 a windowless process's timeBeginPeriod(1) is withdrawn within ~10 s unless it opts out of timer power throttling (measured: Sleep(1) mean 1.5 ms at start, 15.5 ms from 10 s on; the 1 ms rows used the opt-out), so the default-tick rows are what a long-running console tool linking devourer gets even if it asks for 1 ms.

[openipc-ai]

/review

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Channel changes still wait a full tick ✓ Resolved

Description

precise_delay_us calls sleep_for whenever the requested delay exceeds 2 ms, but reserving 2 ms
does not prevent that sleep from running until the next Windows scheduler tick. Kestrel requests a 5
ms delay before reading the channel's EDCCA report, so with the documented default 15.6 ms tick this
path can still wait roughly a full tick before it reaches the deadline loop.

Code

src/PreciseDelay.h[R25-26]

+  if (us > kSpinUs)
+    std::this_thread::sleep_for(std::chrono::microseconds(us - kSpinUs));

Evidence

The new header documents a default 15.6 ms Windows tick but sleeps for us - 2000 on requests over
2 ms. The channel path makes a 5000 µs request, producing a 3000 µs sleep; if that sleep wakes after
the tick, the deadline loop cannot recover the overshoot.

src/PreciseDelay.h[5-11]
src/PreciseDelay.h[20-30]
src/kestrel/HalKestrel.cpp[2304-2311]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Windows long-delay branch reserves only 2 ms for spinning. A 5 ms Kestrel delay therefore still uses a sleep that can overshoot to the default 15.6 ms scheduler tick.
## Fix Focus Areas
- src/PreciseDelay.h[22-28]
- src/kestrel/HalKestrel.cpp[2308-2310]
## Recommended Fix
Avoid `sleep_for` for requests shorter than the effective Windows timer granularity. For longer requests, leave sufficient time after the sleep to meet the deadline without relying on a sub-tick wakeup; verify the 5 ms call under default timer settings.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[josephnef]

Approving. Reviewed cc0f895 (two commits, learned-bound revision).

What I checked

• The Windows branch is pure std::chrono/std::thread, so I compiled it on Linux with _WIN32 forced and timed 40 calls per request size. Idle: 1 us → 1–2 us, 50 us → 50–51 us, 3/5 ms → exact, 20 ms → 20.000–20.004 ms, 100 ms → 100.000–100.004 ms, zero underruns, and the bound learned down from 16000 to ~1100 us once the 20 ms waits started sleeping. Under a -j32 build on the same box the yield-spin overshot (a 50 us request up to 9 ms) — expected for a yield loop on a saturated host, and no different from what sleep_for would do there.
• Linux/macOS branch is byte-identical sleep_for; the Linux build + ctest (82/82) and the full CI matrix (MSVC, MinGW, macOS, Linux, per-chip configs, sanitizers) are green.
• Merges cleanly with #487 (both touch KestrelFw.cpp).

Notes, none blocking

• Under the default 15.6 ms tick every wait ≤ ~16.5 ms is now a 100%-of-a-core yield-spin, and the bound only learns down after a wait long enough to sleep (> bound + 0.5 ms). So on a host that has timeBeginPeriod(1) active but never issues a >16 ms wait, all delays still spin. Correct, and bring-up is the main consumer, but worth one line in the header comment so nobody is surprised by the CPU profile.
• The two delay_us(1000) settles in enable_efuse_pwr_cut/disable_efuse_pwr_cut now spin 1 ms each on Windows; they are settle times, fine.
• Same sleep_for(microseconds(..)) pattern exists in Jaguar1/2/3, RTL8733B, HalmacCfgParam.cpp, PcieDmaUtil.h, MT7612U. Agree with keeping this PR Kestrel-scoped since that's what was measured; a follow-up issue to route those through precise_delay_us once someone has a Windows number for them would be the right place.