diff --git a/docs/10-fps-limit.md b/docs/10-fps-limit.md index ba4acc8..f8d3465 100644 --- a/docs/10-fps-limit.md +++ b/docs/10-fps-limit.md @@ -38,9 +38,9 @@ gamescope, so our pacer would be pacing a path the frames no longer take. 3. **`-r ` / `--nested-refresh`** — the session's vblank grid. `main.cpp:740-741` stores it as `g_nNestedRefresh` in **mHz**; `0` is the unset sentinel (only negatives are rejected) and every consumer falls back to - `g_nOutputRefresh`. GameNative launches with a constant 1200 Hz - (`GamescopeState.SESSION_GRID_HZ` → `gridArg()`), high enough that the grid - never binds the session. Verified consumers: + `g_nOutputRefresh`. GameNative passes no `-r`, so `g_nNestedRefresh` is 0 and + every consumer below falls back to the mode's rate, which is the display's. + Verified consumers: - `vblankmanager.cpp:75-77` `GetRefresh()` — the vblank grid itself, and `:405` a sleep computed from the same value. - `steamcompmgr.cpp:10606-10607` — `nRealRefreshmHz` → @@ -65,14 +65,12 @@ Method: deltas of gamescope's unconditional 1/s `drm: A23 flip commit ` over ≥23 s window after ~95 s settle, same scene each run. Device 97c32e85, container `xuser-STEAM_1868140`. -gndrm exposes **one** mode: the session's scan-out size at the panel's real rate +gndrm exposes **one** mode: the session's scan-out size at the display's own rate (`PanelDesc::modeinfo` in `gndrm-daemon/src/objects.rs`; the rate comes from -`nativeSetPanelRefreshHz`, or from the frame-generation output rate when that is -higher). The vblank grid is a separate value — the daemon's own -`SESSION_REFRESH_MHZ`, 1200 Hz — so `-r` matches no `vrefresh` by design: A47 -re-runs the lookup by size, and `drm_set_mode` makes the mode's rate the -declared output refresh. `-r` is the grid (paint tick, app-refresh cycle) and -never the client-visible rate. +`nativeSetPanelRefreshHz`). The cap and the frame-generation multiplier never +enter it, so the mode is the rate the compositor paints at and the rate clients +are told. With no `-r` on the launch line, the paint tick, the app-refresh cycle +and the divisor base all derive from that one rate. A divisor cap on the grid lands on its target exactly, and the uncapped ceiling is the client↔gamescope lap — the grid itself never binds. @@ -96,6 +94,16 @@ divisor cap reads its target exactly. re-issues it when the value or the panel's screen type changes (`injector.rs:717-722`); `set_frame_cap_target` (`lib.rs:322`) holds the target so the scan-out channel pushes a `FrameCapNote` to a subscribed shim. +- **Every subscriber is pushed to, not the allocator.** The card listener hands + one client the allocator role at a time (`server::accept_newest`), and a session + has three that open it — the compositor, a wine child and the game — swapping it + for the first ~20 s. A `FrameCapSubscribe` therefore outlives that role: + `gndrm_daemon::pushes` keeps the registration and sends the cap, the panel's + grid and the frame-generation settings to every client that asked for them. A + live cap change reaches the guest that gates on it even after another client + took the slot; a subscriber whose peer is gone is dropped there, and the + connection currently being served stays its own loop's to close. This is what + makes a cap change made mid-session reach an already-connected shim at all. - **The shim enforces it** (`icd_shim/src/scanout.rs:118-146`, `next_acquire_after`): the next acquire may start no sooner than one `1/target` period after the previous *deadline*, so the guest blocks in @@ -105,18 +113,31 @@ divisor cap reads its target exactly. (a 60 cap read 59.78 on an Odin 3 menu, 60.00 once stepped from `prev`). Only a guest a whole period behind is re-based on `now`, so no debt is carried out of a stall. -- **`-r` is not part of it**: the grid is the constant 1200 Hz, and no launch - argument carries the limit. `--framerate-limit` is never built. +- **`-r` is not part of it, and no launch argument carries the limit.** The app + sends the cap to the shim's gate; `--framerate-limit` is never built. Open points: - **Divisor rounding bites the gamescope route only.** `nVblankDivisor = nRefreshHz / nTargetFPS` (`steamcompmgr.cpp:7040`, and `:10621` for the window's limited cycle) lands on the nearest divisor of the grid; the shim's floor is exact (measured: 60.00 commits/s at a 60 cap). -- **The client-visible refresh is the mode's.** `-r` reaches `g_nOutputRefresh` - only transiently (`drm_unset_mode`, `:3680`), before `drm_set_mode` (`:3728`) - overwrites it with the mode's rate — `_GAMESCOPE_DISPLAY_REFRESH_RATE_FEEDBACK` - and `active_display_info` therefore carry the panel's rate, never the grid. +- **The client-visible refresh is the mode's, and the mode is the display's.** + `-r` reaches `g_nOutputRefresh` only transiently (`drm_unset_mode`, `:3680`), + before `drm_set_mode` (`:3728`) overwrites it with the mode's rate — + `_GAMESCOPE_DISPLAY_REFRESH_RATE_FEEDBACK` and `active_display_info` therefore + carry the mode's rate, never the grid. The cap and the frame-generation + multiplier never enter it: the connector advertises the display's own rate, so + the compositor paints at the rate the display latches whatever the session does + (a 40 cap at x4 runs a 120 Hz mode and produces 160 frames a second). The + daemon serves the rate live — the mode is built on every connector read — so a + display change is applied by dirtying gamescope's backend + (`backend_set_dirty`), which re-reads the connector and re-selects the mode. + The QuickMenu never moves it. +- **The frame ring counts produced frames**, not published ones: the shim reports + its acquire and generation totals (`FrameAcquired`), and the daemon turns the + deltas into `onFramePublished`. A cap above the panel therefore reads as more + frames than the panel shows — which is the point, and why the publish count is + kept only as the session's own statistic. - **Engagement feedback**: `gamescope_limiter.state` covers the gamescope route; the shim logs `scanout frame cap: fps=… effective=…` (`scanout.rs:148-158`), `effective` being the target in force once the note is adopted, and reports what diff --git a/docs/11-refresh-model.md b/docs/11-refresh-model.md index 832f1cc..28d4ad2 100644 --- a/docs/11-refresh-model.md +++ b/docs/11-refresh-model.md @@ -47,25 +47,27 @@ the app at N while keeping the grid at the mode's rate — they move together. ## What gndrm does with it -GameNative launches with `-r 1200` (`GamescopeState.SESSION_GRID_HZ` → `gridArg()`), the -grid the daemon's flip timeline is also built from (`SESSION_REFRESH_MHZ`, -`gndrm-daemon/src/objects.rs:68`; `lib.rs:2091`), so the grid never binds a session. - -The **one** mode the connector exposes is the session's scan-out size at the panel's real -rate (`PanelDesc::modeinfo`, `objects.rs:121-160`: `mode_refresh_mhz()` is the rate the app -reported through `nativeSetPanelRefreshHz`, raised to the frame-generation output rate when -that is higher, and `clock`/`htotal`/`vtotal`/`vrefresh` all describe it). The rate clients -see is therefore the panel's: +GameNative passes no `-r`: `g_nNestedRefresh` is 0, so the grid, the app-refresh cycle +and the divisor base all fall back to `g_nOutputRefresh` — the mode's rate. The daemon's +flip timeline is built from that same rate (`VblankTimeline`, one mode period), so the +timeline and the grid agree by construction. + +The **one** mode the connector exposes is the session's scan-out size at the display's +rate (`PanelDesc::modeinfo`, `objects.rs:100-160`, which takes the rate in force; +`clock`/`htotal`/`vtotal`/`vrefresh` all describe it). That rate is the panel's, reported +through `nativeSetPanelRefreshHz`, and the daemon serves it live so a display change is +applied to the running session. Neither the cap nor the frame-generation multiplier +enters it — they move what the session produces, not what the compositor runs at. The +rate clients see is therefore the display's: - **Uncapped:** the loop reaches its measured ceiling (~259 presents/s); the grid is three orders of magnitude above the panel's frame time and never the limit. -- **Mode rate vs panel rate:** equal unless frame generation raises the output rate, in - which case the mode advertises that higher rate and the shim still paces on the panel's. +- **Mode rate vs panel rate:** equal, always (a 40 cap at x4 on a 120 Hz panel runs a + 120 Hz mode and produces 160 frames a second). - **Capping at 30/40/60 against the grid is exact arithmetic** — all three divide 1200, and a divisor cap lands on its target exactly (`-r 60` reads 60.0 presents/s). ## Open — to measure, not assume - `active_display_info`'s rate list is the single mode rate above (there are no dynamic - rates on this connector), so a client sees one entry — the panel's rate, or the - generated one. + rates on this connector), so a client sees one entry — the display's rate. diff --git a/docs/15-fps-limiter-pacing-and-vsync.md b/docs/15-fps-limiter-pacing-and-vsync.md index ad959e2..cb028d1 100644 --- a/docs/15-fps-limiter-pacing-and-vsync.md +++ b/docs/15-fps-limiter-pacing-and-vsync.md @@ -1,752 +1,93 @@ -# 15 — the FPS limiter, frame pacing, and vsync - -Scope: an audit of where frame pacing and the fps cap are decided in -`game.exe -> proton-wine (winex11) -> Xwayland -> gamescope -> gndrm-daemon -> -SurfaceFlinger`, and a design for a correct cap with correct pacing and an -honest refresh model. - -The refresh split described in §1 is implemented: the mode advertises the -panel's rate and the 1200 Hz constant is the grid alone. The pacing arithmetic -below is not — `steamcompmgr.cpp:10621` still applies the cap as a refresh -divisor, whose patch sits in `gamescope/android/patches/disabled/`. Line -references are against the working trees at the A51 patch train. - -## 1. One variable does two jobs - -gamescope derives **both** its vblank grid (the paint tick) and the -client-visible refresh from a single expression: - -``` -g_nNestedRefresh ? g_nNestedRefresh : g_nOutputRefresh -``` - -- the grid — `CVBlankTimer::GetRefresh()`, `gamescope/src/vblankmanager.cpp:75-78`, - consumed at `:87` (`GetNextVBlank`) and `:127` (`CalcNextWakeupTime`); -- app pacing — `g_SteamCompMgrAppRefreshCycle = mHzToRefreshCycle(nRealRefreshmHz)`, - `gamescope/src/steamcompmgr.cpp:10606-10607`; -- the limiter's divisor base — `nRefreshHz`, `gamescope/src/steamcompmgr.cpp:7013`. - -The launcher passes the constant `-r 1200` -(`GameNative/.../GamescopeState.kt:44`, `:64` → `gridArg()`, appended at -`BionicProgramLauncherComponent.java:640`), so `g_nNestedRefresh` wins for all -three and the grid is 1200 Hz — the app-refresh cycle and the divisor base -included. - -The DRM backend sets `g_nOutputRefresh` from the mode it selected, recomputed -rather than read from the label: `drm_set_mode` → -`g_nOutputRefresh = gamescope::GetModeRefresh( mode )` -(`gamescope/src/Backends/DRMBackend.cpp:3727-3728`), and `GetModeRefresh` is -`(clock*1e6/htotal + vtotal/2)/vtotal` (`:229-246`). The daemon's one mode carries -its own rate in `clock`/`htotal`/`vtotal`/`vrefresh` alike -(`gamenative-drm/modules/gndrm-daemon/src/objects.rs:121-160`): the panel's rate as -the app reported it through `nativeSetPanelRefreshHz`, raised to the -frame-generation output rate when that is higher. The recomputed rate is therefore -the panel's, and the 1200 Hz constant -(`SESSION_REFRESH_MHZ`, `objects.rs:68`) is the grid alone. - -Consequences, all structural: - -- the grid interval is `mHzToRefreshCycle(1200 Hz)` ≈ 833 µs — three orders of - magnitude below the panel's frame time, so the composite is never the ceiling - and a guest can present faster than the panel at all; -- the refresh pushed to clients is the panel's (or the generated rate), never the grid; -- every capped rate reachable through gamescope is `1200/N`, which is what the - limiter's parity between a requested target and a landed rate is measured - against there; the shim's own floor is exact. - -The daemon's flip-event timebase is built from the same constant — -`VblankTimeline::interval_ns = 1e12 / SESSION_REFRESH_MHZ` -(`gamenative-drm/modules/gndrm-daemon/src/lib.rs:2091`) — so the timeline and -the grid agree by construction. `objects.rs:53-68` states the intent: the grid is a -deliberately high timer so the only cap on a guest is the user's own fps limit, -while the mode's rate stays truthful to the panel. - -## 2. The current limiter path - -App to gamescope: - -- QuickMenu fps control → - `XServerScreen.applyFpsLimiterToEngines(effectiveFpsLimit())` - (`GameNative/app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:703-727`; - `effectiveFpsLimit()` `:733-734`), invoked at session start (`:796`) and on - every control change (`:745`, `:756`, `:767`). `0` means uncapped. -- gamescope arm: `GamescopeCommandQueue.setFpsLimit(limit)` - (`XServerScreen.kt:724`) stores the value as one domain of the session's - desired state and re-applies the whole state - (`GameNative/app/src/main/java/app/gamenative/display/GamescopeCommandQueue.kt:127`, - `:143-181`), calling `GameNativeDrm.nativeSetFpsLimit(fps)` (`:177`). The - re-apply is why a value stored while the session is frozen still lands. -- JNI: `Java_app_gamenative_display_GameNativeDrm_nativeSetFpsLimit` - (`gamenative-drm/modules/gndrm-android/src/lib.rs:1069-1077`) → - `injector::set_fps_limit` (`gndrm-android/src/injector.rs:258`, `:262`). -- the injector emits `gamescope_control.set_app_target_refresh_cycle(fps, flags)` - when the value or the internal-panel decision changes (`injector.rs:635-646`; - `flags` carries `TargetRefreshCycleFlag::InternalDisplay` for an internal - panel, `:639-643`). -- gamescope handler `gamescope_control_set_app_target_refresh_cycle` - (`gamescope/src/wlserver.cpp:1237-1246`, dispatch `:1436`) → - `steamcompmgr_set_app_refresh_cycle_override` - (`gamescope/src/steamcompmgr.cpp:1273-1279`) → - `update_app_target_refresh_cycle` (`:1237-1246`) sets - `g_nSteamCompMgrTargetFPS` (`:1217`). - -Inside gamescope: - -- **the limiter cycle** (`gamescope/src/steamcompmgr.cpp:10531-10547`): - `nRealRefreshmHz = g_nNestedRefresh ?: g_nOutputRefresh` (`:10528`); - `nTargetFPS = min(target, ConvertmHzToHz(nRealRefreshmHz))` (`:10534-10535`); - VRR arm `mHzToRefreshCycle(ConvertHztomHz(nTargetFPS))` — exactly `1/target` - (`:10537-10539`); static arm - `nVblankDivisor = nRealRefreshHz / nTargetFPS; cycle = AppRefreshCycle * nVblankDivisor` - (`:10541-10546`) — an integer floor. -- **the gate** (`gamescope/src/steamcompmgr.cpp:6934-6973`): `IsVRRActive()` is - false in this port (`:6941` — the daemon advertises no `vrr_capable` and no - mutable `VRR_ENABLED`), so the static arm runs: - `divisor = nRefreshHz / nTargetFPS; if (vblank_idx % divisor != 0) bSendCallback = false` - (`:6963-6969`). `bShouldLimitFPS` is the per-window exemption only - (`steamcompmgr_window_should_limit_fps`, `:1506-1508`; `window_is_limiter_exempt`, - `:1334`), not the cap. -- the effect of `bSendCallback = false` is a **withheld frame-done**: - `steamcompmgr_flush_frame_done` runs only under the vblank guard - (`:10506-10512`), and the Xwayland and XDG done-commit entries gate on the same - predicate (`:8161`, `:8223`). The cap therefore acts by *not answering the - client*, not by sleeping. - -Guest side: - -- the paint gate is `bShouldPaint = vblank && (hasRepaint || hasRepaintNonBasePlane || bForceSyncFlip)` - (`gamescope/src/steamcompmgr.cpp:10762`), and `vblank` comes only from - `GetVBlankTimer().ProcessVBlank()` (`:10159-10164`) — i.e. from a grid tick. -- gamescope re-anchors that grid off our flip event: - `page_flip_handler` → `GetVBlankTimer().MarkVBlank(vblanktime, true)` - (`gamescope/src/Backends/DRMBackend.cpp:808`; - `gamescope/src/vblankmanager.cpp:221-229`). The daemon builds the event at - commit time (`gndrm-daemon/src/commit.rs:499-515`), stamps it with the claimed - vblank in the commit worker (`gndrm-daemon/src/lib.rs:1944-1979`), and emits it - **with the publish** so gamescope's present loop never waits out a - SurfaceFlinger round trip (`lib.rs:1944-1958`). -- the shim never requests `wl_surface.frame` — no `wl_surface_frame` call exists - anywhere under `gamenative-drm/icd_shim/`. It frees a slot on - `wl_buffer.release` (`icd_shim/src/proxy.rs:4835`, `:7043`) and - `vkAcquireNextImageKHR` blocks until a slot is free (`:1047-1069`, - `:1071-1154`). -- the shim declares its effective present mode to gamescope on every commit - (`icd_shim/src/proxy.rs:721`, `:1349`), computed by `effective_present_mode` - (`:6336-6349`), which is what decides whether gamescope's FIFO commit deferral - applies (`gamescope/src/steamcompmgr.cpp:8163-8184` against - `gamescope/src/wlserver.cpp:3408-3418`). - -## 3. Where pacing is decided, and what an early frame does - -The stages, in order, with what each can bound: - -1. **Guest production** — the title's own rate. -2. **Shim slot gate** — `acquire_take_free` hands out the first slot that is - neither held by the app nor awaiting release (`icd_shim/src/proxy.rs:1035-1069`). - A committed slot returns to circulation only on `wl_buffer.release`. This is - the only back-pressure the guest reliably feels. -3. **gamescope release and paint** — a buffer is released, and the app's - frame-done sent, on a grid tick subject to the paint gate and to the limiter - gate (`steamcompmgr.cpp:10762`, `:6934-6973`). `paint_all` (`:10817`) is what - drives the compositor's commit to the DRM device. -4. **Daemon publish** — one `setBuffer` transaction per commit; the flip event - is emitted with the publish, and the publish never waits the display - (`gndrm-daemon/src/lib.rs:1933-1979`). -5. **SurfaceFlinger to panel** — downstream of everything we own, and - coalescing. - -Which stage binds is per title, not per build. A title whose lap through the -shim and the compositor exceeds the panel's frame time is bound by its own lap -(stage 1+2). A fast title is bound by how often gamescope releases its buffers, -which is per grid tick (stage 3). Nothing bounds it at the panel rate: that is -the intent of the 1200 Hz grid (`objects.rs:53-68`). - -**A frame presented early** — the guest obtained a slot and committed before the -panel's next edge. The daemon publishes it, and `VblankTimeline::claim` stamps it -on the next boundary of the session grid at or after `now`, never moving -backwards (`gndrm-daemon/src/lib.rs:1843-1855`), so an early frame is stamped at -the current boundary and a late frame skips whole intervals. Delivery stays -immediate, so an early frame neither queues nor is dropped here; the commit -replaces the layer buffer and SurfaceFlinger composites it at its own next -vblank. The guest cannot know whether it was displayed, because it holds no -frame callback; it learns only that its slot came back. That is why the next -acquire — not the present — is where the guest is paced. - -## 4. Where vsync exists, and where it is only nominal - -**There is no real vsync in this stack.** Nothing on the path observes the -panel's vertical blank: - -- the daemon has no vblank ioctl and no vblank source; it emits a *claimed* - timeline (`gndrm-daemon/src/lib.rs:1810-1856`); -- gamescope's "vblank" is a userspace timer (`vblankmanager.cpp:122-128`), armed - early by `ulOffset = rollingDrawTime + redZone` (`:178`, `:90-94`, and the - comment at `:106`: queueing a flip early is free, the kernel holds it to the - panel cycle), and only *re-anchored* by the flip event's timestamp - (`:221-229`); -- the panel's true blank is SurfaceFlinger's and never reaches us. - -What is nominal but load-bearing: - -- the grid tick is the compositor's only pacing element: the paint gate - (`steamcompmgr.cpp:10762`) and the frame-done (`:10510`) both hang off it. -- the "vsync" a title sees is the shim's release cadence - (`icd_shim/src/proxy.rs:1047-1069`), not a frame callback: a title that - elects to vsync internally is paced by how often gamescope releases its - buffers, which is per grid tick. -- upstream's real client-pacing signal, `wl_surface.frame`, is never requested - here, so the frame-done gamescope sends has no consumer on our ICD. - -**What advertising the true refresh rate changes** for a title above it: the -mode's `clock`/`htotal`/`vtotal` feed `GetModeRefresh` → -`g_nOutputRefresh`, which is what the stack *and the clients* are told — the -Xwayland server mode (`steamcompmgr.cpp:10418`, `:7383`), the -`_GAMESCOPE_DISPLAY_REFRESH_RATE_FEEDBACK` root property (`:9307-9312`), the -`active_display_info` rate list (`wlserver.cpp:1488`) and mangoapp's snapshot -(`steamcompmgr.cpp:9856`). A self-vsyncing title asked to present at the -display's rate would then cap itself correctly. But as long as -`g_nOutputRefresh` also feeds the grid (§1), telling the truth also lowers the -grid to the real rate and clamps *every* present to it — so this is not a -relabel, it is a change of which element bounds the frame rate. That is the -whole reason the two jobs must be separated before the real rate can be -advertised. - -## 5. Defects that make the limiter incorrect - -**D1 — the cap is a vblank-grid modulo, and A32 is disabled.** -`gamescope/src/` carries upstream's static arm, which drops the callback -whenever `vblank_idx % (refresh/target) != 0` (`steamcompmgr.cpp:6965-6967`). -The grid index is unrelated to when the guest committed, so a title already at -its own ceiling loses a whole cycle whenever its commit lands late or off the -grid, and reads *below* its target. The approach of the disabled patch is sound -and salvageable: it gives the static arm the VRR arm's lower-bound delay -(`:6947-6950`, a pure deferral against `last_commit_first_latch_time + -limited_cycle`), which structurally cannot pull the rate below what the guest -produced. The patch is not applied, so the defect is live. - -**D2 — the limiter's cycle integer-divides on the static arm.** -`nVblankDivisor = nRealRefreshHz / nTargetFPS` floors -(`steamcompmgr.cpp:10543`), so on any panel whose refresh is not a multiple of -the target the divisor collapses to 1 and the cap disappears entirely — not a -loose cap, no cap. A32's second hunk removes the branch and uses -`mHzToRefreshCycle(ConvertHztomHz(nTargetFPS))` = `1/target` on both arms -(`:10541-10546`), which is the form the VRR arm already had (`:10539`). - -**D3 — the advertised 1200 Hz is not the panel's rate, and one value does both -jobs.** `SESSION_REFRESH_MHZ` (`objects.rs:63`) feeds the mode's -`clock`/`htotal`/`vtotal` (`:103-104`), the mode label (`:120`), the flip-event -timeline (`lib.rs:1834`) and — through `g_nOutputRefresh` — the grid, the app -pacing and the limiter base. Nothing in the stack knows the panel's real rate: -`PanelDesc` carries only width/height/mm (`objects.rs:69-75`), built from the -JNI arguments (`gndrm-android/src/lib.rs:717-722`), and `nativeStart` takes only -width and height (`lib.rs:996-1009`). The app *does* compute the panel's maximum -(`XServerScreen.kt:306-324`, `detectMaxRefreshRateHz`) but uses it only to clamp -the slider (`:792-794`); it never crosses JNI, and `displayRefreshRateHz()` is -dead on this arm. - -**D4 — VRR is dead, so the limiter always takes the arm D1 and D2 are about.** -The daemon advertises neither the connector's `vrr_capable` nor a mutable CRTC -`VRR_ENABLED`, so `IsVRRActive()` is false at every decision site -(`steamcompmgr.cpp:6941`, `:10537`). A corollary that is not itself a defect but -is the symptom users see: because reachable rates are `1200/N`, a -five-step control whose values do not divide 1200 lands on a different rate than -it displays. - -**D5 — the guest never receives a frame callback, so the cap's grip is -indirect.** The shim's only pacing is the slot gate -(`icd_shim/src/proxy.rs:1039-1046`). The cap reaches the title by slowing the -grid, which slows releases, which throttles acquires — an effective but -second-order mechanism: a title that buffers internally past its slot count, or -that stops presenting, is bounded only by slot count and by the compositor's -own release cadence. - -## 6. Cheapest decisive measurement per proposal - -Each of these is one device arm. None has been run. - -**P1 — is the client-visible refresh truthful?** Read what the client is told: -on gamescope's own Xwayland root (`GAMESCOPE_X11_DISPLAY`, socket at -`/usr/tmp/.X11-unix/X0`), `xprop -root GAMESCOPE_DISPLAY_REFRESH_RATE_FEEDBACK` -(`steamcompmgr.cpp:9307-9312`), or `gamescope_control.active_display_info` -(`wlserver.cpp:1488`). Expect the panel's real rate. **Falsified** if it still -reports the fabricated rate, or if the delivered present ceiling collapses onto -the panel rate — the latter means the grid was clamped rather than decoupled, -which is the whole risk of P1. - -**P2 — did the grid stay separate?** The delivered cadence on a title capable of -over-refresh, limiter off, from `dumpsys SurfaceFlinger --latency` on the -scan-out **buffer** layer. The layer name is `gndrm-scanout#` and must be -re-discovered per capture, because the `BackgroundColorLayer` sibling is listed -first and has no buffer queue: - -```sh -LAYER=$(adb shell dumpsys SurfaceFlinger --list | tr -d '\r' \ - | grep -oE "gndrm-scanout#[0-9]+" | sort -u | head -1) -adb shell dumpsys SurfaceFlinger --latency "$LAYER" -``` - -**Falsified** if the p50 spacing becomes the real frame time (one panel frame) -instead of the pre-change ceiling. - -**P3 — are extra frames actually generated?** The same layer, same title, with -the limiter off, above the panel rate; plus the shim's own `slots=[...]` and -`in_flight=` fields on the release and stall lines -(`icd_shim/src/proxy.rs:245-266`, `:1143-1148`). **Falsified** if the delivered -rate tracks the advertised rate, or if `in_flight` is pinned high with stalls — -the latter localises the ceiling to the slot gate rather than to the compositor. - -**P4 — is the cap a lower bound?** One title, three arms: uncapped, a target just -below the title's own ceiling, and a low target of 40. Take the rate from the -daemon's published-frame cadence (the `onFramePublished` upcall feeds -`FrameTimeRing`: `gndrm-android/src/asc.rs:1173`, `lib.rs:147-148`) and from -`dumpsys SurfaceFlinger --latency`; read per-second deltas, never a session mean. -**Falsified** if the near-ceiling title at its target still reads below its own -uncapped rate, or if the 40 target is not exact. The low arm is what proves -non-inertness: a target the guest already cannot exceed separates nothing. - -**P5 — does a frame callback arrive, and does it change the rate?** Log the first -N callbacks and their spacing at the shim (the existing `logf!` gate, -`GNDRM_LOG_VKSHIM`). **Falsified** if the callbacks do not arrive at the grid -rate, or if introducing them materially changes the delivered rate — which would -mean the slot gate and the callback are double-pacing the guest. - -## 7. What could not be established from the code - -- **Which rate "the real refresh rate" is.** `detectMaxRefreshRateHz` takes - `display.supportedModes.maxOfOrNull { it.refreshRate }` - (`XServerScreen.kt:314`) — the *maximum* the panel advertises, not the rate of - the currently active mode. A panel whose active mode is 60 Hz while its - maximum is 120 Hz would be misreported, and no code here tracks a mid-session - mode change. -- **Every timing figure this audit would need is unmeasured here.** The rates - and ceilings carried in docs `10`–`12` and in the project's skill references - are from earlier sessions; by instruction no device pass was run, so this - document states mechanisms and predictions, not results. In particular the - claim that the uncapped ceiling is the client-vs-compositor lap rather than - the grid is an inference from the slot gate's structure, not a measurement - made now. -- **Whether `-r ` leaves the grid's relationship to the flip-event - timeline intact.** The daemon's `VblankTimeline` claims boundaries at the - session grid rate and gamescope's grid interval is `mHzToRefreshCycle(GetRefresh())` - with `GetRefresh()` preferring `g_nNestedRefresh`; that they agree is a claim - read from the code, and that raising the value does not perturb the anchor in - a way that walks the grid is unverified. -- **Whether the shim's per-commit present-mode declaration is the only route by - which gamescope learns the mode.** `GAMESCOPE_FPS_LIMIT` is also read as an X - root property on gamescope's own Xwayland (`steamcompmgr.cpp:7398`), writable - by the shim or by wine rather than by the app; the two routes are not compared - anywhere. -- **Every reader of `g_nOutputRefresh`.** The consumers listed in §4 are the - ones found by grep; a change to the mode's rate reaches them all at once, and - no exhaustive audit of the variable's uses was made. -- **Whether the daemon's presenter depends on the mode rate at all.** The - presenter publishes on every new-FB commit; the mode rate appears only in - `objects.rs` and `lib.rs`. That the mode rate does not reach the presenter is - an absence of evidence, not a proven independence. - -## 8. The CPU cost of the daemon and the ICD shim - -Scope: what one presented frame costs in the ICD shim and in the userspace DRM -daemon — its commit worker, its presenter, and the LD_PRELOAD ioctl interceptor -— which of those costs is per-frame and which is per-session, what the loop is -bounded by, and what is unmeasurable from the code. No timing figure below is a -measurement: every number is a constant read from the source, a claim already in -the tree, or explicitly absent. - -### 8.1 Per frame in the ICD shim - -The default present route is the override/Wayland surface -(`icd_shim/src/proxy.rs:5881-5892`); the DRI3+Present route is the opt-in. The -cost below is the default route's, with the opt-in route's difference named at -the end. - -**The guest's own threads.** - -- `gn_vkQueuePresentKHR` (`proxy.rs:1382-1494`): one relaxed increment, one - `pres_iv_note` — which takes `PRES_IV_LAST` once, reads `Instant::now()`, then - bumps five relaxed atomics (`proxy.rs:187-205`) — a `find_queue`, one - `vkQueueSubmit` carrying the guest's own `pWaitSemaphores` (`:1432-1472`), and - `present_dispatch` (`:1476`). The dispatch (`:1244-1322`) takes `shared`, - writes slot flags and notifies a condvar. No I/O, no syscall on this path. -- `ring_push("vkQueuePresentKHR", …)` (`proxy.rs:1504-1513`) takes the global - `RING` mutex, and at the 48-entry cap does an `r.remove(0)` (`:80-82`). A - second `ring_push` sits on every acquire (`:1081`). -- `gn_acquire_common` (`proxy.rs:1071-1208`): the `ACQ_US_*` census is four to - six relaxed atomics (`:1183-1194`) plus, when no slot is free, a block in - `cv_free.wait_timeout` on a 4 ms slice (`REAP_SLICE`, `:1033`, `:1117-1128`) - with an `x11_reap` probe between slices (`:1110-1113`). The slot scan - `acquire_take_free` (`:1047-1069`) is an in-memory walk of the slot vector. - -**The shim's present worker** (`worker_main`, `proxy.rs:654-756`): - -- `wait_fences` on the slot's own present fence with `u64::MAX` (`:663-665`) — - a CPU block whose duration is the guest's own render time, not the shim's. -- `slot_make_buffer` is lazy and once per slot (`:695-697`, `:7589`), not per - frame. -- Under `SEND_LOCK` (`:323`, `:710-735`): `swapchain_set_present_mode` (the - per-commit present-mode declaration), `surface_attach`, - `surface_damage_buffer`, `surface_commit`, and `wl_display_flush` — the flush - is the only syscall in the commit, one `write` on the Wayland socket. - -So one presented frame here is one Vulkan submit, one CPU fence wait, one -marshal+flush under a process-wide mutex, two global-mutex `ring_push`es, and a -fixed set of relaxed-atomic bumps. - -**Do the census counters format?** They do not. `ACQ_US_*` and `PRES_IV_*` -(`proxy.rs:159-182`) only `fetch_add`/`fetch_max`. The lines that would render -them (`counters()`, `slot_flags()`, `proxy.rs:246-314`) are arguments to -`logf!`, whose `$e` is evaluated inside the gate's body only -(`icd_shim/src/lib.rs:46-52`), and are additionally sampled 1-in-120 -(`proxy.rs:744`). A session with `GNDRM_LOG_VKSHIM` off formats nothing here; -what remains per frame is the cached-gate atomic load and its branch. - -**Per session, not per frame.** The scan-out allocator exchange — the socket -`sendmsg`/`recvmsg` pair that carries the pool dma-buf over `SCM_RIGHTS`, with -its three-attempt retry (`proxy.rs:7469`, `scanout.rs:124-152`) — runs once per -swapchain slot at swapchain creation (`proxy.rs:865`). -`effective_present_mode` is resolved once at swapchain creation -(`proxy.rs:6336-6349`), and `resolve_override_target` (`proxy.rs:5919`, -`x11.rs:609-620`) and `window_extent` once at surface creation. - -**The tree walk.** `maybe_report_game_window` (`proxy.rs:558-579`) is called on -every present but returns on the `GAME_WINDOW_REPORTED` latch and, before that, -only every 30th present (`GAME_WINDOW_CHECK_EVERY`, `proxy.rs:125`). Its -predicate opens a fresh X connection and does X round trips (`x11.rs:1026-1046`), -so this is at most one X round trip per 30 presents until the game is first seen, -and none afterwards — not a per-frame cost. - -**The opt-in X11 route's per-present delta.** `x11_present_slot` -(`proxy.rs:446-529`) creates the pixmap and idle fence once per slot -(`:493-517`) and, per present, does `xcb_sync_reset_fence` + -`xcb_present_pixmap_checked` + `check_err`; `check_err` is `xcb_request_check` -(`x11.rs:828-836`, `:1385-1426`), a blocking X round trip. `x11_reap` -(`proxy.rs:584-613`) then walks every slot and calls `fence_idle`, itself a -blocking `xcb_sync_query_fence` round trip (`x11.rs:1431-1449`) — so this route -pays one X round trip per in-flight slot per present. On the default route -`x11_reap` returns on its first line (`proxy.rs:585-587`). - -### 8.2 Per frame in the daemon - -**The interceptor.** Per flip gamescope issues exactly one atomic commit -(`gamescope/src/Backends/DRMBackend.cpp:4446` → one `DRM_IOCTL_MODE_ATOMIC`). -The LD_PRELOAD `ioctl` (`modules/gndrm-shim/gndrm_shim.c:1940-1976`) routes it to -`rpc()` (`:643-666`), which holds one global mutex across a `send_frame` -(`sendmsg`) and a `recv_frame` (`recvmsg`) that blocks until the daemon answers. -So one flip is one synchronous request/response on the RPC socket, serialized -process-wide, with a 5 s bound that exists to turn an unanswered request into a -failure rather than a hang (`:110-120`, `:489-495`). The dma-heap redirect on a -scan-out allocation is a separate intercepted path (`:1954`, `:1464`) and is per -buffer, not per frame. A first-time buffer also costs one `AddFB2` -(`DRMBackend.cpp:1939-1949`) and a retired one a `RmFB` (`:2905`); every lesser -ioctl was not audited, so "one per flip" is the steady-state count and not a -proven total. - -**The serve loop.** Event-driven, not polling: `server.rs:377-399` `poll`s the -RPC socket and the flip-event fd with a `-1` timeout. Per ioctl it receives one -frame (`:423`), dispatches (`:440`), builds a response header and frame -allocating a `Vec` (`:442-449`), and sends it (`:450`). When the flip-event fd -is ready it drains the queue and sends each event (`:409-421`), allocating a -`Vec` per event. - -**The commit path.** `ioctl_atomic` (`commit.rs:47`) parses into `Vec`s -(`:284`), validates, and runs `apply_commit` (`:444-525`): apply the property -sets, compute the publish predicate -`publish = crtc_on && new_fb != 0 && new_fb != old_primary_fb` (`:455`), and -build the presenter's liveness set — a fresh `HashSet` over every live and -closed GEM object **on every commit**, whether or not it publishes (`:481-487`). -A `CommitJob` is queued unconditionally (`:517`), so a re-commit of the unchanged -primary FB costs a job and a worker wakeup but no publish. - -**The commit worker** (`lib.rs:1907-1982`). Event-driven: it blocks in -`commit_rx.recv()` (`:1923`) and does nothing when idle. Per job it publishes -(`:1936`) and emits the flip event (`:1966-1979`), re-stamping the claimed vblank -and writing an eventfd; the publish does **not** wait for SurfaceFlinger's -completion, so gamescope's blocked present loop waits only for this worker -(`DRMBackend.cpp:4522-4523`, `:808`, `:824-825`). - -**The presenter** (`asc.rs:815-1175`), on that worker: - -- a latched `AtomicBool` swap (`:831-840`) and a rect-key comparison that formats - only on change (`:886-918`); -- the pool lock (`:926-938`): `retire(live_inos, keep)` plus a `resolve(ino)` hash - lookup. The **AHardwareBuffer handle does not cross any IPC boundary per frame** - — the commit carries the dma-buf *inode* and the presenter resolves it to its - own in-process pool buffer (`:932`, `:971-973`); the only handle that crosses a - socket is the pool fd, once per allocation (`scanout.rs:237-264`); -- `release_retired` → `AHardwareBuffer_release` for buffers whose flip event has - been reported (`:943-944`, `:2212-2222`), outside the pool lock; -- a zero-timeout `ALooper_pollOnce` (`:1001-1007`); -- **the previous-release-fence wait**: the completion callback's stored release fd - is swapped out and polled with a 33 ms budget (`:1009-1019`, - `RELEASE_FENCE_TIMEOUT_MS` `:79`, `wait_fence` `:2468-2476`). This is the one - unbounded-in-principle wait on the worker — bounded at 33 ms, after which the - publish proceeds anyway and warns once; -- `build_show` + `apply_transaction` (`:2261-2305`, `:2329-2335`): the layer - transaction and the `ASurfaceTransaction_apply` binder call, which the code's - own comments call "the blocking SurfaceFlinger round-trip" (`:2257`) and the - one "that must not serialise the allocator" (`:2328`); -- `notify_frame_published` (`:1173` → `lib.rs:147-148`), a JNI upcall on every - new-FB publish. - -**Threads that do not spin.** The commit worker blocks in `recv` (`lib.rs:1923`), -the serve loop `poll`s with a `-1` timeout (`server.rs:399`), the scan-out -acceptor `poll`s three fds with `-1` (`asc.rs:1489-1516`), and the warm-spare -worker blocks on its doorbell condvar (`asc.rs:2098-2133`). Nothing in the daemon -polls. The only timed waits are the 33 ms release-fence poll above and the -`WARM_RETRY_MS` backoff (`asc.rs:2129-2130`), and only the former is on the -publish path. - -### 8.3 What the cost scales with - -Three candidates, and which one bounds: - -- **The synthetic 1200 Hz grid.** The compositor's loop runs once per grid tick: - the vblank timer is re-armed unconditionally after every timer-driven tick, - explicitly "in case pageflip handler doesn't happen so we don't stop vblanking - forever" (`steamcompmgr.cpp:10850-10858`); the loop `poll`s the timer as a - waitable (`:10086`, `:10157-10164`); the interval is - `mHzToRefreshCycle(GetRefresh())` = 1/1200 s (`vblankmanager.cpp:75-78`, - `:262-280`; `objects.rs:63`). The loop body — including the change-gated atom - updates (`steamcompmgr.cpp:10866-10870`, `:9272-9356`) and the per-iteration - mangoapp snapshot calls (`:10846-10848`, the connector snapshots early-returning - unless the backend uses virtual connectors, `:9831-9834`) — is therefore driven - at up to ~1200 iterations/s **whatever the panel does**, and is re-anchored by - every flip event (`DRMBackend.cpp:808`). -- **The guest's own present rate.** The shim's per-frame cost is strictly one - present; the daemon's per-flip cost is strictly one gamescope commit, and - gamescope commits only inside `paint_all`, which runs only when there is a - repaint (`steamcompmgr.cpp:10762`, `:10817`) — i.e. when a client committed. - The project's own figure for that pipeline is ~259 presents/s - (`objects.rs:60-61`, from an earlier session and not re-measured here). -- **The panel's real rate.** Nothing on the loop: `g_nOutputRefresh` is the - panel's rate, but the grid is the daemon's 1200 Hz constant, so the - compositor's tick is not tied to the panel; the panel reaches the stack only - through SurfaceFlinger, downstream of everything we own. - -So the loop is bounded by the **guest's present rate** for the per-frame path -(shim present, daemon commit, presenter publish), and by the **1200 Hz grid** for -a fixed-rate floor of compositor loop iterations that runs whether or not a frame -is painted. The limiter does not change that floor: it withholds the client's -frame-done, so it lowers the guest's present rate and with it the per-frame work, -while the grid keeps ticking. - -**Cheapest change that reduces the cost.** Lower the grid to the panel rate — -stage 1's separation, whose minimum form is the single constant everything else -derives from (`objects.rs:63`, consumed at `lib.rs:1834` and through -`g_nOutputRefresh` at `vblankmanager.cpp:77`, `steamcompmgr.cpp:10528`, `:6938`). -That cuts the fixed floor by the ratio of 1200 Hz to the panel rate (10×–20× for -a 60–120 Hz panel) with no change to the per-frame path. Stage 3b (a real frame -callback in the shim, `proxy.rs:1035-1069`) is the alternative if the grid must -stay high: it supersedes the per-frame pacing rather than the floor. The -per-frame path has no cheap win of its own — its dominant items are the guest's -fence wait and one blocking round trip to the daemon per flip, both structural. - -### 8.4 What is gone, and what was never measured - -**The whole-device GPU drain is gone.** There is no `vkQueueWaitIdle` call site -in the shim: the pointer is resolved (`proxy.rs:6830`) and never invoked, and the -only device-idle in the tree is in `gn_vkDestroyDevice` (`:6889-6890`). The -comment at `:451-456` states the replacement contract — the slot's own fence is -signalled by the shim's present submit, which waits the guest's present -semaphores, so no drain is needed and taking one "would destroy all CPU/GPU -overlap". - -**The residual risk is a guest that presents with `waitSemaphoreCount == 0`.** -The present submit's waits come straight from `pi.pWaitSemaphores` -(`proxy.rs:1438-1458`), and the slot fence is that submit's completion fence -(`:1468`). With no wait semaphores and a single swapchain, the submit carries no -waits and no signal chain (`signal_chain = i + 1 < nsc`, `:1445`), so the fence -signals as soon as the empty submit completes — the worker's `wait_fences` -(`:663-665`) then returns at once and the buffer is committed while the GPU may -still be writing it. Under the removed drain that could not happen. Nothing in -the code guards the zero-wait case, and no session is known to have exercised it. - -**Never measured.** The `slot_signature` byte-walk — a full CPU `mmap` of the -slot with a per-pixel loop over `w*h` and two `DMA_BUF_IOCTL_SYNC` ioctls -(`proxy.rs:345-389`) — is bounded to the first 24 presents of each swapchain -(`X11_TRACE_N`, `:217`), and `slot_mem_signature` (`:395-439`) to the first -three; neither has a timing figure, and both are off after their budget. The -blocking X round trips on the guest-surface route (the per-present -`xcb_request_check` and the per-slot `xcb_sync_query_fence` of §8.1) have never -been timed. The acquire-dwell and present-interval censuses were added to be read -from a real session (`proxy.rs:153-175`), and no session's output is in this -document. - -### 8.5 Thermal and power - -The device is a handheld, so a fixed-rate floor that runs whether or not a frame -is painted is a sustained cost, and the 1200 Hz grid is exactly that: the -compositor's timer wakes ~1200 times a second, re-arms itself -(`steamcompmgr.cpp:10850-10858`), runs its loop body, and is re-anchored by every -flip event (`DRMBackend.cpp:808`) — independent of the panel, which is 60–120 Hz. -On top of that sits the per-frame path, whose per-flip cost includes a blocking -IPC round trip to the daemon inside gamescope's own present call -(`DRMBackend.cpp:4522-4523`; `lib.rs:1966-1979`) and a SurfaceFlinger binder round -trip on the daemon's worker (`asc.rs:2329-2335`). Neither is a busy-wait, but -both are on the critical path of every frame, so the CPU time and the achievable -rate scale with how often a frame is produced, not with how little an idle -display needs. - -What bounds it is the separation of §8.3: a grid at the panel's rate removes the -10×–20× oversampling of the compositor loop, while the per-frame path stays -bounded by the guest's own present rate, which the limiter already lowers. The -unmeasured question for a power budget is the split between the two — whether the -sustained load is the grid's wakeups or the per-frame path — and that is what -§8.6 separates. - -### 8.6 The cheapest decisive measurement - -One device arm, not run here. On a fixed scene, limiter off, take per-thread CPU -time (`/proc//task//stat` utime+stime, or `top -H`) for the gamescope -process and for the app process, in two sessions: one with the synthetic mode at -1200 Hz (the current build) and one with the mode's rate describing the panel. -Compare the gamescope vblank/compositor thread against the daemon's commit-worker -thread and the shim's present worker. **If the compositor thread's CPU scales with -the grid while the commit worker's and the present worker's do not, the cost is -the grid's wakeups and §8.3's floor is confirmed. If the daemon and shim threads -track the grid too, the per-frame path is being driven by the grid rather than by -the guest, and stage 1 alone would not fix it.** This needs no instrumentation the -tree does not already have, and it separates the two candidates in one run. - -## 9. Proposal - -The design is one change of kind, staged: **separate the two jobs the single -refresh variable does**, so the grid becomes a property of the session and the -client-visible refresh a property of the panel; then make the limiter a lower -bound on the target frame time; then make the two guest-facing back-pressures -honest. Nothing here is implemented (scope note above); each stage names its own -falsifier in §6. - -**Target behaviour.** - -- *A correct limiter.* With a target enabled the delivered rate is exactly - `1/target` for any panel rate — no integer-division collapse of the divisor, - and no reading *below* the guest's own rate when the guest is already near the - target. With the target off the guest is free above the target up to its own - ceiling. -- *Correct pacing.* The compositor's paint and its frame-done hang off the - session grid, not the panel (§3), so a guest below the panel rate is never - gated by a panel tick and one that runs faster composites per frame. -- *Vsync where it can be had.* Nothing on this stack observes the panel's blank - (§4), so the honest claim is not "vsync" but **a truthful client-visible - refresh**: a self-vsyncing title then caps itself correctly, while the stack's - own pacing stays on the grid. -- *Over-refresh preserved.* A title that can exceed the panel keeps compositing - per frame; the grid stays above the present pipeline's ceiling. - -**The first stage is a cost change, not only an arithmetic one.** The -compositor's loop iterates at up to ~1200 Hz *whatever the panel does*: the -vblank timer is re-armed unconditionally after every tick -(`steamcompmgr.cpp:10850-10858`, `ArmNextVBlank(true)`, so the session never -stops vblanking) and gamescope's present blocks on the daemon's flip event -(`DRMBackend.cpp:4522`), so the grid — not the panel — is the sustained cost -floor (§8.3). The limiter does not lower that floor: it caps by *withholding the -frame-done* (§2), which lowers the guest's rate while the grid keeps ticking -(§8.3). Passing the panel's real rate in is therefore not merely the fix for the -three-jobs-one-variable defect in §1; it collapses the fixed-rate floor of -compositor iterations, a 10×–20× oversampling of a 60–120 Hz panel, because the -single constant everything derives from — `SESSION_REFRESH_MHZ` -(`objects.rs:63`, consumed at `lib.rs:1834` and through `g_nOutputRefresh` at -`vblankmanager.cpp:77`, `steamcompmgr.cpp:10528`, `:6938`) — falls to the panel's -rate. That is the case for stage 1, and it is why stage 1 is stated first even -though stage 2 (the limiter patch) is independent of it: the limiter's -correctness does not depend on the grid's value, but the cost floor does, and -only stage 1 moves it. - -**Stage 1 — separate the grid from the client-visible rate.** - -*1a. Carry the panel's real rate to the daemon and use it for the mode — was P1.* -Add `refresh_mhz` to `PanelDesc` (`objects.rs:69-75`) and use it, not -`SESSION_REFRESH_MHZ`, for the mode's `clock`/`htotal`/`vtotal` and its `vrefresh` -(`modeinfo()` `:91-125`), keeping `SESSION_REFRESH_MHZ` (`:63`) as the grid -constant. `GetModeRefresh` then returns the real rate, so `g_nOutputRefresh` -becomes truthful for every client-visible consumer listed in §4 while the grid -stays high. `VblankTimeline::new` (`gndrm-daemon/src/lib.rs:1834`) keeps its -interval on the session grid, not the mode rate, so the timeline and the grid -still agree by construction. The panel's rate is already computed app-side -(`XServerScreen.kt:306-324`) but never crosses JNI; that is the cheapest thread. -*Risk:* this is exactly the coupling §4 warns about — `g_nOutputRefresh` also -feeds the grid, so 1a *alone* would drop the grid onto the panel rate and clamp -every present to it. 1a is therefore not shippable alone; it lands with 1b, and -the failure it risks is the grid being clamped rather than decoupled. -*Cost payoff:* §8.3 and §8.5 — the fixed compositor floor falls by the -grid-to-panel ratio, the only cheap win in the whole cost audit (§8). - -*1b. Hold the grid on the grid value, explicitly — was P2.* Pass `-r ` from the launcher so `g_nNestedRefresh` is the grid and wins at -`vblankmanager.cpp:77`, `steamcompmgr.cpp:10528` and `:6938`. `-r` also feeds -`init_drm(..., g_nNestedRefresh)` (`DRMBackend.cpp:3899`) → `preferred_refresh` -(`:1499-1503`) → `find_mode` (`:1383-1385`); on a connector exposing a single -mode whose `vrefresh` is now the real rate that exact match fails and the fallback -`find_mode(0, 0, 0)` (`:1397-1399`) keeps the mode, so `drm_set_mode` (`:1406`, -`:3684`) recomputes `g_nOutputRefresh` from the mode — the real rate. The -`preferred_refresh` assignment in `drm_unset_mode` (`:3635-3636`) is reached only -on the headless connector-loss path (`:1348-1351`), not the connected one. -Result: grid = the grid value, client-visible refresh = the real panel rate, and -the mode label agrees with the recomputed rate. -*Risk:* the single-mode fallback is read from the code, not measured — if -`find_mode` instead rejects the mode, the session starts headless. -*Falsifier:* measurements P1 and P2 (§6). - -**Stage 2 — make the limiter a lower bound on the target frame time — was P4.** -Re-cut A32 against current `src/`, two hunks: the gate's static arm becomes the -VRR arm's deferral (`steamcompmgr.cpp:6961-6970`), and the limiter cycle becomes -`mHzToRefreshCycle(ConvertHztomHz(nTargetFPS))` on both arms (`:10541-10546`). -With those, the cap is `1/target` whatever the advertised grid is, the integer -divisor is gone, and "capped exactly when enabled, free above the target when -not" is true for any panel rate. The deferral is sound for the static arm because -it is a pure lower bound against -`last_commit_first_latch_time + g_SteamCompMgrLimitedAppRefreshCycle` -(`:6947-6950`, fudge `k_ulVRRScheduleFudge` `:6949`), which structurally cannot -pull the rate below what the guest produced; the cycle unification is the second -hunk and the one that generalises, since it removes D2's collapse entirely. -*Risk:* the patch is parked at -`gamescope/android/patches/disabled/0032-A32-fps-cap-lower-bound-delay.patch` and -is not applied; the re-cut is against a moving `src/` (HEAD `f3c3b96` here), so a -hunk that no longer reverse-checks stops the build at `PATCH DOES NOT APPLY`, and -the regenerated patch must be re-applied and the train re-verified as a whole. -*Falsifier:* measurement P4 (§6) — the near-ceiling title must not read below its -own uncapped rate, and the low arm (40) must be exact; a fix that improves near -the ceiling but degrades where the cap is exact is a failure, not a fix. - -**Stage 3 — make the guest-facing back-pressures honest.** - -*3a. Keep over-refresh possible, and do not silently re-cap — was P3.* Once the -grid is separate, nothing in the paint path needs loosening: over-refresh is -already possible because the grid is high. What must be *right* rather than -patched are the two guest-facing back-pressures — the shim's slot gate -(`icd_shim/src/proxy.rs:1047-1069`) and gamescope's FIFO commit deferral -(`steamcompmgr.cpp:8163-8184`). The deferral applies only when the shim declares -FIFO (`icd_shim/src/proxy.rs:721`, `:1349`, resolved by `effective_present_mode` -`:6336-6349` against `wlserver.cpp:3408-3418`); the container default is mailbox, -and the declaration must stay the *effective* mode so a title is not silently -re-capped by a deferral it did not ask for. Relaxing the slot gate itself is not -an option: the scan hands out the first free slot, so a free-running variant -starves the compositor with one repeated buffer (`icd_shim/src/proxy.rs:1041-1046`). -*Falsifier:* measurement P3 (§6). - -*3b. Optionally, request `wl_surface.frame` in the shim — was P5.* On its own -merits only: it makes gamescope's frame-done reach the client — upstream's own -pacing signal — so the cap would grip the guest directly rather than through the -slot gate. It is a new mechanism, not a condition to relax: it supersedes the -slot gate as the pacer, so it must be measured before it is trusted, and it is -the answer if the grid's wakeup cost ever shows up in power. -*Risk:* double-pacing — the slot gate and the callback both throttling the guest. -*Falsifier:* measurement P5 (§6) — callbacks must arrive at the grid rate and the -delivered rate must not change when they are introduced. - -Exact touch points: - -| stage | file | location | change | -|---|---|---|---| -| 1a | `GameNative/app/.../xserver/XServerScreen.kt` | `:306-324` | source the panel rate (already computed) and pass it to the session start/retarget | -| 1a | `GameNative/app/.../display/GameNativeDrm.kt` | `nativeStart` / `nativeRetargetSurface` declarations (`:93` region) | add a `refreshHz` parameter | -| 1a | `gamenative-drm/modules/gndrm-android/src/lib.rs` | `nativeStart` `:996-1009`, `nativeRetargetSurface` `:1024-1036` | accept and forward the rate; build `PanelDesc` at `:717-722` | -| 1a | `gamenative-drm/modules/gndrm-daemon/src/objects.rs` | `PanelDesc` `:69-75`, `modeinfo()` `:91-125` | add `refresh_mhz`; use it for `clock` and `vrefresh`; keep `SESSION_REFRESH_MHZ` (`:63`) as the grid | -| 1a | `gamenative-drm/modules/gndrm-daemon/src/lib.rs` | `VblankTimeline::new` `:1834` | keep the interval on the session grid, not the mode rate | -| 1b | `GameNative/app/.../components/BionicProgramLauncherComponent.java` | `:591-594` | add `-r ` to the gamescope launch line | -| 1b | `gamescope/src/vblankmanager.cpp` | `:75-78` | no change; verified `g_nNestedRefresh` wins | -| 1b | `gamescope/src/Backends/DRMBackend.cpp` | `:1383-1385`, `:1397-1399`, `:3674-3685` | no change; verify the single-mode fallback is taken | -| 2 | `gamescope/android/patches/gamescope/0032-*.patch` | re-cut from `android/patches/disabled/0032-...` | gate `steamcompmgr.cpp:6961-6970`; cycle `:10541-10546` | -| 3a | (no code) | — | hold the shim's effective present-mode declaration; do not relax the slot gate | -| 3b | `gamenative-drm/icd_shim/src/proxy.rs` | slot acquire/release `:1035-1069`, `:7043` | request `wl_surface.frame` and consume the callback before freeing the slot | +# 15 — the cap, frame pacing, and vsync + +Where a frame's rate is decided on the paced path, what each place can express, and +where vsync is real rather than nominal: +`game.exe -> proton-wine (winex11) -> Xwayland -> gamescope -> gndrm-daemon -> SurfaceFlinger`. + +Line references are against the trees as they stand, at the A51 patch train plus the +cursor, scan-out and mode patches. + +## 1. Three places a rate is decided + +| place | what it holds | set by | +|---|---|---| +| the shim's acquire gate | the user's cap, exactly | `FrameCapNote{fps}` from the daemon (`docs/19`) | +| gamescope's own limiter | `nRefreshHz / nTargetFPS`, a divisor of the mode's rate | `debug_set_fps_limit`, sent only when generation is off | +| the presenter's latch grid | the display's latch points, as measured | `VsyncNote` from the daemon (`docs/17`) | + +They do different jobs, and only the first is a cap in the strict sense: the gate parks +an acquire before a frame is drawn, so the guest converges on the requested rate by +arithmetic rather than by a deadline it can miss. The second is a divisor, so with +generation off the app hands gamescope the limit and lets it express what it can: +`nVblankDivisor = nRefreshHz / min(target, nRefreshHz)`. + +## 2. The mode is the display's rate + +The connector exposes one mode: the session's scan-out size at the display's own rate +(`PanelDesc::modeinfo`, `objects.rs`, whose rate argument is the launcher's report +through `nativeSetPanelRefreshHz`). Nothing else enters it — not the cap, not the +frame-generation multiplier — so `g_nOutputRefresh` is the refresh the compositor runs +at and the refresh clients are told, and it changes only when the display does. The +daemon builds the mode on every connector read, so a display change reaches a running +session when gamescope re-polls (`backend_set_dirty`, see `docs/11`). + +The launcher passes no `-r`: `g_nNestedRefresh` is 0, so every consumer of it falls back +to `g_nOutputRefresh` (`vblankmanager.cpp:75-78`, `steamcompmgr.cpp:10606-10607`, +`:7013`) — one rate for the paint tick, the app-refresh cycle and the divisor base, and +that rate is the display's. + +## 3. Vsync, where it is real + +- **FIFO.** A present is requeued until the display releases a buffer, so the guest is + paced to the panel whether or not it asked to be. The shim answers + `vkWaitForPresentKHR` on that release, or two refresh periods after the commit when no + release report arrives first. +- **Mailbox and immediate** are applied behind the guest's back by + `MESA_VK_WSI_PRESENT_MODE`, while the guest still believes it has FIFO and waits on + every frame. A guest that knew its real mode would not wait at all, so the wait is + answered at the commit rather than holding the guest's frame-latency loop for two + periods (`answers_present_wait_at_commit`). +- **A replaced frame.** Under mailbox a frame can be replaced before it is shown. It is + never committed, released or latched under that id, so its wait is answered when it is + discarded instead of timing out. +- **Generated frames** are committed by the shim itself on the display's latch points + (`docs/17`); they carry no present id and no wait. +- **The flip event** the daemon emits carries the vblank the frame lands on + (`VblankTimeline`, one mode period), not the instant it is built, so gamescope reads a + completion time rather than a delivery time. + +## 4. What the daemon owns + +- The latch grid: `ASurfaceTransactionStats_getLatchTime` on every publish, the phase + tracked in `vsync.rs`, and a `VsyncNote` (`anchor_ns, period_ns, lead_ns`) pushed + whenever it moves by more than 300 µs, and once on subscribe. +- The lead: how far ahead of a latch point a commit has to be made to be the frame that + point takes, driven so the margin sits at half a period. +- The flip event's timestamp, as above. + +The daemon derives no rate. The launcher hands it the display's rate and the cap, and it +reports back only what it measures. + +## 5. Measured on device + +Retroid Pocket Nova (120 Hz panel, a 60 Hz mode available), Hades II, Vulkan through +zink: + +- **The gate holds the cap.** Cap 40: 40.2 acquires a second over 30 s, 0.1 skipped + batches a second, `gate_avg_us` ≈ 22 ms against a 25 ms period. +- **Generation follows the multiplier, the display does not.** Cap 40 with frame + generation at x2 / x3 / x4 produces and counts 80 / 120 / 160 frames a second, while + the mode stays at the display's rate (120 Hz, or 60 Hz on a 60 Hz panel). +- **A cap above the display's rate** reads as produced: 190 at a 190 cap and ~300 + uncapped on a 120 Hz mode, with publishes still bounded by the display. +- **Swapchain recreations survive.** Six toggles of Vsync (`mode=0` ↔ `mode=2`), no + `slot init failed`, the session continuing at its rate. +- **A display change reaches the session.** Forcing the panel to 60 Hz reported 60 and + re-selected the mode once (`selecting mode 1280x960@60Hz`). + +## 6. What this document does not own + +- The gate's arithmetic and the cap's plumbing: `docs/10`, `docs/19`. +- `g_nOutputRefresh` against `-r`, and the mode: `docs/11`. +- Frame generation, the latch grid and the lead loop: `docs/17`, `docs/18`. +- The present-latency budget: `docs/12`. diff --git a/docs/17-lsfg-frame-generation.md b/docs/17-lsfg-frame-generation.md index 9c99c9f..eb03fff 100644 --- a/docs/17-lsfg-frame-generation.md +++ b/docs/17-lsfg-frame-generation.md @@ -72,26 +72,29 @@ batches) reports both. SurfaceFlinger takes the newest buffer a layer holds at each of its latch points, one panel period apart, and shows it at the vsync after. Nothing between -the shim and the panel paces to that grid — gamescope's tick is the 1200 Hz -session grid, the daemon's `ASurfaceTransaction_apply` carries no present time — +the shim and the panel paces to that grid — gamescope's tick is its mode's rate, +the display's, and the daemon's `ASurfaceTransaction_apply` carries no present time — so a presenter that commits on its own timer beats against it: every so often two commits fall inside one period and the first is replaced before it is shown, and a rate above the panel's (60 × 3 on a 120 Hz panel) drops frames in a drifting pattern. Three rules keep one frame per period, in order: -- **Budget.** A source period at `cap` Hz holds `panel / cap` panel periods, one - of which is the source frame: `pacing::panel_budget` caps the generations per - batch at that minus one (60 → 120 Hz: x2 whatever the multiplier; 40 → 120: - x3; 30 → 60: x2). The rate is the user's cap, or the measured source interval - without one (a source hovering at the threshold, 64 fps on 120 Hz, flips - between x2 and none from batch to batch). When the panel cannot hold the cap - twice at all (60 on 90 Hz, 60 on 60 Hz), `pacing::source_pace` lowers the - source rate the release floor holds the guest to, to half the panel's: 45 → 90, - 30 → 60. The panel's rate is the one the app reported - (`LsfgConfig.refresh_mhz`), not a measurement: while the daemon is the only - thing publishing, the latch samples are spaced by its own rate, so a panel - SurfaceFlinger runs below that rate (an LTPO panel that dropped to 60 Hz) is - still paced at the reported rate and shows every other frame, as before. +- **Multiplier.** A batch generates `multiplier - 1` frames and the source frame + after them, one batch per acquired source frame, so the session produces + `cap × multiplier` frames whatever the display can latch: a 40 cap at x4 on a + 120 Hz panel produces 160 and the panel shows 120 of them. Generation is not + capped by the panel: the mode is the display's rate and never moves for a cap or + a multiplier, so capping the output by it would only make x4 at 40 read 120 and + say nothing about what the session was doing. The rate is the user's cap, or + the measured source interval without one (a source hovering at the threshold, + 64 fps on 120 Hz, flips between x2 and none from batch to batch), and the + display never reshapes it: a 40 cap runs 40 acquires a second on a 60 Hz panel + as much as on a 120 Hz one, and the multiplier decides what is generated on + top. The panel's rate is the one the app reported (`LsfgConfig.refresh_mhz`), + not a measurement: while the daemon is the only thing publishing, the latch + samples are spaced by its own rate, so a panel SurfaceFlinger runs below that + rate (an LTPO panel that dropped to 60 Hz) is still paced at the reported rate + and shows every other frame, as before. - **Grid.** The daemon reads `ASurfaceTransactionStats_getLatchTime` on every completion, tracks the grid's phase (`vsync.rs`), and pushes a `VsyncNote` (`anchor_ns, period_ns, lead_ns`) on the scan-out channel whenever it moves @@ -100,9 +103,12 @@ drifting pattern. Three rules keep one frame per period, in order: takes an unknown frame for a dead channel. The shim gives the frames of a batch consecutive latch points from the one after the previous queued frame's, or the first a commit can still reach when that has passed - (`pacing::Grid::first_latch`), and commits `lead_ns` ahead of each. Without a - note the frames are spread over the source period on the shim's own clock, as - before; the budget and the order rules below apply either way. + (`pacing::Grid::first_latch`), and commits `lead_ns` ahead of each. A batch + with more frames than the panel has points in a source period goes out on the + shim's clock spread evenly over that period instead: on the grid the source + frame would follow the batch's last point, a whole period late, and the queue + would carry that lateness forever. Without a note the frames are spread the + same way; the order rules below apply either way. - **Lead.** The daemon stamps when its apply returned and measures how far ahead of the latch it was; a loop moves the lead so that margin sits at half a period, the middle of the window between two latch points, with the error diff --git a/docs/19-fps-limiter-acquire-rework.md b/docs/19-fps-limiter-acquire-rework.md index 009dc80..4b6eeed 100644 --- a/docs/19-fps-limiter-acquire-rework.md +++ b/docs/19-fps-limiter-acquire-rework.md @@ -31,9 +31,9 @@ work (already merged, in `master`). target frame time instead of a whole number of panel cycles. 2. `DRMBackend.cpp`: the `-r` mode lookup re-finds the mode by size when the grid matches no mode's `vrefresh`. - Parking the patch as a whole would also revert (2), and `GamescopeState.gridArg()` - passes `-r 1200` on every launch — a grid that matches no mode. **A34 must be - split**: (1) is parked, (2) stays as its own patch (`A47`, next free number). + Parking the patch as a whole would also revert (2), and the launcher passed a + nested-refresh grid on every launch — one that matched no mode. **A34 was split**: + (1) is parked, (2) stays as its own patch (`A47`). - **The cap** is enforced at release: `scanout::note_release` (`icd_shim/src/scanout.rs:161`) advances a process-wide schedule and stamps `slot.floor_ns`/`release_held`; `acquire_take_free` frees a held slot when the floor passes. The guest is @@ -103,23 +103,23 @@ what produced the 49 ms holds measured on Ori). now has no limiter to arm. It can stay (harmless, and keeps the API stable), or be dropped in a follow-up once the rework has device evidence. -### 3.3 Count acquisitions, but keep the frame ring on publish +### 3.3 Count what the session produced -Two different numbers, two different events: +Two numbers, two events, and the ring takes the *produced* one: -- **Acquisitions** — the rate the cap controls. The shim keeps a running total and - sends it cumulatively (`FrameAcquired`); the daemon records it for the census. -- **Publications** — the rate the panel shows. The daemon publishes every frame - that reaches SurfaceFlinger, generated frames included, and that is what the - app's frame ring (`onFramePublished` → `FrameRating`) must be fed: its fps is - computed from the *number of calls* in a second. +- **Produced** — acquired frames plus the frames generation inserted. The shim + keeps both as running totals (`FrameAcquired { count, generated }`, sent once + per acquired frame) and the daemon turns their deltas into the app's ring + (`onFramePublished` → `FrameRating`). +- **Published** — the rate the display latched. The daemon publishes each frame + that reaches SurfaceFlinger and keeps its own count for the session log + (`asc: N frames published`), but it does not feed the ring. -Feeding the ring from acquisitions looks correct until generation runs: at a -×2 multiplier the panel shows 120 while the guest acquires 60, so the overlay -drops to half. The ring therefore stays on the publish path, and the acquire -count is a census signal only. `PowerManager.frameSampleStride` keeps its meaning -(one ring entry per displayed frame, sampled every multiplier-th to report the -source rate the tuner compares against). +The ring has to count production, because the mode is the panel's own rate: at a +60 cap, ×2 generation and a 120 Hz panel the two agree, while a 90 cap ×2 +produces 180 against a 120 Hz display and the ring says so. Counting publishes +instead made the overlay read the compositor's output — a number the panel rate +bounds — and hid every frame the display could not take. ## 4. Task list @@ -150,7 +150,7 @@ source rate the tuner compares against). 7. Split A34: `0034-A47-drm-mode-lookup-by-size.patch` from its `DRMBackend.cpp` hunk; move the whole A34 file to `disabled/`. -8. Verify the train: 29 patches apply, A47 reverses cleanly, `-r 1200` still +8. Verify the train: 29 patches apply, A47 reverses cleanly, a launch still resolves a mode (build log or device). 9. No app-side change in this branch. @@ -218,8 +218,6 @@ alternating, two runs each. - Should `nativeSetFpsLimit` keep calling `injector::set_fps_limit` once A34 is parked? Keeping it is harmless and keeps the JNI contract; dropping it is a larger app change with no benefit until a non-Vulkan cap exists. -- Should the app's frame ring be fed by the shim's acquire count *or* by the - daemon's own presentation count for the overlay? The acquire count matches the - cap; the presentation count matches the panel. Pick acquire for now (it is what - the limiter controls), and revisit if the overlay's fps disagrees with a - capture. +- ~~Should the app's frame ring be fed by the shim's acquire count *or* by the + daemon's own presentation count?~~ Answered: by production — acquire plus + generation (§3.3). The presentation count is kept as a session statistic. diff --git a/docs/README.md b/docs/README.md index ce572f3..e642196 100644 --- a/docs/README.md +++ b/docs/README.md @@ -44,7 +44,7 @@ winlator X11 path (which remains the default for normal launches). 12. [12-present-latency-budget.md](12-present-latency-budget.md) — the present-latency budget: what each frame on the paced path pays, and why the flip event is emitted with the publish. 13. [13-single-layer-windowing.md](13-single-layer-windowing.md) — the window/layer model on the single-layer path: what plays layer 0/1/N here, why the content-override mechanism exists, the per-patch layer audit, and the target architecture. 14. [14-guest-surface-commit-stall.md](14-guest-surface-commit-stall.md) — the guest-surface route's queued defect: the commit stream stalls when the guest's Vulkan swapchain comes up, the two candidate causes, and the instrumentation that would decide between them. -15. [15-fps-limiter-pacing-and-vsync.md](15-fps-limiter-pacing-and-vsync.md) — the fps limiter's deadline grid and how it meets the panel's vsync. +15. [15-fps-limiter-pacing-and-vsync.md](15-fps-limiter-pacing-and-vsync.md) — where a frame's rate is decided: the acquire gate, gamescope's divisor, and the presenter's latch grid; and where vsync is real. 16. [16-hdr-verification-and-gaps.md](16-hdr-verification-and-gaps.md) — what HDR verification measured, and the gaps left open. 17. [17-lsfg-frame-generation.md](17-lsfg-frame-generation.md) — plan for a `gamenative_drm` LSFG backend: the app's own engine inside the ICD shim, reading the container's `Lossless.dll` and building its own shader cache, reusing `LsfgVkManager` and the quick-menu tab, with the artifacts and the measurements that decide it. 18. [18-lsfg-driver-facts.md](18-lsfg-driver-facts.md) — the properties the LSFG presenter rests on, each one silent when broken: queue-family matching, the engine writing only into its own images, the blit capture, destination depth, guest-only release pacing, the `Rgba8` storage binding, and the settings channel that carries the DLL path. diff --git a/icd_shim/src/lsfg/mod.rs b/icd_shim/src/lsfg/mod.rs index 9826896..68a0f5a 100644 --- a/icd_shim/src/lsfg/mod.rs +++ b/icd_shim/src/lsfg/mod.rs @@ -348,8 +348,11 @@ pub fn skip(what: &str) -> bool { /// comes from here, not from a `FrameCapNote`. #[must_use] pub fn source_cap() -> Option { - let (enabled, multiplier, cap) = live(); - (enabled && cap > 0).then(|| gndrm_lsfg::pacing::source_pace(cap, panel_hz(), multiplier)) + let (enabled, _multiplier, cap) = live(); + // The user's cap, as asked: the display does not reshape it, so a 40 cap is + // 40 acquires a second on a 60 Hz panel as much as on a 120 Hz one, and the + // multiplier decides what is generated on top. + (enabled && cap > 0).then_some(cap) } /// `(source, output)` rates in whole hertz while generation is on. diff --git a/icd_shim/src/proxy.rs b/icd_shim/src/proxy.rs index e483e95..c4df106 100644 --- a/icd_shim/src/proxy.rs +++ b/icd_shim/src/proxy.rs @@ -978,6 +978,15 @@ unsafe fn lsfg_generate(sc: *mut GnSwapchain, src_idx: i32) -> bool { GEN_SKIPS.fetch_add(1, Ordering::Relaxed); return false; } + // One batch per source frame. The guest's presents are not the cap — a + // mailbox guest presents again as soon as a buffer frees — so a batch per + // present would generate for frames that were never acquired and put the + // output above `cap × multiplier`. + let acquired = ACQ_COUNT.load(Ordering::Relaxed); + if acquired == unsafe { (*gd).gen_last_acq } { + GEN_SKIPS.fetch_add(1, Ordering::Relaxed); + return false; + } // The queue ahead is bounded: a guest running ahead of its cap must not // stack frames further and further into the future. let period = unsafe { (*gd).gen_period_ns }; @@ -1063,18 +1072,6 @@ unsafe fn lsfg_generate(sc: *mut GnSwapchain, src_idx: i32) -> bool { if planned < capacity { GEN_SHORT_PLAN.fetch_add(1, Ordering::Relaxed); } - // No more than the panel can show between two source frames: a frame above - // that is committed and never latched, and which frames those are drifts. - let source_hz = crate::lsfg::source_cap().unwrap_or( - (1_000_000_000 + period / 2) - .checked_div(period) - .unwrap_or(0) as u32, - ); - let generations = generations.min(gndrm_lsfg::pacing::panel_budget( - crate::lsfg::panel_hz(), - source_hz, - capacity, - )); let (Some(alloc_cmd), Some(free_cmd), Some(submit)) = (f.alloc_cmd, f.free_cmd, f.submit) else { @@ -1364,14 +1361,19 @@ unsafe fn lsfg_generate(sc: *mut GnSwapchain, src_idx: i32) -> bool { (0, p) => p, (c, p) => c.max(p), }; - // On the panel's latch grid once the daemon has reported one: the - // frames take consecutive latch points from the one after the previous - // queued frame's (or the first a commit can still reach, when that has + // On the panel's latch grid when the batch fits it: the frames take + // consecutive latch points from the one after the previous queued + // frame's (or the first a commit can still reach, when that has // passed), so one frame lands per panel period and none replaces - // another unshown. The budget above keeps a source period's frames - // within the periods it holds. Without a grid, the frames are spread - // over the source period on this clock. - let grid = crate::lsfg::latch_grid(); + // another unshown. A batch with more frames than the panel has points + // in a source period goes out on this clock spread over the period + // instead: on the grid the source frame would follow the batch's last + // point, the queue would carry a period per extra frame and the + // multiplier the user asked for would never arrive. What the panel + // cannot latch is the compositor's to drop. + let grid = crate::lsfg::latch_grid().filter(|g| { + g.period_ns == 0 || (built as u64 + 1) * g.period_ns <= source_period + }); let step = source_period / (built as u64 + 1); let base = now.max(unsafe { (*gd).gen_last_deadline }); let first_latch = grid.map(|g| g.first_latch(now, unsafe { (*gd).gen_last_latch_ns })); @@ -1406,6 +1408,7 @@ unsafe fn lsfg_generate(sc: *mut GnSwapchain, src_idx: i32) -> bool { }); (*gd).gen_last_deadline = real_at; (*gd).gen_last_latch_ns = real_latch; + (*gd).gen_last_acq = acquired; } queued = true; } @@ -2408,7 +2411,13 @@ unsafe fn gn_acquire_common( // One line per frame for the app's frame ring: the count is cumulative, so // a message the socket refuses is carried by the next one. let acquired = ACQ_COUNT.fetch_add(1, Ordering::Relaxed) + 1; - crate::scanout::send_frame_acquired(acquired); + // With what generation has inserted so far: the pair is what the session has + // produced, and the daemon turns the deltas into the app's frame ring. Sent + // once per acquired frame, so the ring's granularity is one frame. + crate::scanout::send_frame_acquired( + acquired, + GEN_COMMITS.load(Ordering::Relaxed).into(), + ); let acq_ms = acq_t0.elapsed().as_millis(); let acq_us = acq_t0.elapsed().as_micros() as usize; ACQ_US_N.fetch_add(1, Ordering::Relaxed); @@ -6536,6 +6545,10 @@ pub struct GnDevice { /// The panel latch point the last queued frame was placed on, so the next /// one takes a later point; 0 before any frame was placed on the grid. pub gen_last_latch_ns: u64, + /// The acquire count when the last batch ran. A batch belongs to a source + /// frame, so a guest presenting faster than its cap must not add batches of + /// its own: those would multiply the surplus instead of the cap. + pub gen_last_acq: u64, } #[repr(C)] @@ -9627,6 +9640,7 @@ unsafe extern "C" fn gn_vkCreateDevice( gen_period_ns: 0, lsfg: ptr::null_mut(), gen_last_latch_ns: 0, + gen_last_acq: 0, }); // Real-device function resolution never uses the hooks' // vkGetDeviceProcAddr: its answers are vulkan::driver::* trampolines diff --git a/icd_shim/src/scanout.rs b/icd_shim/src/scanout.rs index 234f9f8..41d90ec 100644 --- a/icd_shim/src/scanout.rs +++ b/icd_shim/src/scanout.rs @@ -647,12 +647,15 @@ pub(crate) fn fd_link(fd: RawFd) -> String { /// Non-blocking: this runs on the guest's present thread once per frame, and a /// full socket must cost a dropped message rather than a stall. Losing one is /// harmless — the count is cumulative and the next message carries the total. -pub(crate) fn send_frame_acquired(count: u64) { +pub(crate) fn send_frame_acquired(count: u64, generated: u64) { let ch = CHAN.lock().unwrap_or_else(|e| e.into_inner()); if *ch < 0 { return; } - send_frame_nb(*ch, &gndrm_uapi::wire::FrameAcquired { count }.encode()); + send_frame_nb( + *ch, + &gndrm_uapi::wire::FrameAcquired { count, generated }.encode(), + ); } /// Mark a frame's present id for the daemon, immediately before the commit that diff --git a/modules/gndrm-android/src/asc.rs b/modules/gndrm-android/src/asc.rs index 069c135..a781b08 100644 --- a/modules/gndrm-android/src/asc.rs +++ b/modules/gndrm-android/src/asc.rs @@ -295,6 +295,35 @@ static PRESENT_DONE: Mutex> = Mutex::new(Vec::new()); /// the compositor shows. static ACQUIRED_SEEN: AtomicU64 = AtomicU64::new(0); +/// Frames frame generation had inserted when the producer last reported, so the +/// delta the app's ring sees includes them: they are produced, never acquired. +static GENERATED_SEEN: AtomicU64 = AtomicU64::new(0); + +/// The most produced frames one report may add to the ring. One report is sent +/// per acquired frame, so a healthy delta is one frame plus the multiplier; this +/// only bounds what a corrupt count could make the loop do. +const MAX_PRODUCED_PER_MESSAGE: u64 = 4096; +/// Monotonic time of the last producer report, in ms since the daemon's epoch +/// (0 = never). A session whose frames never go through the ICD shim (Xwayland +/// GL, GDI, software rendering) sends no reports, so the publish path feeds the +/// frame ring for it instead; once reports flow, the publish path goes quiet. +static LAST_PRODUCER_REPORT_MS: AtomicU64 = AtomicU64::new(0); +/// How long without a producer report before publishes count again. +const PRODUCER_REPORT_GRACE_MS: u64 = 1000; + +fn monotonic_ms() -> u64 { + static EPOCH: OnceLock = OnceLock::new(); + let epoch = *EPOCH.get_or_init(Instant::now); + // Never 0: 0 means "no report yet" in LAST_PRODUCER_REPORT_MS. + Instant::now().duration_since(epoch).as_millis() as u64 + 1 +} + +/// Whether the frame ring is currently fed by producer reports. +fn producer_reports_live() -> bool { + let last = LAST_PRODUCER_REPORT_MS.load(Ordering::Relaxed); + last != 0 && monotonic_ms().saturating_sub(last) <= PRODUCER_REPORT_GRACE_MS +} + /// Eventfd the completion callback writes to wake the scan-out thread. -1 until /// the channel is served, which is before any marked frame can exist. static PRESENT_FD: AtomicI32 = AtomicI32::new(-1); @@ -1610,12 +1639,16 @@ impl AscState { self.frames += 1; if fb.id != 0 { crate::clear_booting_splash_on_first_frame(); - // One entry per frame that reaches the panel, generated frames - // included: the app's frame ring is a *display* rate, and only this - // path knows about the ones the presenter inserted. The shim's - // acquire count is the source rate, which is a different number - // whenever frame generation runs. - crate::notify_frame_published(); + // The frame ring counts what the session produced, from the + // producer reports (see the `FrameAcquired` branch in + // `serve_scanout`): keeping both on one counter is what made a cap + // above the panel read as fps the panel never showed. A session + // without a producing shim (Xwayland GL, GDI, software rendering) + // sends no reports at all, and for it a publish is the only frame + // count there is. + if !producer_reports_live() { + crate::notify_frame_published(); + } } } @@ -2490,17 +2523,19 @@ pub fn serve_scanout( doorbell: &Arc, ) -> ScanoutChannel { let d = diag::diag(); + // This connection is the allocator until it ends; a failed push to it is + // left for this loop to close (see `gndrm_daemon::pushes`), and it stops + // being served when the loop returns however that happens. + gndrm_daemon::pushes::set_served(fd); + let _served = ServedGuard(fd); let mut buf = [0u8; ScanoutAllocReq::LEN]; - // Whether this connection opted in to frame-cap notes with a - // [`FrameCapSubscribe`]. A connection that did not is sent exactly the - // frames it was sent before, so an older shim sharing this allocator is - // never handed a frame it would refuse to frame. - let mut cap_aware = false; + // Whether a connection opted in is `gndrm_daemon::pushes`' business: a + // subscription outlives this loop, so the notes cannot be gated on it here. // Whether this connection reads the panel's latch grid, pushed whenever it // moves. Only a shim that declared so is sent one: an older shim's reply // reader takes a frame it does not know for a dead channel. Cleared when // this connection is done, whichever way it returns. - let mut vsync_aware = VsyncSubscription(false); + let vsync_fd = crate::vsync::changed_fd(); // Wake this loop when a completion lands: the callback runs on // SurfaceFlinger's thread and may not write the channel itself. @@ -2520,41 +2555,32 @@ pub fn serve_scanout( // makes the current fd the newest one; watching `cap_fd` is what lets a // target the user changed mid-session reach a shim that is not handing // a buffer over at that instant. - let mut pfds = [ - libc::pollfd { + // Retired subscribers keep reporting (see `pushes`), and this loop is + // where the daemon spends its time: their descriptors are polled beside + // the served one, and their frames read in the drain below. + let retired = gndrm_daemon::pushes::fds_except(fd); + // Slots 0..6 keep their meaning whatever the descriptors are: a `-1` + // eventfd stays in its row (poll ignores it) rather than shifting the + // indices the branches below read. + let mut pfds = Vec::with_capacity(6 + retired.len()); + for fd in [fd, listener, stop, cap_fd, vsync_fd, present_fd] { + pfds.push(libc::pollfd { fd, events: libc::POLLIN, revents: 0, - }, - libc::pollfd { - fd: listener, - events: libc::POLLIN, - revents: 0, - }, - libc::pollfd { - fd: stop, - events: libc::POLLIN, - revents: 0, - }, - libc::pollfd { - fd: cap_fd, - events: libc::POLLIN, - revents: 0, - }, - libc::pollfd { - fd: vsync_fd, - events: libc::POLLIN, - revents: 0, - }, - libc::pollfd { - fd: present_fd, + }); + } + for fd in &retired { + pfds.push(libc::pollfd { + fd: *fd, events: libc::POLLIN, revents: 0, - }, - ]; - // SAFETY: six pollfds waited on indefinitely; a negative descriptor - // (no eventfd) is ignored by `poll`. - let ready = unsafe { libc::poll(pfds.as_mut_ptr(), 6, -1) }; + }); + } + // SAFETY: `pfds` holds live descriptors (`len` of them); a negative one + // (no eventfd yet) is ignored by `poll`. + let count = u32::try_from(pfds.len()).unwrap_or(u32::MAX); + let ready = unsafe { libc::poll(pfds.as_mut_ptr(), count, -1) }; if ready < 0 { let e = io::Error::last_os_error(); if e.raw_os_error() == Some(libc::EINTR) { @@ -2566,6 +2592,19 @@ pub fn serve_scanout( if pfds[2].revents != 0 { return ScanoutChannel::Stop; } + // Retired connections: read what they report, and forget one that has + // gone away. Their allocator requests are not answered — the pool + // belongs to this connection. + for (i, fd) in retired.iter().enumerate() { + if pfds[6 + i].revents == 0 { + continue; + } + if !drain_retired(*fd) { + let _ = gndrm_daemon::pushes::forget(*fd); + // SAFETY: the peer closed; this registry held the only claim. + unsafe { libc::close(*fd) }; + } + } if pfds[3].revents != 0 { // The target changed. Drain the eventfd first, so a count left // behind cannot wake this loop again. The note is pushed even when @@ -2573,32 +2612,12 @@ pub fn serve_scanout( // skipped push would be lost — the shim reads frames by their magic // whatever order they arrive in, so a note ahead of the reply it // waits for is in step. - drain_eventfd(cap_fd); - if cap_aware { - let note = cap_now().note(); - if send_resp(fd, ¬e.encode(), None).is_err() { - return ScanoutChannel::Closed; - } - // The same event carries a settings change; the two are pushed - // together because one wake cannot say which of them moved. - if let Some(settings) = crate::lsfg_settings() { - if send_resp(fd, &settings.encode(), None).is_err() { - return ScanoutChannel::Closed; - } - } - } + flush_cap(cap_fd); } if pfds[4].revents != 0 { // The panel's latch grid moved: its phase, or the lead the shim // should commit by. Drained first, like the cap's. - drain_eventfd(vsync_fd); - if vsync_aware.0 { - if let Some(note) = crate::vsync::note() { - if send_resp(fd, ¬e.encode(), None).is_err() { - return ScanoutChannel::Closed; - } - } - } + flush_vsync(vsync_fd); } if pfds[5].revents != 0 { // A publish completed. Its id and latch time were queued by the @@ -2679,8 +2698,12 @@ pub fn serve_scanout( continue; } if let Some(sub) = FrameCapSubscribe::decode(&buf[..n]) { + // Registered before the replies: from here the pushes follow this + // connection for the rest of the session, whether or not it keeps + // the allocator role. The replies below are its starting state. + gndrm_daemon::pushes::add(fd, sub.caps); + sync_vsync_subscription(); if sub.caps & CAP_FRAME_CAP != 0 { - cap_aware = true; let note = cap_now().note(); if send_resp(fd, ¬e.encode(), None).is_err() { return ScanoutChannel::Closed; @@ -2701,7 +2724,6 @@ pub fn serve_scanout( PRESENT_FD.store(present_fd, Ordering::Relaxed); } if sub.caps & CAP_VSYNC_NOTE != 0 { - vsync_aware.set(true); // The panel's latch grid, when it has been measured: a presenter // that connects mid-session starts on it. if let Some(note) = crate::vsync::note() { @@ -2720,17 +2742,7 @@ pub fn serve_scanout( continue; } if let Some(acq) = FrameAcquired::decode(&buf[..n]) { - // The shim's running acquire total — the rate the cap controls, as - // opposed to the publish count above, which is the rate the panel - // shows. Recorded for the census; not fed to the app's ring, since - // with frame generation running the two differ by the multiplier. - let previous = ACQUIRED_SEEN.swap(acq.count, Ordering::Relaxed); - if previous == 0 && acq.count != 0 { - log_info!( - "{}", - gndrm_obf::obf!("asc: acquire counter connected (count={})", acq.count) - ); - } + note_produced(acq); continue; } if let Some(mark) = PresentMark::decode(&buf[..n]) { @@ -3672,23 +3684,13 @@ fn build_hide( } } -/// Whether the connection in service reads [`gndrm_uapi::wire::VsyncNote`], -/// mirrored into `vsync` for as long as it is, so the lead is measured only -/// while a shim places frames by it. -struct VsyncSubscription(bool); +/// Marks `fd` as the served connection for as long as this loop runs, so a push +/// that fails against it is left for the loop to close rather than closed twice. +struct ServedGuard(RawFd); -impl VsyncSubscription { - fn set(&mut self, on: bool) { - self.0 = on; - crate::vsync::set_subscribed(on); - } -} - -impl Drop for VsyncSubscription { +impl Drop for ServedGuard { fn drop(&mut self) { - if self.0 { - crate::vsync::set_subscribed(false); - } + gndrm_daemon::pushes::clear_served(self.0); } } @@ -3945,6 +3947,120 @@ fn cap_now() -> FrameCap { .unwrap_or_else(PoisonError::into_inner) } +/// Feed the app's frame ring from one producer report. +/// +/// The count is what the session *produced* — frames the guest acquired plus the +/// ones frame generation inserted — which is the number the display cannot always +/// take. The publish path counts what the display actually latched, and that can +/// never exceed its rate. +pub(crate) fn note_produced(acq: FrameAcquired) { + let previous = ACQUIRED_SEEN.swap(acq.count, Ordering::Relaxed); + let generated_seen = GENERATED_SEEN.swap(acq.generated, Ordering::Relaxed); + LAST_PRODUCER_REPORT_MS.store(monotonic_ms(), Ordering::Relaxed); + if previous == 0 && acq.count != 0 { + log_info!( + "{}", + gndrm_obf::obf!("asc: acquire counter connected (count={})", acq.count) + ); + // The frames behind this first total were already counted by the + // publish path while no reports flowed; counting them again here would + // put a one-off jump in the ring. + return; + } + // A total that went backwards is a new producer on this channel, so + // `saturating_sub` reports all of it as new. Bounded: one message is sent per + // acquired frame, so the plausible delta is one frame plus the multiplier, + // and a corrupt value must not spin here. + let produced = acq + .count + .saturating_sub(previous) + .saturating_add(acq.generated.saturating_sub(generated_seen)); + let ones = produced.min(MAX_PRODUCED_PER_MESSAGE); + for _ in 0..ones { + crate::notify_frame_published(); + } + if produced > ones { + log_warn!( + "{}", + gndrm_obf::obf!("asc: produced delta {produced} clamped to {ones} for the frame ring") + ); + } +} + +/// Read what a connection that lost the allocator role still sends, and `false` +/// when it has gone away. +/// +/// A subscription outlives that role, so the pushes reach a retired connection — +/// but its reports have to be read by someone too, and the loop that used to read +/// them has returned. Two kinds matter: the producer's running totals, which feed +/// the app's frame ring, and a present mark, which answers a guest's present wait. +/// Allocation is not served here: the pool belongs to whichever connection holds +/// it, so an alloc request on a retired connection is left to the shim's own +/// timeout, exactly as it was before any of this. +pub(crate) fn drain_retired(fd: RawFd) -> bool { + let mut buf = [0u8; ScanoutAllocReq::LEN]; + // SAFETY: `fd` is a connection this module owns until it is forgotten, and + // `buf` is one full frame; DONTWAIT leaves an unarrived frame for later. + let n = unsafe { libc::recv(fd, buf.as_mut_ptr().cast(), buf.len(), libc::MSG_DONTWAIT) }; + if n == 0 { + return false; + } + if n < 0 { + let e = io::Error::last_os_error(); + // EAGAIN and EWOULDBLOCK are the same value on Linux. + return e.raw_os_error() == Some(libc::EAGAIN); + } + let n = n as usize; + if let Some(acq) = FrameAcquired::decode(&buf[..n]) { + note_produced(acq); + } else if let Some(mark) = PresentMark::decode(&buf[..n]) { + note_present_mark(mark); + } else if ScanoutAllocReq::decode(&buf[..n]).is_some() { + // An allocation on a retired connection. The pool belongs to the + // connection in service, so this one cannot be answered here — and + // leaving it unanswered is what hangs the guest: it blocks until its + // exchange times out, retries on the same dead channel, and a title + // recreating its swapchain (a vsync toggle, a resolution change) never + // finishes. Closing the connection instead makes the shim see a dead + // channel, reconnect, and take the allocator back — and its connect + // re-subscribes, so the pushes resume on the new descriptor. + return false; + } + true +} + +/// Push the current frame cap, and the frame-generation settings that ride with +/// it, to every subscriber. Both go out because one wake cannot say which of +/// them moved. +/// +/// Every subscriber, not only the connection in service: the guest that gates on +/// the target may have handed the allocator role on long ago (`pushes`). +pub(crate) fn flush_cap(cap_fd: RawFd) { + drain_eventfd(cap_fd); + gndrm_daemon::pushes::push(CAP_FRAME_CAP, &cap_now().note().encode()); + if let Some(settings) = crate::lsfg_settings() { + gndrm_daemon::pushes::push(CAP_FRAME_CAP, &settings.encode()); + } + sync_vsync_subscription(); +} + +/// Push the panel's latch grid to every subscriber that asked for it. +pub(crate) fn flush_vsync(vsync_fd: RawFd) { + drain_eventfd(vsync_fd); + if let Some(note) = crate::vsync::note() { + gndrm_daemon::pushes::push(CAP_VSYNC_NOTE, ¬e.encode()); + } + sync_vsync_subscription(); +} + +/// Tell the latch-grid tracker whether anyone still reads its note. Called from +/// every place the subscriber set can change — a subscribe, a push that reaped a +/// peer that had gone, a retirement, a teardown — because `pushes` is the only +/// place that knows, and `vsync` is deliberately crate-root-free. +pub(crate) fn sync_vsync_subscription() { + crate::vsync::set_subscribed(gndrm_daemon::pushes::wants(CAP_VSYNC_NOTE)); +} + /// Drain the cap-change eventfd, so a count left behind cannot wake the scan-out /// channel again on the next poll. /// An eventfd for a wake the callback thread can raise: non-blocking, so a diff --git a/modules/gndrm-android/src/injector.rs b/modules/gndrm-android/src/injector.rs index e9cae54..a3d6077 100644 --- a/modules/gndrm-android/src/injector.rs +++ b/modules/gndrm-android/src/injector.rs @@ -298,6 +298,9 @@ const ALLOWED_COMMANDS: &[&str] = &[ "set_look", "screenshot", "debug_force_repaint", + // Dirties the backend and re-polls it: the connector's mode list is read + // again, so a rate the app changed mid-session becomes the mode. + "backend_set_dirty", ]; /// Whether the session is suspended. Under the `auto`/`manual` suspend policy diff --git a/modules/gndrm-android/src/lib.rs b/modules/gndrm-android/src/lib.rs index d81c40a..3e27b06 100644 --- a/modules/gndrm-android/src/lib.rs +++ b/modules/gndrm-android/src/lib.rs @@ -19,12 +19,10 @@ //! the retained last frame, keeping the listen sockets, the object stores and //! the gamescope connection alive; `nativeStop()` stays the session-exit path. //! -//! The session's vblank grid is not a caller concern: the daemon owns it as -//! its own constant (`gndrm_daemon::SESSION_REFRESH_MHZ`). The panel's real -//! refresh rate *is* one: the app supplies it through -//! `nativeSetPanelRefreshHz` before `nativeStart`, and it becomes the emulated -//! mode's rate — the refresh advertised to clients — while the grid stays the -//! daemon's constant. +//! The session's vblank grid is not a caller concern: it is the emulated mode's +//! own rate, and the mode's rate is the display's. The app supplies it through +//! `nativeSetPanelRefreshHz` before `nativeStart`, and it is what the connector +//! advertises and what the daemon's flip timeline claims. //! //! The mode's *size* is the other session-level input. `nativeStart` takes it //! from the window unless the app named one through `nativeSetScanoutGeometry` @@ -53,7 +51,7 @@ mod vsync; use std::io; use std::os::fd::RawFd; -use std::sync::atomic::{AtomicBool, AtomicI32, AtomicU32, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicI32, Ordering}; use std::sync::mpsc; use std::sync::{Arc, Mutex, OnceLock, Weak}; use std::thread; @@ -123,7 +121,7 @@ pub(crate) fn lsfg_settings() -> Option { // The panel rate is the daemon's, not the app's: it arrives through // `nativeSetPanelRefreshHz` before the session starts. The presenter paces on // its own grid and only reports against this. - settings.refresh_mhz = PANEL_REFRESH_MHZ.load(Ordering::Relaxed); + settings.refresh_mhz = gndrm_daemon::advertised_refresh_mhz(); LSFG_CONFIGURED.load(Ordering::Relaxed).then_some(settings) } @@ -133,9 +131,10 @@ pub(crate) fn lsfg_pacing() -> bool { lsfg_settings().is_some_and(|s| s.enabled != 0 && s.multiplier >= 2) } -/// The panel's refresh in millihertz as the app reported it, 0 when it has not. +/// The display's rate in millihertz as the app reported it, 0 when it has not. The +/// daemon owns it: it is the connector's mode, and it follows the display. pub(crate) fn panel_refresh_mhz() -> u32 { - PANEL_REFRESH_MHZ.load(Ordering::Relaxed) + gndrm_daemon::advertised_refresh_mhz() } /// `GameNativeDrm.nativeSetLsfgConfig(enabled, multiplier, flowPerMille, performance, fpsLimit, dllPath)`. @@ -183,17 +182,6 @@ pub extern "system" fn Java_app_gamenative_display_GameNativeDrm_nativeSetLsfgCo } } -/// The panel's real refresh rate in millihertz, supplied by the app through -/// [`Java_app_gamenative_display_GameNativeDrm_nativeSetPanelRefreshHz`] before -/// the session starts; 0 until then. -/// -/// It is a session-level input, not per-frame: the emulated mode is built once -/// when `nativeStart` assembles the [`PanelDesc`], and gamescope learns it then. -/// A value set after `nativeStart` does not reach the running session. It is -/// process-lifetime (not reset per session) so an app that supplies it once -/// still gets it for every later session. -static PANEL_REFRESH_MHZ: AtomicU32 = AtomicU32::new(0); - /// The scan-out size the app asked the session to present at, supplied through /// [`Java_app_gamenative_display_GameNativeDrm_nativeSetScanoutGeometry`] before /// the session starts; `None` until then, and then the window's size is the mode. @@ -309,10 +297,15 @@ pub(crate) fn clear_booting_splash() { } } -/// Notify the app that the daemon published a new frame, so its frame-time ring -/// sees gamescope's frames — the X11 present hook never ticks for them. Called -/// on the commit worker per new frame; best-effort, and a no-op on the app side -/// when the metrics collector is not running. +/// Tell the app's frame-time ring that the session produced one frame: one the +/// guest acquired, or one frame generation inserted. +/// +/// Called from the scan-out channel, once per frame the producer reports — not +/// from the publish path. What the display latched is a different number and the +/// smaller one: the mode is the panel's own rate, so a session capped above it +/// produces more frames than the panel can show, and the ring is what makes that +/// visible. Best-effort, and a no-op on the app side when the ring is not +/// running. pub(crate) fn notify_frame_published() { let _ = call_app_static("onFramePublished"); } @@ -631,23 +624,41 @@ fn server_thread_inner( // on the main gndrm thread) stay alive. let caught = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { loop { - let mut pfds = [ - libc::pollfd { - fd: listener, + // The note eventfds are watched here too: a subscriber + // that lost the allocator role keeps its registration, + // and between connections this loop is the only one that + // can push to it. `-1` (no eventfd yet) is ignored. + // Retired subscribers report while no connection is + // being served, so their descriptors are watched here + // beside the listener (see `gndrm_daemon::pushes`). + let retired = gndrm_daemon::pushes::fds_except(-1); + let mut pfds = Vec::with_capacity(4 + retired.len()); + for fd in [ + listener, + signal, + cap_fd, + crate::vsync::changed_fd(), + ] { + pfds.push(libc::pollfd { + fd, events: libc::POLLIN, revents: 0, - }, - libc::pollfd { - fd: signal, + }); + } + for fd in &retired { + pfds.push(libc::pollfd { + fd: *fd, events: libc::POLLIN, revents: 0, - }, - ]; + }); + } // Parked waiting for a connection: this row is what proves // the daemon never accepted a request a shim timed out on. diag::diag().set_role(diag::Role::Scanout, diag::State::ScanoutAccepting); - // SAFETY: two valid pollfds waited on indefinitely. - let n = unsafe { libc::poll(pfds.as_mut_ptr(), 2, -1) }; + // SAFETY: `pfds` holds live descriptors (`len` of them); + // a negative one is ignored by `poll`. + let count = u32::try_from(pfds.len()).unwrap_or(u32::MAX); + let n = unsafe { libc::poll(pfds.as_mut_ptr(), count, -1) }; if n < 0 { let e = io::Error::last_os_error(); if e.raw_os_error() == Some(libc::EINTR) { @@ -660,6 +671,25 @@ fn server_thread_inner( log_info!("{}", gndrm_obf::obf!("scan-out acceptor stopping")); return; } + // A push owes nothing to the allocator: it goes out + // whether or not a connection is being served. + if pfds[2].revents != 0 { + asc::flush_cap(cap_fd); + } + if pfds[3].revents != 0 { + asc::flush_vsync(crate::vsync::changed_fd()); + } + for (i, fd) in retired.iter().enumerate() { + if pfds[4 + i].revents == 0 { + continue; + } + if !asc::drain_retired(*fd) { + let _ = gndrm_daemon::pushes::forget(*fd); + // SAFETY: the peer closed; the registry held the + // only claim on the descriptor. + unsafe { libc::close(*fd) }; + } + } if pfds[0].revents == 0 { continue; } @@ -692,8 +722,30 @@ fn server_thread_inner( } } log_info!("{}", gndrm_obf::obf!("scan-out channel closed (fd={fd})")); - // SAFETY: this thread accepted the fd and is its only owner. - unsafe { libc::close(fd) }; + // A connection that asked for pushes keeps its + // registration when it only lost the allocator role: the + // cap, the grid and the frame-generation settings are + // still owed to whoever gates on them, and the guest + // that lost the slot is usually the one that does. The + // registry closes it when a push finds the peer gone + // (`gndrm_daemon::pushes`). Everything else — a + // connection that never subscribed, and any teardown — + // is closed here, as before. + asc::sync_vsync_subscription(); + if matches!(outcome, asc::ScanoutChannel::Superseded) + && gndrm_daemon::pushes::holds(fd) + { + log_info!( + "{}", + gndrm_obf::obf!( + "scan-out channel {fd} keeps its push subscription" + ) + ); + } else { + let _ = gndrm_daemon::pushes::forget(fd); + // SAFETY: this thread accepted the fd and is its only owner. + unsafe { libc::close(fd) }; + } if matches!(outcome, asc::ScanoutChannel::Stop) { log_info!("{}", gndrm_obf::obf!("scan-out acceptor stopping")); return; @@ -809,6 +861,10 @@ fn server_thread_inner( // SAFETY: the acceptor is joined; nothing else uses this descriptor. unsafe { libc::close(signal) }; } + // The acceptor is joined: no subscriber is being served, so every push + // registration can be closed with the session. + gndrm_daemon::pushes::close_all(); + asc::sync_vsync_subscription(); if cap_fd >= 0 { // Published before the close, so a late `nativeSetFpsLimit` never writes // a descriptor that has been reaped by the kernel. @@ -943,27 +999,14 @@ fn start( // window's so the display's compositor upscales instead of the session // compositing at the panel's pixels (see `SCANOUT_GEOMETRY`). let (panel_w, panel_h) = scanout_geometry(win_w, win_h); + // The rate is the daemon's own live state, set by `nativeSetPanelRefreshHz`: + // the mode is built from it on every connector read, so a display change + // reaches the running session. let panel = PanelDesc { width: panel_w, height: panel_h, mm_width: 0, mm_height: 0, - // Frame generation presents more frames than the panel accepts, and - // gamescope paces its own paint loop off this mode rate: advertising only - // the panel's rate caps the output at the panel and discards every - // generated frame above it. The mode therefore carries the rate the - // session is asked to produce, floored at the panel's own. - refresh_mhz: { - let panel_hz = PANEL_REFRESH_MHZ.load(Ordering::Relaxed); - let output_hz = lsfg_settings().map_or(0, |s| { - s.refresh_mhz.max( - s.fps_limit - .saturating_mul(s.multiplier) - .saturating_mul(1000), - ) - }); - panel_hz.max(output_hz) - }, }; let fds = Arc::new(Mutex::new(AcceptedFds { rpc: -1, @@ -1411,7 +1454,9 @@ fn set_panel_refresh_hz(refresh_hz: jint) { return; } eprintln!("gndrm-daemon: panel refresh {} Hz", refresh_hz); - PANEL_REFRESH_MHZ.store(refresh_hz as u32 * 1000, Ordering::Relaxed); + // The connector's mode: gamescope re-reads it when its backend is dirtied, so + // this is what makes a display change the mode rather than a number. + gndrm_daemon::set_advertised_refresh_mhz(refresh_hz as u32 * 1000); // The presenter paces against this: a session that started before the mode was // known would otherwise keep an unclamped plan until the user changes a setting. LSFG_SETTINGS diff --git a/modules/gndrm-android/src/vsync.rs b/modules/gndrm-android/src/vsync.rs index bda9c6b..a24e907 100644 --- a/modules/gndrm-android/src/vsync.rs +++ b/modules/gndrm-android/src/vsync.rs @@ -23,12 +23,19 @@ use std::sync::Mutex; use gndrm_uapi::wire::VsyncNote; -/// Whether the scan-out connection in service declared it reads the note. -/// The lead is measured only then: a shim that commits on its own clock gives -/// margins that do not follow the lead, and the loop would walk it to a clamp. +/// Whether any client reads the note. The lead is measured only then: a shim +/// that commits on its own clock gives margins that do not follow the lead, and +/// the loop would walk it to a clamp. +/// +/// Kept as a local flag rather than read from the subscription registry, because +/// this file is compiled on its own by the host harness +/// (`tests/vsync.rs`) and must reach nothing in the crate root. The scan-out +/// side refreshes it whenever the set of subscribers changes +/// (`asc::sync_vsync_subscription`): a subscriber keeps its registration after +/// it hands the allocator role on, and it is still placing frames by this note. static SUBSCRIBED: AtomicBool = AtomicBool::new(false); -/// Record whether the connection in service reads the note. +/// Record whether anyone reads the note (see [`SUBSCRIBED`]). pub(crate) fn set_subscribed(on: bool) { SUBSCRIBED.store(on, Ordering::Relaxed); } diff --git a/modules/gndrm-daemon/src/edid.rs b/modules/gndrm-daemon/src/edid.rs index 381c9f7..dff63d3 100644 --- a/modules/gndrm-daemon/src/edid.rs +++ b/modules/gndrm-daemon/src/edid.rs @@ -279,7 +279,7 @@ mod tests { use super::*; fn mode() -> DrmModeModeinfo { - PanelDesc::default().modeinfo() + PanelDesc::default().modeinfo(crate::UNKNOWN_PANEL_HZ) } #[test] @@ -289,9 +289,8 @@ mod tests { height: 960, mm_width: 0, mm_height: 0, - refresh_mhz: 120_000, }; - let mode = panel.modeinfo(); + let mode = panel.modeinfo(120_000); let edid = build(None, &mode, &panel); let pc = u32::from(edid[54]) | (u32::from(edid[55]) << 8); // The descriptor's timings imply the connector's rate, not a nominal one. @@ -306,9 +305,8 @@ mod tests { height: 1080, mm_width: 0, mm_height: 0, - refresh_mhz: 500_000, }; - let mode = panel.modeinfo(); + let mode = panel.modeinfo(500_000); assert!( mode.clock > 655_350, "the mode must exceed the 10 kHz field" diff --git a/modules/gndrm-daemon/src/lib.rs b/modules/gndrm-daemon/src/lib.rs index 4f370eb..8851624 100644 --- a/modules/gndrm-daemon/src/lib.rs +++ b/modules/gndrm-daemon/src/lib.rs @@ -22,6 +22,7 @@ mod objects; #[doc(hidden)] // exposed for integration tests; not public API pub mod pod; mod props; +pub mod pushes; pub mod scanout; pub mod server; @@ -38,10 +39,12 @@ use gndrm_uapi::wire::{errno, ScanoutLayout, SCANOUT_LAYOUT_ENV}; pub use frame_cap::FrameCap; pub use objects::{CrtcState, Framebuffer, GemObject, PanelDesc, PlaneState, Stores}; -pub use objects::{CONNECTOR_ID, CRTC_ID, ENCODER_ID, PLANE_CURSOR_ID, PLANE_PRIMARY_ID}; +pub use objects::{ + CONNECTOR_ID, CRTC_ID, ENCODER_ID, PLANE_CURSOR_ID, PLANE_PRIMARY_ID, UNKNOWN_PANEL_HZ, +}; pub use props::find_prop_id; -use objects::{Blob, Syncobj, SESSION_REFRESH_MHZ}; +use objects::{Blob, Syncobj}; use pod::*; use crate::diag::{diag, Role, State}; @@ -535,6 +538,40 @@ type ClockFn = Box (u32, u32) + Send + Sync>; /// re-stamps a page-flip event at emission (see [`commit_worker_loop`]); the /// indirection lets [`Device::set_clock`] (tests) reach the worker too. type SharedClock = Arc>; + +/// The rate the connector advertises, in millihertz; 0 while the launcher has +/// not reported one. +/// +/// Process state, not per-connection: gamescope reads the connector's modes +/// again whenever its backend is dirtied, and a display that changes mid-run has +/// to be able to change the mode the session runs on. A snapshot taken when the +/// session started could not. The cap and the frame-generation multiplier never +/// reach it: they move what the session produces, not what the compositor runs at. +static ADVERTISED_REFRESH_MHZ: std::sync::atomic::AtomicU32 = + std::sync::atomic::AtomicU32::new(0); + +/// Set the rate the connector advertises. Called before a session starts and +/// again whenever the launcher's answer changes. +pub fn set_advertised_refresh_mhz(mhz: u32) { + ADVERTISED_REFRESH_MHZ.store(mhz, Ordering::Relaxed); +} + +/// The rate the connector advertises, 0 when the launcher never reported one. +#[must_use] +pub fn advertised_refresh_mhz() -> u32 { + ADVERTISED_REFRESH_MHZ.load(Ordering::Relaxed) +} + +/// The rate a mode is built at: the advertised one, or [`objects::UNKNOWN_PANEL_HZ`] +/// when the launcher never reported one. +#[must_use] +fn mode_refresh_mhz() -> u32 { + match advertised_refresh_mhz() { + 0 => objects::UNKNOWN_PANEL_HZ, + mhz => mhz, + } +} + /// Diagnostics sink for rejections (see `Device::set_logger`). pub type LogFn = Box; /// Sink for a framebuffer geometry the client registered (`ADDFB2`), so a @@ -549,7 +586,6 @@ pub type AddFbGeometryFn = Box; /// The emulated device (one per connected client / open file description). pub struct Device { panel: PanelDesc, - mode: DrmModeModeinfo, stores: Stores, // client caps @@ -651,11 +687,21 @@ pub struct Device { } impl Device { + /// The connector's mode at the rate in force right now. + /// + /// Read per request rather than stored: the launcher can change the rate + /// while the session runs, and gamescope re-reads the connector when it is + /// told the backend is dirty. + #[must_use] + fn mode(&self) -> DrmModeModeinfo { + self.panel.modeinfo(mode_refresh_mhz()) + } + /// New device for `panel`; presents to `presenter` on a dedicated commit /// worker thread. pub fn new(panel: PanelDesc, presenter: Box) -> Self { let mut stores = Stores::default(); - let mode = panel.modeinfo(); + let mode = panel.modeinfo(mode_refresh_mhz()); // Infallible: a fresh store has the whole blob space free. let in_formats_blob = stores .alloc_blob() @@ -717,7 +763,6 @@ impl Device { .ok(); Self { panel, - mode, stores, cap_atomic: false, cap_universal_planes: false, @@ -1265,7 +1310,7 @@ impl Device { c.y = 0; c.gamma_size = 0; c.mode_valid = u32::from(self.crtc.mode_id != 0); - c.mode = self.mode; + c.mode = self.mode(); let mut ordered = Vec::new(); pod_write(&mut ordered, &c); ordered.extend_from_slice(&body); @@ -1281,7 +1326,7 @@ impl Device { } let mut body = Vec::new(); let encoders = [ENCODER_ID]; - let modes = [self.mode]; + let modes = [self.mode()]; let (prop_ids, prop_vals) = self.connector_props(); let count_enc = Self::two_call(&mut body, c.encoders_ptr, c.count_encoders, &encoders); let count_modes = Self::two_call(&mut body, c.modes_ptr, c.count_modes, &modes); @@ -2175,9 +2220,10 @@ struct VblankTimeline { } impl VblankTimeline { + /// One vblank of the mode in force: the display's own period. fn new() -> Self { Self { - interval_ns: 1_000_000_000_000 / u64::from(SESSION_REFRESH_MHZ), + interval_ns: 1_000_000_000_000 / u64::from(mode_refresh_mhz()), claimed_ns: 0, started: false, } diff --git a/modules/gndrm-daemon/src/objects.rs b/modules/gndrm-daemon/src/objects.rs index d92f351..a03a9f6 100644 --- a/modules/gndrm-daemon/src/objects.rs +++ b/modules/gndrm-daemon/src/objects.rs @@ -50,25 +50,17 @@ const FIRST_SYNCOBJ_HANDLE: u32 = 0x4000; /// Exclusive end of the syncobj handle space. const END_SYNCOBJ_HANDLE: u32 = 0x5000; -/// The session's vblank grid, in millihertz (1200 Hz = 1_200_000). -/// -/// gamescope builds its vblank grid — the compositor's paint and frame-done -/// tick — from the refresh it is launched with (`-r`), and the daemon builds its -/// flip-event timeline from this constant, so the grid and the timeline agree by -/// construction. It is deliberately high: the grid is a timer, so a finite value -/// is required, and 1200 Hz is ~4.6x the present pipeline's measured throughput -/// (~259 presents/s), so it cannot bind and a guest can over-refresh the panel. -/// The only real cap on the guest is the user's own fps limit in the QuickMenu; -/// the physical panel is downstream and coalesces whatever the session sends. +/// The mode's rate when the app never reported one: 60 Hz, the one rate every +/// panel latches. An unknown rate gets the conservative answer rather than a +/// fabricated high number that would have the compositor paint frames nothing +/// can show. +pub const UNKNOWN_PANEL_HZ: u32 = 60_000; + +/// Physical panel description: the surface size the session presents into. /// -/// This is NOT the emulated mode's refresh. The mode's rate is the panel's real -/// rate when the caller supplies one ([`PanelDesc::refresh_mhz`]) and falls back -/// to this constant otherwise; keeping the two separate is what lets the refresh -/// advertised to clients be truthful without clamping the grid onto the panel. -pub const SESSION_REFRESH_MHZ: u32 = 1_200_000; - -/// Physical panel description: the surface size the session presents into and, -/// when known, the panel's real refresh rate. +/// The rate the connector advertises is not part of it: that is the display's +/// ([`crate::panel_refresh_mhz`]) and changes when the display does, whereas this +/// is what the surface was built with. #[derive(Debug, Clone, Copy)] pub struct PanelDesc { pub width: u16, @@ -76,11 +68,6 @@ pub struct PanelDesc { /// Physical size in millimetres (0 = unknown). pub mm_width: u32, pub mm_height: u32, - /// The panel's real refresh rate in millihertz as supplied by the caller, or - /// 0 when it is not known. It sets the emulated mode's `clock` and - /// `vrefresh` only; the session's vblank grid stays [`SESSION_REFRESH_MHZ`]. - /// See [`PanelDesc::mode_refresh_mhz`]. - pub refresh_mhz: u32, } impl Default for PanelDesc { @@ -90,35 +77,18 @@ impl Default for PanelDesc { height: 720, mm_width: 0, mm_height: 0, - refresh_mhz: 0, } } } impl PanelDesc { - /// The emulated mode's refresh rate in millihertz: the panel's real rate when - /// the caller supplied one, else [`SESSION_REFRESH_MHZ`]. + /// Build the connector's mode at `refresh_mhz`. /// - /// The mode's rate and the session's vblank grid are separate values on - /// purpose. The mode's rate is what gamescope recomputes into - /// `g_nOutputRefresh` and advertises to every client — the Xwayland mode, the - /// refresh-rate feedback property, `active_display_info` — so it must be the - /// panel's truth. The grid is the compositor's paint/frame-done tick and the - /// daemon's flip-event interval, and it must stay above the present - /// pipeline's ceiling so a guest can over-refresh the panel. One constant - /// served both jobs, which forced a fabricated rate onto the clients. - #[must_use] - pub fn mode_refresh_mhz(&self) -> u32 { - if self.refresh_mhz == 0 { - SESSION_REFRESH_MHZ - } else { - self.refresh_mhz - } - } - - /// Build the single preferred mode exposed on the connector. + /// The rate is the display's, not this description's: the launcher reports + /// the panel's, and a display change arrives while the session runs, so the + /// caller passes the rate in force rather than this snapshot holding it. #[must_use] - pub fn modeinfo(&self) -> DrmModeModeinfo { + pub fn modeinfo(&self, refresh_mhz: u32) -> DrmModeModeinfo { let hdisplay = self.width; let vdisplay = self.height; // Generic CVT-ish timings: porch values only matter for real panels; @@ -129,11 +99,10 @@ impl PanelDesc { let vsync_start = vdisplay + 3; let vsync_end = vdisplay + 6; let vtotal = vdisplay + 20; - // The mode's rate is the panel's, not the session grid: clock, htotal and - // vtotal must agree with `vrefresh`, because gamescope recomputes the rate - // from them (`GetModeRefresh`) and the recomputed value is what reaches - // clients. - let refresh_mhz = self.mode_refresh_mhz(); + // clock, htotal and vtotal must agree with `vrefresh`, because gamescope + // recomputes the rate from them (`GetModeRefresh`) and the recomputed + // value is what reaches clients. + // // clock[kHz] = total_px * refresh / 1000, rounded. let clock = (u64::from(htotal) * u64::from(vtotal) * u64::from(refresh_mhz) / 1_000_000) as u32; @@ -387,20 +356,27 @@ mod tests { #[test] fn mode_rate_is_the_panel_rate_when_supplied() { - let mode = PanelDesc { - refresh_mhz: 90_000, - ..PanelDesc::default() - } - .modeinfo(); + let mode = PanelDesc::default().modeinfo(90_000); assert_eq!(mode.vrefresh, 90); assert_eq!(recomputed_mhz(&mode), 90_000); - assert_ne!(mode.vrefresh, SESSION_REFRESH_MHZ.div_ceil(1000)); } #[test] - fn mode_rate_falls_back_to_the_session_grid_when_absent() { - let mode = PanelDesc::default().modeinfo(); - assert_eq!(mode.vrefresh, SESSION_REFRESH_MHZ.div_ceil(1000)); - assert_eq!(recomputed_mhz(&mode), u64::from(SESSION_REFRESH_MHZ)); + fn an_unreported_panel_gets_the_conservative_rate() { + // The mode is only ever the panel's own rate, so an unknown panel gets + // one every display latches rather than a fabricated high number. + let mode = PanelDesc::default().modeinfo(UNKNOWN_PANEL_HZ); + assert_eq!(mode.vrefresh, UNKNOWN_PANEL_HZ.div_ceil(1000)); + assert_eq!(recomputed_mhz(&mode), u64::from(UNKNOWN_PANEL_HZ)); + } + + #[test] + fn the_mode_is_the_panel_rate_whatever_the_cap_is() { + // The cap is at acquire and never reaches the mode: a session capped at + // 30 on a 120 Hz panel still advertises 120, so the compositor paints at + // the rate the display can latch. + let mode = PanelDesc::default().modeinfo(120_000); + assert_eq!(mode.vrefresh, 120); + assert_eq!(recomputed_mhz(&mode), 120_000); } } diff --git a/modules/gndrm-daemon/src/pushes.rs b/modules/gndrm-daemon/src/pushes.rs new file mode 100644 index 0000000..5ad4f29 --- /dev/null +++ b/modules/gndrm-daemon/src/pushes.rs @@ -0,0 +1,364 @@ +//! Push delivery to the clients that asked for it. +//! +//! The card listener hands the *allocator* role to one connection at a time +//! ([`crate::server::accept_newest`]), but a push is not the allocator's: the +//! frame cap, the panel's latch grid and the frame-generation settings belong to +//! every client that gates its own behaviour on them. A guest that connects and +//! is later superseded by another client keeps its registration here, so it +//! keeps learning — which is what a live cap change needs. +//! +//! The single slot could not carry that. The note used to be sent to whoever +//! held the allocator at the instant it changed, gated on that connection having +//! subscribed; a session has three clients that open the listener (the +//! compositor, a wine child, the game), they supersede each other for the first +//! ~20 s, and the one that ends up holding the slot is not necessarily the one +//! that gates. A subscriber that lost the slot was closed with it, so its cap +//! froze at whatever it read when it connected. +//! +//! Delivery is fire-and-forget on a non-blocking send. A sink whose peer is gone +//! fails that send, and the failure is the only reaper: the connection is closed +//! here and forgotten — except the one currently served, which its own loop +//! closes when the failed push surfaces there. + +use std::os::fd::RawFd; +use std::sync::atomic::{AtomicI32, Ordering}; +use std::sync::{Mutex, PoisonError}; + +#[cfg(test)] +use gndrm_uapi::wire::{CAP_FRAME_CAP, CAP_PRESENT_ID, CAP_VSYNC_NOTE}; + +/// One connection that asked to be pushed to: its descriptor and the cap bits it +/// declared in its subscribe. +#[derive(Debug, Clone, Copy)] +struct Sink { + fd: RawFd, + caps: u32, +} + +/// Every registered sink. Process-lifetime: a subscription outlives the +/// allocator slot, and a new subscribe on a reused descriptor replaces the old +/// entry rather than shadowing it. +static SINKS: Mutex> = Mutex::new(Vec::new()); + +/// The descriptor the acceptor is serving right now, `-1` when it is parked on +/// the listener. Only used to keep [`push`] from closing a descriptor another +/// loop still owns. +static SERVED_FD: AtomicI32 = AtomicI32::new(-1); + +/// Register `fd` for the pushes its `caps` bits name. A second subscribe on the +/// same descriptor (a shim reconnecting onto a reused number) replaces the caps. +pub fn add(fd: RawFd, caps: u32) { + let mut sinks = SINKS.lock().unwrap_or_else(PoisonError::into_inner); + if let Some(sink) = sinks.iter_mut().find(|s| s.fd == fd) { + sink.caps = caps; + } else { + sinks.push(Sink { fd, caps }); + } +} + +/// Forget `fd`; `true` when it was registered, i.e. when this registry — not the +/// caller — is the descriptor's only owner and the caller must therefore close +/// it only if this returned `false`. +#[must_use] +pub fn forget(fd: RawFd) -> bool { + let mut sinks = SINKS.lock().unwrap_or_else(PoisonError::into_inner); + let before = sinks.len(); + sinks.retain(|s| s.fd != fd); + sinks.len() != before +} + +/// The registered descriptors, minus `except`: what a loop serving one +/// connection should keep reading for the others (a retired subscriber still +/// reports what it produces — see [`crate::pushes`]). +#[must_use] +pub fn fds_except(except: RawFd) -> Vec { + SINKS + .lock() + .unwrap_or_else(PoisonError::into_inner) + .iter() + .map(|s| s.fd) + .filter(|fd| *fd != except) + .collect() +} + +/// Whether any registered sink asked for pushes of `cap`'s kind. This is what +/// keeps a measurement running while the client that wants it is not the one +/// holding the allocator: the grid is only worth tracking if someone reads it. +#[must_use] +pub fn wants(cap: u32) -> bool { + SINKS + .lock() + .unwrap_or_else(PoisonError::into_inner) + .iter() + .any(|s| s.caps & cap != 0) +} + +/// Whether `fd` is registered, without disturbing it. +#[must_use] +pub fn holds(fd: RawFd) -> bool { + SINKS + .lock() + .unwrap_or_else(PoisonError::into_inner) + .iter() + .any(|s| s.fd == fd) +} + +/// Record the descriptor the acceptor is serving, so a failed push to it is left +/// for that loop to close. +pub fn set_served(fd: RawFd) { + SERVED_FD.store(fd, Ordering::Relaxed); +} + +/// Clear the served descriptor if it is still `fd` (a later connection may +/// already have replaced it). +pub fn clear_served(fd: RawFd) { + let _ = SERVED_FD.compare_exchange(fd, -1, Ordering::Relaxed, Ordering::Relaxed); +} + +/// Send one encoded `frame` to every sink that declared `cap` in its subscribe. +/// +/// Non-blocking, and it must be: these descriptors are blocking sockets and the +/// caller is the acceptor thread, so a push into a full buffer would stall +/// allocation behind a client that is not reading. `EAGAIN` therefore keeps the +/// sink — the next push carries the current state anyway, and nothing is owed in +/// between. Any other failure means the peer is gone: the sink is dropped, and +/// its descriptor closed unless the acceptor is serving it. +pub fn push(cap: u32, frame: &[u8]) { + let served = SERVED_FD.load(Ordering::Relaxed); + let mut sinks = SINKS.lock().unwrap_or_else(PoisonError::into_inner); + sinks.retain(|sink| { + if sink.caps & cap == 0 { + return true; + } + // SAFETY: `sink.fd` is a descriptor this registry owns until it is + // dropped or closed here, and `frame` outlives the call. MSG_NOSIGNAL + // keeps a peer that has gone away from raising SIGPIPE in the daemon. + let sent = unsafe { + libc::send( + sink.fd, + frame.as_ptr().cast(), + frame.len(), + libc::MSG_NOSIGNAL | libc::MSG_DONTWAIT, + ) + }; + if sent == frame.len() as isize { + return true; + } + let full = sent < 0 + && std::io::Error::last_os_error().raw_os_error() == Some(libc::EAGAIN); + if full { + return true; + } + if sink.fd != served { + // SAFETY: the failed send proved the peer unusable, and this + // registry holds the only claim on the descriptor. + unsafe { libc::close(sink.fd) }; + } + false + }); +} + +/// Close and forget every sink (session teardown: nothing is left to push to). +pub fn close_all() { + let mut sinks = SINKS.lock().unwrap_or_else(PoisonError::into_inner); + for sink in sinks.drain(..) { + // SAFETY: this registry is the descriptor's only owner. + unsafe { libc::close(sink.fd) }; + } + SERVED_FD.store(-1, Ordering::Relaxed); +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A connected `SOCK_SEQPACKET` pair, the transport the scan-out channel is. + fn pair() -> (RawFd, RawFd) { + let mut fds = [0 as RawFd; 2]; + // SAFETY: two out-params in a live array. + let rc = unsafe { + libc::socketpair( + libc::AF_UNIX, + libc::SOCK_SEQPACKET | libc::SOCK_CLOEXEC, + 0, + fds.as_mut_ptr(), + ) + }; + assert_eq!(rc, 0); + (fds[0], fds[1]) + } + + fn recv_one(fd: RawFd) -> Option> { + let mut buf = [0u8; 64]; + // SAFETY: `fd` is one end of a pair and `buf` is one frame. + let n = unsafe { libc::recv(fd, buf.as_mut_ptr().cast(), buf.len(), libc::MSG_DONTWAIT) }; + (n > 0).then(|| buf[..n as usize].to_vec()) + } + + /// The registry is process-wide, so these tests take turns: each one starts + /// from an empty registry and holds the lock until it ends. + static TEST_LOCK: Mutex<()> = Mutex::new(()); + + fn reset() -> std::sync::MutexGuard<'static, ()> { + let guard = TEST_LOCK.lock().unwrap_or_else(PoisonError::into_inner); + close_all(); + guard + } + + #[test] + fn every_subscriber_receives_the_push() { + let _g = reset(); + let (a, peer_a) = pair(); + let (b, peer_b) = pair(); + add(a, CAP_FRAME_CAP); + add(b, CAP_FRAME_CAP); + push(CAP_FRAME_CAP, b"cap"); + assert_eq!(recv_one(peer_a).as_deref(), Some(&b"cap"[..])); + assert_eq!(recv_one(peer_b).as_deref(), Some(&b"cap"[..])); + assert!(holds(a) && holds(b)); + // SAFETY: the test owns every descriptor it opened. + unsafe { + [a, b, peer_a, peer_b].iter().for_each(|fd| { + libc::close(*fd); + }) + }; + } + + #[test] + fn a_sink_only_gets_the_kinds_it_declared() { + let _g = reset(); + let (cap_only, peer) = pair(); + add(cap_only, CAP_FRAME_CAP); + push(CAP_VSYNC_NOTE, b"grid"); + assert!(recv_one(peer).is_none(), "a cap-only sink got a grid note"); + push(CAP_FRAME_CAP, b"fps"); + assert_eq!(recv_one(peer).as_deref(), Some(&b"fps"[..])); + push(CAP_PRESENT_ID, b"done"); + assert!(recv_one(peer).is_none(), "a cap-only sink got a present id"); + // SAFETY: the test owns every descriptor it opened. + unsafe { + [cap_only, peer].iter().for_each(|fd| { + libc::close(*fd); + }) + }; + } + + #[test] + fn a_dead_sink_is_dropped_and_its_descriptor_closed() { + let _g = reset(); + let (live, peer_live) = pair(); + let (dead, peer_dead) = pair(); + add(live, CAP_FRAME_CAP); + add(dead, CAP_FRAME_CAP); + // The peer goes away: the pair's other end is closed, so a send fails. + // SAFETY: the test owns `peer_dead`. + unsafe { libc::close(peer_dead) }; + push(CAP_FRAME_CAP, b"cap"); + assert!(holds(live)); + assert!(!holds(dead), "a sink whose peer is gone must be dropped"); + assert_eq!(recv_one(peer_live).as_deref(), Some(&b"cap"[..])); + // SAFETY: closing a descriptor this registry already closed is the bug + // this test guards against; a second close would show as EBADF. + let rc = unsafe { libc::fcntl(dead, libc::F_GETFD) }; + assert_eq!(rc, -1, "the dropped sink's descriptor was left open"); + // SAFETY: the test owns the remaining descriptors. + unsafe { + [live, peer_live].iter().for_each(|fd| { + libc::close(*fd); + }) + }; + } + + #[test] + fn a_full_buffer_keeps_the_sink() { + let _g = reset(); + let (busy, peer) = pair(); + // A tiny send buffer, so a few frames fill it and the next send is + // `EAGAIN` — a client that is merely behind, not gone. + let small: libc::c_int = 128; + // SAFETY: setsockopt on a socket this test owns. + unsafe { + libc::setsockopt( + busy, + libc::SOL_SOCKET, + libc::SO_SNDBUF, + (&raw const small).cast(), + core::mem::size_of::() as libc::socklen_t, + ) + }; + add(busy, CAP_FRAME_CAP); + for _ in 0..64 { + push(CAP_FRAME_CAP, &[0u8; 64]); + } + assert!( + holds(busy), + "a sink that is behind must not be dropped as if its peer had gone" + ); + // SAFETY: the test owns every descriptor it opened. + unsafe { + [busy, peer].iter().for_each(|fd| { + libc::close(*fd); + }) + }; + } + + #[test] + fn the_served_descriptor_is_left_to_its_own_loop() { + let _g = reset(); + let (served, peer) = pair(); + add(served, CAP_FRAME_CAP); + set_served(served); + // SAFETY: the test owns `peer`. + unsafe { libc::close(peer) }; + push(CAP_FRAME_CAP, b"cap"); + assert!(!holds(served), "a failed push still drops the registration"); + // SAFETY: probe only, no ownership taken. + let rc = unsafe { libc::fcntl(served, libc::F_GETFD) }; + assert_ne!(rc, -1, "the served descriptor must stay open for its loop"); + clear_served(served); + // SAFETY: the test owns `served`. + unsafe { libc::close(served) }; + } + + #[test] + fn a_second_subscribe_on_one_descriptor_replaces_its_caps() { + let _g = reset(); + let (fd, peer) = pair(); + add(fd, CAP_FRAME_CAP); + add(fd, CAP_VSYNC_NOTE); + push(CAP_FRAME_CAP, b"fps"); + assert!( + recv_one(peer).is_none(), + "the old caps survived the resubscribe" + ); + push(CAP_VSYNC_NOTE, b"grid"); + assert_eq!(recv_one(peer).as_deref(), Some(&b"grid"[..])); + // SAFETY: the test owns every descriptor it opened. + unsafe { + [fd, peer].iter().for_each(|x| { + libc::close(*x); + }) + }; + } + + #[test] + fn teardown_closes_what_is_left() { + let _g = reset(); + let (a, peer_a) = pair(); + let (b, peer_b) = pair(); + add(a, CAP_FRAME_CAP); + add(b, CAP_VSYNC_NOTE); + close_all(); + // SAFETY: probe only. + assert_eq!(unsafe { libc::fcntl(a, libc::F_GETFD) }, -1); + // SAFETY: probe only. + assert_eq!(unsafe { libc::fcntl(b, libc::F_GETFD) }, -1); + assert!(!holds(a) && !holds(b)); + // SAFETY: the test owns the peers. + unsafe { + [peer_a, peer_b].iter().for_each(|fd| { + libc::close(*fd); + }) + }; + } +} diff --git a/modules/gndrm-daemon/src/server.rs b/modules/gndrm-daemon/src/server.rs index d01bc19..5021039 100644 --- a/modules/gndrm-daemon/src/server.rs +++ b/modules/gndrm-daemon/src/server.rs @@ -7,7 +7,10 @@ //! contract: the first accepted connection is the RPC channel, the second is //! the event channel (matches `gndrm-shim`'s connect order); every later //! connection is the single scan-out allocator channel (newest wins). One -//! card client at a time — gamescope. +//! allocator at a time, but not one subscriber: the pushes a connection asks +//! for (frame cap, latch grid, frame-generation settings) are delivered to +//! every registered subscriber for the rest of the session, whether or not it +//! still holds the allocator slot — see [`crate::pushes`]. //! - **render node** — [`render_socket_name`] (the main name + `-render`). //! Served by [`serve_render_listener`], one [`Device`] per RPC/event pair, //! for as many concurrent clients as connect. Xwayland's glamor/dri3/drm diff --git a/modules/gndrm-daemon/tests/golden.rs b/modules/gndrm-daemon/tests/golden.rs index 0f63251..64f436c 100644 --- a/modules/gndrm-daemon/tests/golden.rs +++ b/modules/gndrm-daemon/tests/golden.rs @@ -12,7 +12,7 @@ use gndrm_daemon::pod::{pod_read, pod_write}; use gndrm_daemon::{ find_prop_id, select_scanout_layout, Device, DispatchResult, Framebuffer, NullPresenter, PanelDesc, PlaneState, Presenter, CONNECTOR_ID, CRTC_ID, ENCODER_ID, PLANE_CURSOR_ID, - PLANE_PRIMARY_ID, + PLANE_PRIMARY_ID, UNKNOWN_PANEL_HZ, }; use gndrm_uapi::drm::*; use gndrm_uapi::mode::*; @@ -502,7 +502,9 @@ fn connector_encoder_plane_queries() { assert_eq!(ext.len(), want); let m: DrmModeModeinfo = pod_read(&ext[4..]).unwrap(); assert_eq!((m.hdisplay, m.vdisplay), (1280, 720)); - assert_eq!(m.vrefresh, 1200); + // The mode is the panel's own rate; no panel was reported in this harness, + // so the conservative default. + assert_eq!(m.vrefresh, UNKNOWN_PANEL_HZ.div_ceil(1000)); assert_ne!(m.ty & DRM_MODE_TYPE_PREFERRED, 0); // Unknown connector id. @@ -769,7 +771,7 @@ fn property_queries_golden() { #[test] fn blob_lifecycle() { let mut d = dev(); - let mode = PanelDesc::default().modeinfo(); + let mode = PanelDesc::default().modeinfo(gndrm_daemon::UNKNOWN_PANEL_HZ); let mut req = pod(&DrmModeCreateBlob { length: core::mem::size_of::() as u32, ..Default::default() @@ -1464,7 +1466,7 @@ fn commit_worker_panic_is_contained_and_non_fatal() { struct ModeBlob(u32); fn create_mode_blob(d: &mut Device) -> ModeBlob { - let mode = PanelDesc::default().modeinfo(); + let mode = PanelDesc::default().modeinfo(gndrm_daemon::UNKNOWN_PANEL_HZ); let mut req = pod(&DrmModeCreateBlob { length: core::mem::size_of::() as u32, ..Default::default() @@ -1664,11 +1666,10 @@ fn atomic_test_only_then_modeset_then_flip() { assert_eq!(events[0].sequence, 2); assert_eq!(events[0].user_data, 0xBEEF); // The claimed vblank advanced by exactly one mode interval (the mocked clock - // does not move), which is what keeps gamescope's grid on the mode's rate: - // the 1200 Hz session constant is 833 333 ns per frame. + // does not move): the mode is the 60 Hz fallback, 16 666 666 ns per frame. assert_eq!( (events[0].tv_sec, events[0].tv_usec), - (1000, 1_333), + (1000, 17_166), "the next claim is one interval after the previous vblank" ); diff --git a/modules/gndrm-lsfg/src/pacing.rs b/modules/gndrm-lsfg/src/pacing.rs index f49a773..1d6f119 100644 --- a/modules/gndrm-lsfg/src/pacing.rs +++ b/modules/gndrm-lsfg/src/pacing.rs @@ -5,8 +5,7 @@ //! so a timer that is not phased to those points beats against them: every so //! often two commits fall inside one period and the second replaces the first //! before it is shown. This module turns an instant a frame is wanted at into -//! the latch point it lands on and the commit instant that reaches it, and -//! bounds how many frames a source period can carry at all. +//! the latch point it lands on and the commit instant that reaches it. /// The panel's latch grid, as the daemon reports it. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -57,49 +56,6 @@ impl Grid { } } -/// The rate the source is paced to while generating `multiplier` frames per -/// source frame for a `panel_hz` panel: the user's `cap`, unless the panel -/// cannot show two frames per source frame at it, in which case half the -/// panel's rate, so the panel is filled evenly (a 60 cap on a 90 Hz panel runs -/// the game at 45 and shows 90; on a 120 Hz panel it stays 60 and shows 120). -/// A cap of 0 (none) or an unknown panel is returned as is. -#[must_use] -pub fn source_pace(cap: u32, panel_hz: u32, multiplier: u32) -> u32 { - if cap == 0 || panel_hz == 0 || multiplier < 2 { - return cap; - } - // The tolerance `panel_budget` applies: a 120 Hz panel reported as 119 still - // holds a 60 cap twice. - if u64::from(panel_hz) * 17 / 16 >= 2 * u64::from(cap) { - cap - } else { - panel_hz / 2 - } -} - -/// How many generated frames a panel can show between two source frames, at -/// most `capacity`. -/// -/// A panel shows one frame per period; a source period at `source_hz` holds -/// `panel_hz / source_hz` of them, one of which is the source frame itself. -/// Anything above that is committed and never shown, and which frames are -/// dropped drifts, so the output looks uneven. A rate a hair under an exact -/// multiple (a 120 Hz panel reporting 119.88, a guest measured at 61 against a -/// 60 cap) still gets the multiple: the presenter's queue bound absorbs the -/// difference as an occasional skipped batch. 0 for either rate means it is -/// unknown, which leaves the capacity alone. -#[must_use] -pub fn panel_budget(panel_hz: u32, source_hz: u32, capacity: u32) -> u32 { - if panel_hz == 0 || source_hz == 0 { - return capacity; - } - let slots = u64::from(panel_hz) * 17 / 16 / u64::from(source_hz); - u32::try_from(slots) - .unwrap_or(u32::MAX) - .saturating_sub(1) - .min(capacity) -} - #[cfg(test)] mod tests { use super::*; @@ -175,45 +131,4 @@ mod tests { assert_eq!(g.first_latch(100, 0), 107); assert_eq!(g.commit_for(107), 100); } - - #[test] - fn source_pace_halves_the_panel_when_the_cap_does_not_fit_twice() { - assert_eq!(source_pace(60, 120, 2), 60); - assert_eq!(source_pace(60, 119, 3), 60); - assert_eq!(source_pace(60, 90, 2), 45); - assert_eq!(source_pace(90, 144, 2), 72); - assert_eq!(source_pace(60, 60, 2), 30); - assert_eq!(source_pace(30, 60, 2), 30); - assert_eq!(source_pace(60, 90, 1), 60); - assert_eq!(source_pace(0, 90, 2), 0); - assert_eq!(source_pace(60, 0, 2), 60); - } - - #[test] - fn panel_budget_counts_the_slots_a_source_period_leaves() { - assert_eq!(panel_budget(120, 60, 3), 1); - assert_eq!(panel_budget(120, 40, 3), 2); - assert_eq!(panel_budget(120, 30, 3), 3); - assert_eq!(panel_budget(120, 30, 2), 2); - assert_eq!(panel_budget(60, 60, 3), 0); - assert_eq!(panel_budget(60, 30, 3), 1); - assert_eq!(panel_budget(90, 45, 3), 1); - assert_eq!(panel_budget(90, 60, 3), 0); - assert_eq!(panel_budget(144, 72, 3), 1); - assert_eq!(panel_budget(144, 48, 3), 2); - } - - #[test] - fn panel_budget_tolerates_a_rate_just_over_the_multiple() { - assert_eq!(panel_budget(120, 61, 3), 1); - assert_eq!(panel_budget(120, 62, 3), 1); - assert_eq!(panel_budget(119, 60, 3), 1); - assert_eq!(panel_budget(120, 41, 3), 2); - } - - #[test] - fn panel_budget_leaves_an_unknown_rate_alone() { - assert_eq!(panel_budget(0, 60, 3), 3); - assert_eq!(panel_budget(120, 0, 3), 3); - } } diff --git a/modules/gndrm-uapi/src/wire.rs b/modules/gndrm-uapi/src/wire.rs index e712deb..e8e95b6 100644 --- a/modules/gndrm-uapi/src/wire.rs +++ b/modules/gndrm-uapi/src/wire.rs @@ -586,6 +586,11 @@ pub const FRAME_ACQUIRED_MAGIC: u32 = 0x474E_4151; pub struct FrameAcquired { /// Frames acquired since this shim connected. pub count: u64, + /// Frames frame generation inserted since this shim connected. They are + /// never acquired — the presenter's engine makes them from a frame the guest + /// already finished — so a consumer counting what the session *produced* has + /// to add them to `count`. Zero when generation is off. + pub generated: u64, } impl FrameAcquired { @@ -596,6 +601,7 @@ impl FrameAcquired { let mut b = [0u8; Self::LEN]; b[0..4].copy_from_slice(&FRAME_ACQUIRED_MAGIC.to_le_bytes()); b[8..16].copy_from_slice(&self.count.to_le_bytes()); + b[16..24].copy_from_slice(&self.generated.to_le_bytes()); b } @@ -608,10 +614,39 @@ impl FrameAcquired { } Some(Self { count: u64::from_le_bytes(b[8..16].try_into().ok()?), + generated: u64::from_le_bytes(b[16..24].try_into().ok()?), }) } } +#[cfg(test)] +mod frame_acquired_tests { + use super::FrameAcquired; + + #[test] + fn the_generated_total_rides_with_the_acquire_total() { + // Both totals are cumulative and travel together: one report per acquired + // frame is what the app's ring turns into "produced". + let report = FrameAcquired { + count: 900, + generated: 1800, + }; + let decoded = FrameAcquired::decode(&report.encode()).expect("round trip"); + assert_eq!(decoded.count, 900); + assert_eq!(decoded.generated, 1800); + } + + #[test] + fn a_report_without_generation_leaves_the_field_zero() { + let decoded = FrameAcquired { + count: 7, + generated: 0, + }; + let back = FrameAcquired::decode(&decoded.encode()).expect("round trip"); + assert_eq!(back.generated, 0); + } +} + /// Present-mark magic: ASCII "GNPM". Shim -> daemon, on the scan-out channel. /// /// The shim tells the daemon which present id the frame it is about to commit