From d8a3c70fe004df72fe521bc6a5519c2a53c06a94 Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Mon, 2 Nov 2026 11:40:09 +0530 Subject: [PATCH 1/7] daemon: flip events carry the display's real vsync, read from Choreographer The flip events were stamped on a timeline of the daemon's own: a constant walk of 1e12 / mode_refresh_mhz() from CLOCK_MONOTONIC, 16 666 666 ns on a "60 Hz" mode. gamescope re-arms its composite timer from every stamp (DRMBackend.cpp page_flip_handler -> MarkVBlank; vblankmanager.cpp: wakeup = stamp + interval - draw time - red zone), so the stamps decide where in the display's cycle it commits. The Retroid Pocket 6 panel latches at 59.97-59.99 Hz, so the walk slid across SurfaceFlinger's latch every half minute or so; while the composite sat on the latch edge, ~1 ms of post jitter put two buffers in one latch window and SurfaceFlinger dropped the older one (HUD 58.7-59.3 fps, SurfaceFlinger 54-57, at a 60 cap). A KMS driver stamps the hardware vblank and none of this happens on a PC. The stamps now follow the display. A thread with an ALooper keeps one Choreographer callback posted per vsync while a session runs (AChoreographer_postVsyncCallback on API 33+, postFrameCallback64 on 29-32) and parks between sessions. The daemon keeps the last 32 vsyncs, counts each step in whole periods (a missed callback is two steps, not one long one) and fits the period by least squares over them, so the period is the measured one and never the app's integer Hz. A step that is not a whole number of periods, or a full ring with no single-period step, re-seeds the fit, so a live mode switch is followed; mode_refresh_mhz() still serves the advertised rate to the connector. Which instant is stamped: KMS has one instant per cycle, the vblank, which is both the point a flip must be queued by and the start of scan-out. Android splits it into SurfaceFlinger's latch and the present one or more periods later. gamescope treats the stamp as the point its commit has to beat by its red zone (1.65 ms) plus its rolling draw time, so the stamps sit on the latch grid: the preferred frame timeline's deadline on API 33+, moved by where SurfaceFlinger actually latched the daemon's publishes against it (latch-after-deadline, smoothed from every publish's ASurfaceTransactionStats_getLatchTime). A present-time stamp would aim gamescope's margin at an instant whose phase to the latch is the compositor's work duration. Each publish claims the first latch at or after it, the latch that takes it, even when that is after Choreographer's deadline. The stamp is that latch minus a lead. gamescope's red zone was sized for a kernel commit; here a commit still crosses the commit worker and a binder apply with ~1 ms of spread. Stamped on the deadline itself (RP6, SWD2, cap 60) the applies landed 1.04 / 2.13 / 4.47 ms (p10 / p50 / p90) ahead of the latch, the tail crossed it, and SurfaceFlinger showed 58.65 fps on a 59.967 Hz panel. The lead is measured: every completed publish's margin (latch - apply, folded into [-P/4, 3P/4)) feeds a quantile loop (+9 steps of 20 us below target, -1 above) that holds the 10th percentile at 3 ms, or 30 % of the period on faster panels; start 2 ms, cap P/3. GNDRM_GS_LATCH_LEAD_US (app process environment) fixes it instead. The cost is latency: about 2 ms more between gamescope's commit and the latch than a deadline stamp on that device. The stamps are earlier than the light by the lead plus SurfaceFlinger's pipeline; what reads them (gamescope's grid, mangoapp's frame time) uses their spacing, which a constant offset does not change. The VsyncNote every scan-out client receives now carries the measured period while Choreographer is locked and fresh (the mode's advertised rate otherwise); a period move pushes the note only above 1 us, since the fit wanders by a few microseconds and the 300 us phase push re-anchors long before that adds up. Fallbacks: on API 29-32 the measured period is used with the latch grid's phase (vsync.rs), or the vsync time's own phase until a latch was seen; with Choreographer silent for a second the latch grid alone is followed at the mode's period; with neither reporting yet, the previous walk of the mode's interval. A claim is never earlier than the previous one and never repeats an instant: a second frame inside a period claims the next instant, a late frame skips whole periods, and a source switch keeps at least half a period between claims; claims moved past their latch by that rule are counted. Ungated lines: vsync: flip events on the display's vsync: source=choreographer-deadline period=16675.123us (59.969 Hz, mode 60.000 Hz) latch-vs-grid=+250us once per lock (the first, and the first after a rate change), and every 10 s while publishes complete: vsync: check source=choreographer-deadline period=16675.672us (59.967 Hz) publishes=598 vsyncs=600 late=0 pushed=0 margin p10/p50/p90=+3.0ms/+4.1ms/+5.9ms lead=+2.8ms latch-after-deadline=+0.4ms late = publishes that missed their latch, pushed = claims moved by the ordering rule, publishes below vsyncs at a cap at or above the panel's rate = gamescope skipping a wakeup. - modules/gndrm-daemon/src/vblank.rs (new): VsyncRing (period fit), Grid, DisplayVsync (source selection, freshness, measured_period_ns), LeadLoop, fold_margin, VsyncReport, the process-global feed and VblankTimeline, moved out of lib.rs and taking the grid; unit tests for the fit, rate changes, pauses, source selection, staleness, the claim rules, the lead and the report. - modules/gndrm-daemon/src/lib.rs: the commit worker claims on the display grid, with the mode's interval as the walk. - modules/gndrm-android/src/choreographer.rs (new): the vsync thread and the Choreographer FFI (dlsym'd from libandroid), started by nativeStart and stopped by nativeStop; the lock and check lines. - modules/gndrm-android/src/vsync.rs: the latch tracker's anchor feeds the daemon, every completion feeds note_publish_latch, the note's period is the measured one (PERIOD_STEP_NS). - modules/gndrm-android/src/ffi.rs: sym() is crate-visible for the new table. - modules/gndrm-android/src/lib.rs: module wiring and the header's account of the grid. - modules/gndrm-daemon/tests/golden.rs: the first-claim comment names the no-source case (no assertion changes). - docs/15-fps-limiter-pacing-and-vsync.md: section 3 on the vsync source, the latch grid, the claim, the lead and its cost, fallbacks and ordering; section 4 on the vsync timeline, the note's period and the two lines. docs/11-refresh-model.md: the flip timeline is no longer "one mode period". --- docs/11-refresh-model.md | 5 +- docs/15-fps-limiter-pacing-and-vsync.md | 76 +- modules/gndrm-android/src/choreographer.rs | 309 ++++++ modules/gndrm-android/src/ffi.rs | 2 +- modules/gndrm-android/src/lib.rs | 15 +- modules/gndrm-android/src/vsync.rs | 29 +- modules/gndrm-daemon/src/lib.rs | 67 +- modules/gndrm-daemon/src/vblank.rs | 1068 ++++++++++++++++++++ modules/gndrm-daemon/tests/golden.rs | 3 +- 9 files changed, 1497 insertions(+), 77 deletions(-) create mode 100644 modules/gndrm-android/src/choreographer.rs create mode 100644 modules/gndrm-daemon/src/vblank.rs diff --git a/docs/11-refresh-model.md b/docs/11-refresh-model.md index 28d4ad2..5884c47 100644 --- a/docs/11-refresh-model.md +++ b/docs/11-refresh-model.md @@ -49,8 +49,9 @@ the app at N while keeping the grid at the mode's rate — they move together. 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. +flip events carry the display's measured vsync, not that integer rate (`docs/15` §3), and +gamescope re-anchors its grid on every event, so the grid follows the display's real +period while the mode carries its advertised one. 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; diff --git a/docs/15-fps-limiter-pacing-and-vsync.md b/docs/15-fps-limiter-pacing-and-vsync.md index cb028d1..3fffff3 100644 --- a/docs/15-fps-limiter-pacing-and-vsync.md +++ b/docs/15-fps-limiter-pacing-and-vsync.md @@ -52,21 +52,81 @@ that rate is the display's. 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. +- **The flip event** carries the display's vsync, as a KMS driver's carries the + hardware vblank. gamescope re-arms its composite timer from every event (wakeup = + stamp + interval − draw time − red zone, `vblankmanager.cpp`), so the stamp decides + where in the display's cycle gamescope finishes; a timeline of its own, however + regular, slides against the display. The source is `AChoreographer` + (`gndrm-android/src/choreographer.rs`): one thread with an `ALooper` keeps a callback + posted per vsync while a session runs, and the daemon fits the period to the measured + vsync spacing over the last 32 vsyncs (`gndrm-daemon/src/vblank.rs`, `VsyncRing`). The + mode's integer rate never enters it: a "60 Hz" panel that latches at 59.973 Hz is + stamped at 59.973 Hz, and a live mode change is followed when the spacing changes. +- **Which instant is stamped.** KMS has one instant per cycle: the vblank is both the + point a flip must be queued by and the point scan-out starts. Android splits it into + `SurfaceFlinger`'s latch and the present one or more periods later. gamescope uses + the stamp as the point its next commit must beat by its red zone, so the stamps sit + on the **latch** grid, not the present: the preferred frame timeline's deadline on + API 33+ (`AChoreographer_postVsyncCallback`), moved by where `SurfaceFlinger` + actually latched the daemon's publishes against it (`latch-after-deadline`, smoothed + from `ASurfaceTransactionStats_getLatchTime`). A present-time stamp would aim + gamescope's margin at an instant whose phase to the latch is the compositor's work + duration. A publish is claimed for the first latch at or after its apply returned — + the latch that takes it, even when that is after Choreographer's deadline; claiming + the next one would move gamescope's next wakeup a whole period out and leave a latch + with no new frame. +- **The lead.** The stamp is that latch minus a lead. gamescope's 1.65 ms red zone was + sized for a kernel commit; here a commit still crosses the daemon's commit worker and + a binder apply, with about 1 ms of spread. Stamped on the deadline itself (Retroid + Pocket 6, SWD2, cap 60, 2026-11-02) the applies landed 1.0 / 2.1 / 4.5 ms (p10 / p50 / + p90) ahead of the latch, the tail crossed it, and SurfaceFlinger showed 58.65 fps on a + 59.967 Hz panel. The lead moves gamescope's whole schedule earlier by the same amount. + It is measured, not fixed: each completed publish's margin (latch − apply) feeds a + quantile loop that holds the 10th percentile at 3 ms (30 % of a period on faster + panels), starting from 2 ms and capped at a third of a period. `GNDRM_GS_LATCH_LEAD_US` + in the app process's environment fixes it instead. The cost is latency: every frame + waits the lead longer between gamescope's commit and the latch (about 2 ms more than a + deadline stamp on that device). The stamps are earlier than the light by the lead plus + the compositor's pipeline; consumers read differences between stamps (frame time), + which a constant offset does not touch. +- **Fallbacks.** On API 29-32 Choreographer reports the vsync time only + (`AChoreographer_postFrameCallback64`): its measured period is used with the latch + grid's phase (`vsync.rs`, from `ASurfaceTransactionStats_getLatchTime`), or the vsync + time's own phase until a latch was seen. With Choreographer silent for a second the + latch grid alone is followed, at the mode's period; with neither reporting yet, the + events walk the mode's interval from the clock. +- **Ordering.** A claim is never earlier than the previous one and never repeats an + instant: a second frame inside one period claims the next instant, and a late frame + skips whole periods. Switching sources keeps the rule (the next claim is at least half + a period after the last). ## 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. + whenever its phase moves by more than 300 µs or its period by more than 1 µs, and once + on subscribe. Its period is the one measured from Choreographer while that is locked + (the mode's advertised rate otherwise), so the shim's vblank gate and frame + generation extrapolate the display's real rate. Its anchor is also the + flip timeline's phase where Choreographer gives no deadline, and its fallback. - 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. +- The display's vsync timeline: Choreographer's vsyncs on a thread of its own while a + session runs, the period fitted to their spacing, the phase from the frame deadline + (or the latch grid), and the flip event's timestamp on it, as above. One ungated line + marks each lock — `vsync: flip events on the display's vsync: source=… period=…us + (… Hz, mode … Hz) latch-vs-grid=…` — the first, and the first after a rate change. + Another, every 10 s while publishes complete, is the cross-check: `vsync: check + source=… period=…us (… Hz) publishes=… vsyncs=… late=… pushed=… margin + p10/p50/p90=…/…/… lead=… latch-after-deadline=…`. `late` counts publishes that missed + the latch they were aimed at; `pushed`, flip events moved past their latch by the + one-claim-per-instant rule; publishes below vsyncs at a cap at or above the panel's + rate is gamescope skipping a wakeup. +- The stamp lead, as above. + +The daemon derives no rate for the mode. The launcher hands it the display's rate and the +cap; the connector advertises that rate, and the daemon reports back only what it +measures — the latch grid to the presenter, the vsync timeline in the flip events. ## 5. Measured on device diff --git a/modules/gndrm-android/src/choreographer.rs b/modules/gndrm-android/src/choreographer.rs new file mode 100644 index 0000000..4e4cf82 --- /dev/null +++ b/modules/gndrm-android/src/choreographer.rs @@ -0,0 +1,309 @@ +//! The display's vsync, read from `AChoreographer` and handed to the daemon's +//! flip timeline (`gndrm_daemon::vblank`). +//! +//! A KMS driver stamps page-flip events with the hardware vblank; this process +//! has no vblank interrupt, but it has the vsync `SurfaceFlinger` hands every +//! app. One thread owns an `ALooper` and a Choreographer and keeps one callback +//! posted per vsync while a session runs: +//! +//! - API 33+: `AChoreographer_postVsyncCallback`. The callback data carries the +//! vsync time and the preferred frame timeline's deadline, the instant a +//! buffer must reach `SurfaceFlinger` by to make that timeline's present — +//! the instant the flip events stamp (see `gndrm_daemon::vblank`). +//! - API 29-32: `AChoreographer_postFrameCallback64`, the vsync time only; the +//! phase then comes from the latch grid (`vsync.rs`). +//! +//! The daemon fits the period to the measured spacing, so the integer rate the +//! app reports never reaches the timeline. Between sessions the thread stays +//! parked in its looper with no callback posted: no wakeups. + +use std::ffi::{c_int, c_void}; +use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; + +use gndrm_daemon::vblank::{self, VsyncLock, VsyncReport}; + +/// Opaque `AChoreographer`, `AChoreographerFrameCallbackData`. +type Choreographer = c_void; +type FrameData = c_void; + +type FnGetInstance = unsafe extern "C" fn() -> *mut Choreographer; +type VsyncCallback = unsafe extern "C" fn(data: *const FrameData, user: *mut c_void); +type FnPostVsyncCallback = + unsafe extern "C" fn(ch: *mut Choreographer, cb: VsyncCallback, user: *mut c_void) -> c_int; +type FrameCallback64 = unsafe extern "C" fn(frame_time_nanos: i64, user: *mut c_void); +type FnPostFrameCallback64 = + unsafe extern "C" fn(ch: *mut Choreographer, cb: FrameCallback64, user: *mut c_void) -> c_int; +type FnGetFrameTime = unsafe extern "C" fn(data: *const FrameData) -> i64; +type FnGetPreferredIndex = unsafe extern "C" fn(data: *const FrameData) -> usize; +type FnGetDeadline = unsafe extern "C" fn(data: *const FrameData, index: usize) -> i64; +type FnLooperWake = unsafe extern "C" fn(looper: *mut c_void); + +/// API 33's per-vsync data accessors. +#[derive(Clone, Copy)] +struct VsyncApi { + post: FnPostVsyncCallback, + frame_time: FnGetFrameTime, + preferred: FnGetPreferredIndex, + deadline: FnGetDeadline, +} + +#[derive(Clone, Copy)] +struct Api { + get_instance: FnGetInstance, + vsync: Option, + frame64: Option, + prepare: crate::ffi::FnALooperPrepare, + poll_once: crate::ffi::FnALooperPollOnce, + wake: FnLooperWake, +} + +impl Api { + fn load() -> Option { + // SAFETY: dlopen of the platform library this process already has + // loaded (RTLD_NOLOAD), with a plain dlopen as the fallback; each + // symbol is cast to the signature `` and + // `` declare for it. + unsafe { + let mut handle = libc::dlopen( + c"libandroid.so".as_ptr(), + libc::RTLD_NOW | libc::RTLD_NOLOAD, + ); + if handle.is_null() { + handle = libc::dlopen(c"libandroid.so".as_ptr(), libc::RTLD_NOW); + } + if handle.is_null() { + return None; + } + use crate::ffi::sym; + let vsync = match ( + sym::(handle, c"AChoreographer_postVsyncCallback"), + sym::(handle, c"AChoreographerFrameCallbackData_getFrameTimeNanos"), + sym::( + handle, + c"AChoreographerFrameCallbackData_getPreferredFrameTimelineIndex", + ), + sym::( + handle, + c"AChoreographerFrameCallbackData_getFrameTimelineDeadlineNanos", + ), + ) { + (Some(post), Some(frame_time), Some(preferred), Some(deadline)) => Some(VsyncApi { + post, + frame_time, + preferred, + deadline, + }), + _ => None, + }; + let frame64 = + sym::(handle, c"AChoreographer_postFrameCallback64"); + if vsync.is_none() && frame64.is_none() { + return None; + } + Some(Self { + get_instance: sym(handle, c"AChoreographer_getInstance")?, + vsync, + frame64, + prepare: sym(handle, c"ALooper_prepare")?, + poll_once: sym(handle, c"ALooper_pollOnce")?, + wake: sym(handle, c"ALooper_wake")?, + }) + } + } +} + +/// Whether a session wants vsyncs: the callback is re-posted only while set. +static WANTED: AtomicBool = AtomicBool::new(false); +/// Whether the thread was spawned (once per process). +static SPAWNED: AtomicBool = AtomicBool::new(false); +/// Whether a callback is posted and has not run yet. Touched on the thread only. +static OUTSTANDING: AtomicBool = AtomicBool::new(false); +/// The thread's looper, to wake it when a session starts; null until prepared. +static LOOPER: AtomicPtr = AtomicPtr::new(std::ptr::null_mut()); +/// The resolved table, set once by the thread before its first post. +static API: std::sync::OnceLock = std::sync::OnceLock::new(); + +/// Follow the display's vsync for a session. Spawns the thread on first use. +pub(crate) fn start() { + WANTED.store(true, Ordering::Release); + if SPAWNED.swap(true, Ordering::AcqRel) { + let looper = LOOPER.load(Ordering::Acquire); + if let (false, Some(api)) = (looper.is_null(), API.get()) { + // SAFETY: the looper the thread prepared; it lives as long as the + // thread, which never exits once it holds one. + unsafe { (api.wake)(looper) }; + } + return; + } + let spawned = std::thread::Builder::new() + .name("gndrm-vsync".into()) + .spawn(|| { + let _ = std::panic::catch_unwind(run); + }); + if spawned.is_err() { + SPAWNED.store(false, Ordering::Release); + } +} + +/// Stop following: the next callback is the last. +pub(crate) fn stop() { + WANTED.store(false, Ordering::Release); +} + +fn run() { + let Some(api) = Api::load() else { + crate::log_always!( + "{}", + gndrm_obf::obf!( + "vsync: no Choreographer vsync callback here; flip events follow the latch grid" + ) + ); + return; + }; + let api = *API.get_or_init(|| api); + // SAFETY: prepares (or returns) this thread's looper; Choreographer needs one. + let looper = unsafe { (api.prepare)(0) }; + // SAFETY: called on a thread that has a looper, as the NDK requires. + let ch = unsafe { (api.get_instance)() }; + if looper.is_null() || ch.is_null() { + crate::log_always!( + "{}", + gndrm_obf::obf!("vsync: Choreographer unavailable; flip events follow the latch grid") + ); + return; + } + LOOPER.store(looper, Ordering::Release); + loop { + if WANTED.load(Ordering::Acquire) && !OUTSTANDING.load(Ordering::Relaxed) { + // SAFETY: `ch` is this thread's Choreographer; the callbacks are + // `extern "C"` functions that live for the process and take no + // user pointer. + let rc = unsafe { + match (api.vsync, api.frame64) { + (Some(v), _) => (v.post)(ch, on_vsync, std::ptr::null_mut()), + (None, Some(post)) => post(ch, on_frame, std::ptr::null_mut()), + (None, None) => -1, + } + }; + if rc != 0 { + crate::log_always!( + "{}", + gndrm_obf::obf!("vsync: posting a Choreographer callback failed ({rc})") + ); + return; + } + OUTSTANDING.store(true, Ordering::Relaxed); + } + // SAFETY: this thread's looper; null out-pointers are allowed. Returns + // after a callback ran or a wake. + unsafe { + (api.poll_once)( + -1, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + ) + }; + } +} + +/// API 33+: one vsync with its frame timelines. +unsafe extern "C" fn on_vsync(data: *const FrameData, _user: *mut c_void) { + OUTSTANDING.store(false, Ordering::Relaxed); + let _ = std::panic::catch_unwind(|| { + let Some(v) = API.get().and_then(|a| a.vsync) else { + return; + }; + // SAFETY: `data` is valid for the duration of this callback. + let (vsync, deadline) = unsafe { + let idx = (v.preferred)(data); + ((v.frame_time)(data), (v.deadline)(data, idx)) + }; + if vsync > 0 { + let deadline = u64::try_from(deadline).ok().filter(|&d| d > 0); + feed(vsync as u64, deadline); + } + }); +} + +/// API 29-32: the vsync time only. +unsafe extern "C" fn on_frame(frame_time_nanos: i64, _user: *mut c_void) { + OUTSTANDING.store(false, Ordering::Relaxed); + let _ = std::panic::catch_unwind(|| { + if frame_time_nanos > 0 { + feed(frame_time_nanos as u64, None); + } + }); +} + +fn feed(vsync_ns: u64, deadline_ns: Option) { + if let Some(lock) = vblank::note_choreographer_vsync(vsync_ns, deadline_ns) { + log_lock(&lock); + } +} + +/// One line per lock: the first, and the first after each rate change. Not a +/// per-frame path, so it is ungated — this process never sees the guest's +/// `GNDRM_LOG_*`, and the line is what says the real vsync is in use. +fn log_lock(lock: &VsyncLock) { + let p = lock.grid.period_ns; + let mode_mhz = crate::panel_refresh_mhz(); + let rate_mhz = 1_000_000_000_000 / p.max(1); + let offset = match lock.latch_offset_ns { + Some(o) => gndrm_obf::obf!("{:+}us", o / 1000).to_string(), + None => gndrm_obf::obf!("none yet").to_string(), + }; + crate::log_always!( + "{}", + gndrm_obf::obf!( + "vsync: flip events on the display's vsync: source={} period={}.{:03}us ({}.{:03} Hz, mode {}.{:03} Hz) latch-vs-grid={}", + lock.grid.source.name(), + p / 1000, + p % 1000, + rate_mhz / 1000, + rate_mhz % 1000, + mode_mhz / 1000, + mode_mhz % 1000, + offset + ) + ); +} + +/// Signed nanoseconds as `+1.234ms`. +fn ms(ns: i64) -> String { + let sign = if ns < 0 { '-' } else { '+' }; + let a = ns.unsigned_abs(); + gndrm_obf::obf!("{sign}{}.{:03}ms", a / 1_000_000, (a / 1000) % 1000).to_string() +} + +/// The cross-check, once per 10 s while publishes complete (see +/// `vblank::DisplayVsync::note_publish`). Ungated for the same reason as the +/// lock line: it is what a device run reads to know the timeline holds. +pub(crate) fn log_report(r: &VsyncReport) { + let p = r.period_ns; + let rate_mhz = 1_000_000_000_000 / p.max(1); + let lad = match r.latch_after_deadline_ns { + Some(d) => ms(d), + None => gndrm_obf::obf!("n/a").to_string(), + }; + crate::log_always!( + "{}", + gndrm_obf::obf!( + "vsync: check source={} period={}.{:03}us ({}.{:03} Hz) publishes={} vsyncs={} late={} pushed={} margin p10/p50/p90={}/{}/{} lead={} latch-after-deadline={}", + r.source.map_or("none", |s| s.name()), + p / 1000, + p % 1000, + rate_mhz / 1000, + rate_mhz % 1000, + r.publishes, + r.vsyncs, + r.late, + r.pushed, + ms(r.margin_ns[0]), + ms(r.margin_ns[1]), + ms(r.margin_ns[2]), + ms(r.lead_ns as i64), + lad + ) + ); +} diff --git a/modules/gndrm-android/src/ffi.rs b/modules/gndrm-android/src/ffi.rs index c27fd0d..e985b90 100644 --- a/modules/gndrm-android/src/ffi.rs +++ b/modules/gndrm-android/src/ffi.rs @@ -490,7 +490,7 @@ impl ScApi { /// # Safety /// `handle` must be a live `dlopen` handle and `T` must be the exact C /// signature the platform exports under `name`. -unsafe fn sym(handle: *mut c_void, name: &core::ffi::CStr) -> Option { +pub(crate) unsafe fn sym(handle: *mut c_void, name: &core::ffi::CStr) -> Option { // SAFETY: caller guarantees the handle; `name` is NUL-terminated. let p = unsafe { libc::dlsym(handle, name.as_ptr()) }; if p.is_null() { diff --git a/modules/gndrm-android/src/lib.rs b/modules/gndrm-android/src/lib.rs index 3e27b06..de79a49 100644 --- a/modules/gndrm-android/src/lib.rs +++ b/modules/gndrm-android/src/lib.rs @@ -19,10 +19,12 @@ //! 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: 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 session's vblank grid is not a caller concern. The mode's rate is the +//! display's: the app supplies it through `nativeSetPanelRefreshHz` before +//! `nativeStart`, and it is what the connector advertises. The flip events' +//! timestamps follow the display's own vsync, read from `AChoreographer` while a +//! session runs (`choreographer.rs`), with the latch grid (`vsync.rs`) as the +//! fallback and a walk of the mode's interval until either has reported. //! //! The mode's *size* is the other session-level input. `nativeStart` takes it //! from the window unless the app named one through `nativeSetScanoutGeometry` @@ -39,6 +41,7 @@ #![cfg(target_os = "android")] mod asc; +mod choreographer; mod dump; pub mod ffi; mod foreign; @@ -1117,6 +1120,9 @@ fn start( asc, thread: Some(handle), }); + // The display's vsync, for the flip events' timestamps (see + // `choreographer.rs`): followed while a session runs. + choreographer::start(); 0 } @@ -1127,6 +1133,7 @@ fn stop() { .take(); let Some(mut rt) = rt else { return }; log_info!("{}", gndrm_obf::obf!("stopping ({})", rt.name)); + choreographer::stop(); // Unblock accept()/recvmsg() regardless of which phase the thread is in. // SAFETY: fds are valid or -1; shutdown only unblocks, the thread // remains the owner that closes them. diff --git a/modules/gndrm-android/src/vsync.rs b/modules/gndrm-android/src/vsync.rs index a24e907..0549da9 100644 --- a/modules/gndrm-android/src/vsync.rs +++ b/modules/gndrm-android/src/vsync.rs @@ -88,6 +88,10 @@ pub(crate) fn now_ns() -> u64 { /// A phase or lead move below this is not worth a push: the shim extrapolates /// the grid by its period, and the margin the lead aims for is a third of one. const PUSH_STEP_NS: u64 = 300_000; +/// A period move below this is not worth a push: the measured period wanders by +/// a few microseconds between fits, and the phase push above re-anchors the +/// shim's extrapolation long before such an error adds up. +const PERIOD_STEP_NS: u64 = 1_000; /// The shortest lead worth asking for. const MIN_LEAD_NS: u64 = 500_000; /// Samples off the grid before the grid is taken to have moved. @@ -177,7 +181,7 @@ impl Tracker { Some(p) => { let phase = wrap(note.anchor_ns as i64 - p.anchor_ns as i64, period as i64).unsigned_abs(); - p.period_ns != note.period_ns + u64::from(p.period_ns.abs_diff(note.period_ns)) > PERIOD_STEP_NS || phase > PUSH_STEP_NS || u64::from(p.lead_ns.abs_diff(note.lead_ns)) > PUSH_STEP_NS } @@ -205,11 +209,16 @@ impl Tracker { } } -/// The panel period in nanoseconds from the rate the app reported, 0 when it -/// has not. The samples cannot give the period on their own: while the daemon -/// is the only thing publishing, their spacing is the daemon's own rate. +/// The panel period in nanoseconds: the display's measured vsync spacing while +/// Choreographer reports it (`gndrm_daemon::vblank`), else the rate the app +/// reported, 0 when neither. The samples cannot give the period on their own: +/// while the daemon is the only thing publishing, their spacing is the daemon's +/// own rate. #[cfg(target_os = "android")] -fn period_ns() -> u64 { +fn period_ns(now_ns: u64) -> u64 { + if let Some(p) = gndrm_daemon::vblank::measured_period_ns(now_ns) { + return p; + } let mhz = crate::panel_refresh_mhz(); if mhz == 0 { 0 @@ -223,7 +232,7 @@ fn period_ns() -> u64 { /// callback, on `SurfaceFlinger`'s thread. #[cfg(target_os = "android")] pub(crate) fn sample(latch_ns: u64, applied_ns: u64) { - let period = period_ns(); + let period = period_ns(latch_ns); if period == 0 || latch_ns == 0 { return; } @@ -232,6 +241,8 @@ pub(crate) fn sample(latch_ns: u64, applied_ns: u64) { let mut t = TRACKER.lock().unwrap_or_else(|e| e.into_inner()); let first = t.pushed.is_none(); let note = t.track(latch_ns, applied_ns, period, pacing); + // The flip events' fallback timeline, and Choreographer's cross-check. + gndrm_daemon::vblank::note_latch_grid(t.anchor_ns, period); if first && note.is_some() { log_info!( "{}", @@ -243,6 +254,10 @@ pub(crate) fn sample(latch_ns: u64, applied_ns: u64) { } note }; + // The flip events' lead and the periodic cross-check (one line per 10 s). + if let Some(r) = gndrm_daemon::vblank::note_publish_latch(applied_ns, latch_ns) { + crate::choreographer::log_report(&r); + } if note.is_some() { let fd = changed_fd(); if fd >= 0 { @@ -257,7 +272,7 @@ pub(crate) fn sample(latch_ns: u64, applied_ns: u64) { /// The note to hand a shim now, `None` before any latch was seen. #[cfg(target_os = "android")] pub(crate) fn note() -> Option { - let period = period_ns(); + let period = period_ns(now_ns()); TRACKER .lock() .unwrap_or_else(|e| e.into_inner()) diff --git a/modules/gndrm-daemon/src/lib.rs b/modules/gndrm-daemon/src/lib.rs index 8851624..88f1da4 100644 --- a/modules/gndrm-daemon/src/lib.rs +++ b/modules/gndrm-daemon/src/lib.rs @@ -25,6 +25,7 @@ mod props; pub mod pushes; pub mod scanout; pub mod server; +pub mod vblank; use std::collections::{HashSet, VecDeque}; use std::os::fd::RawFd; @@ -2198,55 +2199,6 @@ impl Drop for CommitJob { } } -/// The vblank timeline a page-flip event claims. -/// -/// A real KMS page-flip event is delivered at the vblank the flip takes effect -/// on, and gamescope re-anchors its entire vblank grid off that timestamp -/// (`Backends/DRMBackend.cpp` `page_flip_handler` → `vblankmanager` `MarkVBlank`). -/// As the display, gndrm owns that schedule: events are quantized to the mode's -/// own rate — one interval apart while frames keep up, stepping whole intervals -/// when one is late — which is what a real display's vblank timeline looks like. -/// -/// Stamping the *emit* instant instead re-anchors that grid slightly early on -/// every frame. The grid is armed early by design (its aim-early offset, so a -/// real kernel can hold the flip to the panel cycle), so the error integrates -/// into the grid's rate: every divisor-based fps cap then runs fast by roughly -/// that offset over one interval — measured as +7 % on this port across four -/// divisors (targets 20/24/30/40 → 21/26/32/43). -struct VblankTimeline { - interval_ns: u64, - claimed_ns: u64, - started: bool, -} - -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(mode_refresh_mhz()), - claimed_ns: 0, - started: false, - } - } - - /// The vblank this frame lands on: the next boundary of the mode's rate at or - /// after `now_ns`. Never moves backwards, so a late frame skips whole vblanks - /// instead of claiming a time in the past. - fn claim(&mut self, now_ns: u64) -> u64 { - if !self.started { - self.started = true; - self.claimed_ns = now_ns; - return now_ns; - } - let mut t = self.claimed_ns + self.interval_ns; - while t < now_ns { - t += self.interval_ns; - } - self.claimed_ns = t; - t - } -} - /// The commit worker: a thin panic barrier around [`commit_worker_loop`]. /// /// A panic anywhere on this worker — most plausibly inside the presenter — is @@ -2310,8 +2262,8 @@ fn commit_worker_loop( // Atomics only, so this cannot perturb the flip path. let d = diag(); d.set_role(Role::Commit, State::CommitWaitingJob); - // The vblank schedule the flip events claim (see `VblankTimeline`). - let mut vblank = VblankTimeline::new(); + // The vblank schedule the flip events claim (see `vblank::VblankTimeline`). + let mut timeline = vblank::VblankTimeline::default(); while let Ok(mut job) = commit_rx.recv() { if shutdown.load(Ordering::Relaxed) { break; @@ -2374,10 +2326,17 @@ fn commit_worker_loop( } if let Some(mut ev) = job.event.take() { // Stamp the vblank this frame lands on, not the instant the event is - // built or emitted (see `VblankTimeline`). Delivery stays immediate, - // so gamescope's present loop never waits on a display round trip. + // built or emitted: the display's latch deadline when its timeline + // is known, else a walk of the mode's interval (see + // `vblank::VblankTimeline`). Delivery stays immediate, so + // gamescope's present loop never waits on a display round trip. let (sec, usec) = (clock.lock().unwrap_or_else(PoisonError::into_inner))(); - let claimed = vblank.claim(u64::from(sec) * 1_000_000_000 + u64::from(usec) * 1_000); + let now = u64::from(sec) * 1_000_000_000 + u64::from(usec) * 1_000; + let claimed = timeline.claim( + now, + vblank::display_grid(now), + 1_000_000_000_000 / u64::from(mode_refresh_mhz()), + ); ev.tv_sec = (claimed / 1_000_000_000) as u32; ev.tv_usec = ((claimed % 1_000_000_000) / 1_000) as u32; flip_queue diff --git a/modules/gndrm-daemon/src/vblank.rs b/modules/gndrm-daemon/src/vblank.rs new file mode 100644 index 0000000..ba9b512 --- /dev/null +++ b/modules/gndrm-daemon/src/vblank.rs @@ -0,0 +1,1068 @@ +//! The display's vblank grid, as the page-flip events report it. +//! +//! A KMS driver stamps a page-flip event with the hardware vblank the flip took +//! effect on, and gamescope schedules its next composite from that stamp +//! (`DRMBackend.cpp` `page_flip_handler` → `MarkVBlank`, re-armed on every +//! event: wakeup = stamp + interval − draw time − red zone). The stamps have to +//! be the display's own timeline, at the display's own rate, or gamescope's +//! composite phase walks across the display's latch point. +//! +//! On Android the display's timeline reaches the app process through +//! `AChoreographer` (`gndrm-android`'s `choreographer.rs` feeds +//! [`note_choreographer_vsync`]); the latch times `SurfaceFlinger` reports for +//! the daemon's own publishes are the fallback ([`note_latch_grid`]). Neither +//! source is a KMS vblank, so this module decides which instant stands in for +//! one: the latch deadline, the point a publish must reach `SurfaceFlinger` by +//! to be the frame its next latch takes. That is what a KMS vblank is to the +//! kernel's commit path — the point a flip must be queued by to be scanned out +//! on the next cycle — and gamescope's own margin (its red zone) then lands +//! every commit ahead of the latch rather than ahead of a present time whose +//! phase against the latch is arbitrary. +//! +//! The stamp sits a measured lead ahead of that latch ([`LeadLoop`]): gamescope's +//! red zone was sized for a kernel commit, and the daemon's apply path needs more. +//! +//! The period comes from measured vsync spacing ([`VsyncRing`]), never from the +//! integer rate the mode advertises: a panel sold as 60 Hz latches at 59.97 or +//! 59.99, and a timeline walking 16 666 666 ns slides across the latch point +//! once every half a minute or so. + +use std::sync::{Mutex, PoisonError}; + +/// Vsyncs the period is fitted over. +const RING: usize = 32; +/// Vsyncs the ring needs before its period is trusted. +const MIN_LOCK: usize = 8; +/// A source that has said nothing for this long is not followed any more. +const STALE_NS: u64 = 1_000_000_000; +/// The slowest display period taken as a period rather than a gap (10 Hz). +const MAX_PERIOD_NS: u64 = 100_000_000; +/// The fastest (1 kHz): anything closer together is a duplicate callback. +const MIN_PERIOD_NS: u64 = 1_000_000; + +/// Recent display vsyncs and the period fitted through them. +/// +/// Each sample is placed on a vsync count from the ring's first one: the step to +/// the previous sample divided by the period, rounded, so a callback that came a +/// vsync late counts as two steps rather than one long one. The period is the +/// least-squares slope of time against count over the ring, which a single +/// late timestamp barely moves. +#[derive(Debug, Default)] +pub struct VsyncRing { + /// `(timestamp_ns, vsync count from the first sample)`, oldest first. + samples: std::collections::VecDeque<(u64, u64)>, + /// The fitted period; before the first fit, the seed the steps are counted + /// with. 0 before any step was seen. + period_ns: u64, + /// Whether `period_ns` is a fit over [`MIN_LOCK`] vsyncs or more of the + /// current rate. Survives a gap in the callbacks, not a rate change. + locked: bool, +} + +/// What one [`VsyncRing::push`] changed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RingEvent { + /// Nothing worth reporting. + None, + /// The ring locked onto a period it was not locked onto before: the first + /// lock, or the first after a rate change. + Locked(u64), +} + +impl VsyncRing { + /// The fitted period, `None` until the ring is locked. + #[must_use] + pub fn period_ns(&self) -> Option { + self.locked.then_some(self.period_ns) + } + + /// Fold in one vsync timestamp (`CLOCK_MONOTONIC` ns). + pub fn push(&mut self, t_ns: u64) -> RingEvent { + let Some(&(last_t, last_n)) = self.samples.back() else { + self.samples.push_back((t_ns, 0)); + return RingEvent::None; + }; + if t_ns <= last_t || t_ns - last_t < MIN_PERIOD_NS { + return RingEvent::None; + } + let step = t_ns - last_t; + if self.period_ns == 0 { + if step > MAX_PERIOD_NS { + // Not a period: the callbacks paused. Count from here. + self.restart(t_ns); + return RingEvent::None; + } + self.period_ns = step; + } + let period = self.period_ns; + if step > 8 * period { + // A gap (the callbacks paused, or the thread was starved): counting + // across it would ask a period for more precision than it has. The + // fitted period and the lock stay; the count starts again. + self.restart(t_ns); + return RingEvent::None; + } + let k = (step + period / 2) / period; + let residual = step.abs_diff(k * period); + if k == 0 || residual > period / 4 { + // The step is not a whole number of periods: the display changed + // rate. Seed from this step and lock again. + self.reseed(t_ns, step); + return RingEvent::None; + } + self.samples.push_back((t_ns, last_n + k)); + if self.samples.len() > RING { + self.samples.pop_front(); + } + // A full ring without one single-vsync step is counting a rate at a + // multiple of the period it holds (the display halved its rate): take + // the shortest step instead. + if self.samples.len() == RING + && self + .samples + .iter() + .zip(self.samples.iter().skip(1)) + .all(|(a, b)| b.1 - a.1 >= 2) + { + let shortest = self + .samples + .iter() + .zip(self.samples.iter().skip(1)) + .map(|(a, b)| b.0 - a.0) + .min() + .unwrap_or(step); + self.reseed(t_ns, shortest); + return RingEvent::None; + } + if self.samples.len() < MIN_LOCK { + return RingEvent::None; + } + let fitted = self.fit(); + let was = self.locked.then_some(self.period_ns); + self.period_ns = fitted; + self.locked = true; + match was { + None => RingEvent::Locked(fitted), + Some(_) => RingEvent::None, + } + } + + /// Restart the count at `t_ns`, keeping the period and the lock. + fn restart(&mut self, t_ns: u64) { + self.samples.clear(); + self.samples.push_back((t_ns, 0)); + } + + /// Restart the count at `t_ns` on a new seed period, unlocked. + fn reseed(&mut self, t_ns: u64, seed_ns: u64) { + self.restart(t_ns); + self.period_ns = if (MIN_PERIOD_NS..=MAX_PERIOD_NS).contains(&seed_ns) { + seed_ns + } else { + 0 + }; + self.locked = false; + } + + /// Least-squares slope of timestamp against vsync count over the ring. + fn fit(&self) -> u64 { + let (t0, n0) = self.samples[0]; + let len = self.samples.len() as f64; + let (mut sx, mut sy, mut sxx, mut sxy) = (0.0f64, 0.0f64, 0.0f64, 0.0f64); + for &(t, n) in &self.samples { + let x = (n - n0) as f64; + let y = (t - t0) as f64; + sx += x; + sy += y; + sxx += x * x; + sxy += x * y; + } + let den = len * sxx - sx * sx; + if den <= 0.0 { + return self.period_ns; + } + let slope = (len * sxy - sx * sy) / den; + if slope.is_finite() && slope >= MIN_PERIOD_NS as f64 && slope <= MAX_PERIOD_NS as f64 { + slope.round() as u64 + } else { + self.period_ns + } + } +} + +/// Where a [`Grid`]'s phase and period came from. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum GridSource { + /// Choreographer's frame deadline (API 33+), measured period. + ChoreographerDeadline, + /// The latch grid's phase with Choreographer's measured period (API 29-32, + /// where Choreographer reports the vsync time only). + ChoreographerLatch, + /// Choreographer's vsync time and measured period, with no latch seen yet. + ChoreographerVsync, + /// The latch grid alone: its phase, and the mode's advertised period. + Latch, +} + +impl GridSource { + /// A short name for the log. + #[must_use] + pub fn name(self) -> &'static str { + match self { + GridSource::ChoreographerDeadline => "choreographer-deadline", + GridSource::ChoreographerLatch => "choreographer+latch", + GridSource::ChoreographerVsync => "choreographer-vsync", + GridSource::Latch => "latch", + } + } +} + +/// A display timeline: the latch instants `anchor_ns + k * period_ns`, and the +/// lead a flip event is stamped ahead of the latch it lands on. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Grid { + pub anchor_ns: u64, + pub period_ns: u64, + pub source: GridSource, + /// How far ahead of its latch a flip event is stamped (see [`LeadLoop`]). + pub lead_ns: u64, +} + +impl Grid { + /// The first grid instant at or after `t_ns`. + #[must_use] + pub fn at_or_after(&self, t_ns: u64) -> u64 { + let p = i128::from(self.period_ns.max(1)); + let off = i128::from(t_ns) - i128::from(self.anchor_ns); + // ceil(off / p) for either sign. + let k = off.div_euclid(p) + i128::from(off.rem_euclid(p) != 0); + (i128::from(self.anchor_ns) + k * p) as u64 + } +} + +/// The signed distance from `x` to the nearest instant of a `period` grid +/// through 0, in `[-period/2, period/2)`. +#[must_use] +pub fn wrap(x: i64, period: i64) -> i64 { + let half = period / 2; + (x + half).rem_euclid(period) - half +} + +/// A signed margin folded into `[-period/4, 3*period/4)`: a publish that +/// landed more than three quarters of a period ahead of a latch is read as late +/// for the latch before, and one late by more than a quarter as very early. +/// The lead keeps margins near a few milliseconds, so the first never happens +/// while it holds; the second (a stall) only nudges the lead down one step. +#[must_use] +pub fn fold_margin(x: i64, period: i64) -> i64 { + let q = period / 4; + (x + q).rem_euclid(period) - q +} + +/// The default lead before the loop has measured anything. On the Retroid +/// Pocket 6 (2026-11-02) a stamp on the deadline itself left the publish +/// 1.0 / 2.1 / 4.5 ms (p10 / p50 / p90) ahead of the latch. +const START_LEAD_NS: u64 = 2_000_000; +/// The margin the loop holds the 10th percentile at: three times the ~1 ms +/// spread of the apply path, capped at 30 % of a period for fast panels. +const TARGET_P10_NS: u64 = 3_000_000; +/// One step of the quantile loop. +const LEAD_STEP_NS: u64 = 20_000; +/// The override, in microseconds: a fixed lead, no loop. +pub const LEAD_ENV: &str = "GNDRM_GS_LATCH_LEAD_US"; + +/// How far ahead of `SurfaceFlinger`'s latch a flip event is stamped. +/// +/// gamescope commits about its red zone (1.65 ms) ahead of the stamp it was +/// handed one period earlier. That was sized for a kernel commit; here the +/// commit still crosses the daemon's commit worker and a binder apply before it +/// reaches `SurfaceFlinger`, with ~1 ms of jitter, so the tail of the applies +/// crossed the latch. The lead moves every stamp, and with it gamescope's +/// whole schedule, earlier by the same amount. +/// +/// The loop measures each publish's margin (latch time minus apply time) and +/// moves the lead so that one publish in ten lands less than +/// [`TARGET_P10_NS`] ahead of its latch: a stochastic quantile tracker (+9 +/// steps on a margin below target, −1 step above). The price is latency: a +/// frame waits the lead longer between gamescope's commit and the latch. +#[derive(Debug, Clone, Copy)] +pub struct LeadLoop { + lead_ns: u64, + fixed: bool, +} + +impl Default for LeadLoop { + fn default() -> Self { + Self { + lead_ns: START_LEAD_NS, + fixed: false, + } + } +} + +impl LeadLoop { + /// A loop held at `lead_ns` (the [`LEAD_ENV`] override). + #[must_use] + pub fn fixed(lead_ns: u64) -> Self { + Self { + lead_ns, + fixed: true, + } + } + + /// The lead in force, clamped to a third of `period`. + #[must_use] + pub fn lead_ns(&self, period: u64) -> u64 { + if self.fixed { + self.lead_ns + } else { + self.lead_ns.min(period / 3) + } + } + + /// The 10th-percentile margin the loop aims for at `period`. + #[must_use] + pub fn target_ns(period: u64) -> u64 { + TARGET_P10_NS.min(period * 3 / 10) + } + + /// Fold in one publish's margin (folded, see [`fold_margin`]). + pub fn update(&mut self, margin_ns: i64, period: u64) { + if self.fixed || period == 0 { + return; + } + if margin_ns < Self::target_ns(period) as i64 { + self.lead_ns += 9 * LEAD_STEP_NS; + } else { + self.lead_ns = self.lead_ns.saturating_sub(LEAD_STEP_NS); + } + self.lead_ns = self.lead_ns.min(period / 3); + } +} + +/// How often the cross-check is reported. +const REPORT_NS: u64 = 10_000_000_000; + +/// One window's cross-check (see [`DisplayVsync::note_publish`]). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct VsyncReport { + pub source: Option, + pub period_ns: u64, + /// Latch minus apply, folded: p10, p50, p90. + pub margin_ns: [i64; 3], + /// Publishes that missed the latch they were aimed at (negative margin). + pub late: u32, + pub publishes: u32, + pub vsyncs: u32, + /// Flip events pushed past the first latch after their publish by the + /// one-claim-per-instant rule (a second publish inside one period). + pub pushed: u32, + pub lead_ns: u64, + /// `SurfaceFlinger`'s latch against Choreographer's deadline, when both are + /// known. + pub latch_after_deadline_ns: Option, +} + +#[derive(Debug, Default)] +struct Window { + start_ns: u64, + margins: Vec, + late: u32, + vsyncs: u32, +} + +/// Claims pushed by the ordering rule, counted by the commit worker. +static PUSHED: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + +/// Everything the display has told the daemon about its timeline. +#[derive(Debug, Default)] +pub struct DisplayVsync { + ring: VsyncRing, + /// The latest Choreographer vsync and, when it reported one, the preferred + /// frame timeline's deadline. + chor: Option<(u64, Option)>, + /// The latch grid: anchor and the period it was tracked at. + latch: Option<(u64, u64)>, + /// `SurfaceFlinger`'s latch against the deadline grid, smoothed over the + /// publishes' latch times: the deadline is the latch Choreographer + /// predicts, this is where it is taken. + latch_after_deadline: Option, + lead: LeadLoop, + window: Window, +} + +/// What [`DisplayVsync::note_vsync`] reports back for the log. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct VsyncLock { + /// The grid the flip events follow from now on. + pub grid: Grid, + /// The latch grid's phase against it, when a latch has been seen: how far + /// after the stamped instant `SurfaceFlinger` latched. + pub latch_offset_ns: Option, +} + +impl DisplayVsync { + /// One Choreographer vsync: its timestamp and, on API 33+, the preferred + /// frame timeline's deadline. Returns the lock to report when the ring + /// locked onto a period. + pub fn note_vsync(&mut self, vsync_ns: u64, deadline_ns: Option) -> Option { + let ev = self.ring.push(vsync_ns); + self.chor = Some((vsync_ns, deadline_ns)); + self.window.vsyncs += 1; + match ev { + RingEvent::Locked(_) => { + let grid = self.grid(vsync_ns)?; + let latch_offset_ns = self.latch.map(|(anchor, _)| { + wrap(anchor as i64 - grid.anchor_ns as i64, grid.period_ns as i64) + }); + Some(VsyncLock { + grid, + latch_offset_ns, + }) + } + RingEvent::None => None, + } + } + + /// The latch grid's current anchor and the period it is tracked at. + pub fn note_latch(&mut self, anchor_ns: u64, period_ns: u64) { + if anchor_ns != 0 && period_ns != 0 { + self.latch = Some((anchor_ns, period_ns)); + } + } + + /// The measured period while Choreographer is locked and fresh at `now_ns`. + #[must_use] + pub fn measured_period_ns(&self, now_ns: u64) -> Option { + let (vsync, _) = self.chor?; + (now_ns.saturating_sub(vsync) < STALE_NS) + .then(|| self.ring.period_ns()) + .flatten() + } + + /// Hold the lead at `lead_ns` instead of measuring it. + pub fn fix_lead(&mut self, lead_ns: u64) { + self.lead = LeadLoop::fixed(lead_ns); + } + + /// One completed publish: the instant its apply returned and the latch + /// `SurfaceFlinger` took it at. Moves the lead, tracks the latch against + /// the deadline, and every [`REPORT_NS`] returns the window's cross-check. + pub fn note_publish(&mut self, applied_ns: u64, latch_ns: u64) -> Option { + let grid = self.grid(latch_ns); + if let Some(g) = grid { + let period = g.period_ns as i64; + if let (Some((_, Some(deadline))), Some(_)) = (self.chor, self.ring.period_ns()) { + let e = wrap(latch_ns as i64 - deadline as i64, period); + self.latch_after_deadline = Some(match self.latch_after_deadline { + None => e, + // Folded around the current estimate, so a phase near half + // a period does not average two edges into the middle. + Some(d) => d + wrap(e - d, period) / 8, + }); + } + if applied_ns != 0 { + let margin = fold_margin(latch_ns as i64 - applied_ns as i64, period); + self.lead.update(margin, g.period_ns); + if margin < 0 { + self.window.late += 1; + } + if self.window.margins.len() < 4096 { + self.window.margins.push(margin); + } + } + } + if self.window.start_ns == 0 || latch_ns < self.window.start_ns { + self.window = Window { + start_ns: latch_ns, + ..Window::default() + }; + return None; + } + let span = latch_ns - self.window.start_ns; + if span < REPORT_NS { + return None; + } + let w = std::mem::replace( + &mut self.window, + Window { + start_ns: latch_ns, + ..Window::default() + }, + ); + let pushed = PUSHED.swap(0, std::sync::atomic::Ordering::Relaxed); + if span > 2 * REPORT_NS { + // The window spans a gap between sessions: nothing to report. + return None; + } + let mut m = w.margins; + m.sort_unstable(); + let q = |f: usize| m.get(m.len() * f / 10).copied().unwrap_or(0); + let period_ns = grid.map_or(0, |g| g.period_ns); + Some(VsyncReport { + source: grid.map(|g| g.source), + period_ns, + margin_ns: [q(1), q(5), q(9)], + late: w.late, + publishes: m.len() as u32, + vsyncs: w.vsyncs, + pushed, + lead_ns: self.lead.lead_ns(period_ns), + latch_after_deadline_ns: self.latch_after_deadline, + }) + } + + /// The grid flip events follow at `now_ns`, `None` when no source is fresh. + /// + /// Its instants are `SurfaceFlinger`'s latch points. Choreographer's + /// measured period wins whenever it is locked and fresh. Its phase is the + /// frame deadline when it reports one, moved by where the publishes were + /// actually latched against it; otherwise the latch grid's, and the vsync + /// time only until a latch has been seen. With Choreographer silent, the + /// latch grid alone is followed. + #[must_use] + pub fn grid(&self, now_ns: u64) -> Option { + let fresh = |t: u64| now_ns.saturating_sub(t) < STALE_NS; + if let (Some(period), Some((vsync, deadline))) = (self.ring.period_ns(), self.chor) { + if fresh(vsync) { + let latch = self.latch.filter(|&(a, _)| fresh(a)); + let (anchor_ns, source) = match (deadline, latch) { + (Some(d), _) => ( + d.saturating_add_signed(self.latch_after_deadline.unwrap_or(0)), + GridSource::ChoreographerDeadline, + ), + (None, Some((a, _))) => (a, GridSource::ChoreographerLatch), + (None, None) => (vsync, GridSource::ChoreographerVsync), + }; + return Some(Grid { + anchor_ns, + period_ns: period, + source, + lead_ns: self.lead.lead_ns(period), + }); + } + } + let (anchor_ns, period_ns) = self.latch.filter(|&(a, _)| fresh(a))?; + Some(Grid { + anchor_ns, + period_ns, + source: GridSource::Latch, + lead_ns: self.lead.lead_ns(period_ns), + }) + } +} + +static DISPLAY: Mutex> = Mutex::new(None); + +fn with_display(f: impl FnOnce(&mut DisplayVsync) -> R) -> R { + let mut g = DISPLAY.lock().unwrap_or_else(PoisonError::into_inner); + f(g.get_or_insert_with(|| { + let mut d = DisplayVsync::default(); + // Read once per process: the daemon lives in the app process, so the + // variable has to be in the app's environment, not the guest's. + if let Some(us) = std::env::var(LEAD_ENV) + .ok() + .and_then(|v| v.trim().parse::().ok()) + { + d.fix_lead(us * 1000); + } + d + })) +} + +/// The display's measured period while Choreographer is locked and fresh at +/// `now_ns`, `None` otherwise. +#[must_use] +pub fn measured_period_ns(now_ns: u64) -> Option { + with_display(|d| d.measured_period_ns(now_ns)) +} + +/// Feed one completed publish (see [`DisplayVsync::note_publish`]). +pub fn note_publish_latch(applied_ns: u64, latch_ns: u64) -> Option { + with_display(|d| d.note_publish(applied_ns, latch_ns)) +} + +/// Feed one display vsync from the platform's vsync source (see +/// [`DisplayVsync::note_vsync`]). +pub fn note_choreographer_vsync(vsync_ns: u64, deadline_ns: Option) -> Option { + with_display(|d| d.note_vsync(vsync_ns, deadline_ns)) +} + +/// Feed the latch grid's current anchor (see [`DisplayVsync::note_latch`]). +pub fn note_latch_grid(anchor_ns: u64, period_ns: u64) { + with_display(|d| d.note_latch(anchor_ns, period_ns)); +} + +/// The grid the flip events follow now, `None` when no source is fresh. +#[must_use] +pub fn display_grid(now_ns: u64) -> Option { + with_display(|d| d.grid(now_ns)) +} + +/// The vblank timeline a page-flip event claims. +/// +/// gamescope re-anchors its whole vblank schedule on each event's timestamp, so +/// the timestamp is the latch the frame lands on — the first latch instant of +/// the display's [`Grid`] at or after the publish returned — moved earlier by +/// the grid's lead (see [`LeadLoop`]). A frame published between the deadline +/// Choreographer predicted and the latch `SurfaceFlinger` actually took is +/// claimed for that latch, not the next: claiming the next would push +/// gamescope's next wakeup a whole period out and leave one latch with no new +/// frame. While no grid is known the timeline walks the mode's advertised +/// interval from the clock, as before any source reported. +/// +/// Stamping the emit instant instead would re-anchor gamescope's grid early on +/// every frame (it arms early by design), and every divisor cap then ran fast +/// by that offset: +7 % measured across targets 20/24/30/40. +/// +/// Either way a claim never moves backwards and never repeats: a second frame +/// inside one period claims the next instant, and a late frame skips whole +/// periods instead of claiming a time in the past. +#[derive(Debug, Default)] +pub struct VblankTimeline { + claimed_ns: u64, + started: bool, +} + +impl VblankTimeline { + /// The vblank a frame published at `now_ns` lands on. `grid` is the + /// display's timeline when one is known, `interval_ns` the mode's advertised + /// period, walked when it is not. + pub fn claim(&mut self, now_ns: u64, grid: Option, interval_ns: u64) -> u64 { + let interval_ns = interval_ns.max(1); + let t = match grid { + Some(g) => { + // At least half a period after the last claim: one claim per + // grid instant, whatever grid (or walk) the last one was on. + let floor = if self.started { + self.claimed_ns + g.lead_ns + g.period_ns / 2 + 1 + } else { + 0 + }; + let landed = g.at_or_after(now_ns); + let latch = g.at_or_after(now_ns.max(floor)); + if latch != landed { + PUSHED.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + } + latch.saturating_sub(g.lead_ns) + } + None if !self.started => now_ns, + None => { + let mut t = self.claimed_ns + interval_ns; + if t < now_ns { + t += (now_ns - t).div_ceil(interval_ns) * interval_ns; + } + t + } + }; + let t = if self.started { + t.max(self.claimed_ns + 1) + } else { + t + }; + self.started = true; + self.claimed_ns = t; + t + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The Retroid Pocket 6's "60 Hz" panel, as measured. + const P: u64 = 16_670_000; + const T0: u64 = 1_000_000_000_000; + + fn locked_ring(period: u64) -> (VsyncRing, u64) { + let mut r = VsyncRing::default(); + let mut t = T0; + let mut locked = None; + for _ in 0..RING { + if let RingEvent::Locked(p) = r.push(t) { + locked = Some(p); + } + t += period; + } + assert_eq!(locked, Some(period)); + (r, t) + } + + #[test] + fn the_period_is_the_measured_spacing_not_the_mode() { + let (r, _) = locked_ring(P); + assert_eq!(r.period_ns(), Some(P)); + } + + #[test] + fn no_period_before_enough_vsyncs() { + let mut r = VsyncRing::default(); + for i in 0..MIN_LOCK as u64 - 1 { + assert_eq!(r.push(T0 + i * P), RingEvent::None); + } + assert_eq!(r.period_ns(), None); + assert_eq!(r.push(T0 + (MIN_LOCK as u64 - 1) * P), RingEvent::Locked(P)); + } + + #[test] + fn skipped_and_jittered_callbacks_still_fit_the_period() { + let mut r = VsyncRing::default(); + let mut n = 0u64; + for i in 0..200u64 { + // Every fifth callback misses a vsync; timestamps jitter by 50 us. + n += if i % 5 == 4 { 2 } else { 1 }; + let jitter = if i % 2 == 0 { 50_000 } else { 0 }; + r.push(T0 + n * P + jitter); + } + let p = r.period_ns().expect("locked"); + assert!(p.abs_diff(P) < 2_000, "fitted {p}"); + } + + #[test] + fn a_rate_change_relocks_on_the_new_period() { + let (mut r, mut t) = locked_ring(P); + let fast = 8_324_000; + let mut relocked = None; + for _ in 0..RING { + if let RingEvent::Locked(p) = r.push(t) { + relocked = Some(p); + } + t += fast; + } + assert_eq!(relocked, Some(fast)); + assert_eq!(r.period_ns(), Some(fast)); + } + + #[test] + fn a_halved_rate_relocks_too() { + // 120 Hz to 60: every step is two of the old periods, which would count + // as a skipped vsync forever if the ring did not notice. + let fast = 8_324_000; + let (mut r, mut t) = locked_ring(fast); + let mut relocked = None; + for _ in 0..3 * RING { + if let RingEvent::Locked(p) = r.push(t) { + relocked = Some(p); + } + t += 2 * fast + 6_000; + } + assert_eq!(relocked, Some(2 * fast + 6_000)); + } + + #[test] + fn a_pause_keeps_the_lock() { + let (mut r, t) = locked_ring(P); + assert_eq!(r.push(t + 2_000_000_000), RingEvent::None); + assert_eq!(r.period_ns(), Some(P)); + } + + #[test] + fn grid_instants_round_up_on_either_side_of_the_anchor() { + let g = Grid { + anchor_ns: 1_000, + period_ns: 100, + source: GridSource::Latch, + lead_ns: 0, + }; + assert_eq!(g.at_or_after(1_000), 1_000); + assert_eq!(g.at_or_after(1_001), 1_100); + assert_eq!(g.at_or_after(950), 1_000); + assert_eq!(g.at_or_after(899), 900); + assert_eq!(g.at_or_after(100), 100); + } + + fn display_locked(deadline_lead: Option) -> (DisplayVsync, u64) { + let mut d = DisplayVsync::default(); + let mut t = T0; + for _ in 0..MIN_LOCK { + d.note_vsync(t, deadline_lead.map(|l| t + l)); + t += P; + } + (d, t - P) + } + + #[test] + fn the_deadline_is_the_stamped_instant_when_reported() { + let (d, last) = display_locked(Some(P + 3_000_000)); + let g = d.grid(last + 1_000).expect("grid"); + assert_eq!(g.source, GridSource::ChoreographerDeadline); + assert_eq!(g.period_ns, P); + assert_eq!(g.anchor_ns, last + P + 3_000_000); + // Instants fall on the deadline's phase. + assert_eq!(g.at_or_after(last + 1_000), last + 3_000_000); + } + + #[test] + fn without_a_deadline_the_latch_phase_is_used_with_the_measured_period() { + let (mut d, last) = display_locked(None); + assert_eq!( + d.grid(last).map(|g| g.source), + Some(GridSource::ChoreographerVsync) + ); + d.note_latch(last + 4_000_000, 16_666_666); + let g = d.grid(last + 5_000_000).expect("grid"); + assert_eq!(g.source, GridSource::ChoreographerLatch); + assert_eq!(g.anchor_ns, last + 4_000_000); + assert_eq!(g.period_ns, P); + } + + #[test] + fn the_latch_grid_alone_is_the_fallback_and_stale_sources_drop_out() { + let mut d = DisplayVsync::default(); + assert_eq!(d.grid(T0), None); + d.note_latch(T0, 16_666_666); + let g = d.grid(T0 + 1_000).expect("grid"); + assert_eq!(g.source, GridSource::Latch); + assert_eq!(g.period_ns, 16_666_666); + assert_eq!(d.grid(T0 + STALE_NS + 1), None); + + let (d, last) = display_locked(Some(P)); + assert!(d.grid(last + STALE_NS - 1).is_some()); + assert_eq!(d.grid(last + STALE_NS + 1), None); + } + + #[test] + fn the_lock_reports_the_latch_offset() { + let mut d = DisplayVsync::default(); + let mut t = T0; + d.note_latch(T0 + 500_000, 16_666_666); + let mut lock = None; + for _ in 0..MIN_LOCK { + if let Some(l) = d.note_vsync(t, Some(t + 2 * P)) { + lock = Some(l); + } + t += P; + } + let lock = lock.expect("locked"); + assert_eq!(lock.grid.source, GridSource::ChoreographerDeadline); + assert_eq!(lock.latch_offset_ns, Some(500_000)); + } + + #[test] + fn claims_follow_the_display_grid_not_the_mode() { + let grid = Grid { + anchor_ns: T0, + period_ns: P, + source: GridSource::ChoreographerDeadline, + lead_ns: 0, + }; + let mut tl = VblankTimeline::default(); + // gamescope publishes ~3 ms before each deadline, every frame, for + // two minutes: every claim is the deadline it made, so the stamps walk + // the display's 16.670 ms and not the mode's 16.667 ms. + let mut prev = 0; + for k in 1..7_200u64 { + let publish = T0 + k * P - 3_000_000; + let c = tl.claim(publish, Some(grid), 16_666_666); + assert_eq!(c, T0 + k * P, "frame {k}"); + if prev != 0 { + assert_eq!(c - prev, P); + } + prev = c; + } + } + + #[test] + fn a_late_frame_skips_whole_periods_and_never_moves_back() { + let grid = Grid { + anchor_ns: T0, + period_ns: P, + source: GridSource::ChoreographerDeadline, + lead_ns: 0, + }; + let mut tl = VblankTimeline::default(); + assert_eq!(tl.claim(T0 + P - 1_000, Some(grid), 16_666_666), T0 + P); + // Published 1.5 periods late: lands two periods on. + assert_eq!( + tl.claim(T0 + 2 * P + P / 2, Some(grid), 16_666_666), + T0 + 3 * P + ); + // Two publishes inside one period: the second takes the next instant. + assert_eq!( + tl.claim(T0 + 3 * P + 10, Some(grid), 16_666_666), + T0 + 4 * P + ); + assert_eq!( + tl.claim(T0 + 3 * P + 20, Some(grid), 16_666_666), + T0 + 5 * P + ); + } + + #[test] + fn switching_grids_never_moves_back() { + let mut tl = VblankTimeline::default(); + // The walk claims first (no source yet), then a grid whose instants sit + // a third of a period before the walk's. + let first = tl.claim(T0, None, 16_666_666); + assert_eq!(first, T0); + let grid = Grid { + anchor_ns: T0 - P / 3, + period_ns: P, + source: GridSource::Latch, + lead_ns: 0, + }; + let c = tl.claim(T0 + 1_000, Some(grid), 16_666_666); + assert!(c > first + P / 2, "{c}"); + assert_eq!(c, T0 - P / 3 + P); + // And back to the walk: one interval after the last claim. + assert_eq!(tl.claim(c, None, 16_666_666), c + 16_666_666); + } + + #[test] + fn the_walk_is_unchanged_without_a_grid() { + let mut tl = VblankTimeline::default(); + let i = 16_666_666; + assert_eq!(tl.claim(T0, None, i), T0); + assert_eq!(tl.claim(T0, None, i), T0 + i); + assert_eq!(tl.claim(T0 + 3 * i + 5, None, i), T0 + 4 * i); + } + + #[test] + fn fold_reads_a_quarter_period_late_as_late() { + let p = 100; + assert_eq!(fold_margin(10, p), 10); + assert_eq!(fold_margin(70, p), 70); + assert_eq!(fold_margin(80, p), -20); + assert_eq!(fold_margin(99, p), -1); + assert_eq!(fold_margin(-30, p), 70); + } + + #[test] + fn a_lead_stamps_ahead_of_the_latch_and_keeps_the_rules() { + let grid = Grid { + anchor_ns: T0, + period_ns: P, + source: GridSource::ChoreographerDeadline, + lead_ns: 3_000_000, + }; + let mut tl = VblankTimeline::default(); + // Published 1 ms before the latch: stamped 3 ms before it, i.e. in the + // past, as a KMS event's vblank is by the time it is read. + assert_eq!( + tl.claim(T0 + P - 1_000_000, Some(grid), 16_666_666), + T0 + P - 3_000_000 + ); + // The next frame, on time. + assert_eq!( + tl.claim(T0 + 2 * P - 2_000_000, Some(grid), 16_666_666), + T0 + 2 * P - 3_000_000 + ); + // Late (just after the latch): the latch after. + assert_eq!( + tl.claim(T0 + 3 * P + 100, Some(grid), 16_666_666), + T0 + 4 * P - 3_000_000 + ); + } + + /// Model the session: gamescope commits `red_zone` ahead of the stamp it + /// was handed one period later, the apply lands `pipeline` after that with + /// a deterministic spread, `SurfaceFlinger` latches on the grid. Returns + /// the steady lead and the share of publishes that missed their latch. + fn run_session(pipeline: i64, spread: i64, frames: u64) -> (u64, f64, i64) { + let mut d = DisplayVsync::default(); + let mut tl = VblankTimeline::default(); + let mut t = T0; + for _ in 0..MIN_LOCK { + d.note_vsync(t, Some(t + P)); + t += P; + } + let mut stamp = tl.claim(t, d.grid(t), 16_666_666); + let (mut late, mut counted) = (0u64, 0u64); + let mut margins = Vec::new(); + for k in 0..frames { + d.note_vsync(t, Some(t + P)); + t += P; + // A spread of -spread..+spread, in a fixed pseudo-random order. + let jitter = ((k * 7919) % 101) as i64 * 2 * spread / 100 - spread; + let applied = (stamp + P - 1_650_000) as i64 + pipeline + jitter; + let applied = applied as u64; + let g = d.grid(applied).expect("grid"); + let latch = Grid { lead_ns: 0, ..g }.at_or_after(applied); + stamp = tl.claim(applied, Some(g), 16_666_666); + d.note_publish(applied, latch); + if k > frames / 2 { + counted += 1; + let aimed = stamp + g.lead_ns; + if latch != aimed { + late += 1; + } + margins.push(latch as i64 - applied as i64); + } + } + margins.sort_unstable(); + ( + d.lead.lead_ns(P), + late as f64 / counted as f64, + margins[margins.len() / 10], + ) + } + + #[test] + fn the_lead_holds_the_tenth_percentile_at_its_target() { + for pipeline in [0i64, 1_000_000, 2_500_000] { + let (lead, late, p10) = run_session(pipeline, 2_000_000, 6_000); + assert!(late < 0.001, "pipeline {pipeline}: late {late} lead {lead}"); + assert!( + (p10 - TARGET_P10_NS as i64).abs() < 400_000, + "pipeline {pipeline}: p10 {p10} lead {lead}" + ); + } + } + + #[test] + fn a_fixed_lead_does_not_move() { + let mut l = LeadLoop::fixed(4_000_000); + for _ in 0..100 { + l.update(-1, P); + } + assert_eq!(l.lead_ns(P), 4_000_000); + } + + #[test] + fn a_publish_after_the_deadline_but_before_the_latch_claims_that_latch() { + let mut d = DisplayVsync::default(); + d.fix_lead(0); + let mut t = T0; + for _ in 0..MIN_LOCK { + d.note_vsync(t, Some(t + P)); + t += P; + } + // `SurfaceFlinger` takes every publish 1.5 ms after the deadline. + for k in 0..64u64 { + let latch = T0 + (MIN_LOCK as u64 + 1 + k) * P + 1_500_000; + d.note_publish(latch - 4_000_000, latch); + } + let g = d.grid(t).expect("grid"); + let mut tl = VblankTimeline::default(); + let deadline = t + 3 * P; + // Published 1 ms after the deadline: still the latch 0.5 ms later. + let c = tl.claim(deadline + 1_000_000, Some(g), 16_666_666); + assert!(c.abs_diff(deadline + 1_500_000) < 50_000, "{c}"); + } + + #[test] + fn the_cross_check_reports_every_ten_seconds() { + let mut d = DisplayVsync::default(); + let mut t = T0; + let mut reports = Vec::new(); + for k in 0..1_300u64 { + d.note_vsync(t, Some(t + P)); + // Every 60th vsync carries no publish. + if k > MIN_LOCK as u64 && k % 60 != 0 { + let latch = t + P; + if let Some(r) = d.note_publish(latch - 3_000_000, latch) { + reports.push(r); + } + } + t += P; + } + assert_eq!(reports.len(), 2); + let r = &reports[0]; + assert_eq!(r.source, Some(GridSource::ChoreographerDeadline)); + assert_eq!(r.period_ns, P); + assert_eq!(r.margin_ns, [3_000_000; 3]); + assert_eq!(r.late, 0); + assert!(r.vsyncs > r.publishes, "{r:?}"); + assert!((590..=600).contains(&r.vsyncs), "{r:?}"); + } +} diff --git a/modules/gndrm-daemon/tests/golden.rs b/modules/gndrm-daemon/tests/golden.rs index 64f436c..d5aaeab 100644 --- a/modules/gndrm-daemon/tests/golden.rs +++ b/modules/gndrm-daemon/tests/golden.rs @@ -1631,7 +1631,8 @@ fn atomic_test_only_then_modeset_then_flip() { assert_eq!(ev.crtc_id, CRTC_ID); assert_eq!(ev.sequence, 1); // The timestamp is the vblank this frame claims, not the emit instant: the - // first claim is the clock's own reading (see `VblankTimeline`). + // first claim is the clock's own reading while no display timeline is known + // (see `vblank::VblankTimeline`). assert_eq!((ev.tv_sec, ev.tv_usec), (1000, 500)); // State is already visible to queries (applied synchronously on commit). From 7bdcbae4368942fe7a0f491350f7fea3e018b42f Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Mon, 2 Nov 2026 11:40:34 +0530 Subject: [PATCH 2/7] shim: the shim declares FIFO while a cap is set, as gamescope's WSI layer does; gamescope's limiter is the cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a PC, gamescope's WSI layer creates the driver swapchain as mailbox and implements FIFO itself: per present it declares the app's own mode to gamescope through gamescope_swapchain.set_present_mode, or FIFO while gamescope's limiter is engaged. gamescope latches one FIFO commit per limited vblank (steamcompmgr.cpp handle_done_commits_xwayland) and the client is paced only by wl_buffer.release. MESA_VK_WSI_PRESENT_MODE reaches Mesa below the layer and never changes what is declared. The shim is that layer on gamescope's override route (its own wl_surface wrapped in a gamescope_swapchain), but it let MESA_VK_WSI_PRESENT_MODE decide the mode, and every container carries mailbox there as a default for the X11 path. So every commit was declared mailbox, which gamescope's limiter never holds, and an acquire gate on the shim's own clock stood in for the cap. That gate beat against the display (57-59 fps at cap 60), and with mailbox there was no queue to absorb a late frame. On the override route, per present: - frame generation on: mailbox, exactly as before (generated frames are committed without a declaration, and a real frame declared FIFO would queue behind them); - a cap in force (the daemon's FrameCapNote, the value the app also hands gamescope's limiter): FIFO; - otherwise: the guest's own mode. The same rule picks the present discipline and both set_present_mode sites. The env no longer leads the offered mode list there, and the extra mailbox slot follows the guest's request (or frame generation at creation). A present wait is answered at commit when the guest asked for mailbox or immediate, or its frames go out as mailbox; a FIFO guest declared FIFO keeps the release-or-grace answer. Off the override route (the opt-in X11 Present route) nothing changes. The time gate now runs on the override route only while frame generation sets the source rate (the app disarms gamescope's limiter then); otherwise gamescope's limiter is the cap. A cap that is not the display's rate over a whole number (40 or 45 on 60 Hz) runs at gamescope's next divisor rate above it, upstream gamescope's behaviour, left to it for now. One ungated line every 10 s while frames are presented: gn-vkshim: present mode: declared=fifo requested=fifo route=gamescope cap=60 lsfg=0 acquires=… commits=… declared_fifo=… declared_other=… releases=… - icd_shim/src/proxy.rs: live_present_mode / choose_live_mode, declared_present_mode, mode_census; effective_present_mode ignores the env on the override route; present_dispatch, both declarations, the present-wait rule, the slot count, the offered modes and the time gate follow; tests. - icd_shim/src/scanout.rs: cap_in_force, cap_fps_note. - docs/10-fps-limit.md, docs/15-fps-limiter-pacing-and-vsync.md, docs/README.md: the cap is gamescope's limiter on the shim's FIFO declaration; the acquire gate is described as frame generation's source-rate gate. --- docs/10-fps-limit.md | 26 ++- docs/15-fps-limiter-pacing-and-vsync.md | 39 +++-- docs/README.md | 2 +- icd_shim/src/proxy.rs | 214 +++++++++++++++++++++--- icd_shim/src/scanout.rs | 11 ++ 5 files changed, 241 insertions(+), 51 deletions(-) diff --git a/docs/10-fps-limit.md b/docs/10-fps-limit.md index f8d3465..ae00beb 100644 --- a/docs/10-fps-limit.md +++ b/docs/10-fps-limit.md @@ -104,15 +104,23 @@ divisor cap reads its target exactly. 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 - `vkAcquireNextImageKHR` — the one point where waiting costs neither rendered - work nor display latency. The deadline, not the wake: the wait returns a hair - after it, and `max(prev, now) + period` made that hair part of every period - (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. +- **gamescope's limiter enforces it; the shim declares FIFO while it is set.** On a + PC, gamescope's WSI layer declares FIFO to gamescope (`gamescope_swapchain. + set_present_mode`) while its limiter is engaged, gamescope latches one FIFO commit per + limited vblank (`steamcompmgr.cpp` `handle_done_commits_xwayland`), and the client is + paced by `wl_buffer.release`. The shim is that layer here: on gamescope's override + route it declares FIFO on every commit while the daemon's `FrameCapNote` carries a + cap, the guest's own mode otherwise, and mailbox while frame generation runs + (`icd_shim/src/proxy.rs` `live_present_mode`). `MESA_VK_WSI_PRESENT_MODE`, a container + default for the X11 path, no longer decides that route: honouring it declared every + commit mailbox, which the limiter never holds, and that is why an acquire gate on the + shim's own clock used to stand in for the cap. That time gate + (`scanout::next_acquire_after`) now runs only while frame generation sets the source + rate, and on the opt-in X11 Present route, which has no compositor declaration. + gamescope's fixed-rate limiter passes every `refresh / target`-th vblank with an integer + divisor, so a cap that does not divide the display's rate (40 or 45 on 60 Hz, 45 or 50 + on 120 Hz) runs at the next divisor rate above it; that is upstream gamescope's + behaviour and is left to it for now. - **`-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. diff --git a/docs/15-fps-limiter-pacing-and-vsync.md b/docs/15-fps-limiter-pacing-and-vsync.md index 3fffff3..fa701b5 100644 --- a/docs/15-fps-limiter-pacing-and-vsync.md +++ b/docs/15-fps-limiter-pacing-and-vsync.md @@ -11,15 +11,15 @@ cursor, scan-out and mode patches. | place | what it holds | set by | |---|---|---| -| the shim's acquire gate | the user's cap, exactly | `FrameCapNote{fps}` from the daemon (`docs/19`) | +| the shim's declared present mode | FIFO while a cap is set, so gamescope's limiter holds each frame to its limited vblank | `FrameCapNote{fps}` from the daemon (`docs/10`) | | 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)`. +They work together: the app hands gamescope the cap, gamescope's limiter passes every +`nRefreshHz / target`-th vblank (`nVblankDivisor = nRefreshHz / min(target, nRefreshHz)`), +and the shim's FIFO declaration is what makes the limiter hold the guest's commits. Under +frame generation the app disarms gamescope's limiter and the shim's time gate holds the +source rate instead (`docs/17`). ## 2. The mode is the display's rate @@ -39,14 +39,16 @@ 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`). + paced to the panel whether or not it asked to be. On gamescope's override route the + mode declared per commit is FIFO while a cap is set, the guest's own otherwise, and + mailbox under frame generation; `MESA_VK_WSI_PRESENT_MODE` decides it only off that + route (`docs/10`). The shim answers `vkWaitForPresentKHR` on the release, or two + refresh periods after the commit when no release report arrives first. +- **Mailbox and immediate.** A guest that asked for either does not wait, and neither + would one whose frames go out as mailbox (frame generation, or the env off the + override route while the guest believes it has FIFO), so those waits are 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. @@ -89,6 +91,15 @@ that rate is the display's. deadline stamp on that device). The stamps are earlier than the light by the lead plus the compositor's pipeline; consumers read differences between stamps (frame time), which a constant offset does not touch. +- **The guest is paced by gamescope's FIFO latch, not by a gate of its own.** With a + cap set, the shim declares FIFO on every commit, as gamescope's WSI layer does on a PC + while the limiter is engaged; gamescope latches one FIFO commit per limited vblank, on + the display's vblank stream above, and hands the previous buffer back with + `wl_buffer.release`, which is what frees the guest's next acquire. A gate on the shim's + own clock (exactly 1e9 / N ns) beat against the display's 59.97-59.99 Hz and, with the + env's mailbox declaration, left no queue to absorb a late frame. A cap that is not the + display's rate over a whole number runs at gamescope's next divisor rate above it + (`docs/10`). - **Fallbacks.** On API 29-32 Choreographer reports the vsync time only (`AChoreographer_postFrameCallback64`): its measured period is used with the latch grid's phase (`vsync.rs`, from `ASurfaceTransactionStats_getLatchTime`), or the vsync diff --git a/docs/README.md b/docs/README.md index e642196..3fd0f68 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) — where a frame's rate is decided: the acquire gate, gamescope's divisor, and the presenter's latch grid; and where vsync is real. +15. [15-fps-limiter-pacing-and-vsync.md](15-fps-limiter-pacing-and-vsync.md) — where a frame's rate is decided: gamescope's limiter on the shim's FIFO declaration, 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/proxy.rs b/icd_shim/src/proxy.rs index c4df106..6593c7d 100644 --- a/icd_shim/src/proxy.rs +++ b/icd_shim/src/proxy.rs @@ -233,6 +233,15 @@ const X11_TRACE_N: usize = 24; /// so it is bounded to the first few presents only. const X11_VIEW_N: u32 = 3; static RATE_STALL: Mutex> = Mutex::new(None); +// The ungated present-mode census (see `mode_census`): commits declared to +// gamescope as FIFO and as anything else, and every compositor release. Kept +// apart from the gated WSI census, whose counters only move while +// `GNDRM_LOG_VKSHIM` is set. +static DECL_FIFO_N: AtomicUsize = AtomicUsize::new(0); +static DECL_OTHER_N: AtomicUsize = AtomicUsize::new(0); +static REL_ALL_N: AtomicUsize = AtomicUsize::new(0); +static RATE_MODE: Mutex> = Mutex::new(None); +const MODE_CENSUS_EVERY: Duration = Duration::from_secs(10); static RATE_WAIT: Mutex> = Mutex::new(None); /// The last (semaphore, value) the guest asked us to *signal*. A GPU wait for a /// value the driver never reaches shows up as `want` > `last_signalled`. @@ -1719,7 +1728,12 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { // wait at all, and holding it couples the guest's frame-latency loop // to a refresh period it does not pace by. if (*s).wait_id != 0 { - let at_commit = answers_present_wait_at_commit((*sc).present_mode); + // A guest that asked for mailbox or immediate does not wait, and + // neither would one whose frames go out as mailbox (frame + // generation): both are answered here. A FIFO guest declared FIFO + // keeps the release-or-grace answer. + let at_commit = answers_present_wait_at_commit((*sc).present_mode) + || answers_present_wait_at_commit(live_present_mode(sc)); let id = if at_commit { let _g = (*sc).shared.lock().unwrap(); core::mem::replace(&mut (*s).wait_id, 0) @@ -1783,7 +1797,7 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { // the compositor as the shim applies it. wlproto::swapchain_set_present_mode( (*(*sc).surf).swapchain_obj, - gamescope_present_mode((*sc).present_mode), + gamescope_present_mode(declared_present_mode(sc)), ); wlproto::surface_attach(parent, (*s).buf, 0, 0); wlproto::surface_damage_buffer( @@ -1909,12 +1923,19 @@ unsafe extern "C" fn gn_vkCreateSwapchainKHR( )); } } - let mode = effective_present_mode(ci); + // On gamescope's override route the guest's own mode is the one this + // swapchain keeps (see `effective_present_mode`); what each commit declares + // is decided per present (`live_present_mode`). + let gamescope_route = unsafe { !(*surf).swapchain_obj.is_null() }; + let mode = effective_present_mode(ci, gamescope_route); // Mesa's wayland WSI derives minImageCount from the present mode // (wsi_common_wayland.c:1188): the extra image exists so one can be queued // for scan-out while the app renders the next. Mailbox therefore carries - // one more slot than fifo; fifo keeps its existing count. - let extra_slot = u32::from(mode == VK_PRESENT_MODE_MAILBOX_KHR); + // one more slot than fifo; fifo keeps its existing count. Frame generation + // presents as mailbox, so a swapchain made while it runs keeps that count. + let extra_slot = u32::from( + mode == VK_PRESENT_MODE_MAILBOX_KHR || (gamescope_route && crate::lsfg::live().0), + ); let mut n_slots = ci.minImageCount + 1 + extra_slot; if n_slots < 2 { n_slots = 2; @@ -2077,7 +2098,7 @@ unsafe extern "C" fn gn_vkCreateSwapchainKHR( ci.imageColorSpace, ci.clipped, mode, - gamescope_present_mode(mode) + gamescope_present_mode(live_present_mode(sc)) )); } VK_SUCCESS @@ -2262,10 +2283,17 @@ unsafe fn gn_acquire_common( [timeout as usize, sem as usize, fence as usize, 0, 0], ); let deadline = (timeout != u64::MAX).then(|| Instant::now() + Duration::from_nanos(timeout)); - // The cap, applied here: a frame that may not start yet waits before it is - // given an image, so nothing rendered is thrown away and nothing already on - // its way to the panel is delayed. - let period = crate::scanout::period_ns(crate::scanout::target_fps()); + // The time gate: a frame that may not start yet waits before it is given an + // image. On gamescope's override route it runs only while frame generation + // sets the source rate (the app disarms gamescope's limiter then); any other + // cap is gamescope's limiter's, which holds the FIFO commits this shim + // declares while a cap is set (`live_present_mode`). Off that route the cap + // has no compositor to hold it, and the gate keeps it as before. + let period = if unsafe { (*(*sc).surf).swapchain_obj.is_null() } { + crate::scanout::period_ns(crate::scanout::target_fps()) + } else { + crate::lsfg::source_cap().map_or(0, crate::scanout::period_ns) + }; let mut idx: i32 = -1; let mut timed_out = false; // First instant this acquire was parked by the deadline, 0 while it never @@ -2593,11 +2621,11 @@ unsafe fn present_sync_commit(sc: *mut GnSwapchain, slot: *mut GnSlot, idx: i32) if !unsafe { (*slot).buf.is_null() } { let _sg = SEND_LOCK.lock().unwrap(); unsafe { - // Declare this swapchain's *effective* present mode, as on the - // worker path above (gamescope reads the mode once per commit). + // Declare the mode in force, as on the worker path above + // (gamescope reads the mode once per commit). wlproto::swapchain_set_present_mode( (*(*sc).surf).swapchain_obj, - gamescope_present_mode((*sc).present_mode), + gamescope_present_mode(declared_present_mode(sc)), ); wlproto::surface_attach(parent, (*slot).buf, 0, 0); wlproto::surface_damage_buffer( @@ -2616,7 +2644,8 @@ unsafe fn present_sync_commit(sc: *mut GnSwapchain, slot: *mut GnSlot, idx: i32) /// Route a present to the discipline its effective present mode names. unsafe fn present_dispatch(sc: *mut GnSwapchain, slot: *mut GnSlot, idx: i32) { - match unsafe { (*sc).present_mode } { + mode_census(sc); + match unsafe { live_present_mode(sc) } { VK_PRESENT_MODE_IMMEDIATE_KHR => unsafe { present_immediate(sc, slot, idx) }, VK_PRESENT_MODE_MAILBOX_KHR => unsafe { present_mailbox(sc, slot, idx) }, VK_PRESENT_MODE_FIFO_RELAXED_KHR => unsafe { present_fifo_relaxed(sc, slot, idx) }, @@ -6138,6 +6167,39 @@ mod tests { } } + #[test] + fn the_declared_mode_is_fifo_while_a_cap_is_set_as_gamescopes_layer_does() { + let (m, f, i) = ( + VK_PRESENT_MODE_MAILBOX_KHR, + VK_PRESENT_MODE_FIFO_KHR, + VK_PRESENT_MODE_IMMEDIATE_KHR, + ); + // Capped: FIFO whatever the guest asked for. + assert_eq!(choose_live_mode(f, false, true), f); + assert_eq!(choose_live_mode(m, false, true), f); + assert_eq!(choose_live_mode(i, false, true), f); + // Uncapped: the guest's own mode. + assert_eq!(choose_live_mode(f, false, false), f); + assert_eq!(choose_live_mode(m, false, false), m); + assert_eq!(choose_live_mode(i, false, false), i); + // Frame generation: mailbox, capped or not. + assert_eq!(choose_live_mode(f, true, true), m); + assert_eq!(choose_live_mode(f, true, false), m); + } + + #[test] + fn the_env_mode_does_not_decide_the_gamescope_route() { + // SAFETY: no other test reads this variable. + unsafe { std::env::set_var("MESA_VK_WSI_PRESENT_MODE", "mailbox") }; + let mut ci: VkSwapchainCreateInfoKHR = unsafe { core::mem::zeroed() }; + ci.presentMode = VK_PRESENT_MODE_FIFO_KHR; + assert_eq!(effective_present_mode(&ci, true), VK_PRESENT_MODE_FIFO_KHR); + assert_eq!(effective_present_mode(&ci, false), VK_PRESENT_MODE_MAILBOX_KHR); + ci.presentMode = 1000; + assert_eq!(effective_present_mode(&ci, true), VK_PRESENT_MODE_FIFO_KHR); + unsafe { std::env::remove_var("MESA_VK_WSI_PRESENT_MODE") }; + } + #[test] fn present_wait_is_answered_at_commit_only_for_unpaced_modes() { assert!(answers_present_wait_at_commit(VK_PRESENT_MODE_MAILBOX_KHR)); @@ -6914,6 +6976,7 @@ extern "C" fn on_buffer_release(data: *mut c_void, _buf: *mut wl_proxy) { (*sc).cv_free.notify_all(); let wait_id = core::mem::replace(&mut (*slot).wait_id, 0); drop(_g); + REL_ALL_N.fetch_add(1, Ordering::Relaxed); if wait_id != 0 { crate::present::complete_on_release(wait_id, monotonic_ns()); } @@ -8995,14 +9058,22 @@ fn parse_present_mode_override(val: Option<&str>) -> Option { Some(mode) } -/// The present mode the shim honours for a swapchain: the -/// MESA_VK_WSI_PRESENT_MODE override wins over the app's requested -/// `presentMode`, falling back to fifo (mesa wsi_common.c:1521). -fn effective_present_mode(ci: &VkSwapchainCreateInfoKHR) -> VkPresentModeKHR { - if let Some(m) = - parse_present_mode_override(std::env::var("MESA_VK_WSI_PRESENT_MODE").ok().as_deref()) - { - return m; +/// The present mode a swapchain keeps: the guest's requested `presentMode`, +/// falling back to fifo (mesa wsi_common.c:1521). Off gamescope's override route +/// the MESA_VK_WSI_PRESENT_MODE override wins over it, as in Mesa. +/// +/// On the override route the env is not consulted. There the shim is what +/// gamescope's WSI layer is on a PC, and that layer declares the app's own mode +/// to gamescope whatever Mesa below it was told: the env reaches only Mesa. The +/// variable comes with every container as a default for the X11 path; honouring +/// it here declared every commit MAILBOX, which gamescope's limiter never holds. +fn effective_present_mode(ci: &VkSwapchainCreateInfoKHR, gamescope_route: bool) -> VkPresentModeKHR { + if !gamescope_route { + if let Some(m) = + parse_present_mode_override(std::env::var("MESA_VK_WSI_PRESENT_MODE").ok().as_deref()) + { + return m; + } } match ci.presentMode { VK_PRESENT_MODE_IMMEDIATE_KHR @@ -9021,6 +9092,90 @@ fn answers_present_wait_at_commit(mode: VkPresentModeKHR) -> bool { mode == VK_PRESENT_MODE_MAILBOX_KHR || mode == VK_PRESENT_MODE_IMMEDIATE_KHR } +/// The mode a present on `sc` runs under and declares, decided per present the +/// way gamescope's WSI layer does on a PC: there the layer declares FIFO while +/// gamescope's limiter is engaged and the app's own mode otherwise, and +/// gamescope latches one FIFO commit per limited vblank, so the client is paced +/// by `wl_buffer.release` and gamescope's limiter is the cap. +/// +/// On the override route: +/// - frame generation on: mailbox, as before. Generated frames are committed +/// with no declaration, and a real frame declared FIFO would queue behind +/// them. +/// - a cap in force (the daemon's `FrameCapNote`, the same value the app hands +/// gamescope's limiter): FIFO. +/// - otherwise: the guest's own mode. +/// +/// Off the override route there is nothing to declare it to, and the mode is the +/// one the swapchain was made with. +pub(crate) unsafe fn live_present_mode(sc: *mut GnSwapchain) -> VkPresentModeKHR { + let requested = unsafe { (*sc).present_mode }; + if unsafe { (*(*sc).surf).swapchain_obj.is_null() } { + return requested; + } + choose_live_mode( + requested, + crate::lsfg::live().0, + crate::scanout::cap_in_force(), + ) +} + +/// [`live_present_mode`]'s rule on the override route. +fn choose_live_mode(requested: VkPresentModeKHR, generating: bool, capped: bool) -> VkPresentModeKHR { + if generating { + VK_PRESENT_MODE_MAILBOX_KHR + } else if capped { + VK_PRESENT_MODE_FIFO_KHR + } else { + requested + } +} + +/// The mode a commit on `sc` declares to gamescope, counted for the census. +unsafe fn declared_present_mode(sc: *mut GnSwapchain) -> VkPresentModeKHR { + let mode = unsafe { live_present_mode(sc) }; + if mode == VK_PRESENT_MODE_FIFO_KHR { + DECL_FIFO_N.fetch_add(1, Ordering::Relaxed); + } else { + DECL_OTHER_N.fetch_add(1, Ordering::Relaxed); + } + mode +} + +fn mode_name(mode: VkPresentModeKHR) -> &'static str { + match mode { + VK_PRESENT_MODE_IMMEDIATE_KHR => "immediate", + VK_PRESENT_MODE_MAILBOX_KHR => "mailbox", + VK_PRESENT_MODE_FIFO_KHR => "fifo", + VK_PRESENT_MODE_FIFO_RELAXED_KHR => "fifo_relaxed", + _ => "?", + } +} + +/// Ungated, every 10 s while frames are presented: the mode declared to +/// gamescope and why, and the counts a device run checks the pacing against. +/// The launcher does not pass the container's environment to gamescope's +/// clients, so a gated line could not be read there. +unsafe fn mode_census(sc: *mut GnSwapchain) { + if !rate_ok(&RATE_MODE, MODE_CENSUS_EVERY) { + return; + } + let route = unsafe { !(*(*sc).surf).swapchain_obj.is_null() }; + crate::logi!(gndrm_obf::obf!( + "present mode: declared={} requested={} route={} cap={} lsfg={} acquires={} commits={} declared_fifo={} declared_other={} releases={}", + mode_name(unsafe { live_present_mode(sc) }), + mode_name(unsafe { (*sc).present_mode }), + if route { "gamescope" } else { "x11" }, + crate::scanout::cap_fps_note(), + u8::from(crate::lsfg::live().0), + N_ACQ.load(Ordering::Relaxed), + N_COMMIT_OK.load(Ordering::Relaxed), + DECL_FIFO_N.load(Ordering::Relaxed), + DECL_OTHER_N.load(Ordering::Relaxed), + REL_ALL_N.load(Ordering::Relaxed), + )); +} + /// The wire value for `gamescope_swapchain.set_present_mode` that declares /// `mode` — the mode `present_dispatch` honours — to the compositor. /// @@ -9083,16 +9238,21 @@ unsafe extern "C" fn gn_vkGetPhysicalDeviceSurfacePresentModesKHR( ) -> VkResult { HB_PMODES.fetch_add(1, Ordering::Relaxed); wsi_hb(); - if unsafe { find_surface(surface) }.is_null() { + let surf = unsafe { find_surface(surface) }; + if surf.is_null() { crate::logf!(gndrm_obf::obf!( "proxy surface present-modes query: {surface:#x} not one of ours" )); return VK_ERROR_SURFACE_LOST_KHR; } - let modes_list = offered_present_modes( - &SERVED_PRESENT_MODES, - parse_present_mode_override(std::env::var("MESA_VK_WSI_PRESENT_MODE").ok().as_deref()), - ); + // The env's mode leads the list only where it decides the mode + // (`effective_present_mode`): not on gamescope's override route. + let override_mode = if unsafe { (*surf).swapchain_obj.is_null() } { + parse_present_mode_override(std::env::var("MESA_VK_WSI_PRESENT_MODE").ok().as_deref()) + } else { + None + }; + let modes_list = offered_present_modes(&SERVED_PRESENT_MODES, override_mode); unsafe { if modes.is_null() { *count = modes_list.len() as u32; diff --git a/icd_shim/src/scanout.rs b/icd_shim/src/scanout.rs index 41d90ec..91f6522 100644 --- a/icd_shim/src/scanout.rs +++ b/icd_shim/src/scanout.rs @@ -169,6 +169,17 @@ pub(crate) fn target_fps() -> u32 { CAP_FPS.load(Ordering::Relaxed) } +/// Whether the daemon's cap note sets a cap, whatever frame generation does: +/// the value the app also hands gamescope's limiter. +pub(crate) fn cap_in_force() -> bool { + CAP_FPS.load(Ordering::Relaxed) != FPS_UNCAPPED +} + +/// The cap the daemon's note carries, [`FPS_UNCAPPED`] when none. +pub(crate) fn cap_fps_note() -> u32 { + CAP_FPS.load(Ordering::Relaxed) +} + /// How many frame-cap notes have been adopted on this connection, for the same /// census. pub(crate) fn notes_seen() -> usize { From 307d6956bedde4387f095cece35e1f543ab71980 Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Mon, 2 Nov 2026 11:40:34 +0530 Subject: [PATCH 3/7] shim: answer the present wait from gamescope's presentation feedback With FIFO declared, a guest that waits on every frame (DXVK with in-game VSync on: FIFO, present wait, frame latency 1) ran at 35 fps at a 60 cap on a 60 Hz panel, 364 of 526 intervals two vblanks long. The shim answered vkWaitForPresentKHR from the compositor's release, which under FIFO comes when the next frame is latched, a vblank after this one was shown, or two refresh periods after the commit; the guest's next frame started a vblank late every time. Mesa's Wayland WSI answers the present wait from wp_presentation: each present that carries an id requests a wp_presentation_feedback on the surface, and presented or discarded signals that id. gamescope offers wp_presentation (version 1, CLOCK_MONOTONIC), sends presented for the commit it latched with the refresh it targets, and discarded for a commit it replaced or never showed (including a surface that goes away). The shim now does the same on the override route: - wp_presentation is bound with the gamescope swapchain factory (not on the guest-surface route); its clock_id is recorded. - A FIFO commit that carries a wait requests a feedback before wl_surface.commit, under the send lock, with the id as the listener's data. The id is registered as feedback-pending before the commit, so nothing can answer it first. - presented answers the wait at the reported time (CLOCK_MONOTONIC; the receipt time otherwise); discarded answers it at once. The object is destroyed in either event. - While a feedback is pending, the release does not answer the id and the two-period grace does not apply; a feedback that never arrives is answered 100 ms after the commit, well inside the guest's one-second bound. - An id is answered once: record() ignores ids no longer armed, so a second answer from another path (the daemon's latch report, a release) leaves nothing stale. - Without the global, off the override route, or when the feedback request fails, the release and the grace answer as before. The mailbox/immediate answer-at-commit rule (drm #37) is unchanged, and so is frame generation's path (its commits carry no feedback). The ungated present-mode line adds feedback=on|off presented= discarded= fallback= (waits answered by release, grace or the safety). - icd_shim/src/wlproto.rs: wp_presentation / wp_presentation_feedback interfaces, presentation_feedback(). - icd_shim/src/proxy.rs: bind, clock and feedback listeners, the request in commit_real, census fields. - icd_shim/src/present.rs: expect_feedback, feedback_presented / feedback_discarded, cancel_feedback, the pending rule for release and grace, counters; tests. - docs/15-fps-limiter-pacing-and-vsync.md: the FIFO present-wait answer. --- docs/15-fps-limiter-pacing-and-vsync.md | 12 +- icd_shim/src/present.rs | 162 ++++++++++++++++++++++-- icd_shim/src/proxy.rs | 125 +++++++++++++++++- icd_shim/src/wlproto.rs | 95 ++++++++++++++ 4 files changed, 382 insertions(+), 12 deletions(-) diff --git a/docs/15-fps-limiter-pacing-and-vsync.md b/docs/15-fps-limiter-pacing-and-vsync.md index fa701b5..3bba450 100644 --- a/docs/15-fps-limiter-pacing-and-vsync.md +++ b/docs/15-fps-limiter-pacing-and-vsync.md @@ -42,8 +42,16 @@ that rate is the display's. paced to the panel whether or not it asked to be. On gamescope's override route the mode declared per commit is FIFO while a cap is set, the guest's own otherwise, and mailbox under frame generation; `MESA_VK_WSI_PRESENT_MODE` decides it only off that - route (`docs/10`). The shim answers `vkWaitForPresentKHR` on the release, or two - refresh periods after the commit when no release report arrives first. + route (`docs/10`). On that route the shim answers `vkWaitForPresentKHR` from + gamescope's presentation feedback, as Mesa's Wayland WSI does: every commit that + carries a wait requests a `wp_presentation_feedback`, and the wait is answered on + `presented` (at the refresh it reports) or `discarded`. The release comes a vblank + later under FIFO, when the next frame is latched, and a guest that waits on every + frame (DXVK with VSync on) turned that into two vblanks per frame (35 fps at a 60 + cap), so while a feedback is pending neither the release nor the grace answers; a + feedback that never comes is answered 100 ms after the commit. Without the global + (or off the override route) the wait is answered on the release, or two refresh + periods after the commit when no release report arrives first. - **Mailbox and immediate.** A guest that asked for either does not wait, and neither would one whose frames go out as mailbox (frame generation, or the env off the override route while the guest believes it has FIFO), so those waits are answered at diff --git a/icd_shim/src/present.rs b/icd_shim/src/present.rs index 3cbe942..e4b4551 100644 --- a/icd_shim/src/present.rs +++ b/icd_shim/src/present.rs @@ -7,7 +7,17 @@ //! latch time for its vsync grid already, so the answer is the panel's, not a //! guess made from the release fence. //! -//! Only ids that were armed — seen in a present — are tracked, and both lists are +//! On gamescope's override route the answer is gamescope's own presentation +//! feedback, as in Mesa's Wayland WSI: the shim requests a +//! `wp_presentation_feedback` with every commit that carries a wait, and the +//! wait is answered on `presented` (at the presentation time it reports) or on +//! `discarded` (the frame will never be shown under that id). While a feedback +//! is pending for an id, the compositor's release and the two-period grace do +//! not answer it: they are the fallback for a compositor without the global, +//! and answering from them is a vblank late under FIFO, which a guest waiting +//! on every frame turns into two vblanks per frame. +//! +//! Only ids that were armed — seen in a present — are tracked, and the lists are //! bounded: a guest that marks frames and never waits cannot grow them. use core::ffi::c_void; @@ -37,6 +47,10 @@ struct Waits { /// Ids whose frame has been committed, with the commit time: the latest a /// wait on them is answered is two refresh periods after that. committed: Vec<(u64, u64)>, + /// Ids whose commit asked gamescope for presentation feedback that has not + /// arrived: only `presented` / `discarded` answers them (or, should it + /// never come, [`FEEDBACK_SAFETY_NS`] after the commit). + feedback: Vec, } /// Bound on both lists: the ids in flight are a handful of frames, so anything @@ -47,8 +61,78 @@ static WAITS: Mutex = Mutex::new(Waits { armed: Vec::new(), done: Vec::new(), committed: Vec::new(), + feedback: Vec::new(), }); +/// How long after its commit an id whose feedback never came is answered +/// anyway: far beyond any presentation gamescope makes, so it never answers +/// early, and well inside the guest's one-second bound. +const FEEDBACK_SAFETY_NS: u64 = 100_000_000; + +/// Presentation feedback outcomes, and waits answered by a fallback (release, +/// grace or the feedback safety), for the shim's ungated census. +static PRESENTED_N: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); +static DISCARDED_N: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); +static FALLBACK_N: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +/// `(presented, discarded, answered by a fallback)` so far. +pub(crate) fn feedback_counts() -> (usize, usize, usize) { + ( + PRESENTED_N.load(Ordering::Relaxed), + DISCARDED_N.load(Ordering::Relaxed), + FALLBACK_N.load(Ordering::Relaxed), + ) +} + +/// The commit carrying `id` asked gamescope for presentation feedback. Called +/// before that commit is sent, so no release or grace can answer it first. +pub(crate) fn expect_feedback(id: u64) { + if id == 0 { + return; + } + let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); + if w.feedback.len() >= CAP { + w.feedback.remove(0); + } + w.feedback.push(id); +} + +/// gamescope's `wp_presentation_feedback.presented` for the commit carrying +/// `id`, at `present_ns`. +pub(crate) fn feedback_presented(id: u64, present_ns: u64) { + PRESENTED_N.fetch_add(1, Ordering::Relaxed); + feedback_done(id, present_ns); +} + +/// gamescope's `wp_presentation_feedback.discarded` for the commit carrying +/// `id`: it will never be shown, and its wait is answered now. +pub(crate) fn feedback_discarded(id: u64, now_ns: u64) { + DISCARDED_N.fetch_add(1, Ordering::Relaxed); + feedback_done(id, now_ns); +} + +/// The feedback for `id` could not be requested after all: the release and the +/// grace answer it again. +pub(crate) fn cancel_feedback(id: u64) { + if id == 0 { + return; + } + let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); + w.feedback.retain(|&f| f != id); +} + +fn feedback_done(id: u64, ns: u64) { + { + let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); + w.feedback.retain(|&f| f != id); + } + record(id, ns); +} + +fn feedback_pending(w: &Waits, id: u64) -> bool { + w.feedback.contains(&id) +} + /// Two refresh periods after a commit, the frame has been latched whatever the /// compositor does with the buffer afterwards. fn commit_grace_ns() -> u64 { @@ -124,16 +208,20 @@ pub(crate) fn arm(id: u64, sc: usize) { w.armed.push((id, sc)); } -/// A completed present, reported by the daemon with the panel's latch time. +/// A completed present: presentation feedback, the daemon's latch report, a +/// commit under mailbox, or a fallback. An id no longer armed was answered +/// already and is left alone, so a second answer never queues a stale one. pub(crate) fn record(id: u64, latch_ns: u64) { if id == 0 { return; } let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); - if let Some(i) = w.armed.iter().position(|&(armed, _)| armed == id) { - w.armed.remove(i); - } + let Some(i) = w.armed.iter().position(|&(armed, _)| armed == id) else { + return; + }; + w.armed.remove(i); w.committed.retain(|&(c, _)| c != id); + w.feedback.retain(|&f| f != id); if w.done.len() >= CAP { w.done.remove(0); } @@ -161,8 +249,18 @@ fn note_outcome(id: u64, what: &str, t0: std::time::Instant) { /// The compositor released the frame's buffer: it has been taken into the /// output, and the next refresh shows it. Answers the wait when no latch report /// came first; a daemon that did report is left as the authority. +/// +/// Not while presentation feedback is pending for it: under FIFO the release of +/// a frame comes when the next one is latched, a vblank after it was shown. pub(crate) fn complete_on_release(id: u64, release_ns: u64) { - discard(id, release_ns); + let answers = { + let w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); + !feedback_pending(&w, id) && w.armed.iter().any(|&(armed, _)| armed == id) + }; + if answers { + FALLBACK_N.fetch_add(1, Ordering::Relaxed); + record(id, release_ns); + } } /// The frame carrying `id` will never reach the panel under that id: a newer @@ -223,16 +321,23 @@ pub(crate) fn wait(id: u64, sc: usize, timeout_ns: u64) -> bool { return true; } // A committed frame counts as latched two refresh periods after its - // commit, whether or not its buffer ever comes back. + // commit, whether or not its buffer ever comes back; one whose + // presentation feedback is pending only at the feedback safety. + let grace = if feedback_pending(&w, id) { + FEEDBACK_SAFETY_NS + } else { + commit_grace_ns() + }; let latch_by = w .committed .iter() .find(|&&(c, _)| c == id) - .map(|&(_, ns)| ns.saturating_add(commit_grace_ns())); + .map(|&(_, ns)| ns.saturating_add(grace)); if let Some(latch_ns) = latch_by { let now_ns = crate::proxy::monotonic_ns(); if now_ns >= latch_ns { drop(w); + FALLBACK_N.fetch_add(1, Ordering::Relaxed); record(id, latch_ns); let mut w2 = WAITS.lock().unwrap_or_else(|e| e.into_inner()); if let Some(i) = w2.done.iter().position(|&(done, _)| done == id) { @@ -427,6 +532,47 @@ mod tests { assert!(wait(201, 6, 0)); } + #[test] + fn presentation_feedback_answers_and_the_release_does_not_preempt_it() { + arm(401, 8); + expect_feedback(401); + committed(401, crate::proxy::monotonic_ns().saturating_sub(50_000_000), false); + // The release comes first (the next frame was latched): no answer. + complete_on_release(401, crate::proxy::monotonic_ns()); + // Nor does the two-period grace, 50 ms after the commit. + assert!(!wait(401, 8, 0)); + feedback_presented(401, 777); + assert!(wait(401, 8, 0)); + // A late duplicate from another path leaves nothing behind. + record(401, 1); + assert!(!WAITS.lock().unwrap().done.iter().any(|&(id, _)| id == 401)); + } + + #[test] + fn a_discarded_frame_is_answered() { + arm(501, 9); + expect_feedback(501); + committed(501, crate::proxy::monotonic_ns(), false); + feedback_discarded(501, crate::proxy::monotonic_ns()); + assert!(wait(501, 9, 0)); + } + + #[test] + fn a_feedback_that_never_comes_is_answered_by_the_safety() { + arm(601, 10); + expect_feedback(601); + committed(601, crate::proxy::monotonic_ns().saturating_sub(FEEDBACK_SAFETY_NS + 1), false); + assert!(wait(601, 10, 0)); + } + + #[test] + fn without_feedback_the_release_still_answers() { + arm(701, 11); + committed(701, crate::proxy::monotonic_ns(), false); + complete_on_release(701, crate::proxy::monotonic_ns()); + assert!(wait(701, 11, 0)); + } + #[test] fn a_replaced_frame_is_answered_without_a_commit() { // The mailbox drops a frame before it is ever committed: recording it diff --git a/icd_shim/src/proxy.rs b/icd_shim/src/proxy.rs index 6593c7d..2210714 100644 --- a/icd_shim/src/proxy.rs +++ b/icd_shim/src/proxy.rs @@ -38,7 +38,7 @@ use crate::wlproto::{self, wl_display, wl_proxy}; use gndrm_uapi::wire::ScanoutLayout; use std::collections::{HashMap, VecDeque}; use std::ffi::{c_char, c_void, CStr, CString}; -use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU32, AtomicU64, AtomicUsize, Ordering}; // First-N diagnostics for the present pipeline handoffs (acquire -> // present -> worker commit -> compositor release). Bounded counters, so @@ -1727,6 +1727,9 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { // wait is answered here: a guest that knew its real mode would not // wait at all, and holding it couples the guest's frame-latency loop // to a refresh period it does not pace by. + // The present id this commit asks gamescope's presentation feedback for, + // 0 when none (see `crate::present`). + let mut feedback_id = 0u64; if (*s).wait_id != 0 { // A guest that asked for mailbox or immediate does not wait, and // neither would one whose frames go out as mailbox (frame @@ -1740,6 +1743,17 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { } else { (*s).wait_id }; + // A FIFO wait on the override route is answered by gamescope's + // presentation feedback, as in Mesa's Wayland WSI. Registered + // before the commit, so neither the release nor the grace can + // answer it first. + if !at_commit + && !(*(*sc).surf).swapchain_obj.is_null() + && !PRESENTATION.load(Ordering::Acquire).is_null() + { + feedback_id = id; + crate::present::expect_feedback(id); + } crate::present::committed(id, monotonic_ns(), at_commit); } // X11 route: the window carries its own content, so the frame is @@ -1776,6 +1790,7 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { crate::logf!(gndrm_obf::obf!( "proxy: wl_buffer creation failed — dropping frame" )); + crate::present::cancel_feedback(feedback_id); let _g = (*sc).shared.lock().unwrap(); (*s).in_flight = false; (*sc).cv_free.notify_all(); @@ -1807,6 +1822,23 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { (*sc).extent.width as i32, (*sc).extent.height as i32, ); + if feedback_id != 0 { + let fb = wlproto::presentation_feedback( + PRESENTATION.load(Ordering::Acquire), + parent, + ); + if fb.is_null() { + crate::present::cancel_feedback(feedback_id); + } else { + // The id rides as the listener's data: no allocation to + // free when the event spends the object. + wlproto::wl_proxy_add_listener( + fb, + FEEDBACK_LISTENER.as_ptr() as *mut _, + feedback_id as usize as *mut c_void, + ); + } + } wlproto::surface_commit(parent); wlproto::wl_display_flush((*(*sc).surf).dpy) }; @@ -6773,6 +6805,19 @@ extern "C" fn registry_global( } else if iface == b"zwp_linux_dmabuf_v1" && version >= 3 { (*w).dmabuf = wlproto::registry_bind(reg, id, &wlproto::ZWP_LINUX_DMABUF_V1_INTERFACE, 3); + } else if iface == b"wp_presentation" && !use_guest_surface() { + // gamescope's presentation feedback, the present-wait answer on the + // override route (see `crate::present`). Bound only there, like the + // swapchain factory. + let p = wlproto::registry_bind(reg, id, &wlproto::WP_PRESENTATION_INTERFACE, 1); + if !p.is_null() { + wlproto::wl_proxy_add_listener( + p, + PRESENTATION_LISTENER.as_ptr() as *mut _, + ptr::null_mut(), + ); + PRESENTATION.store(p, Ordering::Release); + } } else if iface == b"gamescope_swapchain_factory_v2" && !use_guest_surface() { // Capability switch (`GN_VKSHIM_GUEST_SURFACE`): the // gamescope_swapchain_factory_v2 global is bound only while the @@ -6806,6 +6851,77 @@ static REGISTRY_LISTENER: [RawListenerFn; 2] = [ ) }, ]; +/// The bound `wp_presentation`, null while the compositor offers none (or the +/// guest-surface route is in use): then present waits keep the release and grace +/// answers. +static PRESENTATION: AtomicPtr = AtomicPtr::new(ptr::null_mut()); +/// The clock `wp_presentation` stamps with (`CLOCK_MONOTONIC` = 1 on gamescope); +/// `u32::MAX` until announced. +static PRESENTATION_CLOCK: AtomicU32 = AtomicU32::new(u32::MAX); + +extern "C" fn presentation_clock_id(_data: *mut c_void, _p: *mut wl_proxy, clk: u32) { + PRESENTATION_CLOCK.store(clk, Ordering::Relaxed); +} + +static PRESENTATION_LISTENER: [RawListenerFn; 1] = [unsafe { + std::mem::transmute::( + presentation_clock_id, + ) +}]; + +extern "C" fn feedback_sync_output(_data: *mut c_void, _fb: *mut wl_proxy, _output: *mut wl_proxy) {} + +/// `presented`: the frame carrying the present id in `data` was latched for +/// the refresh at `tv`. The feedback object is spent either way. +#[allow(clippy::too_many_arguments)] // the event's own arity +extern "C" fn feedback_presented( + data: *mut c_void, + fb: *mut wl_proxy, + tv_sec_hi: u32, + tv_sec_lo: u32, + tv_nsec: u32, + _refresh: u32, + _seq_hi: u32, + _seq_lo: u32, + _flags: u32, +) { + let id = data as usize as u64; + let ns = if PRESENTATION_CLOCK.load(Ordering::Relaxed) == libc::CLOCK_MONOTONIC as u32 { + ((u64::from(tv_sec_hi) << 32 | u64::from(tv_sec_lo)) * 1_000_000_000) + .saturating_add(u64::from(tv_nsec)) + } else { + monotonic_ns() + }; + crate::present::feedback_presented(id, ns); + unsafe { wlproto::wl_proxy_destroy(fb) }; +} + +/// `discarded`: the frame carrying the present id in `data` will never be +/// shown; its wait is answered now. +extern "C" fn feedback_discarded(data: *mut c_void, fb: *mut wl_proxy) { + crate::present::feedback_discarded(data as usize as u64, monotonic_ns()); + unsafe { wlproto::wl_proxy_destroy(fb) }; +} + +static FEEDBACK_LISTENER: [RawListenerFn; 3] = [ + unsafe { + std::mem::transmute::( + feedback_sync_output, + ) + }, + unsafe { + std::mem::transmute::< + extern "C" fn(*mut c_void, *mut wl_proxy, u32, u32, u32, u32, u32, u32, u32), + RawListenerFn, + >(feedback_presented) + }, + unsafe { + std::mem::transmute::( + feedback_discarded, + ) + }, +]; + static BUFFER_LISTENER: [RawListenerFn; 1] = [unsafe { std::mem::transmute::( on_buffer_release, @@ -9161,8 +9277,9 @@ unsafe fn mode_census(sc: *mut GnSwapchain) { return; } let route = unsafe { !(*(*sc).surf).swapchain_obj.is_null() }; + let (presented, discarded, fallback) = crate::present::feedback_counts(); crate::logi!(gndrm_obf::obf!( - "present mode: declared={} requested={} route={} cap={} lsfg={} acquires={} commits={} declared_fifo={} declared_other={} releases={}", + "present mode: declared={} requested={} route={} cap={} lsfg={} acquires={} commits={} declared_fifo={} declared_other={} releases={} feedback={} presented={} discarded={} fallback={}", mode_name(unsafe { live_present_mode(sc) }), mode_name(unsafe { (*sc).present_mode }), if route { "gamescope" } else { "x11" }, @@ -9173,6 +9290,10 @@ unsafe fn mode_census(sc: *mut GnSwapchain) { DECL_FIFO_N.load(Ordering::Relaxed), DECL_OTHER_N.load(Ordering::Relaxed), REL_ALL_N.load(Ordering::Relaxed), + if PRESENTATION.load(Ordering::Relaxed).is_null() { "off" } else { "on" }, + presented, + discarded, + fallback, )); } diff --git a/icd_shim/src/wlproto.rs b/icd_shim/src/wlproto.rs index 9efd10f..c480e4c 100644 --- a/icd_shim/src/wlproto.rs +++ b/icd_shim/src/wlproto.rs @@ -109,6 +109,7 @@ extern "C" { pub static wl_subcompositor_interface: wl_interface; pub static wl_surface_interface: wl_interface; pub static wl_buffer_interface: wl_interface; + pub static wl_output_interface: wl_interface; } // ----------------------------------------------------------- core requests @@ -361,6 +362,100 @@ pub unsafe fn swapchain_feedback( wl_proxy_marshal_array(sc, 2, args.as_mut_ptr()); } +// ------------------------------------------------- wp_presentation statics +// Transliterated from presentation-time.xml (stable, version 1, the version +// gamescope advertises). gamescope sends `presented` for the commit it latched, +// at that latch, and `discarded` for a commit it replaced or never showed. + +#[allow(clippy::missing_transmute_annotations)] +mod presentation_statics { + use super::{wl_interface, wl_message}; + use std::os::raw::c_char; + + const fn cs(s: &'static [u8]) -> *const c_char { + s.as_ptr() as *const c_char + } + + struct SyncIfaces([*const wl_interface; 3]); + unsafe impl Sync for SyncIfaces {} + // feedback args: wl_surface (object), wp_presentation_feedback (new_id); + // sync_output's arg: wl_output. + static TYPES: SyncIfaces = SyncIfaces([ + &raw const super::wl_surface_interface, + &raw const WP_PRESENTATION_FEEDBACK_INTERFACE, + &raw const super::wl_output_interface, + ]); + + static PRESENTATION_REQUESTS: [wl_message; 2] = [ + wl_message { + name: cs(b"destroy\0"), + signature: cs(b"\0"), + types: std::ptr::null(), + }, + wl_message { + name: cs(b"feedback\0"), + signature: cs(b"on\0"), + types: TYPES.0.as_ptr(), + }, + ]; + + static PRESENTATION_EVENTS: [wl_message; 1] = [wl_message { + name: cs(b"clock_id\0"), + signature: cs(b"u\0"), + types: std::ptr::null(), + }]; + + pub static WP_PRESENTATION_INTERFACE: wl_interface = wl_interface { + name: cs(b"wp_presentation\0"), + version: 1, + method_count: 2, + methods: PRESENTATION_REQUESTS.as_ptr(), + event_count: 1, + events: PRESENTATION_EVENTS.as_ptr(), + }; + + static FEEDBACK_EVENTS: [wl_message; 3] = [ + wl_message { + name: cs(b"sync_output\0"), + signature: cs(b"o\0"), + types: unsafe { TYPES.0.as_ptr().add(2) }, + }, + wl_message { + name: cs(b"presented\0"), + signature: cs(b"uuuuuuu\0"), + types: std::ptr::null(), + }, + wl_message { + name: cs(b"discarded\0"), + signature: cs(b"\0"), + types: std::ptr::null(), + }, + ]; + + pub static WP_PRESENTATION_FEEDBACK_INTERFACE: wl_interface = wl_interface { + name: cs(b"wp_presentation_feedback\0"), + version: 1, + method_count: 0, + methods: std::ptr::null(), + event_count: 3, + events: FEEDBACK_EVENTS.as_ptr(), + }; +} + +pub use presentation_statics::{WP_PRESENTATION_FEEDBACK_INTERFACE, WP_PRESENTATION_INTERFACE}; + +/// wp_presentation.feedback (opcode 1): surface, new_id +/// wp_presentation_feedback. Applies to the surface's next commit. +pub unsafe fn presentation_feedback(pres: *mut wl_proxy, surface: *mut wl_proxy) -> *mut wl_proxy { + let mut args = [wl_argument { o: surface }, wl_argument { u: 0 }]; + wl_proxy_marshal_array_constructor( + pres, + 1, + args.as_mut_ptr(), + &WP_PRESENTATION_FEEDBACK_INTERFACE, + ) +} + // ------------------------------------------- zwp_linux_dmabuf_v1 statics // Transliterated from the wayland-scanner output (linux-dmabuf-v1); only // the members a version-3 bind can speak are referenced at runtime. From ee1efa2ec416763f96d161920f12c2418de00714 Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Mon, 2 Nov 2026 11:56:57 +0530 Subject: [PATCH 4/7] docs: the vsync note's measured period serves frame generation --- docs/15-fps-limiter-pacing-and-vsync.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/15-fps-limiter-pacing-and-vsync.md b/docs/15-fps-limiter-pacing-and-vsync.md index 3bba450..f2c80f1 100644 --- a/docs/15-fps-limiter-pacing-and-vsync.md +++ b/docs/15-fps-limiter-pacing-and-vsync.md @@ -125,8 +125,8 @@ that rate is the display's. tracked in `vsync.rs`, and a `VsyncNote` (`anchor_ns, period_ns, lead_ns`) pushed whenever its phase moves by more than 300 µs or its period by more than 1 µs, and once on subscribe. Its period is the one measured from Choreographer while that is locked - (the mode's advertised rate otherwise), so the shim's vblank gate and frame - generation extrapolate the display's real rate. Its anchor is also the + (the mode's advertised rate otherwise), so frame generation extrapolates + the display's real rate. Its anchor is also the flip timeline's phase where Choreographer gives no deadline, and its fallback. - 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. From 1b16523cfef7de8d7a8b093f39cd3ef9c20e068c Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Mon, 2 Nov 2026 14:02:28 +0530 Subject: [PATCH 5/7] daemon: hold the vsync source to the NDK's contract and the mode's rate Review fixes for the Choreographer vsync source: - AChoreographer_postVsyncCallback and AChoreographer_postFrameCallback64 return void (, NDK 27.3). They were declared to return an int and the thread exited on a non-zero value, which was whatever the callee left in w0. Both are now declared `-> ()` with no check. - The thread no longer has an exit after it publishes its looper. A callback that has not come within a second is posted once more (logged once), and the flip events fall back to the latch grid on their own while the vsync is stale. The looper is held with ALooper_acquire for the life of the process, and any end of the thread (a panic included) clears LOOPER and SPAWNED, so start() never wakes a freed looper and the next session spawns a new thread. - The fit is bounded by the mode's advertised period. Steps are counted against it and a fit more than 2 % from it is rejected (the previous period stays, logged once per run): a callback thread starved into delivering every second vsync counts two periods per step instead of locking the timeline at twice the period. A new advertised mode (nativeSetPanelRefreshHz on a display change) starts the fit again. With no mode reported the old spacing rules apply. - The callbacks stop while a session is suspended and resume with it (nativeInjectorSetSuspended). - GNDRM_GS_LATCH_LEAD_US is clamped to half a period: a lead of a period or more would stamp every event before the previous latch. --- modules/gndrm-android/src/choreographer.rs | 94 +++++++++--- modules/gndrm-android/src/lib.rs | 12 +- modules/gndrm-daemon/src/vblank.rs | 162 +++++++++++++++++++-- 3 files changed, 238 insertions(+), 30 deletions(-) diff --git a/modules/gndrm-android/src/choreographer.rs b/modules/gndrm-android/src/choreographer.rs index 4e4cf82..d8abb32 100644 --- a/modules/gndrm-android/src/choreographer.rs +++ b/modules/gndrm-android/src/choreographer.rs @@ -28,15 +28,19 @@ type FrameData = c_void; type FnGetInstance = unsafe extern "C" fn() -> *mut Choreographer; type VsyncCallback = unsafe extern "C" fn(data: *const FrameData, user: *mut c_void); +// Both post functions return `void` (``): there is no +// status to read, so a callback that never comes is caught by the poll timeout +// in `run`, not by a return value. type FnPostVsyncCallback = - unsafe extern "C" fn(ch: *mut Choreographer, cb: VsyncCallback, user: *mut c_void) -> c_int; + unsafe extern "C" fn(ch: *mut Choreographer, cb: VsyncCallback, user: *mut c_void); type FrameCallback64 = unsafe extern "C" fn(frame_time_nanos: i64, user: *mut c_void); type FnPostFrameCallback64 = - unsafe extern "C" fn(ch: *mut Choreographer, cb: FrameCallback64, user: *mut c_void) -> c_int; + unsafe extern "C" fn(ch: *mut Choreographer, cb: FrameCallback64, user: *mut c_void); type FnGetFrameTime = unsafe extern "C" fn(data: *const FrameData) -> i64; type FnGetPreferredIndex = unsafe extern "C" fn(data: *const FrameData) -> usize; type FnGetDeadline = unsafe extern "C" fn(data: *const FrameData, index: usize) -> i64; type FnLooperWake = unsafe extern "C" fn(looper: *mut c_void); +type FnLooperAcquire = unsafe extern "C" fn(looper: *mut c_void); /// API 33's per-vsync data accessors. #[derive(Clone, Copy)] @@ -55,6 +59,7 @@ struct Api { prepare: crate::ffi::FnALooperPrepare, poll_once: crate::ffi::FnALooperPollOnce, wake: FnLooperWake, + acquire: FnLooperAcquire, } impl Api { @@ -107,6 +112,7 @@ impl Api { prepare: sym(handle, c"ALooper_prepare")?, poll_once: sym(handle, c"ALooper_pollOnce")?, wake: sym(handle, c"ALooper_wake")?, + acquire: sym(handle, c"ALooper_acquire")?, }) } } @@ -118,8 +124,17 @@ static WANTED: AtomicBool = AtomicBool::new(false); static SPAWNED: AtomicBool = AtomicBool::new(false); /// Whether a callback is posted and has not run yet. Touched on the thread only. static OUTSTANDING: AtomicBool = AtomicBool::new(false); -/// The thread's looper, to wake it when a session starts; null until prepared. +/// Set by every callback, cleared by the thread: a callback arrived. +static FIRED: AtomicBool = AtomicBool::new(false); +/// The thread's looper, to wake it when a session starts; null until prepared +/// and again once the thread has ended. The thread takes a reference on it +/// (`ALooper_acquire`) that is never dropped, so a `start` that read the pointer +/// just before the thread ended still wakes a live object, not a freed one. static LOOPER: AtomicPtr = AtomicPtr::new(std::ptr::null_mut()); +/// How long a posted callback may take before it is posted again: far beyond +/// any refresh period, so only a callback that was lost (or a display that is +/// off) reaches it. +const CALLBACK_TIMEOUT_MS: c_int = 1000; /// The resolved table, set once by the thread before its first post. static API: std::sync::OnceLock = std::sync::OnceLock::new(); @@ -129,8 +144,8 @@ pub(crate) fn start() { if SPAWNED.swap(true, Ordering::AcqRel) { let looper = LOOPER.load(Ordering::Acquire); if let (false, Some(api)) = (looper.is_null(), API.get()) { - // SAFETY: the looper the thread prepared; it lives as long as the - // thread, which never exits once it holds one. + // SAFETY: the looper the thread prepared, kept alive by the + // reference the thread took on it (see `LOOPER`). unsafe { (api.wake)(looper) }; } return; @@ -139,6 +154,10 @@ pub(crate) fn start() { .name("gndrm-vsync".into()) .spawn(|| { let _ = std::panic::catch_unwind(run); + // However the thread ended, the next session starts a new one. + LOOPER.store(std::ptr::null_mut(), Ordering::Release); + OUTSTANDING.store(false, Ordering::Relaxed); + SPAWNED.store(false, Ordering::Release); }); if spawned.is_err() { SPAWNED.store(false, Ordering::Release); @@ -172,44 +191,71 @@ fn run() { ); return; } + // SAFETY: the looper this thread prepared; the reference is never + // released (see `LOOPER`). + unsafe { (api.acquire)(looper) }; LOOPER.store(looper, Ordering::Release); + // Whether a callback was posted again after a timeout and none has come + // since: a callback that never comes is posted again once, not every + // second, so a display that is off cannot pile them up. + let mut reposted = false; + let mut logged_lost = false; loop { + if FIRED.swap(false, Ordering::Relaxed) { + reposted = false; + } if WANTED.load(Ordering::Acquire) && !OUTSTANDING.load(Ordering::Relaxed) { // SAFETY: `ch` is this thread's Choreographer; the callbacks are // `extern "C"` functions that live for the process and take no // user pointer. - let rc = unsafe { + unsafe { match (api.vsync, api.frame64) { (Some(v), _) => (v.post)(ch, on_vsync, std::ptr::null_mut()), (None, Some(post)) => post(ch, on_frame, std::ptr::null_mut()), - (None, None) => -1, + (None, None) => {} } - }; - if rc != 0 { - crate::log_always!( - "{}", - gndrm_obf::obf!("vsync: posting a Choreographer callback failed ({rc})") - ); - return; } OUTSTANDING.store(true, Ordering::Relaxed); } // SAFETY: this thread's looper; null out-pointers are allowed. Returns - // after a callback ran or a wake. - unsafe { + // after a callback ran, a wake, or the timeout. + let r = unsafe { (api.poll_once)( - -1, + if OUTSTANDING.load(Ordering::Relaxed) { CALLBACK_TIMEOUT_MS } else { -1 }, std::ptr::null_mut(), std::ptr::null_mut(), std::ptr::null_mut(), ) }; + const ALOOPER_POLL_TIMEOUT: c_int = -3; + if r == ALOOPER_POLL_TIMEOUT + && OUTSTANDING.load(Ordering::Relaxed) + && WANTED.load(Ordering::Acquire) + && !reposted + { + // The callback was not delivered within a second. The flip + // events fall back to the latch grid by themselves once the + // vsync goes stale; post once more so the stream resumes if the + // callback was lost rather than the display stopped. + if !logged_lost { + logged_lost = true; + crate::log_always!( + "{}", + gndrm_obf::obf!( + "vsync: no Choreographer callback for 1 s; posting again, flip events follow the latch grid meanwhile" + ) + ); + } + OUTSTANDING.store(false, Ordering::Relaxed); + reposted = true; + } } } /// API 33+: one vsync with its frame timelines. unsafe extern "C" fn on_vsync(data: *const FrameData, _user: *mut c_void) { OUTSTANDING.store(false, Ordering::Relaxed); + FIRED.store(true, Ordering::Relaxed); let _ = std::panic::catch_unwind(|| { let Some(v) = API.get().and_then(|a| a.vsync) else { return; @@ -229,6 +275,7 @@ unsafe extern "C" fn on_vsync(data: *const FrameData, _user: *mut c_void) { /// API 29-32: the vsync time only. unsafe extern "C" fn on_frame(frame_time_nanos: i64, _user: *mut c_void) { OUTSTANDING.store(false, Ordering::Relaxed); + FIRED.store(true, Ordering::Relaxed); let _ = std::panic::catch_unwind(|| { if frame_time_nanos > 0 { feed(frame_time_nanos as u64, None); @@ -240,6 +287,19 @@ fn feed(vsync_ns: u64, deadline_ns: Option) { if let Some(lock) = vblank::note_choreographer_vsync(vsync_ns, deadline_ns) { log_lock(&lock); } + // Once per run of rejected fits, so not a per-frame line. + if let Some((fit, mode)) = vblank::take_rejected_fit() { + crate::log_always!( + "{}", + gndrm_obf::obf!( + "vsync: measured period {}.{:03}us is more than 2% from the mode's {}.{:03}us; kept the previous period", + fit / 1000, + fit % 1000, + mode / 1000, + mode % 1000 + ) + ); + } } /// One line per lock: the first, and the first after each rate change. Not a diff --git a/modules/gndrm-android/src/lib.rs b/modules/gndrm-android/src/lib.rs index de79a49..baa1f4f 100644 --- a/modules/gndrm-android/src/lib.rs +++ b/modules/gndrm-android/src/lib.rs @@ -1540,7 +1540,17 @@ pub extern "system" fn Java_app_gamenative_display_GameNativeDrm_nativeInjectorS _class: JClass, suspended: jboolean, ) { - let _ = std::panic::catch_unwind(|| injector::set_suspended(suspended != 0)); + let _ = std::panic::catch_unwind(|| { + let suspended = suspended != 0; + injector::set_suspended(suspended); + // A suspended session draws nothing: no vsync callbacks for it. Resumed, + // they come back for a session that is still running. + if suspended { + choreographer::stop(); + } else if RUNTIME.lock().is_ok_and(|rt| rt.is_some()) { + choreographer::start(); + } + }); } /// `GameNativeDrm.nativeGamescopeCommand(command)` — run one allowlisted diff --git a/modules/gndrm-daemon/src/vblank.rs b/modules/gndrm-daemon/src/vblank.rs index ba9b512..62e38fa 100644 --- a/modules/gndrm-daemon/src/vblank.rs +++ b/modules/gndrm-daemon/src/vblank.rs @@ -39,6 +39,9 @@ const STALE_NS: u64 = 1_000_000_000; const MAX_PERIOD_NS: u64 = 100_000_000; /// The fastest (1 kHz): anything closer together is a duplicate callback. const MIN_PERIOD_NS: u64 = 1_000_000; +/// How far a fitted period may sit from the mode's advertised one (2 %), in +/// per mille: a "60 Hz" panel latches at 59.97, never at 30 or 120. +const MODE_TOLERANCE_PER_MILLE: u64 = 20; /// Recent display vsyncs and the period fitted through them. /// @@ -57,6 +60,15 @@ pub struct VsyncRing { /// Whether `period_ns` is a fit over [`MIN_LOCK`] vsyncs or more of the /// current rate. Survives a gap in the callbacks, not a rate change. locked: bool, + /// The mode's advertised period, 0 when unknown. When known, the steps are + /// counted against it and a fit outside [`MODE_TOLERANCE_PER_MILLE`] of it + /// is rejected: a callback thread starved for a stretch delivers every + /// second vsync, which counts as two steps of the mode's period instead of + /// one step of twice it. + mode_ns: u64, + /// A fit is being rejected; set on the first rejection of a run, so it is + /// reported once. + rejecting: bool, } /// What one [`VsyncRing::push`] changed. @@ -67,6 +79,9 @@ pub enum RingEvent { /// The ring locked onto a period it was not locked onto before: the first /// lock, or the first after a rate change. Locked(u64), + /// A fit this far from the mode's period was rejected; the period stays. + /// Reported on the first rejection of a run. + Rejected(u64), } impl VsyncRing { @@ -76,6 +91,24 @@ impl VsyncRing { self.locked.then_some(self.period_ns) } + /// The mode's advertised period (0 when unknown). A change starts the fit + /// again from it: the app reports the new mode when the display changes. + pub fn set_mode(&mut self, mode_ns: u64) { + if mode_ns == self.mode_ns { + return; + } + self.mode_ns = mode_ns; + self.samples.clear(); + self.period_ns = mode_ns; + self.locked = false; + self.rejecting = false; + } + + fn within_mode(&self, period: u64) -> bool { + self.mode_ns == 0 + || period.abs_diff(self.mode_ns) * 1000 <= self.mode_ns * MODE_TOLERANCE_PER_MILLE + } + /// Fold in one vsync timestamp (`CLOCK_MONOTONIC` ns). pub fn push(&mut self, t_ns: u64) -> RingEvent { let Some(&(last_t, last_n)) = self.samples.back() else { @@ -105,6 +138,12 @@ impl VsyncRing { let k = (step + period / 2) / period; let residual = step.abs_diff(k * period); if k == 0 || residual > period / 4 { + if self.mode_ns != 0 { + // With the mode known, a rate change arrives as a new mode + // (`set_mode`); this is a stray callback. Count from here. + self.restart(t_ns); + return RingEvent::None; + } // The step is not a whole number of periods: the display changed // rate. Seed from this step and lock again. self.reseed(t_ns, step); @@ -117,7 +156,8 @@ impl VsyncRing { // A full ring without one single-vsync step is counting a rate at a // multiple of the period it holds (the display halved its rate): take // the shortest step instead. - if self.samples.len() == RING + if self.mode_ns == 0 + && self.samples.len() == RING && self .samples .iter() @@ -138,6 +178,16 @@ impl VsyncRing { return RingEvent::None; } let fitted = self.fit(); + if !self.within_mode(fitted) { + let first = !self.rejecting; + self.rejecting = true; + return if first { + RingEvent::Rejected(fitted) + } else { + RingEvent::None + }; + } + self.rejecting = false; let was = self.locked.then_some(self.period_ns); self.period_ns = fitted; self.locked = true; @@ -310,11 +360,13 @@ impl LeadLoop { } } - /// The lead in force, clamped to a third of `period`. + /// The lead in force, clamped to a third of `period` (half of it for the + /// fixed override: a lead of a period or more would stamp every event + /// before the previous latch). #[must_use] pub fn lead_ns(&self, period: u64) -> u64 { if self.fixed { - self.lead_ns + self.lead_ns.min(period / 2) } else { self.lead_ns.min(period / 3) } @@ -389,6 +441,8 @@ pub struct DisplayVsync { latch_after_deadline: Option, lead: LeadLoop, window: Window, + /// A fit the ring just rejected, for the log (taken once). + rejected_fit: Option, } /// What [`DisplayVsync::note_vsync`] reports back for the log. @@ -405,8 +459,17 @@ impl DisplayVsync { /// One Choreographer vsync: its timestamp and, on API 33+, the preferred /// frame timeline's deadline. Returns the lock to report when the ring /// locked onto a period. - pub fn note_vsync(&mut self, vsync_ns: u64, deadline_ns: Option) -> Option { + pub fn note_vsync( + &mut self, + vsync_ns: u64, + deadline_ns: Option, + mode_ns: u64, + ) -> Option { + self.ring.set_mode(mode_ns); let ev = self.ring.push(vsync_ns); + if let RingEvent::Rejected(p) = ev { + self.rejected_fit = Some(p); + } self.chor = Some((vsync_ns, deadline_ns)); self.window.vsyncs += 1; match ev { @@ -420,7 +483,7 @@ impl DisplayVsync { latch_offset_ns, }) } - RingEvent::None => None, + RingEvent::None | RingEvent::Rejected(_) => None, } } @@ -584,8 +647,19 @@ pub fn note_publish_latch(applied_ns: u64, latch_ns: u64) -> Option /// Feed one display vsync from the platform's vsync source (see /// [`DisplayVsync::note_vsync`]). +/// +/// The mode's advertised period bounds the fit (see [`VsyncRing::set_mode`]). pub fn note_choreographer_vsync(vsync_ns: u64, deadline_ns: Option) -> Option { - with_display(|d| d.note_vsync(vsync_ns, deadline_ns)) + let mhz = u64::from(crate::advertised_refresh_mhz()); + let mode_ns = 1_000_000_000_000u64.checked_div(mhz).unwrap_or(0); + with_display(|d| d.note_vsync(vsync_ns, deadline_ns, mode_ns)) +} + +/// A fit the ring rejected as too far from the mode's period since the last +/// call, with the mode's period: `(fit, mode)`. Reported once per run of +/// rejections. +pub fn take_rejected_fit() -> Option<(u64, u64)> { + with_display(|d| d.rejected_fit.take().map(|p| (p, d.ring.mode_ns))) } /// Feed the latch grid's current anchor (see [`DisplayVsync::note_latch`]). @@ -775,7 +849,7 @@ mod tests { let mut d = DisplayVsync::default(); let mut t = T0; for _ in 0..MIN_LOCK { - d.note_vsync(t, deadline_lead.map(|l| t + l)); + d.note_vsync(t, deadline_lead.map(|l| t + l), 0); t += P; } (d, t - P) @@ -828,7 +902,7 @@ mod tests { d.note_latch(T0 + 500_000, 16_666_666); let mut lock = None; for _ in 0..MIN_LOCK { - if let Some(l) = d.note_vsync(t, Some(t + 2 * P)) { + if let Some(l) = d.note_vsync(t, Some(t + 2 * P), 0) { lock = Some(l); } t += P; @@ -963,14 +1037,14 @@ mod tests { let mut tl = VblankTimeline::default(); let mut t = T0; for _ in 0..MIN_LOCK { - d.note_vsync(t, Some(t + P)); + d.note_vsync(t, Some(t + P), 0); t += P; } let mut stamp = tl.claim(t, d.grid(t), 16_666_666); let (mut late, mut counted) = (0u64, 0u64); let mut margins = Vec::new(); for k in 0..frames { - d.note_vsync(t, Some(t + P)); + d.note_vsync(t, Some(t + P), 0); t += P; // A spread of -spread..+spread, in a fixed pseudo-random order. let jitter = ((k * 7919) % 101) as i64 * 2 * spread / 100 - spread; @@ -1024,7 +1098,7 @@ mod tests { d.fix_lead(0); let mut t = T0; for _ in 0..MIN_LOCK { - d.note_vsync(t, Some(t + P)); + d.note_vsync(t, Some(t + P), 0); t += P; } // `SurfaceFlinger` takes every publish 1.5 ms after the deadline. @@ -1046,7 +1120,7 @@ mod tests { let mut t = T0; let mut reports = Vec::new(); for k in 0..1_300u64 { - d.note_vsync(t, Some(t + P)); + d.note_vsync(t, Some(t + P), 0); // Every 60th vsync carries no publish. if k > MIN_LOCK as u64 && k % 60 != 0 { let latch = t + P; @@ -1065,4 +1139,68 @@ mod tests { assert!(r.vsyncs > r.publishes, "{r:?}"); assert!((590..=600).contains(&r.vsyncs), "{r:?}"); } + + #[test] + fn a_starved_thread_cannot_halve_the_rate_the_mode_names() { + // 120 Hz mode; the callback thread delivers only every second vsync + // for 64 straight vsyncs. Each step counts two of the mode's periods, + // and the period stays 8.33 ms. + let p120 = 8_333_333u64; + let mut r = VsyncRing::default(); + r.set_mode(p120); + let mut t = T0; + for _ in 0..RING { + r.push(t); + t += p120; + } + assert_eq!(r.period_ns(), Some(p120)); + for _ in 0..2 * RING { + r.push(t); + t += 2 * p120; + } + assert_eq!(r.period_ns(), Some(p120)); + } + + #[test] + fn a_fit_outside_the_mode_band_is_rejected_once() { + // The mode says 60 Hz; the callbacks come at 16.0 ms (4 % off). + let mut r = VsyncRing::default(); + r.set_mode(16_666_667); + let mut events = Vec::new(); + for i in 0..64u64 { + let e = r.push(T0 + i * 16_000_000); + if e != RingEvent::None { + events.push(e); + } + } + assert_eq!(events, vec![RingEvent::Rejected(16_000_000)]); + assert_eq!(r.period_ns(), None); + } + + #[test] + fn a_new_mode_starts_the_fit_again() { + let mut r = VsyncRing::default(); + r.set_mode(16_666_667); + let mut t = T0; + for _ in 0..RING { + r.push(t); + t += P; + } + assert_eq!(r.period_ns(), Some(P)); + r.set_mode(8_333_333); + assert_eq!(r.period_ns(), None); + let mut locked = None; + for _ in 0..RING { + if let RingEvent::Locked(p) = r.push(t) { + locked = Some(p); + } + t += 8_337_836; + } + assert_eq!(locked, Some(8_337_836)); + } + + #[test] + fn the_fixed_lead_is_clamped_to_half_a_period() { + assert_eq!(LeadLoop::fixed(50_000_000).lead_ns(P), P / 2); + } } From e03a8c6116906968adf04e18dba246329c1f390d Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Mon, 2 Nov 2026 14:02:28 +0530 Subject: [PATCH 6/7] shim: key present-wait state by swapchain; no declaration without a gamescope swapchain Review fixes for the present-wait and declaration paths: - Present ids restart at 1 on every swapchain a guest creates (DXVK recreates on a resize or a mode change), and the pending feedback, the commit list, the answers and the release path were keyed by id alone, so an old swapchain's late `discarded` (its surface torn down) or release could answer the new swapchain's frame early. Every entry is now keyed by (id, swapchain); the feedback listener carries a per-request token instead of the id; destroying a swapchain drops its pending feedback, commits and unconsumed answers. The daemon's latch report, which names an id only, answers the armed frame with that id. - commit_real and present_sync_commit declared the present mode on gamescope_swapchain unconditionally; on the guest-surface route (or a gamescope without the swapchain factory) that proxy is null. The declaration is now skipped there. --- icd_shim/src/present.rs | 266 ++++++++++++++++++++++++++++------------ icd_shim/src/proxy.rs | 56 +++++---- 2 files changed, 215 insertions(+), 107 deletions(-) diff --git a/icd_shim/src/present.rs b/icd_shim/src/present.rs index e4b4551..95c6ed7 100644 --- a/icd_shim/src/present.rs +++ b/icd_shim/src/present.rs @@ -41,18 +41,30 @@ pub const EXT_PRESENT_WAIT: &core::ffi::CStr = c"VK_KHR_present_wait"; /// Every present id armed by a present and not yet completed, oldest first, and /// the completions the daemon has reported and no wait has consumed. +/// +/// Every entry is keyed by the id *and* its swapchain: present ids restart at 1 +/// on every swapchain a guest creates (DXVK recreates on a resize or a mode +/// change), so an id alone would let the old swapchain's late answer complete +/// the new one's frame. struct Waits { + /// `(id, swapchain)`. armed: Vec<(u64, usize)>, - done: Vec<(u64, u64)>, - /// Ids whose frame has been committed, with the commit time: the latest a - /// wait on them is answered is two refresh periods after that. - committed: Vec<(u64, u64)>, - /// Ids whose commit asked gamescope for presentation feedback that has not + /// `(id, swapchain, ns)`. + done: Vec<(u64, usize, u64)>, + /// Frames committed, with the commit time: the latest a wait on them is + /// answered is two refresh periods after that. `(id, swapchain, ns)`. + committed: Vec<(u64, usize, u64)>, + /// Commits that asked gamescope for presentation feedback that has not /// arrived: only `presented` / `discarded` answers them (or, should it - /// never come, [`FEEDBACK_SAFETY_NS`] after the commit). - feedback: Vec, + /// never come, [`FEEDBACK_SAFETY_NS`] after the commit). `(token, id, + /// swapchain)`; the token is the feedback listener's data, unique per + /// request. + feedback: Vec<(u64, u64, usize)>, } +/// The next feedback token; 0 is never handed out. +static NEXT_TOKEN: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); + /// Bound on both lists: the ids in flight are a handful of frames, so anything /// older is a guest that marked frames and never waited. const CAP: usize = 64; @@ -84,53 +96,64 @@ pub(crate) fn feedback_counts() -> (usize, usize, usize) { ) } -/// The commit carrying `id` asked gamescope for presentation feedback. Called -/// before that commit is sent, so no release or grace can answer it first. -pub(crate) fn expect_feedback(id: u64) { +/// The commit carrying `id` on swapchain `sc` asks gamescope for presentation +/// feedback. Called before that commit is sent, so no release or grace can +/// answer it first. Returns the token the feedback's events carry (0 for id 0). +pub(crate) fn expect_feedback(id: u64, sc: usize) -> u64 { if id == 0 { - return; + return 0; } + let token = NEXT_TOKEN.fetch_add(1, Ordering::Relaxed); let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); if w.feedback.len() >= CAP { w.feedback.remove(0); } - w.feedback.push(id); + w.feedback.push((token, id, sc)); + token } -/// gamescope's `wp_presentation_feedback.presented` for the commit carrying -/// `id`, at `present_ns`. -pub(crate) fn feedback_presented(id: u64, present_ns: u64) { +/// gamescope's `wp_presentation_feedback.presented` for the commit `token` +/// names, at `present_ns`. +pub(crate) fn feedback_presented(token: u64, present_ns: u64) { PRESENTED_N.fetch_add(1, Ordering::Relaxed); - feedback_done(id, present_ns); + feedback_done(token, present_ns); } -/// gamescope's `wp_presentation_feedback.discarded` for the commit carrying -/// `id`: it will never be shown, and its wait is answered now. -pub(crate) fn feedback_discarded(id: u64, now_ns: u64) { +/// gamescope's `wp_presentation_feedback.discarded` for the commit `token` +/// names: it will never be shown, and its wait is answered now. +pub(crate) fn feedback_discarded(token: u64, now_ns: u64) { DISCARDED_N.fetch_add(1, Ordering::Relaxed); - feedback_done(id, now_ns); + feedback_done(token, now_ns); } -/// The feedback for `id` could not be requested after all: the release and the -/// grace answer it again. -pub(crate) fn cancel_feedback(id: u64) { - if id == 0 { +/// The feedback `token` names could not be requested after all: the release +/// and the grace answer its frame again. +pub(crate) fn cancel_feedback(token: u64) { + if token == 0 { return; } let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); - w.feedback.retain(|&f| f != id); + w.feedback.retain(|&(t, _, _)| t != token); } -fn feedback_done(id: u64, ns: u64) { - { +fn feedback_done(token: u64, ns: u64) { + let key = { let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); - w.feedback.retain(|&f| f != id); + let i = w.feedback.iter().position(|&(t, _, _)| t == token); + i.map(|i| { + let (_, id, sc) = w.feedback.remove(i); + (id, sc) + }) + }; + // A token no longer pending belongs to a swapchain that went away, or a + // frame another path answered: nothing to do. + if let Some((id, sc)) = key { + record_for(id, sc, ns); } - record(id, ns); } -fn feedback_pending(w: &Waits, id: u64) -> bool { - w.feedback.contains(&id) +fn feedback_pending(w: &Waits, id: u64, sc: usize) -> bool { + w.feedback.iter().any(|&(_, i, s)| i == id && s == sc) } /// Two refresh periods after a commit, the frame has been latched whatever the @@ -140,8 +163,9 @@ fn commit_grace_ns() -> u64 { 2 * (1_000_000_000 / hz) } -/// The frame carrying `id` has been committed to the compositor. -pub(crate) fn note_commit(id: u64, commit_ns: u64) { +/// The frame carrying `id` on swapchain `sc` has been committed to the +/// compositor. +pub(crate) fn note_commit(id: u64, sc: usize, commit_ns: u64) { if id == 0 { return; } @@ -149,7 +173,7 @@ pub(crate) fn note_commit(id: u64, commit_ns: u64) { if w.committed.len() >= CAP { w.committed.remove(0); } - w.committed.push((id, commit_ns)); + w.committed.push((id, sc, commit_ns)); drop(w); WAIT_CV.notify_all(); } @@ -167,11 +191,11 @@ static WAIT_CV: Condvar = Condvar::new(); /// guest that knew its real mode would not wait at all: DXVK skips the present /// wait for mailbox and immediate. Holding the wait for the grace there throttles /// the guest's frame-latency loop to `(start-to-commit + 2 periods) / (L + 1)`. -pub(crate) fn committed(id: u64, commit_ns: u64, answer_now: bool) { +pub(crate) fn committed(id: u64, sc: usize, commit_ns: u64, answer_now: bool) { if answer_now { - record(id, commit_ns); + record_for(id, sc, commit_ns); } else { - note_commit(id, commit_ns); + note_commit(id, sc, commit_ns); } } @@ -211,21 +235,42 @@ pub(crate) fn arm(id: u64, sc: usize) { /// A completed present: presentation feedback, the daemon's latch report, a /// commit under mailbox, or a fallback. An id no longer armed was answered /// already and is left alone, so a second answer never queues a stale one. +/// +/// The daemon's latch report names an id only; it answers the oldest armed +/// frame with that id, which is the live swapchain's (an old swapchain's ids +/// are dropped when it is destroyed). pub(crate) fn record(id: u64, latch_ns: u64) { + if id == 0 { + return; + } + let sc = { + let w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); + w.armed + .iter() + .find(|&&(armed, _)| armed == id) + .map(|&(_, sc)| sc) + }; + if let Some(sc) = sc { + record_for(id, sc, latch_ns); + } +} + +/// [`record`] for the frame `id` of swapchain `sc`. +pub(crate) fn record_for(id: u64, sc: usize, latch_ns: u64) { if id == 0 { return; } let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); - let Some(i) = w.armed.iter().position(|&(armed, _)| armed == id) else { + let Some(i) = w.armed.iter().position(|&k| k == (id, sc)) else { return; }; w.armed.remove(i); - w.committed.retain(|&(c, _)| c != id); - w.feedback.retain(|&f| f != id); + w.committed.retain(|&(c, s, _)| (c, s) != (id, sc)); + w.feedback.retain(|&(_, f, s)| (f, s) != (id, sc)); if w.done.len() >= CAP { w.done.remove(0); } - w.done.push((id, latch_ns)); + w.done.push((id, sc, latch_ns)); drop(w); WAIT_CV.notify_all(); } @@ -252,14 +297,14 @@ fn note_outcome(id: u64, what: &str, t0: std::time::Instant) { /// /// Not while presentation feedback is pending for it: under FIFO the release of /// a frame comes when the next one is latched, a vblank after it was shown. -pub(crate) fn complete_on_release(id: u64, release_ns: u64) { +pub(crate) fn complete_on_release(id: u64, sc: usize, release_ns: u64) { let answers = { let w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); - !feedback_pending(&w, id) && w.armed.iter().any(|&(armed, _)| armed == id) + !feedback_pending(&w, id, sc) && w.armed.contains(&(id, sc)) }; if answers { FALLBACK_N.fetch_add(1, Ordering::Relaxed); - record(id, release_ns); + record_for(id, sc, release_ns); } } @@ -268,17 +313,8 @@ pub(crate) fn complete_on_release(id: u64, release_ns: u64) { /// was. A wait on it is answered now (`wp_presentation`'s "discarded"), so the /// guest does not sit out the bound in `wait`. An id no longer armed was /// answered already and is left alone. -pub(crate) fn discard(id: u64, ns: u64) { - if id == 0 { - return; - } - let armed = { - let w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); - w.armed.iter().any(|&(armed, _)| armed == id) - }; - if armed { - record(id, ns); - } +pub(crate) fn discard(id: u64, sc: usize, ns: u64) { + record_for(id, sc, ns); } /// The swapchain is going away: nothing armed on it will be released or @@ -289,6 +325,11 @@ pub(crate) fn forget_swapchain(sc: usize) { let before = w.armed.len(); w.armed.retain(|&(_, owner)| owner != sc); let dropped = before - w.armed.len(); + // Its pending feedback, commits and unconsumed answers go with it: a new + // swapchain reuses the same ids. + w.feedback.retain(|&(_, _, owner)| owner != sc); + w.committed.retain(|&(_, owner, _)| owner != sc); + w.done.retain(|&(_, owner, _)| owner != sc); drop(w); if dropped != 0 { WAIT_CV.notify_all(); @@ -306,24 +347,36 @@ pub(crate) fn wait(id: u64, sc: usize, timeout_ns: u64) -> bool { // mismatched pair degrading to "no wait" instead. `VK_TIMEOUT` is positive, // so a caller that treats negative results as errors carries on. const MAX_WAIT_NS: u64 = 1_000_000_000; - let bound = if timeout_ns == u64::MAX { MAX_WAIT_NS } else { timeout_ns }; + let bound = if timeout_ns == u64::MAX { + MAX_WAIT_NS + } else { + timeout_ns + }; let deadline = Some(std::time::Instant::now() + Duration::from_nanos(bound)); let t0 = std::time::Instant::now(); let mut w = WAITS.lock().unwrap_or_else(|e| e.into_inner()); loop { - if let Some(i) = w.done.iter().position(|&(done, _)| done == id) { + if let Some(i) = w + .done + .iter() + .position(|&(done, s, _)| (done, s) == (id, sc)) + { w.done.remove(i); note_outcome(id, "done", t0); return true; } - if !w.armed.iter().any(|&(armed, owner)| armed == id && owner == sc) { + if !w + .armed + .iter() + .any(|&(armed, owner)| armed == id && owner == sc) + { note_outcome(id, "unarmed", t0); return true; } // A committed frame counts as latched two refresh periods after its // commit, whether or not its buffer ever comes back; one whose // presentation feedback is pending only at the feedback safety. - let grace = if feedback_pending(&w, id) { + let grace = if feedback_pending(&w, id, sc) { FEEDBACK_SAFETY_NS } else { commit_grace_ns() @@ -331,16 +384,20 @@ pub(crate) fn wait(id: u64, sc: usize, timeout_ns: u64) -> bool { let latch_by = w .committed .iter() - .find(|&&(c, _)| c == id) - .map(|&(_, ns)| ns.saturating_add(grace)); + .find(|&&(c, s, _)| (c, s) == (id, sc)) + .map(|&(_, _, ns)| ns.saturating_add(grace)); if let Some(latch_ns) = latch_by { let now_ns = crate::proxy::monotonic_ns(); if now_ns >= latch_ns { drop(w); FALLBACK_N.fetch_add(1, Ordering::Relaxed); - record(id, latch_ns); + record_for(id, sc, latch_ns); let mut w2 = WAITS.lock().unwrap_or_else(|e| e.into_inner()); - if let Some(i) = w2.done.iter().position(|&(done, _)| done == id) { + if let Some(i) = w2 + .done + .iter() + .position(|&(done, s, _)| (done, s) == (id, sc)) + { w2.done.remove(i); } note_outcome(id, "latched", t0); @@ -510,13 +567,18 @@ mod tests { #[test] fn a_fifo_commit_is_answered_only_after_the_grace() { arm(101, 5); - committed(101, crate::proxy::monotonic_ns(), false); + committed(101, 5, crate::proxy::monotonic_ns(), false); // The grace (two refresh periods, at least 2 ms at any rate the shim // accepts) has not passed, so a zero timeout expires. assert!(!wait(101, 5, 0)); // A commit far enough in the past is answered as latched. arm(102, 5); - committed(102, crate::proxy::monotonic_ns().saturating_sub(1_000_000_000), false); + committed( + 102, + 5, + crate::proxy::monotonic_ns().saturating_sub(1_000_000_000), + false, + ); assert!(wait(102, 5, 0)); record(101, 0); assert!(wait(101, 5, 0)); @@ -525,7 +587,7 @@ mod tests { #[test] fn a_mailbox_commit_is_answered_at_the_commit() { arm(201, 6); - committed(201, crate::proxy::monotonic_ns(), true); + committed(201, 6, crate::proxy::monotonic_ns(), true); assert!(wait(201, 6, 0)); // Answered once; nothing is left armed for the release to answer again. assert!(!WAITS.lock().unwrap().armed.iter().any(|&(id, _)| id == 201)); @@ -535,41 +597,56 @@ mod tests { #[test] fn presentation_feedback_answers_and_the_release_does_not_preempt_it() { arm(401, 8); - expect_feedback(401); - committed(401, crate::proxy::monotonic_ns().saturating_sub(50_000_000), false); + let token = expect_feedback(401, 8); + committed( + 401, + 8, + crate::proxy::monotonic_ns().saturating_sub(50_000_000), + false, + ); // The release comes first (the next frame was latched): no answer. - complete_on_release(401, crate::proxy::monotonic_ns()); + complete_on_release(401, 8, crate::proxy::monotonic_ns()); // Nor does the two-period grace, 50 ms after the commit. assert!(!wait(401, 8, 0)); - feedback_presented(401, 777); + feedback_presented(token, 777); assert!(wait(401, 8, 0)); // A late duplicate from another path leaves nothing behind. record(401, 1); - assert!(!WAITS.lock().unwrap().done.iter().any(|&(id, _)| id == 401)); + assert!(!WAITS + .lock() + .unwrap() + .done + .iter() + .any(|&(id, _, _)| id == 401)); } #[test] fn a_discarded_frame_is_answered() { arm(501, 9); - expect_feedback(501); - committed(501, crate::proxy::monotonic_ns(), false); - feedback_discarded(501, crate::proxy::monotonic_ns()); + let token = expect_feedback(501, 9); + committed(501, 9, crate::proxy::monotonic_ns(), false); + feedback_discarded(token, crate::proxy::monotonic_ns()); assert!(wait(501, 9, 0)); } #[test] fn a_feedback_that_never_comes_is_answered_by_the_safety() { arm(601, 10); - expect_feedback(601); - committed(601, crate::proxy::monotonic_ns().saturating_sub(FEEDBACK_SAFETY_NS + 1), false); + expect_feedback(601, 10); + committed( + 601, + 10, + crate::proxy::monotonic_ns().saturating_sub(FEEDBACK_SAFETY_NS + 1), + false, + ); assert!(wait(601, 10, 0)); } #[test] fn without_feedback_the_release_still_answers() { arm(701, 11); - committed(701, crate::proxy::monotonic_ns(), false); - complete_on_release(701, crate::proxy::monotonic_ns()); + committed(701, 11, crate::proxy::monotonic_ns(), false); + complete_on_release(701, 11, crate::proxy::monotonic_ns()); assert!(wait(701, 11, 0)); } @@ -578,12 +655,39 @@ mod tests { // The mailbox drops a frame before it is ever committed: recording it // is what releases a guest waiting on it before the one-second bound. arm(301, 7); - discard(301, crate::proxy::monotonic_ns()); + discard(301, 7, crate::proxy::monotonic_ns()); let t0 = std::time::Instant::now(); assert!(wait(301, 7, u64::MAX)); assert!(t0.elapsed() < Duration::from_millis(100)); // A second discard of an answered id leaves nothing for a later wait. - discard(301, 0); - assert!(!WAITS.lock().unwrap().done.iter().any(|&(id, _)| id == 301)); + discard(301, 7, 0); + assert!(!WAITS + .lock() + .unwrap() + .done + .iter() + .any(|&(id, _, _)| id == 301)); + } + + #[test] + fn a_recreated_swapchains_ids_are_not_answered_by_the_old_ones_feedback() { + // Swapchain 20 had id 1 pending feedback; the guest recreates it as 21, + // whose ids start at 1 again. + arm(1, 20); + let old = expect_feedback(1, 20); + committed(1, 20, crate::proxy::monotonic_ns(), false); + forget_swapchain(20); + arm(1, 21); + let new = expect_feedback(1, 21); + committed(1, 21, crate::proxy::monotonic_ns(), false); + // The old surface's teardown discards its feedback: the new frame + // stays pending. + feedback_discarded(old, crate::proxy::monotonic_ns()); + assert!(!wait(1, 21, 0)); + // And a release of the old swapchain's slot cannot answer it either. + complete_on_release(1, 20, crate::proxy::monotonic_ns()); + assert!(!wait(1, 21, 0)); + feedback_presented(new, 5); + assert!(wait(1, 21, 0)); } } diff --git a/icd_shim/src/proxy.rs b/icd_shim/src/proxy.rs index 2210714..b31d641 100644 --- a/icd_shim/src/proxy.rs +++ b/icd_shim/src/proxy.rs @@ -679,7 +679,7 @@ unsafe fn x11_reap(sc: *mut GnSwapchain) { }; drop(_g); if wait_id != 0 { - crate::present::complete_on_release(wait_id, monotonic_ns()); + crate::present::complete_on_release(wait_id, sc as usize, monotonic_ns()); } if logging { N_REL.fetch_add(1, Ordering::Relaxed); @@ -1729,7 +1729,7 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { // to a refresh period it does not pace by. // The present id this commit asks gamescope's presentation feedback for, // 0 when none (see `crate::present`). - let mut feedback_id = 0u64; + let mut feedback_token = 0u64; if (*s).wait_id != 0 { // A guest that asked for mailbox or immediate does not wait, and // neither would one whose frames go out as mailbox (frame @@ -1751,10 +1751,9 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { && !(*(*sc).surf).swapchain_obj.is_null() && !PRESENTATION.load(Ordering::Acquire).is_null() { - feedback_id = id; - crate::present::expect_feedback(id); + feedback_token = crate::present::expect_feedback(id, sc as usize); } - crate::present::committed(id, monotonic_ns(), at_commit); + crate::present::committed(id, sc as usize, monotonic_ns(), at_commit); } // X11 route: the window carries its own content, so the frame is // presented into it (DRI3 + Present) instead of committed to a @@ -1790,7 +1789,7 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { crate::logf!(gndrm_obf::obf!( "proxy: wl_buffer creation failed — dropping frame" )); - crate::present::cancel_feedback(feedback_id); + crate::present::cancel_feedback(feedback_token); let _g = (*sc).shared.lock().unwrap(); (*s).in_flight = false; (*sc).cv_free.notify_all(); @@ -1810,10 +1809,13 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { // Declaring the effective mode (not a hardcoded FIFO) is what // lets a window's fps limit, or the absence of one, be seen by // the compositor as the shim applies it. - wlproto::swapchain_set_present_mode( - (*(*sc).surf).swapchain_obj, - gamescope_present_mode(declared_present_mode(sc)), - ); + // The guest-surface route has no gamescope swapchain to declare to. + if !(*(*sc).surf).swapchain_obj.is_null() { + wlproto::swapchain_set_present_mode( + (*(*sc).surf).swapchain_obj, + gamescope_present_mode(declared_present_mode(sc)), + ); + } wlproto::surface_attach(parent, (*s).buf, 0, 0); wlproto::surface_damage_buffer( parent, @@ -1822,20 +1824,20 @@ unsafe fn commit_real(sc: *mut GnSwapchain, idx: i32, fence_ms: u128) { (*sc).extent.width as i32, (*sc).extent.height as i32, ); - if feedback_id != 0 { + if feedback_token != 0 { let fb = wlproto::presentation_feedback( PRESENTATION.load(Ordering::Acquire), parent, ); if fb.is_null() { - crate::present::cancel_feedback(feedback_id); + crate::present::cancel_feedback(feedback_token); } else { - // The id rides as the listener's data: no allocation to - // free when the event spends the object. + // The token rides as the listener's data: no allocation + // to free when the event spends the object. wlproto::wl_proxy_add_listener( fb, FEEDBACK_LISTENER.as_ptr() as *mut _, - feedback_id as usize as *mut c_void, + feedback_token as usize as *mut c_void, ); } } @@ -2612,7 +2614,7 @@ unsafe fn present_mailbox(sc: *mut GnSwapchain, slot: *mut GnSlot, idx: i32) { (*sc).cv_put.notify_one(); drop(g); if replaced_id != 0 { - crate::present::discard(replaced_id, monotonic_ns()); + crate::present::discard(replaced_id, sc as usize, monotonic_ns()); } } } @@ -2655,10 +2657,12 @@ unsafe fn present_sync_commit(sc: *mut GnSwapchain, slot: *mut GnSlot, idx: i32) unsafe { // Declare the mode in force, as on the worker path above // (gamescope reads the mode once per commit). - wlproto::swapchain_set_present_mode( - (*(*sc).surf).swapchain_obj, - gamescope_present_mode(declared_present_mode(sc)), - ); + if !(*(*sc).surf).swapchain_obj.is_null() { + wlproto::swapchain_set_present_mode( + (*(*sc).surf).swapchain_obj, + gamescope_present_mode(declared_present_mode(sc)), + ); + } wlproto::surface_attach(parent, (*slot).buf, 0, 0); wlproto::surface_damage_buffer( parent, @@ -2799,7 +2803,7 @@ unsafe extern "C" fn gn_vkQueuePresentKHR(queue: VkQueue, pi: *const VkPresentIn } }; if stale_id != 0 { - crate::present::discard(stale_id, monotonic_ns()); + crate::present::discard(stale_id, sc as usize, monotonic_ns()); } if present_id != 0 { unsafe { @@ -6871,7 +6875,7 @@ static PRESENTATION_LISTENER: [RawListenerFn; 1] = [unsafe { extern "C" fn feedback_sync_output(_data: *mut c_void, _fb: *mut wl_proxy, _output: *mut wl_proxy) {} -/// `presented`: the frame carrying the present id in `data` was latched for +/// `presented`: the frame whose feedback token is `data` was latched for /// the refresh at `tv`. The feedback object is spent either way. #[allow(clippy::too_many_arguments)] // the event's own arity extern "C" fn feedback_presented( @@ -6885,18 +6889,18 @@ extern "C" fn feedback_presented( _seq_lo: u32, _flags: u32, ) { - let id = data as usize as u64; + let token = data as usize as u64; let ns = if PRESENTATION_CLOCK.load(Ordering::Relaxed) == libc::CLOCK_MONOTONIC as u32 { ((u64::from(tv_sec_hi) << 32 | u64::from(tv_sec_lo)) * 1_000_000_000) .saturating_add(u64::from(tv_nsec)) } else { monotonic_ns() }; - crate::present::feedback_presented(id, ns); + crate::present::feedback_presented(token, ns); unsafe { wlproto::wl_proxy_destroy(fb) }; } -/// `discarded`: the frame carrying the present id in `data` will never be +/// `discarded`: the frame whose feedback token is `data` will never be /// shown; its wait is answered now. extern "C" fn feedback_discarded(data: *mut c_void, fb: *mut wl_proxy) { crate::present::feedback_discarded(data as usize as u64, monotonic_ns()); @@ -7094,7 +7098,7 @@ extern "C" fn on_buffer_release(data: *mut c_void, _buf: *mut wl_proxy) { drop(_g); REL_ALL_N.fetch_add(1, Ordering::Relaxed); if wait_id != 0 { - crate::present::complete_on_release(wait_id, monotonic_ns()); + crate::present::complete_on_release(wait_id, sc as usize, monotonic_ns()); } if logging { N_REL.fetch_add(1, Ordering::Relaxed); From 5831f1e17870fb695182f915d454c20043abdd5a Mon Sep 17 00:00:00 2001 From: Utkarsh Dalal Date: Mon, 2 Nov 2026 14:02:29 +0530 Subject: [PATCH 7/7] docs: behaviour changes in gamescope sessions, and when gamescope answers presentation feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docs/15 gains a "Behaviour changes" section (FIFO games vsynced without a cap, mailbox/immediate keep their mode, MESA_VK_WSI_PRESENT_MODE ignored on the gamescope route, non-divisor caps at gamescope's divisor rate, FIFO queue latency, frame generation switched on mid-session, the toggle's uncapped moment) and records what gamescope does with presentation feedback: `presented` every vblank for the last latched commit of every window, visible or not, and `discarded` for a replaced or destroyed commit, so an unfocused game is still answered per vblank and the 100 ms safety is only a backstop. §3 also covers the period's bound to the mode and the callback re-post. --- docs/15-fps-limiter-pacing-and-vsync.md | 50 ++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/docs/15-fps-limiter-pacing-and-vsync.md b/docs/15-fps-limiter-pacing-and-vsync.md index f2c80f1..321c9ed 100644 --- a/docs/15-fps-limiter-pacing-and-vsync.md +++ b/docs/15-fps-limiter-pacing-and-vsync.md @@ -48,8 +48,17 @@ that rate is the display's. `presented` (at the refresh it reports) or `discarded`. The release comes a vblank later under FIFO, when the next frame is latched, and a guest that waits on every frame (DXVK with VSync on) turned that into two vblanks per frame (35 fps at a 60 - cap), so while a feedback is pending neither the release nor the grace answers; a - feedback that never comes is answered 100 ms after the commit. Without the global + cap), so while a feedback is pending neither the release nor the grace answers. Every + pending feedback is keyed by its swapchain, so a recreated swapchain, whose ids start + again at 1, is never answered by the old one's. gamescope answers every commit: it + sends `presented` each vblank for the last latched commit of *every* window, visible + or not ("we return presented for any window not visible … for forward progress", + `steamcompmgr.cpp` before `handle_presented_xwayland`), and `discarded` for a commit + that is replaced or destroyed (`commit_t::~commit_t`) or whose window is gone. An + unfocused game, or one behind a dialog, is therefore still answered once per vblank. + Mesa blocks until the event comes; the shim answers a feedback that never comes + 100 ms after the commit, a backstop for a compositor fault, not a path a working + session takes (the census's `fallback=` counts it). Without the global (or off the override route) the wait is answered on the release, or two refresh periods after the commit when no release report arrives first. - **Mailbox and immediate.** A guest that asked for either does not wait, and neither @@ -68,10 +77,16 @@ that rate is the display's. where in the display's cycle gamescope finishes; a timeline of its own, however regular, slides against the display. The source is `AChoreographer` (`gndrm-android/src/choreographer.rs`): one thread with an `ALooper` keeps a callback - posted per vsync while a session runs, and the daemon fits the period to the measured - vsync spacing over the last 32 vsyncs (`gndrm-daemon/src/vblank.rs`, `VsyncRing`). The - mode's integer rate never enters it: a "60 Hz" panel that latches at 59.973 Hz is - stamped at 59.973 Hz, and a live mode change is followed when the spacing changes. + posted per vsync while a session runs (and not while it is suspended), and the daemon + fits the period to the measured vsync spacing over the last 32 vsyncs + (`gndrm-daemon/src/vblank.rs`, `VsyncRing`). The steps are counted against the mode's + advertised period and a fit is accepted only within 2 % of it: a "60 Hz" panel that + latches at 59.973 Hz is stamped at 59.973 Hz, while a callback thread starved into + delivering every second vsync counts two periods per step instead of locking at twice + the period. A live mode change reaches the daemon as a new mode + (`nativeSetPanelRefreshHz`) and starts the fit again. The post functions return + nothing, so a callback that has not come within a second is posted once more; the + flip events meanwhile fall back to the latch grid on their own. - **Which instant is stamped.** KMS has one instant per cycle: the vblank is both the point a flip must be queued by and the point scan-out starts. Android splits it into `SurfaceFlinger`'s latch and the present one or more periods later. gamescope uses @@ -119,6 +134,29 @@ that rate is the display's. skips whole periods. Switching sources keeps the rule (the next claim is at least half a period after the last). +## Behaviour changes in gamescope sessions + +- **FIFO games without a cap are vsynced.** The env's mailbox used to be declared for + every swapchain; now a guest that asks for FIFO gets FIFO, as on a PC. +- **Mailbox and immediate games keep the mode they asked for** when no cap is set, and + are declared FIFO while one is. +- **`MESA_VK_WSI_PRESENT_MODE` is ignored on the gamescope route** (it still applies to + the X11 Present route), including a value the user picked in the driver settings; on a + PC it reaches only Mesa below gamescope's layer. +- **Caps that do not divide the display's rate** run at gamescope's divisor rate: + `floor(refresh / target)` vblanks, so 40 or 45 on 60 Hz runs at 60 and 50 on 120 Hz at + 60. +- **Queue latency.** Under FIFO a guest faster than the cap fills its free slots, so + up to `slots − 1` frames can wait in gamescope's FIFO list: at low caps that is up to + that many limited periods of input latency, as on a PC. A guest that uses present wait + is bounded by its frame latency. +- **Frame generation turned on mid-session** switches a FIFO guest's swapchain to the + mailbox discipline with the slot count it was created with (`minImageCount + 1`, one + fewer than a swapchain created with generation on) until the guest recreates it. +- **Toggling frame generation** sends the cap change and the generation settings + separately; between the two, for a moment, the guest's own mode is declared with no + gate, so a mailbox/immediate guest can run uncapped for a few frames. + ## 4. What the daemon owns - The latch grid: `ASurfaceTransactionStats_getLatchTime` on every publish, the phase