From 5742ec9548f4d435232857cd4d889e553bc7d495 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 30 Jul 2026 23:13:13 +0200 Subject: [PATCH] =?UTF-8?q?fix(vdisplay/driver):=20the=20audit=20bundle=20?= =?UTF-8?q?=E2=80=94=20one=20timing=20formula,=20honest=20EDID,=20scoped?= =?UTF-8?q?=20watchdog,=20lock-free=20drain,=20D0-resume=20re-init,=20knob?= =?UTF-8?q?bed=20RT=20priority?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One signing pass over the 2026-07-30 audit findings: - Timing math unified (D4): monitor-description and target modes now come from ONE IddSampleDriver-exact builder differing only in vSyncFreqDivider; the virtual-display-rs legacy formula (width-less pixel rate, deliberately fractional vSync) is gone. - EDID (D5): the preferred-timing DTD is built from the SESSION's mode when it fits the encoding (pf-driver-proto's tested builder; 1080p60 stays the fallback); the range-limits descriptor covers everything the driver can advertise (max clock 150 MHz → 2550 MHz, max-H +255 — the old limits were violated by the driver's own 1080p120 default); product code 0 → 1. Deliberately still no HDMI VSDB — documented in the module doc. - INF (D6): UmdfFileObjectPolicy=AllowNullAndUnknownFileObjects added (the sibling drivers all carry it); the dead DeviceGroupId (inert under ProcessSharingDisabled) dropped; the IddCx0102-vs- IddMinimumVersionRequired=10 pairing documented as deliberate — 0102 is the extension's registered identity, not a version request. - Watchdog lifecycle (D7): device cleanup now stops the host-liveness thread (it ran forever and its reap raced device teardown over the same monitor list). - Drain path off the mutex (D8): the per-frame has/take_frame_channel checks (≥60 locks/s per worker on the mutex the whole control plane, the mode DDIs and the watchdog contend) are gated by a delivery generation counter — the steady state takes no lock. - Adapter cache (D9): last-write-wins slot instead of a OnceLock, and a D0 re-entry from a REAL low-power state clears + re-inits — the stale pre-power-cycle handle used to wedge every later IOCTL_ADD. - Realtime GPU priority (D10): IddCxSetRealtimeGPUPriority is now A/B-able without a rebuild (PFVD_NO_RT_GPU, machine env) — no canonical IDD driver raises it, and it preempts the game's and DWM's queues at a level apps can't reach. - Logging (D2): the logger rides file_log_enabled() as a whole — a RELEASE driver without the opt-in no longer OutputDebugStringA's (+2 allocs) per logged event. Co-Authored-By: Claude Fable 5 --- .../drivers/pf-vdisplay/pf_vdisplay.inx | 9 +- .../drivers/pf-vdisplay/src/adapter.rs | 30 +++++- .../drivers/pf-vdisplay/src/callbacks.rs | 21 ++++- .../drivers/pf-vdisplay/src/control.rs | 24 +++++ .../windows/drivers/pf-vdisplay/src/edid.rs | 33 +++++-- .../windows/drivers/pf-vdisplay/src/log.rs | 23 +++-- .../drivers/pf-vdisplay/src/monitor.rs | 91 +++++++++++-------- .../pf-vdisplay/src/swap_chain_processor.rs | 56 ++++++++++-- 8 files changed, 213 insertions(+), 74 deletions(-) diff --git a/packaging/windows/drivers/pf-vdisplay/pf_vdisplay.inx b/packaging/windows/drivers/pf-vdisplay/pf_vdisplay.inx index 4fd4bcf1..704ddfeb 100644 --- a/packaging/windows/drivers/pf-vdisplay/pf_vdisplay.inx +++ b/packaging/windows/drivers/pf-vdisplay/pf_vdisplay.inx @@ -41,7 +41,9 @@ AddReg=pf_vdisplay_HardwareDeviceSettings [pf_vdisplay_HardwareDeviceSettings] HKR, , "UpperFilters", %REG_MULTI_SZ%, "IndirectKmd" -HKR, "WUDF", "DeviceGroupId", %REG_SZ%, "pfVDisplayGroup" +; (no "WUDF"/DeviceGroupId value: it only means anything under ProcessSharingEnabled, and this +; driver sets UmdfHostProcessSharing=ProcessSharingDisabled below — code in monitor.rs/control.rs +; reasons from the dedicated-WUDFHost guarantee, so the group id was dead weight.) ; Only the host (LocalSystem service) + admins may open the control device. Deliberately NO Everyone ; ACE (SudoVDA ships one for its user-mode host): the control plane creates/removes monitors and ; bootstraps the sealed frame channel (IOCTL_SET_FRAME_CHANNEL), so it is not for unprivileged callers. @@ -55,10 +57,15 @@ UmdfService=pf_vdisplay, pf_vdisplay_Install UmdfServiceOrder=pf_vdisplay UmdfKernelModeClientPolicy=AllowKernelModeClients UmdfHostProcessSharing=ProcessSharingDisabled +UmdfFileObjectPolicy=AllowNullAndUnknownFileObjects [pf_vdisplay_Install] UmdfLibraryVersion=$UMDFVERSION$ ServiceBinary=%12%\UMDF\pf_vdisplay.dll +; "IddCx0102" is the extension's REGISTERED IDENTITY string (every IddCx release keeps it — the +; sample drivers ship it unchanged at IddCx 1.10); the real version floor is the binary's exported +; IddMinimumVersionRequired=10 (src/lib.rs). Do not "fix" this to IddCx0110 — that identity does +; not exist and the install would fail. UmdfExtensions=IddCx0102 [WUDFRD_ServiceInstall] diff --git a/packaging/windows/drivers/pf-vdisplay/src/adapter.rs b/packaging/windows/drivers/pf-vdisplay/src/adapter.rs index 14364147..3d935feb 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/adapter.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/adapter.rs @@ -4,7 +4,7 @@ //! (`adapter_init_finished` → [`set_adapter`]). FP16 caps + the obligated `*2`/gamma/hdr callbacks (in //! `callbacks.rs`) together enable HDR. STEP 3. -use std::sync::OnceLock; +use std::sync::Mutex; use wdk_sys::{NTSTATUS, WDFDEVICE, iddcx}; @@ -40,7 +40,11 @@ unsafe impl Send for SendAdapter {} // shared `&SendAdapter` access across threads is sound. unsafe impl Sync for SendAdapter {} -static ADAPTER: OnceLock = OnceLock::new(); +// A slot, NOT a OnceLock: `set_adapter` must be last-write-wins so a D0-resume re-init's fresh +// handle REPLACES the pre-power-cycle one (a OnceLock's second `set` was a silent no-op, leaving +// every later `IddCxMonitorCreate` pointed at a stale adapter). Poison-recovering lock idiom as +// in `monitor.rs` (panic = abort here, so poisoning is unreachable anyway). +static ADAPTER: Mutex> = Mutex::new(None); /// A WDF context type for the adapter object (matches the upstream's `init_context_type`); STEP 4 stores /// adapter state here. `WDF_OBJECT_CONTEXT_TYPE_INFO` holds raw pointers (so a Sync wrapper to allow a @@ -60,7 +64,7 @@ static ADAPTER_CTX: CtxTypeInfo = CtxTypeInfo(wdk_sys::WDF_OBJECT_CONTEXT_TYPE_I /// Build the adapter caps (FP16/HDR-capable) and kick off the async adapter creation. Called from /// `EvtDeviceD0Entry`; idempotent across re-entrant D0 transitions. pub fn init_adapter(device: WDFDEVICE) -> NTSTATUS { - if ADAPTER.get().is_some() { + if adapter().is_some() { return STATUS_SUCCESS; } dbglog!("[pf-vd] init_adapter"); @@ -125,14 +129,30 @@ pub fn init_adapter(device: WDFDEVICE) -> NTSTATUS { } /// Stash the adapter object delivered by `EvtIddCxAdapterInitFinished` (STEP 4 reads it). +/// Last write wins — see [`ADAPTER`]. pub fn set_adapter(adapter: iddcx::IDDCX_ADAPTER) { - let _ = ADAPTER.set(SendAdapter(adapter)); + *ADAPTER + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(SendAdapter(adapter)); +} + +/// Forget the cached adapter. Called on a D0 re-entry from a REAL low-power state +/// (`callbacks::device_d0_entry`): the handle belongs to the pre-power-cycle incarnation, and +/// clearing is what lets `init_adapter` run again instead of short-circuiting on it. +pub fn clear_adapter() { + *ADAPTER + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = None; } /// The created adapter handle, once `EvtIddCxAdapterInitFinished` has fired — for `create_monitor` /// (`IddCxMonitorCreate`) and SET_RENDER_ADAPTER. `None` before adapter init completes. pub(crate) fn adapter() -> Option { - ADAPTER.get().map(|a| a.0) + ADAPTER + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .as_ref() + .map(|a| a.0) } /// Honor the host's `IOCTL_SET_RENDER_ADAPTER`: pin the GPU the IddCx swap-chain renders on. On a hybrid diff --git a/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs b/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs index 6b0476ec..a3e24539 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs @@ -19,9 +19,23 @@ use crate::{ /// (the adapter object is only valid after D0), not driver_add. pub unsafe extern "C" fn device_d0_entry( device: WDFDEVICE, - _previous_state: wdk_sys::WDF_POWER_DEVICE_STATE, + previous_state: wdk_sys::WDF_POWER_DEVICE_STATE, ) -> NTSTATUS { - dbglog!("[pf-vd] device_d0_entry"); + dbglog!("[pf-vd] device_d0_entry (previous_state={previous_state})"); + // A resume from a REAL low-power state (D1/D2/D3 — the initial start reports D3Final): the + // cached adapter handle belongs to the pre-power-cycle incarnation, and `init_adapter` would + // short-circuit on it forever, leaving every later IOCTL_ADD pointed at a stale adapter. The + // MS sample re-inits on every D0 entry; we clear-and-reinit only on genuine resumes so the + // common re-entrant D0 (no power cycle) stays the cheap no-op the doc above promises. + if matches!( + previous_state, + wdk_sys::_WDF_POWER_DEVICE_STATE::WdfPowerDeviceD1 + | wdk_sys::_WDF_POWER_DEVICE_STATE::WdfPowerDeviceD2 + | wdk_sys::_WDF_POWER_DEVICE_STATE::WdfPowerDeviceD3 + ) { + dbglog!("[pf-vd] device_d0_entry: power-cycle resume — re-initializing the adapter"); + crate::adapter::clear_adapter(); + } crate::adapter::init_adapter(device) } @@ -53,6 +67,9 @@ pub unsafe extern "C" fn adapter_init_finished( /// [`crate::monitor::cleanup_for_device_removal`]. pub unsafe extern "C" fn device_cleanup(_object: WDFOBJECT) { dbglog!("[pf-vd] device cleanup — releasing monitors"); + // Stop the host-liveness watchdog FIRST: a reap that fired mid-cleanup would race this + // teardown over the same monitor list (the hazard `monitor.rs` documents). + crate::control::stop_watchdog(); crate::monitor::cleanup_for_device_removal(); } diff --git a/packaging/windows/drivers/pf-vdisplay/src/control.rs b/packaging/windows/drivers/pf-vdisplay/src/control.rs index 8cc9cb9f..4c85a668 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/control.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/control.rs @@ -20,6 +20,10 @@ const WATCHDOG_TIMEOUT_S: u32 = 10; static WATCHDOG_PINGS: AtomicU64 = AtomicU64::new(0); /// Spawns the watchdog thread exactly once (idempotent across re-entrant adapter inits). static WATCHDOG_STARTED: AtomicBool = AtomicBool::new(false); +/// Asks the watchdog thread to exit ([`stop_watchdog`], from device cleanup). The thread consumes +/// the flag (swap) and clears [`WATCHDOG_STARTED`] on the way out, so a later adapter init on a +/// fresh device can re-arm. +static WATCHDOG_STOP: AtomicBool = AtomicBool::new(false); /// Start the host-liveness watchdog (once, from `adapter_init_finished`). /// @@ -43,6 +47,16 @@ pub fn start_watchdog() { let mut last_change = Instant::now(); loop { std::thread::sleep(tick); + // Device cleanup asked us to stop: the WDFDEVICE (and with it every monitor) is going + // away — a reap fired after that point would race `cleanup_for_device_removal` over + // the same monitor list. Consume the flag and un-mark STARTED so a fresh device's + // adapter init can re-arm. (Previously this thread ran forever: it outlived the + // device and was reaped only with the WUDFHost process.) + if WATCHDOG_STOP.swap(false, Ordering::SeqCst) { + WATCHDOG_STARTED.store(false, Ordering::SeqCst); + dbglog!("[pf-vd] watchdog: device cleanup — thread exiting"); + return; + } let cur = WATCHDOG_PINGS.load(Ordering::Relaxed); if cur != last { last = cur; @@ -64,6 +78,16 @@ pub fn start_watchdog() { }); } +/// Ask the watchdog thread to exit (device cleanup). Takes effect within one tick (~3 s); the +/// narrow window where a cleanup-then-re-add lands between the flag and the thread noticing it +/// is unreachable in practice — `ProcessSharingDisabled` gives each device its own WUDFHost, so +/// a new device means a new process with fresh statics. +pub fn stop_watchdog() { + if WATCHDOG_STARTED.load(Ordering::SeqCst) { + WATCHDOG_STOP.store(true, Ordering::SeqCst); + } +} + /// Dispatch one control IOCTL and complete the request. /// /// # Safety diff --git a/packaging/windows/drivers/pf-vdisplay/src/edid.rs b/packaging/windows/drivers/pf-vdisplay/src/edid.rs index 4bdf5c80..385abfe0 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/edid.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/edid.rs @@ -12,8 +12,16 @@ //! serial-number field (base offset 0x0C, little-endian) encodes the per-monitor index so //! `parse_monitor_description` can map an EDID the OS hands back to its monitor; [`Edid::generate_with`] //! patches that serial and recomputes BOTH block checksums (base byte 127 + extension byte 255). The -//! detailed-timing / range-limit descriptors are placeholders — the modes we actually advertise come -//! from the monitor's stored mode list (`monitor.rs` / `callbacks.rs`), not from parsing this EDID. +//! preferred-timing DTD is patched to the SESSION's mode when it fits the encoding (fallback: +//! 1080p60), and the range-limits descriptor is sized to cover everything the driver can advertise +//! — but the modes the OS OFFERS still come from the monitor's stored mode list +//! (`monitor.rs` / `callbacks.rs`), not from parsing this EDID. +//! +//! Deliberately NO HDMI Vendor-Specific Data Block, although `monitor.rs` declares +//! `DISPLAYCONFIG_OUTPUT_TECHNOLOGY_HDMI`: a VSDB exists to carry physical-sink features (physical +//! address for CEC, TMDS limits, deep-color caps) that a virtual display has none of, Windows does +//! not require it to drive the monitor, and inventing a CEC physical address is worse than the +//! cosmetic parser warning its absence costs. use std::array::TryFromSliceError; @@ -26,7 +34,7 @@ const SERIAL_OFFSET: usize = 0x0C; #[rustfmt::skip] const BASE: [u8; 128] = [ 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, // fixed header - 0x41, 0xCB, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // mfr "PNK", product, serial (patched) + 0x41, 0xCB, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, // mfr "PNK", product code 1 (0 = "unset" to EDID tooling), serial (patched) 0xFF, 0x21, 0x01, 0x04, 0xB0, 0x32, 0x1F, 0x78, // week/year, EDID 1.4, 10-bit digital, size, gamma 0x03, 0x78, 0xB1, 0xB5, 0x4A, 0x2B, 0xCC, 0x21, // feature (sRGB-default CLEARED), BT.2020 primaries... 0x0B, 0x50, 0x54, 0x00, 0x00, 0x00, 0x01, 0x01, // ...BT.2020 primaries, established timings, std timings @@ -34,8 +42,8 @@ const BASE: [u8; 128] = [ 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3A, // std timings, DTD 1 (placeholder preferred timing) 0x80, 0x18, 0x71, 0x38, 0x2D, 0x40, 0x58, 0x2C, 0x45, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1E, - 0x00, 0x00, 0x00, 0xFD, 0x00, 0x17, 0xF0, 0x0F, // display range-limits descriptor - 0xFF, 0x0F, 0x00, 0x0A, 0x20, 0x20, 0x20, 0x20, + 0x00, 0x00, 0x00, 0xFD, 0x08, 0x17, 0xF0, 0x0F, // range-limits: offsets H-max+255, 23-240 Hz, min-H 15 kHz... + 0xFF, 0xFF, 0x00, 0x0A, 0x20, 0x20, 0x20, 0x20, // ...max-H 255+255=510 kHz, max clock 2550 MHz (was 150 — below the driver's own 1080p120 default) 0x20, 0x20, 0x00, 0x00, 0x00, 0xFC, 0x00, 0x50, // name descriptor "Punktfunk" 0x75, 0x6E, 0x6B, 0x74, 0x66, 0x75, 0x6E, 0x6B, 0x0A, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0x00, // empty 4th descriptor... @@ -102,12 +110,23 @@ impl Edid { /// `lum` is the CLIENT display's luminance volume — coded into the HDR static-metadata block's /// desired-content bytes (CTA-861.3, via the shared+unit-tested `pf_driver_proto::edid` /// coders) so the OS/apps tone-map to the client's real panel; all-zero keeps the built-in - /// ~993-nit defaults. - pub fn generate_with(serial: u32, lum: ClientLuminance) -> Vec { + /// ~993-nit defaults. `preferred` is the session's `(width, height, refresh)` — when it fits + /// the DTD encoding (≤ 655.35 MHz pixel clock; 4K120-class does not), it REPLACES the + /// hard-coded 1080p60 preferred-timing descriptor, so the EDID's preferred mode is the mode + /// the session actually asked for (`pf_driver_proto::edid::dtd`, unit-tested there). The modes + /// the OS OFFERS still come from the IddCx mode list, not this descriptor. + pub fn generate_with( + serial: u32, + lum: ClientLuminance, + preferred: Option<(u32, u32, u32)>, + ) -> Vec { let mut edid = [0u8; 256]; // Block 0: base. edid[..128].copy_from_slice(&BASE); edid[SERIAL_OFFSET..SERIAL_OFFSET + 4].copy_from_slice(&serial.to_le_bytes()); + if let Some(dtd) = preferred.and_then(|(w, h, r)| pf_driver_proto::edid::dtd(w, h, r)) { + edid[54..72].copy_from_slice(&dtd); + } // Block 1: CTA-861.3 extension (header + colorimetry + HDR static metadata; rest stays 0). edid[128..132].copy_from_slice(&CTA_HEADER); edid[132..136].copy_from_slice(&COLORIMETRY_DB); diff --git a/packaging/windows/drivers/pf-vdisplay/src/log.rs b/packaging/windows/drivers/pf-vdisplay/src/log.rs index 736c237d..362df5fc 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/log.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/log.rs @@ -1,16 +1,17 @@ -//! Minimal driver logger. `OutputDebugStringA` always (ETW/DebugView); the optional world-writable file -//! (`C:\Users\Public\pfvd-driver.log`, readable over SSH) is now OPT-IN — debug builds, or the -//! `PFVD_DEBUG_LOG` env var, only — so a RELEASE build never writes it (audit §4.4: it was an -//! info-leak/DoS surface). Best-effort; ignores all errors. Production driver-state visibility is the -//! SharedHeader `driver_status` channel, not this file. +//! Minimal driver logger, gated as a whole on [`file_log_enabled`] (debug builds, or the +//! `PFVD_DEBUG_LOG` env var): a RELEASE build without the opt-in emits NOTHING — the +//! `OutputDebugStringA` used to fire unconditionally, a syscall + CString + `format!` alloc per +//! logged event on paths that run per IOCTL/frame. The file tee (WUDFHost temp dir, not +//! world-writable — audit §4.4) rides the same gate. Best-effort; ignores all errors. Production +//! driver-state visibility is the SharedHeader `driver_status` channel, not this module. unsafe extern "system" { fn OutputDebugStringA(s: *const u8); } -/// Whether the world-writable bring-up file log is enabled (resolved once). Off in release builds unless -/// `PFVD_DEBUG_LOG` is set. -fn file_log_enabled() -> bool { +/// Whether driver logging (debug string + bring-up file) is enabled (resolved once). Off in release +/// builds unless `PFVD_DEBUG_LOG` is set. `pub(crate)` so `dbglog!` can skip its `format!` too. +pub(crate) fn file_log_enabled() -> bool { use std::sync::OnceLock; static ON: OnceLock = OnceLock::new(); *ON.get_or_init(|| cfg!(debug_assertions) || std::env::var_os("PFVD_DEBUG_LOG").is_some()) @@ -43,6 +44,9 @@ fn file_appender() -> Option<&'static std::sync::Mutex> { } pub fn log(s: &str) { + if !file_log_enabled() { + return; + } if let Ok(c) = std::ffi::CString::new(s) { // SAFETY: `c` is a valid NUL-terminated string for the duration of the call. unsafe { OutputDebugStringA(c.as_ptr().cast()) }; @@ -56,8 +60,9 @@ pub fn log(s: &str) { } } +// The `file_log_enabled()` pre-check skips the `format!` alloc too when logging is off. macro_rules! dbglog { - ($($a:tt)*) => { $crate::log::log(&::std::format!($($a)*)) }; + ($($a:tt)*) => { if $crate::log::file_log_enabled() { $crate::log::log(&::std::format!($($a)*)) } }; } /// Zero-initialise a C POD struct (windows-rs / WDK / IddCx). These are `#[repr(C)]` framework structs diff --git a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs index dba2a84b..371e45ce 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs @@ -120,6 +120,19 @@ pub fn has_monitors() -> bool { !lock_monitors().is_empty() } +/// Frame-channel delivery generation: bumped (Release) by every successful [`set_frame_channel`]. +/// The swap-chain drain loop compares it (Acquire) against its last-seen value and only takes +/// [`MONITOR_MODES`] when a delivery actually landed — the steady state (≥60 loop passes/s per +/// worker, every one of which used to lock a mutex contended by the whole control plane, the mode +/// DDIs and the watchdog) runs lock-free. The counter is global, not per-target: a bump for a +/// sibling target costs one spurious lock peek, and target-scoped state would itself need the lock. +static FRAME_CHANNEL_GEN: core::sync::atomic::AtomicU32 = core::sync::atomic::AtomicU32::new(0); + +/// The current frame-channel delivery generation (see [`FRAME_CHANNEL_GEN`]). +pub fn frame_channel_gen() -> u32 { + FRAME_CHANNEL_GEN.load(core::sync::atomic::Ordering::Acquire) +} + /// Depart every monitor that has existed at least `grace` — the host-gone watchdog reap /// ([`crate::control::start_watchdog`]). The grace skips a just-created monitor (the host adds it, then /// starts pinging) so a momentarily-stale ping timer can't nuke a brand-new monitor. Returns the count @@ -258,45 +271,22 @@ fn default_modes() -> Vec { ] } -/// `DISPLAYCONFIG_VIDEO_SIGNAL_INFO` for a monitor mode (vSyncFreqDivider = 0, per the DDI contract). -pub fn display_info( +/// THE `DISPLAYCONFIG_VIDEO_SIGNAL_INFO` builder — one formula for both mode DDI families +/// (IddSampleDriver-exact): pixel rate = rr·w·h, integer sync rationals, total == active (no +/// fabricated blanking). Monitor (description) and target (scan-out) modes differ ONLY in +/// `vSyncFreqDivider`, which the caller passes (0 / 1 per the DDI contract). +/// +/// Until 2026-07 the monitor side used the virtual-display-rs legacy math instead — a WIDTH-LESS +/// pixel rate (`rr·(h+4)²+1000`) and a deliberately fractional vSync — so the OS saw two +/// disagreeing signal descriptions for the same `(w,h,rr)` tuple, one of them physically +/// meaningless. Numerators computed in u64 and saturated: an 8K240-class input would overflow the +/// u32 rational and panic→abort the extern-"C" mode DDI in a debug build. +fn signal_info( width: u32, height: u32, refresh_rate: u32, + v_sync_freq_divider: u32, ) -> wdk_sys::DISPLAYCONFIG_VIDEO_SIGNAL_INFO { - // Compute in u64 then saturate the u32 rational numerators: the old u32 `refresh*(h+4)^2` overflows - // for a large mode (e.g. 8K@240), which panics→aborts the extern-"C" mode DDI in a debug build. - // Identical for every real mode; only an absurd (also now bounds-rejected) mode saturates. - let clock_rate: u64 = - u64::from(refresh_rate) * u64::from(height + 4) * u64::from(height + 4) + 1000; - let clock_rate_u32 = u32::try_from(clock_rate).unwrap_or(u32::MAX); - let mut si = pod_init!(wdk_sys::DISPLAYCONFIG_VIDEO_SIGNAL_INFO); - si.pixelRate = clock_rate; - si.hSyncFreq = wdk_sys::DISPLAYCONFIG_RATIONAL { - Numerator: clock_rate_u32, - Denominator: height + 4, - }; - si.vSyncFreq = wdk_sys::DISPLAYCONFIG_RATIONAL { - Numerator: clock_rate_u32, - Denominator: (height + 4) * (height + 4), - }; - si.activeSize = wdk_sys::DISPLAYCONFIG_2DREGION { - cx: width, - cy: height, - }; - si.totalSize = wdk_sys::DISPLAYCONFIG_2DREGION { - cx: width + 4, - cy: height + 4, - }; - // union { AdditionalSignalInfo bitfield | videoStandard:u32 }: videoStandard=255, vSyncFreqDivider=0. - si.__bindgen_anon_1.videoStandard = 255; - si.scanLineOrdering = - wdk_sys::DISPLAYCONFIG_SCANLINE_ORDERING::DISPLAYCONFIG_SCANLINE_ORDERING_PROGRESSIVE; - si -} - -/// `IDDCX_TARGET_MODE` for a scan-out mode (vSyncFreqDivider = 1, per the DDI contract). -pub fn target_mode(width: u32, height: u32, refresh_rate: u32) -> iddcx::IDDCX_TARGET_MODE { let region = wdk_sys::DISPLAYCONFIG_2DREGION { cx: width, cy: height, @@ -304,7 +294,7 @@ pub fn target_mode(width: u32, height: u32, refresh_rate: u32) -> iddcx::IDDCX_T let mut si = pod_init!(wdk_sys::DISPLAYCONFIG_VIDEO_SIGNAL_INFO); si.pixelRate = u64::from(refresh_rate) * u64::from(width) * u64::from(height); si.hSyncFreq = wdk_sys::DISPLAYCONFIG_RATIONAL { - Numerator: refresh_rate * height, + Numerator: u32::try_from(u64::from(refresh_rate) * u64::from(height)).unwrap_or(u32::MAX), Denominator: 1, }; si.vSyncFreq = wdk_sys::DISPLAYCONFIG_RATIONAL { @@ -315,12 +305,27 @@ pub fn target_mode(width: u32, height: u32, refresh_rate: u32) -> iddcx::IDDCX_T si.activeSize = region; si.scanLineOrdering = wdk_sys::DISPLAYCONFIG_SCANLINE_ORDERING::DISPLAYCONFIG_SCANLINE_ORDERING_PROGRESSIVE; - // videoStandard=255, vSyncFreqDivider=1 (bits 16..21) => 255 | (1<<16). - si.__bindgen_anon_1.videoStandard = 255 | (1 << 16); + // union { AdditionalSignalInfo bitfield | videoStandard:u32 }: videoStandard=255 (other), + // vSyncFreqDivider in bits 16..21. + si.__bindgen_anon_1.videoStandard = 255 | (v_sync_freq_divider << 16); + si +} + +/// `DISPLAYCONFIG_VIDEO_SIGNAL_INFO` for a monitor mode (vSyncFreqDivider = 0, per the DDI contract). +pub fn display_info( + width: u32, + height: u32, + refresh_rate: u32, +) -> wdk_sys::DISPLAYCONFIG_VIDEO_SIGNAL_INFO { + signal_info(width, height, refresh_rate, 0) +} + +/// `IDDCX_TARGET_MODE` for a scan-out mode (vSyncFreqDivider = 1, per the DDI contract). +pub fn target_mode(width: u32, height: u32, refresh_rate: u32) -> iddcx::IDDCX_TARGET_MODE { let mut tm = pod_init!(iddcx::IDDCX_TARGET_MODE); tm.Size = core::mem::size_of::() as u32; tm.TargetVideoSignalInfo = wdk_sys::DISPLAYCONFIG_TARGET_MODE { - targetVideoSignalInfo: si, + targetVideoSignalInfo: signal_info(width, height, refresh_rate, 1), }; tm } @@ -401,6 +406,10 @@ pub fn set_frame_channel( let mut lock = lock_monitors(); if let Some(m) = lock.iter_mut().find(|m| m.target_id == target_id) { m.frame_channel = Some(ch); + // The channel store above is mutex-ordered; the bump is what lets the drain loop's + // lock-free gate ([`frame_channel_gen`]) notice it. Bump-after-store: a loop pass that + // reads the old generation misses THIS pass and attaches on the next (~16 ms). + FRAME_CHANNEL_GEN.fetch_add(1, core::sync::atomic::Ordering::Release); Ok(()) } else { Err(ch) @@ -767,7 +776,9 @@ pub fn create_monitor( }; // EDID (serial = id) describes the monitor; the OS calls back into parse_monitor_description. - let mut edid = crate::edid::Edid::generate_with(id, client_lum); + // The session's own mode becomes the preferred-timing DTD when it fits the encoding. + let mut edid = + crate::edid::Edid::generate_with(id, client_lum, Some((width, height, refresh))); let mut desc = pod_init!(iddcx::IDDCX_MONITOR_DESCRIPTION); desc.Size = core::mem::size_of::() as u32; desc.Type = iddcx::IDDCX_MONITOR_DESCRIPTION_TYPE::IDDCX_MONITOR_DESCRIPTION_TYPE_EDID; diff --git a/packaging/windows/drivers/pf-vdisplay/src/swap_chain_processor.rs b/packaging/windows/drivers/pf-vdisplay/src/swap_chain_processor.rs index 85b08043..09e2ee21 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/swap_chain_processor.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/swap_chain_processor.rs @@ -69,6 +69,15 @@ fn hr_success(hr: NTSTATUS) -> bool { hr >= 0 } +/// The `IddCxSetRealtimeGPUPriority` A/B knob: `PFVD_NO_RT_GPU` (any value, MACHINE env — the +/// driver runs in WUDFHost as LocalService, so `setx /M PFVD_NO_RT_GPU 1` + a device restart) +/// turns the priority raise OFF. Read once per process, the [`crate::log`] `OnceLock` pattern. +fn realtime_gpu_priority_enabled() -> bool { + use std::sync::OnceLock; + static ON: OnceLock = OnceLock::new(); + *ON.get_or_init(|| std::env::var_os("PFVD_NO_RT_GPU").is_none()) +} + /// A minimal newtype to move a raw pointer / handle across the thread boundary. The wrapped value is a /// raw IddCx swap-chain handle or an event HANDLE (both raw pointers, framework-managed) — sending them /// to the worker is sound because only this thread touches them and the framework synchronises lifetime. @@ -249,7 +258,12 @@ impl SwapChainProcessor { // guaranteed populated (`IddMinimumVersionRequired = 10`, lib.rs); the DDI itself may // still decline (e.g. E_NOTIMPL on pre-WDDM-3.0 hardware) — best-effort, never fatal. // Called while our borrowed device reference is still alive; IddCx uses it synchronously. - if set_ok { + // + // Knobbed (PFVD_NO_RT_GPU, machine env, read in the WUDFHost process): no canonical IDD + // driver raises this priority, and it preempts the game's and DWM's own queues at a level + // apps can't reach — a candidate aggravator in the interval-stutter program that must + // stay A/B-able on a field box without a rebuild. Default ON (today's behavior). + if set_ok && realtime_gpu_priority_enabled() { let mut rt = pod_init!(IDARG_IN_SETREALTIMEGPUPRIORITY); rt.pDevice = dxgi_device.as_raw().cast(); // SAFETY: driver is loaded; `swap_chain` is the live assigned swap-chain whose device @@ -319,6 +333,11 @@ impl SwapChainProcessor { let mut logged_pending = false; let mut logged_frame = false; + // The frame-channel delivery gate (see `monitor::frame_channel_gen`): the loop only takes + // the monitors mutex when a delivery LANDED since it last looked. Seeded one behind the + // current generation so a delivery that arrived before this worker started (host delivered + // ahead of the swap-chain assign) is checked on the very first pass. + let mut seen_chan_gen = crate::monitor::frame_channel_gen().wrapping_sub(1); loop { // Check terminate at the TOP, every iteration. The success branch below does NOT re-check it, // so during a CONTINUOUS frame burst (DWM rendering the freshly-activated desktop) a thread the @@ -330,6 +349,13 @@ impl SwapChainProcessor { break; } + // The lock-free delivery gate: `chan_pending` is true only when a `set_frame_channel` + // landed since the last pass — the two mutex-taking checks below (`has_frame_channel`, + // `take_frame_channel`) used to run EVERY pass (≥60 locks/s per worker on a mutex + // contended by the whole control plane, the mode DDIs and the watchdog); now the + // steady state takes no lock at all. + let chan_gen = crate::monitor::frame_channel_gen(); + let chan_pending = chan_gen != seen_chan_gen; // Re-attach triggers, either of: // * `is_stale` — the host recreated the ring mid-session (HDR flip): it bumps OUR header's // generation and re-delivers; without dropping here we'd keep CopyResource'ing into the @@ -340,7 +366,9 @@ impl SwapChainProcessor { // fire. The host only delivers after fully (re)creating a ring, so a pending delivery // always supersedes whatever we're attached to. if publisher.as_ref().is_some_and(FramePublisher::is_stale) - || (publisher.is_some() && crate::monitor::has_frame_channel(target_id)) + || (publisher.is_some() + && chan_pending + && crate::monitor::has_frame_channel(target_id)) { // Harvest the superseded ring's last-published frame into the stash BEFORE dropping // the publisher: between sessions the driver keeps publishing into the (host-side @@ -351,15 +379,16 @@ impl SwapChainProcessor { } } // Lazy-attach at the loop TOP so we keep trying even while the display is idle (E_PENDING / - // no frames presented yet), not only when a frame is acquired. Polled EVERY iteration (a - // cheap mutex peek, the same cost the pending-delivery check above already pays): attach - // latency is now first-frame latency, since the attach itself republishes the stash. A - // taken delivery is consumed whether the attach succeeds or not (on failure its handles are - // closed inside from_channel, the host's wait-for-attach reads the status code, and any - // retry is a NEW delivery). `target_id` binds the attach: the mapped ring must name THIS - // monitor (proto v3 validation inside from_channel — a cross-delivered ring is refused, - // never published into). + // no frames presented yet), not only when a frame is acquired — gated by `chan_pending` + // (a delivery can only appear via `set_frame_channel`, which bumps the generation): + // attach latency is still first-frame latency, since the attach itself republishes the + // stash. A taken delivery is consumed whether the attach succeeds or not (on failure its + // handles are closed inside from_channel, the host's wait-for-attach reads the status + // code, and any retry is a NEW delivery — with its own bump). `target_id` binds the + // attach: the mapped ring must name THIS monitor (proto v3 validation inside + // from_channel — a cross-delivered ring is refused, never published into). if publisher.is_none() + && chan_pending && let Some(channel) = crate::monitor::take_frame_channel(target_id) && let Ok(mut p) = FramePublisher::from_channel( channel, @@ -384,6 +413,13 @@ impl SwapChainProcessor { } publisher = Some(p); } + // The pending generation was serviced above — whichever branch ran, a lock-taking + // check happened (`has_frame_channel` and/or `take_frame_channel`), so this pass has + // seen everything up to `chan_gen`. A delivery racing in between bumps past it and + // re-arms the gate on the next pass. + if chan_pending { + seen_chan_gen = chan_gen; + } // ...Buffer2 is required once CAN_PROCESS_FP16 is set. AcquireSystemMemoryBuffer=FALSE keeps // the GPU surface (out.MetaData.pSurface) — STEP 6 publishes it into the shared ring in the