From 16618d2cdc18824a2436da4d5f18a62b03775c4d Mon Sep 17 00:00:00 2001 From: Daniel Samson <12231216+daniel-samson@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:24:23 +0100 Subject: [PATCH] =?UTF-8?q?display:=20stop=20calling=20the=20fenced=20pres?= =?UTF-8?q?ent=20'vsync'=20=E2=80=94=20it=20isn't?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The virtio-gpu present fence completes when the device has consumed the frame: real completion feedback, and tear-freedom by snapshot semantics. It is not a vblank — base virtio-gpu 2D has no display-refresh event at all (Linux fakes one with a timer), so nothing paces presents to the monitor. The code and docs claimed vsync anyway; now they don't. - backend.hasVsync -> hasFencedPresent, with an honest doc comment - marker 'display: vsync present ok' -> 'display: fenced present ok' (display-modeset test expectation updated, passes) - display-v2.md gains a 'Fenced is not vsync' note; the vsync claims in both v2 docs are corrected - true vsync arrives with a native driver's vblank IRQ, or approximated by a compositor frame clock --- build.zig | 2 +- docs/display-v2-plan.md | 16 ++++++++------ docs/display-v2.md | 28 +++++++++++++++--------- system/drivers/virtio-gpu/virtio-gpu.zig | 10 +++++---- system/services/display/backend.zig | 12 ++++++---- system/services/display/display.zig | 5 +++-- test/qemu_test.py | 11 +++++----- 7 files changed, 51 insertions(+), 33 deletions(-) diff --git a/build.zig b/build.zig index f0351c0..b9fa826 100644 --- a/build.zig +++ b/build.zig @@ -854,7 +854,7 @@ pub fn build(b: *std.Build) void { // The VGA device still supplies the boot (GOP) framebuffer the compositor starts // on; the virtio-gpu function is discovered by the device-manager stack, its // driver announces a shared scanout, and the compositor upgrades off the GOP - // floor to fenced — vsync, tear-free — native presents (docs/display-v2.md). + // floor to fenced, tear-free native presents (docs/display-v2.md). // This is the interactive twin of the `display-native` test case, and 512M // matches it (the whole driver stack + the compositor's surfaces at once). // QEMU shows one head per adapter: pick the virtio-gpu head in the View menu diff --git a/docs/display-v2-plan.md b/docs/display-v2-plan.md index ca97ce4..849c6e9 100644 --- a/docs/display-v2-plan.md +++ b/docs/display-v2-plan.md @@ -6,7 +6,7 @@ lands on its own and ends in a **verifiable gate** — shaped for a `/loop` run, ## Locked decisions (do not relitigate) -- **First native backend = virtio-gpu** (VM standard: mode-set + present/flush + vsync). +- **First native backend = virtio-gpu** (VM standard: mode-set + fenced present/flush). - **Dynamic hot-attach**: boot on GOP, upgrade to native when the driver **announces** (push, not polling); re-attach across driver restarts; GOP is the floor for "no driver ever," not a live fall-back after a reprogram. @@ -46,7 +46,7 @@ Extract scanout from the compositor so today's path becomes one backend among fu - [x] `system/services/display/backend.zig`: a `Backend` tagged union with `info()`, `surface()` (the cacheable compose target), `present(damage)`, and capability flags - (`canModeSet`/`hasVsync`, both false for GOP). + (`canModeSet`/`hasFencedPresent`, both false for GOP). - [x] The v1 GOP path is now `backend.Gop` (claims the `display` node, WC-maps the LFB, keeps the cacheable back buffer, `present` = the damage-rect WC copy). display.zig composes into `backend.surface()` and calls `backend.present(damage)` — no LFB or @@ -123,7 +123,7 @@ confirm the composited frame landed (`display: native present verified`), while ok` still fires — checked order-independently. Without `-device virtio-gpu-pci` nothing is announced and it stays on GOP: the v1 `display-service`/`display-demo` gates pass unchanged. -## V5 — Mode-setting, EDID, and vsync ✅ +## V5 — Mode-setting, EDID, and fenced presents ✅ - [x] The driver negotiates `VIRTIO_GPU_F_EDID` (when offered) and reads the monitor's EDID, logging its preferred mode; it offers a small mode list over `.scanout` `get_modes`. The @@ -131,14 +131,16 @@ announced and it stays on GOP: the v1 `display-service`/`display-demo` gates pas scanout rectangle (no resource/surface churn) — a runtime resolution change. `runtime.display` gains `modes()` / `setMode()` (display-protocol `get_modes`/`set_mode`, forwarded to the backend). - [x] Every `resource_flush` is issued fenced (`VIRTIO_GPU_FLAG_FENCE`); the device signals the - fence when the frame is on screen, which the used-ring ack the synchronous present waits on - already gates — a tear-free present. -- [x] `backend.VirtioGpu` reports `canModeSet` / `hasVsync` = true. + fence when it has consumed the frame, which the used-ring ack the synchronous present waits + on already gates — a tear-free present. (Completion feedback, **not vblank**: base + virtio-gpu 2D has no display-refresh event, so nothing paces presents to the monitor — + see the "Fenced is not vsync" note in [display-v2.md](display-v2.md).) +- [x] `backend.VirtioGpu` reports `canModeSet` / `hasFencedPresent` = true. **Gate (met):** the `display-modeset` case (reusing the display-native boot) upgrades to virtio-gpu, queries the driver's modes, `setMode`s to a different resolution, and confirms the change by reading the backend's geometry back (`display: mode set to {w}x{h}, verified`); the -fenced present path is exercised and confirmed (`display: vsync present ok`) — both from serial, +fenced present path is exercised and confirmed (`display: fenced present ok`) — both from serial, passing 3/3. The driver also logs the EDID preferred mode (`virtio-gpu: EDID preferred mode …`). ## V6 — Resilience (restart + re-attach) + tests + docs ✅ diff --git a/docs/display-v2.md b/docs/display-v2.md index ada8c3e..15a692f 100644 --- a/docs/display-v2.md +++ b/docs/display-v2.md @@ -2,7 +2,7 @@ **Status: complete (V1–V6).** The compositor boots on the GOP framebuffer and, when a virtio-gpu driver announces itself, hot-attaches a native backend over the shared `shm` -scanout surface — with runtime mode-setting, EDID, and fenced (vsync) presents, and it +scanout surface — with runtime mode-setting, EDID, and fenced presents, and it re-attaches across driver restarts. All serial-gated (see [display-v2-plan.md](display-v2-plan.md)). v1 ([display.md](display.md)) is a compositor that owns the **GOP framebuffer** — it @@ -25,10 +25,10 @@ The compositor itself (layers, back buffer, damage) does not change. Only the la scanout backend (selected at runtime — GOP by default, native when it appears) │ ├─ GopBackend the v1 path: WC copy back→front to the firmware LFB. - │ Always available. No mode-set, no vsync. THE FLOOR. + │ Always available. No mode-set, no present fence. THE FLOOR. │ └─ VirtioGpuBackend talks to a virtio-gpu driver process over a `scanout` - service: present via a shared resource + flush (real vsync), + service: present via a shared resource + fenced flush, EDID mode list, runtime mode-set. ``` @@ -37,8 +37,8 @@ A **backend** is a small interface the compositor calls: - `surface()` → the pixels to compose into and their geometry `{ptr, pitch, format, w, h}` (the LFB for GOP; a shared scanout resource for virtio-gpu), - `present(damage: Rect)` → make the damaged region visible (a no-op-ish WC copy for GOP; - a virtio flush, optionally vsync-fenced, for the native path), -- capability queries — `canModeSet`, `hasVsync` — and, when supported, `modes()` / + a fenced virtio flush for the native path), +- capability queries — `canModeSet`, `hasFencedPresent` — and, when supported, `modes()` / `setMode(m)`. The compositor composes into `surface()` and calls `present(damage)` exactly as it does @@ -98,23 +98,31 @@ compositor when a second backend arrives"). It claims the virtio-gpu PCI functio at a chosen mode for **runtime mode-setting**, - registers a `scanout` service and announces to the display service. -Its `resource_flush` is the real **present** — and gives a genuine **vsync/tear-free** -path a dumb GOP framebuffer can't. +Its `resource_flush` is the real **present** — and gives a **fenced, tear-free** path a +dumb GOP framebuffer can't. + +**Fenced is not vsync.** The fence completes when the device has *consumed* the frame: +real completion feedback, and tear-freedom by snapshot semantics (the host displays +discrete transferred frames, never a half-written surface). It is **not** a vblank — +base virtio-gpu 2D has no display-refresh event at all (Linux's driver for this device +fakes one with a software timer), so nothing paces presents to the monitor's refresh. +Refresh-paced presents need either a native driver's vblank interrupt (delivered over +the existing IRQ-as-IPC path) or the compositor's own frame clock. ## What v2 unlocks — and its honest scope Behind the abstraction, a native backend gives runtime **mode-setting** (resolution / -refresh / bpp), **EDID** enumeration, and **vsync**. But only on devices we have a driver +refresh / bpp), **EDID** enumeration, and **fenced presents**. But only on devices we have a driver for — realistically **VMs** (virtio-gpu, and later maybe Bochs DISPI). Real discrete GPUs need per-vendor KMS-class drivers that aren't getting written, so they **stay on GOP** — which is genuinely fine (v1 on the NVIDIA box is smooth). So v2's real value is twofold: -the **pluggable architecture** (a driver slots in when one exists) and a **rich, vsync'd +the **pluggable architecture** (a driver slots in when one exists) and a **rich, fenced path in VMs**, where danos development happens. The framebuffer floor never goes away. ## Locked decisions - **First native backend: virtio-gpu** — the VM standard; gives mode-set + a real - present/flush (and vsync), and exercises the whole pluggable design. Tested with QEMU + present/flush (fenced), and exercises the whole pluggable design. Tested with QEMU `-device virtio-gpu`. - **Dynamic hot-attach** — boot on GOP, upgrade to native on the driver's announce, re-attach across driver restarts; GOP is the floor for "no driver ever," not a live diff --git a/system/drivers/virtio-gpu/virtio-gpu.zig b/system/drivers/virtio-gpu/virtio-gpu.zig index 4321c2d..bd88f3c 100644 --- a/system/drivers/virtio-gpu/virtio-gpu.zig +++ b/system/drivers/virtio-gpu/virtio-gpu.zig @@ -53,8 +53,9 @@ const offered_modes = [_]Mode{ .{ .width = 640, .height = 480 }, .{ .width = 800 var current_width: u32 = offered_modes[0].width; var current_height: u32 = offered_modes[0].height; -/// Monotonic fence id for fenced (vsync) flushes; the device signals the fence when the flush -/// is complete, which its used-ring ack already gates our synchronous present on. +/// Monotonic fence id for fenced flushes; the device signals the fence when the flush is +/// complete, which its used-ring ack already gates our synchronous present on. Completion +/// feedback, not vblank — nothing here is paced to the display's refresh. var fence_next: u64 = 1; /// Whether the device offered VIRTIO_GPU_F_EDID, so `get_edid` is worth issuing. @@ -492,8 +493,9 @@ fn presentFull() bool { if (command_nodata(@sizeOf(vg.TransferToHost2d)) != ok_nodata) return false; } { - // A fenced flush (vsync): the device signals the fence when the frame is actually on - // screen — which its used-ring ack, what our synchronous submit waits on, already gates. + // A fenced flush: the device signals the fence once it has consumed the frame — which + // its used-ring ack, what our synchronous submit waits on, already gates. Completion + // feedback and a tear-free snapshot, not vblank pacing. const request = requestAt(vg.ResourceFlush); request.* = .{ .hdr = .{ .type = @intFromEnum(vg.CmdType.resource_flush), .flags = vg.flag_fence, .fence_id = fence_next }, diff --git a/system/services/display/backend.zig b/system/services/display/backend.zig index c4c426d..78c9d22 100644 --- a/system/services/display/backend.zig +++ b/system/services/display/backend.zig @@ -26,7 +26,7 @@ var device_table: [64]device.DeviceDescriptor = undefined; /// framebuffer write-combining as the front buffer, and keeps a cacheable back buffer of /// the same geometry as the compose target. `present` streams the damaged rectangle from /// the back buffer to the LFB (sequential WC writes; the LFB is never read). No mode-set, -/// no vsync — the portable floor (docs/display-v2.md). +/// no present fence — the portable floor (docs/display-v2.md). pub const Gop = struct { device_id: u64, front: [*]volatile u8, // the LFB (write-combining) @@ -263,9 +263,13 @@ pub const Backend = union(enum) { .virtio => true, }; } - /// Whether this backend has a vblank/fence for tear-free present (virtio-gpu: yes, V5 — every - /// flush is fenced, so the device signals completion when the frame is actually on screen). - pub fn hasVsync(self: *const Backend) bool { + /// Whether this backend's present is **fenced** — it completes only once the device has + /// consumed the frame (virtio-gpu: every flush carries a fence the used-ring ack waits on). + /// A fence gives completion feedback and tear-free snapshot presents; it is *not* vblank — + /// nothing paces presents to the display's refresh (base virtio-gpu 2D has no vblank event + /// at all). True vsync needs a native driver's vblank interrupt. See docs/display-v2.md, + /// "Fenced is not vsync". + pub fn hasFencedPresent(self: *const Backend) bool { return switch (self.*) { .gop => false, .virtio => true, diff --git a/system/services/display/display.zig b/system/services/display/display.zig index de463cb..65a7c1d 100644 --- a/system/services/display/display.zig +++ b/system/services/display/display.zig @@ -287,7 +287,8 @@ fn attachScanout(stride: u32, width: u32, height: u32, format: u32, capability: /// After the native upgrade is verified, prove the runtime-resolution-change and fenced-present /// paths: query the driver's modes, switch to one that differs from the current, re-composite /// the whole screen at the new size, and confirm the backend now reports that geometry. The -/// present goes through the driver's fenced flush, so a clean present is a vsync present. +/// present goes through the driver's fenced flush, so a clean present is a *fenced* present — +/// completion-acknowledged and tear-free, not vblank-paced (docs/display-v2.md). fn modesetSelfCheck() void { if (!backend.canModeSet()) return; var mode_list: [4]backend_mod.Mode = undefined; @@ -319,7 +320,7 @@ fn modesetSelfCheck() void { if (now.width == wanted.width and now.height == wanted.height) { var line: [80]u8 = undefined; _ = system.write(std.fmt.bufPrint(&line, "display: mode set to {d}x{d}, verified\n", .{ now.width, now.height }) catch "display: mode set, verified\n"); - if (backend.hasVsync()) _ = system.write("display: vsync present ok\n"); + if (backend.hasFencedPresent()) _ = system.write("display: fenced present ok\n"); } else { _ = system.write("display: mode set FAILED (geometry unchanged)\n"); } diff --git a/test/qemu_test.py b/test/qemu_test.py index 1f83f23..0ae317d 100644 --- a/test/qemu_test.py +++ b/test/qemu_test.py @@ -221,15 +221,16 @@ CASES = [ # require all three markers to appear somewhere rather than in a fixed order. "expect": r"(?s)(?=.*display: scanout upgraded to virtio-gpu)(?=.*display: native present verified)(?=.*display-demo: ok)", "fail": r"display: native present FAILED|display: could not|display-demo: (no display|create failed)|CPU EXCEPTION|KERNEL PANIC"}, - # Mode-set + EDID + vsync (v2 V5): same boot as display-native. After upgrading, the - # compositor queries the driver's modes, switches to a different resolution, and confirms the - # backend now reports it; the fenced present path makes it a vsync present. (The driver also - # logs the EDID preferred mode during bring-up.) Reuses the display-native kernel scenario. + # Mode-set + EDID + fenced presents (v2 V5): same boot as display-native. After upgrading, + # the compositor queries the driver's modes, switches to a different resolution, and confirms + # the backend now reports it; each present is fenced — completion-acknowledged and tear-free, + # not vblank-paced (docs/display-v2.md, "Fenced is not vsync"). (The driver also logs the + # EDID preferred mode during bring-up.) Reuses the display-native kernel scenario. {"name": "display-modeset", "build_case": "display-native", "qemu_extra": ["-device", "virtio-gpu-pci"], "mem": "512M", - "expect": r"(?s)(?=.*display: mode set to \d+x\d+, verified)(?=.*display: vsync present ok)", + "expect": r"(?s)(?=.*display: mode set to \d+x\d+, verified)(?=.*display: fenced present ok)", "fail": r"display: mode set FAILED|display: mode-set self-check: |display: native present FAILED|CPU EXCEPTION|KERNEL PANIC"}, # Resilience: driver restart + re-attach (v2 V6). device-manager (in test-scanout-restart # mode) kills the virtio-gpu driver once after it hellos; the restart policy respawns it, it