feat(dri3): serve DRI3 1.4 syncobjs through DRM - #122
Merged
Conversation
yserver caps DRI3 at 1.3 on NVIDIA proprietary via a driver blacklist added in 8c68a28 with an empty commit body, so ImportSyncobj and PresentPixmapSynced are never offered. status.md:316 records the cost: the PresentPixmapSynced branch of the merged deferred-Present design has never run on hardware. Three measurements on the nvidia box settle the design. nvidia-drm serves the whole DRM syncobj path (12/12 on both nodes). vkImportSemaphoreFdKHR on a DRM syncobj fd returns VK_ERROR_INITIALIZATION_FAILED, so the gate's premise was right -- OPAQUE_FD is a driver-private format and the spec never promised it is a syncobj, which also means no capability query can predict this. And libGLX_nvidia resolves import_syncobj plus present_pixmap_synced once per swapchain against a 1.4 server, so a real client here is waiting on explicit sync. Signal through DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL on every driver rather than branching. vkSignalSemaphore is already a host operation and acquire already waits on a kernel eventfd, so the swap is CPU-to-CPU; the GPU-side plan the Phase 4.2 design specified was abandoned in b92b3dd and cannot return while one queue is shared with every client. Splits OwnedSemaphore so the XSync fence half keeps Vulkan, which it genuinely needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six tasks: introduce ImportedSyncobj, split the syncobj registry off the fence registry, move the deferred-acquire polling to the DRM timeline, strip the syncobj half out of OwnedSemaphore, derive the capability from DRM_CAP_SYNCOBJ_TIMELINE, then validate on hardware. The capability flip is deliberately last: it is the step that starts advertising 1.4, so everything it exposes must already be wired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nine blocking findings across both reviewers, all reproduced before accepting. The three that changed content rather than wording: The spec claimed the 2026-05-20 gate was added with no recorded reason. False -- the same commit added a doc comment above the function (vk/device.rs:78-85) naming ERROR_INITIALIZATION_FAILED exactly. The Vulkan probe corroborated a documented rationale rather than discovering one; its value is that the failure still reproduces on 610.57.04. The spec claimed DRM and Vulkan both reject a stale timeline point, so the swap was semantically identical. Measured false: the kernel clamps to max(current, point) and returns success, and the Vulkan side is a VUID rather than an error return. Out-of-order releases are silent in both. That invalidated the risk framing and one hardware-validation check, which predicted an ENOENT that cannot occur. It also cuts in the change's favour at shutdown, where wakes flush in HashMap order. Tasks 2 and 3 merge: the acquire path reads the same registry the split moves, so separate commits left every synced present failing on Mesa at the boundary while still compiling and passing tests. All three docs/status.md citations were wrong -- read while checked out on glx-extension-string-terminator, where that file is longer. Corrected to 297, 548 and 4063, and the plan now says to re-verify after any branch switch. Also: Device::open takes DRM master so no test could open a render node (adds open_render_node); two existing tests break silently and are now named; signal_vk goes dead and would fail clippy; ImportSyncobj logs nothing on success, so the validation grep returned 0 exactly when the feature worked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Problem section claimed PresentPixmapSynced had never run on hardware, citing a status.md line that says "structurally untestable on this box". Dropping that qualifier inverted the risk: status.md:407-424 records the path passing on bee and silence with 2,221 synced requests, 473 deferred and all 473 signalled. This change replaces the release signal and the readiness poll on the one stack where the path is proven, so a Mesa run is now a merge gate rather than a nice-to-have. "Which fd to ask" was left as a preference and the plan then split it: capability on the render node, ImportedSyncobj on platform.device, the KMS node. That advertises 1.4 on the strength of one device and operates another. Now an invariant with the three cases tabulated, including why the wrong one is the easy one to write by accident. Four Xorg protocol deviations become reachable once the 1.3 cap lifts, and they share one root cause: Xorg makes a syncobj a real X resource (dri3/dri3.c:106) while yserver uses a HashMap with no owning client. Verified against the local checkout -- LEGAL_NEW_RESOURCE (dri3_request.c:609), fd<0 -> BadValue (:619), FreeSyncobj through dixLookupResourceByType with client access (:634), VERIFY_DRI3_SYNCOBJ plus BadValue (present_request.c:296-302). Two are not cosmetic: one client can free another's syncobj, and an unknown acquire syncobj leaves the client blocked with neither reply nor error. The teardown leak moves in scope because it shares that cause. Task 1 could not pass its own gate: ImportedSyncobj has no non-test caller until Task 2, so clippy --all-targets -D warnings fails on the lib target with dead_code, which the test module does not rescue. Also found while revising: the plan hardcoded /dev/dri/renderD128 in three test helpers. kms/render_node.rs:1-8 forbids exactly that -- "on multi-GPU hosts that selects the wrong device" -- and the box became one on 2026-08-08, so a run meant to validate Mesa would have exercised nvidia-drm and reported green. Hardware: the Ryzen 7700 iGPU is up with RADV on card1/renderD129, syncobjprobe passes 12/12 there, and cross-device syncobj sharing is measured working both directions. The display now runs off the board port, which unblocks the Mesa run and blocks the NVIDIA one instead. Recorded with it: resolve_drm_device picks the card by connected connector while pick_physical_device scores by device type, so yserver launched here with no overrides now scans out on AMD and renders on NVIDIA. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The plan now, per the round-3 review and the DRM/DRI3 1.4/Present 1.4 documentation: - serves every syncobj ioctl and the DRM_CAP_SYNCOBJ_TIMELINE query on a retained render-node Device (the spec's one-device invariant), never the KMS node - implements the spec's protocol-conformance scope decision as Task 3: owning client, BadAlloc/BadValue error paths, PresentPixmapSynced Value errors, disconnect purge - declares deliberate divergence from Xorg (spec section 'Divergence from Xorg'): no resource-type machinery, own error codes; only PresentPixmapSynced Value errors are protocol-mandated - adds IGT GPU Tools and the two mandatory env overrides to hardware validation (Task 6) - fixes the acquire path as a fifth registry consumer and the wire tests against the permissive install_client fixture .gitignore: exclude the local dri3_1.4 reference docs (not part of the repo).
PresentPixmapSynced's Value error now carries the offending syncobj xid as the bad-value argument when a syncobj lookup fails, and 0 for point/order failures — matching Xorg's VERIFY_DRI3_SYNCOBJ (client->errorValue = id) and present_request.c's unset errorValue. This is the error ARGUMENT, not the code, so it is outside the declared divergence. RecordingBackend's dri3_capabilities was going to hardcode a (1,4)/syncobj surface globally, which would have broken dri3_hidden_when_caps_unsupported (DRI3 must stay absent from QueryExtension on the default backend). It is now a dri3_caps field defaulting to unsupported, with a syncobj_cap_backend() helper the conformance tests opt into.
A DRI3 syncobj is a kernel object and every operation the server needs on one -- signal, query, eventfd -- has a DRM ioctl. Importing it into a VkSemaphore only works where the driver's OPAQUE_FD payload happens to be a DRM syncobj, which is false on NVIDIA proprietary. Device::open_render_node comes along because Device::open takes DRM master, which a render node refuses, so there was no way to reach these ioctls from a test. Not wired up yet; the registries still hold OwnedSemaphore.
FenceFromFD and ImportSyncobj registered two different X resource types into one HashMap of OwnedSemaphore. Only the fence half needs Vulkan (FDFromFence exports a sync_file from the VkSemaphore); the syncobj half needs a DRM handle. Split them so the type mismatch is unrepresentable. PlatformBackend now retains a Device over the render node, and every syncobj ioctl runs on it — the same fd kind DRI3 hands clients — instead of the KMS node (which only answers for the display device on split-GPU boxes). The acquire path moves in the same commit rather than a later one: it looks up the same map, so splitting the two would leave every synced present failing on Mesa at the intermediate commit. FDFromFence now looks up the registry before the Vulkan gate, so an unknown xid reports as unknown rather than as a Vulkan failure.
The spec scoped this in as the minimum that makes DRI3 1.4 safe to advertise: a client can currently free another client's syncobj, and PresentPixmapSynced with an unknown acquire syncobj gets no reply and no error — a silent hang indistinguishable from a server crash. The registry now carries the importing client. ImportSyncobj validates the xid and the fd (BadAlloc); FreeSyncobj enforces ownership (BadValue); and PresentPixmapSynced verifies both syncobjs and the point ordering per presentproto 1.4 (Value errors). Error codes deliberately diverge from Xorg's BadIDChoice/BadAccess: the protocol is silent there and no client branches on the distinction (spec 'Divergence from Xorg'). client_disconnected purges the owning client's syncobjs, closing the teardown leak.
OwnedSemaphore carried a retained DRM handle, an eventfd registration, a timeline query and a host-signal that existed only for the syncobj half, now served by ImportedSyncobj. What remains is the XSync fence resource it always was. import_drm_syncobj goes with it: vkImportSemaphoreFdKHR on a DRM syncobj fd is a Mesa-only accident, not a portable interop path.
supports_dri3_syncobj() blacklisted NVIDIA_PROPRIETARY, capping DRI3 at 1.3 and leaving PresentPixmapSynced untestable on the nvidia box. The blacklist was a correct response to a real failure, recorded in its own doc comment -- vkImportSemaphoreFdKHR rejects DRM syncobj fds on that driver. It stops being the right question once nothing imports them. The capability now comes from DRM_CAP_SYNCOBJ_TIMELINE on the render node, cached at init because dri3_capabilities sits on the per-request path. The capability and every syncobj ioctl run on the same retained render-node Device, per the spec's one-device invariant.
Only the BadAlloc branch logged, so a working explicit-sync client was indistinguishable from no client at all.
The DRI3 syncobj cap moved off the NVIDIA_PROPRIETARY driver branch and now derives from DRM_CAP_SYNCOBJ_TIMELINE on the render node, so DRI3 1.4 and ImportSyncobj/PresentPixmapSynced are advertised on the nvidia box. Hardware validation of the path is still pending.
The PresentPixmapSynced path is no longer 'structurally untestable on this box': the DRI3 syncobj cap derives from DRM_CAP_SYNCOBJ_TIMELINE on the render node, and a full-Mesa run on the Raphael iGPU (card1/RADV) passed the spec's merge gate. yserver on card1 + mpv Vulkan X11 WSI reproduced a 15s testsrc; the log shows DRI3 1.4 advertised, 6 ImportSyncobj imports, 192 synced presents with 1 deferred acquire (subsequently signalled) and 0 fallback warnings. IGT: syncobj_basic 12/12 on nvidia-drm; eventfd/wait/timeline skip because the 7.1 kernel compiles CONFIG_SW_SYNC but does not expose /dev/sw_sync.
The full-Mesa validation on the nvidia box (card1/RADV) passed the spec's merge gate: DRI3 1.4 advertised, 6 ImportSyncobj imports, 192 synced presents with 1 deferred acquire (subsequently signalled), 0 fallback warnings. The final-review fix wave (duplicate-xid rejection and the client-scoped PresentPixmapSynced ownership check) is also in.
With CONFIG_SW_SYNC exposed at /sys/kernel/debug/sync/sw_sync (symlinked to /dev/sw_sync for IGT 2.3), the syncobj_* suite passes 212/212 (0 SKIP) on nvidia-drm: syncobj_basic 12, syncobj_eventfd 10, syncobj_wait 69, syncobj_timeline 121. The earlier eventfd/wait/timeline skips were purely the missing sw_sync device node. IGT's device filter does not match cards in this environment, so amdgpu coverage stays with the yserver imported_syncobj tests (3/3 on renderD129).
ariel3259
force-pushed
the
dri3-syncobj-drm-signal
branch
from
August 10, 2026 22:33
2c1d474 to
d16bed4
Compare
Owner
|
Nice, thx. There are some minor fixups needed, but I'll tackle those rather than ask for a few roundtrips. |
ariel3259
pushed a commit
to ariel3259/yserver
that referenced
this pull request
Aug 12, 2026
Two observability/robustness fixes on the DRI3 1.4 syncobj path landed in joske#122. Neither changes protocol behaviour. **Latch the eventfd fallback.** `Dri3Caps::syncobj` derives from DRM_CAP_SYNCOBJ_TIMELINE, but the acquire path needs DRM_IOCTL_SYNCOBJ_EVENTFD -- a much later addition. FreeBSD's drm-kmod satisfies the first and not the second, so we advertise DRI3 1.4, clients use PresentPixmapSynced, and every acquire then failed the ioctl, logged a warning, and fell back to the 1ms timeline poll. One warning per frame, on a path where per-frame logging is known to perturb the timing it reports (it is why the per-Present timeline is aggregate-only). The ioctl is now skipped once it has proven unavailable, and the warning fires once, naming the consequence for the whole session. Only ENOTTY / ENOSYS / EOPNOTSUPP latch: a transient per-call failure must not disable kernel notification for the rest of the session. Observed on FreeBSD (logs on ZFS, not retrievable from Linux); Linux runs here show zero fallbacks, which is why it did not surface earlier. **Log successful FreeSyncobj.** Only the error paths logged, so an import count could not be matched against a free count and "are retired swapchain syncobjs released?" was unanswerable from a capture. It cost a review round-trip: 18 imports with no visible frees read as a leak. With the line, one mpv session answered it -- 12 imports, 12 frees, exact XID pairing, net zero at exit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ariel3259
pushed a commit
to ariel3259/yserver
that referenced
this pull request
Aug 12, 2026
Two defects found reviewing joske#122, both "identity tracked by XID where Xorg tracks it by reference", neither reachable by a well-behaved client: 1. The release syncobj is re-resolved by XID at completion (backend.rs:1198, :19450) rather than pinned at accept. A free+reimport of that XID while the Present is parked makes us signal the replacement: the real waiter hangs and an unrelated object gets a spurious signal. The acquire path already pins the Arc correctly (backend.rs:13188), so release is the outlier. 2. The syncobj XID is never registered in the server's XID namespace, so xid_in_use and used_xids_in are blind to it -- CreatePixmap can reuse a live syncobj's XID (Xorg: BadIDChoice), and XC-MISC GetXIDRange can hand it out. They are linked: xcb allocates XIDs monotonically so clients do not recycle their own, and XC-MISC is the realistic path to reuse -- which defect 2 leaves blind. Fixing 2 makes 1 unreachable; fixing 1 makes it impossible. Plan covers the design (carry Arc<dyn SyncobjHandle> in PresentWake, dropping Copy -- CompletedPresentEvent is already only Clone so the ripple is ~12 sites), the core-side XID mirror, and a regression test that frees and reimports the release XID while a Present is parked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ariel3259
pushed a commit
to ariel3259/yserver
that referenced
this pull request
Aug 12, 2026
…xid_occupied Second codex pass. Four new corrections, all verified in source: 1. A NINTH site: process_request.rs:9359 does `let wake = pending.request.wake()` before the copy-failure reroute, distinct from the XID signal at :9394. Also records that production has exactly two PendingPresentPixmap constructions (:9957 ordinary, :10266 synced) and that the destructuring at :9149 must carry the wake through. 2. The authoritative occupancy predicate ALREADY EXISTS: ServerState::xid_occupied (server.rs:1677), whose doc says outright "extend this (and the xid_occupied_covers_every_namespace test) when adding an XID-keyed map". joske#122 added an XID-keyed map and did not -- so defect 2 is a missed documented obligation, not a design gap. What is missing is a legal_new_resource helper; most creation handlers bypass xid_occupied entirely, which is why ImportSyncobj can still collide with extension-only maps. 3. Close-down is now ANSWERED rather than deferred. Xorg RETAINS syncobjs: dri3_syncobj_type is created without RC_NEVERRETAIN (dri3/dri3.c:106), so they follow ordinary resource semantics under RetainPermanent/RetainTemporary. Our unconditional purge in Backend::client_disconnected is therefore wrong for retained clients, and KillClient cannot currently name a zombie via a syncobj. 4. RecordingBackend needs more than identity-bearing handles: enqueue_present_- completion is a no-op default, signal_present_wake does not signal a retained handle, and try_present_direct discards the event. Only the four pre-completion paths are writable without that machinery; normal completion and direct scanout are not. The plan no longer claims otherwise. Also: dropping Copy affects four shared-reference matches, not two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ariel3259
pushed a commit
to ariel3259/yserver
that referenced
this pull request
Aug 12, 2026
… namespace (codex) Authored by codex; committed on its behalf. Follows the plan in docs/superpowers/plans/2026-08-12-dri3-syncobj-identity.md, with one improvement over it: syncobj records live in ResourceTable as a ninth namespace rather than a ServerState mirror. That removes the two-map drift the plan worried about, and makes xid_in_use, collect_xids_in, ownership lookup and close-down retention apply for free. Two defects from the joske#122 review, both "identity tracked by XID where Xorg tracks it by reference", neither reachable by a well-behaved client: 1. The release syncobj was re-resolved by XID at completion, so a free+reimport of that XID while a Present was parked made us signal the replacement -- the real waiter hangs, an unrelated object gets a spurious signal. Pending Presents now pin the exact Arc at accept and no deferred path re-resolves. PendingPresentRequest::wake() is deleted so the compiler enforces it. 2. Syncobj XIDs were absent from the server's XID namespace, so CreatePixmap could reuse a live syncobj's XID (Xorg: BadIDChoice) and XC-MISC could hand it out. They now participate in collision checks, XC-MISC allocation, ownership lookup and retained-client cleanup. Close-down semantics now match Xorg, which retains syncobjs (dri3_syncobj_type carries no RC_NEVERRETAIN, dri3/dri3.c:106): the unconditional purge in Backend::client_disconnected is gone, disconnect hands core an explicit freed_dri3_syncobjs list, and KillClient(syncobj XID) / KillClient(AllTemporary) destroy retained syncobjs. Adds identity-reuse, reverse-XID-collision, retention, namespace and KillClient regressions. Verified here, not just reported: cargo +nightly fmt clean; cargo clippy --all-targets -- -D warnings clean; 2273 passed / 177 ignored / 0 failed (+16 over baseline). Hardware: a full mpv --gpu-api=vulkan swapchain lifetime on silence -- 2683 PresentPixmapSynced accepted, 204 deferred acquires all signalled, 6 imports and 6 frees pairing to net zero at exit, no protocol errors. Not covered by any of that: the identity bug itself needs XID reuse mid-flight, which no real client does, so it rests on the unit tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DRI3 1.4 syncobj: signal through DRM, not Vulkan
Serves DRI3 1.4 syncobjs through the kernel's DRM syncobj interface on every driver, replacing the Vulkan timeline-semaphore path. The NVIDIA blacklist that capped DRI3 at 1.3 is deleted;
PresentPixmapSyncedis advertised whenever the kernel reportsDRM_CAP_SYNCOBJ_TIMELINE.What changed
ImportedSyncobj— a DRM-backed syncobj resource (syncobj_timeline_signal/query,syncobj_eventfd,fd_to_syncobj) plusDevice::open_render_node.Device(never the KMS node).OwnedSemaphore; syncobjs move toHashMap<u32, (ClientId, Arc<ImportedSyncobj>)>.PresentPixmapSyncedValue errors, disconnect purge, duplicate-import and cross-client ownership checks. Error codes follow Xorg (BadIDChoice, BadDrawable, BadValue, BadAccess, BadAlloc).Dri3Caps::syncobjderives fromDRM_CAP_SYNCOBJ_TIMELINEon the render node;supports_dri3_syncobj()blacklist removed.timeline_semaphore(true)feature removed.Validation (nvidia box, 2026-08-10)
DRI3::QueryVersion -> 1.4, 6ImportSyncobjimports, 192 synced presents with 1 acquire deferred and subsequently signalled, 0unknown syncobj, 0DRM eventfd unavailablefallback warnings — the spec's merge gate is satisfied.syncobj_*suite passes 212/212 (0 SKIP) on nvidia-drm withCONFIG_SW_SYNCexposed via/sys/kernel/debug/sync/sw_sync.cargo clippy --all-targets -- -D warningsclean; yserver-core 1139 and yserver 780 tests pass (plus 62 DRM#[ignore]passing on real render nodes).Internal divergence from Xorg (not client-visible)
yserver does not clone Xorg's resource-type machinery for DRI3 syncobjs: they live in a backend
HashMap<u32, (ClientId, Arc<ImportedSyncobj>)>with an owner field, instead of first-class X resources. This is internal only — error codes follow Xorg, and the observable behaviour (ownership checks, disconnect purge, protocol validation) is reproduced via the owner field. Justified by the HLD non-goal "being a drop-in clone of Xorg internals". See the spec's "Divergence from Xorg" section.dri3_1.4/is not part of this PRA local reference-documentation directory (DRI3/Present protocols, kernel DRM docs) gathered during research. Excluded via
.gitignore(/dri3_1.4/); the derived knowledge lives in the spec and plan in this repo.