From ca92dab6fd768b918fd510c1163ead06e13d1df2 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 5 Aug 2026 20:03:25 +0200 Subject: [PATCH] =?UTF-8?q?fix(pf-vkdecode):=20prefer=20DEVICE=5FLOCAL,=20?= =?UTF-8?q?never=20require=20it=20=E2=80=94=20NVIDIA=20runs;=20both=20DPB?= =?UTF-8?q?=20modes=20hardware-green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session-memory and image allocation now honor each binding's memoryTypeBits with DEVICE_LOCAL preferred, not required: NVIDIA 610.88 legally places a video-session binding in host-visible-only memory and the hard requirement refused the whole device. The bitstream ring keeps its hard HOST_VISIBLE|COHERENT need. Smoke test gains PF_VKD_SMOKE_VENDOR device pinning + attribution and a final-state print (DPB mode now observed, not inferred). On-glass matrix after this fix (.173, vendor-pinned): NVIDIA 4090 PASSES in COINCIDE mode — the first end-to-end run of the RESULT_STATUS query path, ~44 per-frame driver verdicts on the recording pattern that hangs RADV's VCN — and Adrenalin re-passes in distinct mode unchanged. With RADV's distinct pass, both DPB arrangements and three of four desktop drivers are now hardware-validated; Intel remains a clean caps refusal (no SAMPLED on decode outputs — its rung stays D3D11VA). Gates: fmt clean, clippy -D warnings zero, 45+27+53 green both platforms. --- crates/pf-vkdecode/src/device.rs | 54 +++++++++++++++++++++++++++ crates/pf-vkdecode/src/images.rs | 6 ++- crates/pf-vkdecode/src/session.rs | 7 +++- crates/pf-vkdecode/tests/gpu_smoke.rs | 37 ++++++++++++++++++ 4 files changed, 100 insertions(+), 4 deletions(-) diff --git a/crates/pf-vkdecode/src/device.rs b/crates/pf-vkdecode/src/device.rs index 4c34dc4f..9cb16037 100644 --- a/crates/pf-vkdecode/src/device.rs +++ b/crates/pf-vkdecode/src/device.rs @@ -133,6 +133,26 @@ pub(crate) fn find_memory_type( }) } +/// First memory type matching `bits` that also carries `prefer`; when none does, +/// the first type matching `bits` at all. A driver constrains `memoryTypeBits` to +/// where the allocation can legally live — NVIDIA (610.88) reports some video- +/// session bindings host-visible-ONLY, which is spec-legal, so a hard `prefer` +/// requirement there is unsatisfiable by construction. Still an +/// [`AllocError::NoMemoryType`] when `bits` selects nothing whatsoever (that +/// miss-is-error contract stays; only the property preference softens). Mapped +/// staging paths (the bitstream ring) must NOT use this: they require +/// `HOST_VISIBLE|HOST_COHERENT` as a hard property, not a preference. +pub(crate) fn find_memory_type_preferring( + props: &vk::PhysicalDeviceMemoryProperties, + bits: u32, + prefer: vk::MemoryPropertyFlags, +) -> Result { + match find_memory_type(props, bits, prefer) { + Ok(index) => Ok(index), + Err(_) => find_memory_type(props, bits, vk::MemoryPropertyFlags::empty()), + } +} + impl std::fmt::Display for DeviceError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -403,6 +423,40 @@ mod tests { ); } + #[test] + fn preferring_picks_the_preferred_type_and_falls_back_inside_the_bits() { + let mut props = vk::PhysicalDeviceMemoryProperties { + memory_type_count: 4, + ..Default::default() + }; + props.memory_types[0].property_flags = + vk::MemoryPropertyFlags::HOST_VISIBLE | vk::MemoryPropertyFlags::HOST_COHERENT; + props.memory_types[1].property_flags = vk::MemoryPropertyFlags::DEVICE_LOCAL; + props.memory_types[2].property_flags = vk::MemoryPropertyFlags::DEVICE_LOCAL; + props.memory_types[3].property_flags = + vk::MemoryPropertyFlags::HOST_VISIBLE | vk::MemoryPropertyFlags::HOST_COHERENT; + + // The preferred property wins over a lower-indexed non-preferred type. + assert_eq!( + find_memory_type_preferring(&props, 0b0011, vk::MemoryPropertyFlags::DEVICE_LOCAL), + Ok(1) + ); + // The NVIDIA session-binding shape: `memoryTypeBits` names only a + // host-visible type — honor the bits instead of erroring. + assert_eq!( + find_memory_type_preferring(&props, 0b1000, vk::MemoryPropertyFlags::DEVICE_LOCAL), + Ok(3) + ); + // Bits selecting nothing remain a hard miss, never index 0. + assert_eq!( + find_memory_type_preferring(&props, 0b0000, vk::MemoryPropertyFlags::DEVICE_LOCAL), + Err(AllocError::NoMemoryType { + type_bits: 0b0000, + flags: vk::MemoryPropertyFlags::empty() + }) + ); + } + #[test] fn the_queue_submit_guard_brackets_the_lock() { use std::sync::atomic::AtomicI32; diff --git a/crates/pf-vkdecode/src/images.rs b/crates/pf-vkdecode/src/images.rs index cf6a20d4..a84f2be9 100644 --- a/crates/pf-vkdecode/src/images.rs +++ b/crates/pf-vkdecode/src/images.rs @@ -29,7 +29,7 @@ use crate::caps::H264ProfileChain; use crate::caps::COINCIDE_USAGE; use crate::caps::DPB_USAGE; use crate::caps::OUTPUT_USAGE; -use crate::device::find_memory_type; +use crate::device::find_memory_type_preferring; use crate::device::AllocError; use crate::device::DecodeDevice; @@ -401,7 +401,9 @@ unsafe fn create_video_image( // SAFETY: `image` was just created on this device. let req = unsafe { dev.ash().get_image_memory_requirements(image) }; let props = dev.memory_properties(); - let type_index = match find_memory_type( + // DEVICE_LOCAL preferred, any advertised type accepted (same rationale as the + // session bindings: `memoryTypeBits` is the driver's placement contract). + let type_index = match find_memory_type_preferring( &props, req.memory_type_bits, vk::MemoryPropertyFlags::DEVICE_LOCAL, diff --git a/crates/pf-vkdecode/src/session.rs b/crates/pf-vkdecode/src/session.rs index d56b9b66..6702b75e 100644 --- a/crates/pf-vkdecode/src/session.rs +++ b/crates/pf-vkdecode/src/session.rs @@ -27,7 +27,7 @@ use tracing::debug; use crate::caps::DecodeCaps; use crate::caps::H264ProfileChain; -use crate::device::find_memory_type; +use crate::device::find_memory_type_preferring; use crate::device::AllocError; use crate::device::DecodeDevice; use crate::params::pps_to_std; @@ -288,7 +288,10 @@ impl VideoSession { let mut binds = Vec::with_capacity(reqs.len()); for rq in &reqs { let mr = rq.memory_requirements; - let type_index = find_memory_type( + // DEVICE_LOCAL preferred, any type from `memoryTypeBits` accepted: + // NVIDIA (610.88) constrains some session bindings to host-visible-only + // types, and the driver knows where its own session state belongs. + let type_index = find_memory_type_preferring( &props, mr.memory_type_bits, vk::MemoryPropertyFlags::DEVICE_LOCAL, diff --git a/crates/pf-vkdecode/tests/gpu_smoke.rs b/crates/pf-vkdecode/tests/gpu_smoke.rs index be162e2d..317023ca 100644 --- a/crates/pf-vkdecode/tests/gpu_smoke.rs +++ b/crates/pf-vkdecode/tests/gpu_smoke.rs @@ -83,12 +83,29 @@ fn decodes_48_aus_holding_four_frames_like_the_real_client() { let instance = unsafe { entry.create_instance(&instance_ci, None) }.expect("create a Vulkan 1.3 instance"); + // Optional vendor pin (`PF_VKD_SMOKE_VENDOR`, hex `0x1002` or decimal): multi-GPU + // boxes enumerate several decode-capable devices and first-match hides all but + // one — the pin makes a run attributable to a specific vendor's driver. + let vendor_filter: Option = std::env::var("PF_VKD_SMOKE_VENDOR").ok().map(|raw| { + let trimmed = raw.trim(); + trimmed + .strip_prefix("0x") + .or_else(|| trimmed.strip_prefix("0X")) + .map_or_else(|| trimmed.parse(), |hex| u32::from_str_radix(hex, 16)) + .unwrap_or_else(|_| panic!("PF_VKD_SMOKE_VENDOR is not a PCI vendor id: {raw:?}")) + }); + // ---- physical device with an H.264 decode queue family ---- // SAFETY: live instance. let physical_devices = unsafe { instance.enumerate_physical_devices() }.expect("enumerate physical devices"); let mut picked: Option<(vk::PhysicalDevice, u32, u32)> = None; for pd in physical_devices { + // SAFETY: `pd` was just enumerated from this instance. + let props = unsafe { instance.get_physical_device_properties(pd) }; + if vendor_filter.is_some_and(|vendor| props.vendor_id != vendor) { + continue; + } // SAFETY: `pd` was just enumerated from this instance. let ext_props = unsafe { instance.enumerate_device_extension_properties(pd) }.unwrap_or_default(); @@ -167,6 +184,23 @@ fn decodes_48_aus_holding_four_frames_like_the_real_client() { let (pd, decode_qf, graphics_qf) = picked.expect("a physical device with VK_KHR_video_decode_h264 and a decode queue"); + // Attribution header: which device (and driver) this run actually exercised. + { + let mut driver_props = vk::PhysicalDeviceDriverProperties::default(); + let mut props2 = vk::PhysicalDeviceProperties2::default().push_next(&mut driver_props); + // SAFETY: live physical device; the chain fills the Vulkan 1.2 core + // driver-identity struct. + unsafe { instance.get_physical_device_properties2(pd, &mut props2) }; + let props = props2.properties; + eprintln!( + "picked: {:?} vendor=0x{:04x} driver={:?} info={:?}", + props.device_name_as_c_str().unwrap_or(c"?"), + props.vendor_id, + driver_props.driver_name_as_c_str().unwrap_or(c"?"), + driver_props.driver_info_as_c_str().unwrap_or(c"?"), + ); + } + // ---- logical device: decode (+ graphics) queues, video + sync features ---- let priorities = [1.0f32]; let mut queue_infos = vec![vk::DeviceQueueCreateInfo::default() @@ -281,6 +315,9 @@ fn decodes_48_aus_holding_four_frames_like_the_real_client() { delivered >= 40, "expected at least 40 delivered frames from 48 AUs, got {delivered}" ); + // The DPB mode the caps derivation chose — a passing run should say so + // too (failure paths already carry it via the same snapshot). + eprintln!("final state: {}", decoder.debug_snapshot()); } // ---- teardown (decoder is gone; its Drop drained the queue) ----