diff --git a/libs/scrap/src/common/drm_reader.rs b/libs/scrap/src/common/drm_reader.rs index e4516cdef..b271e3f22 100644 --- a/libs/scrap/src/common/drm_reader.rs +++ b/libs/scrap/src/common/drm_reader.rs @@ -51,7 +51,7 @@ pub struct DisplaySnapshot { /// Returns true only if `path` canonicalizes to a node directly under /dev/dri/. /// This is the realpath gate the libdrmtap helper applied but the in-process /// (direct) path does not, so the service must apply it itself. -fn device_under_dev_dri(path: &str) -> bool { +pub(super) fn device_under_dev_dri(path: &str) -> bool { match std::fs::canonicalize(path) { Ok(p) => p.parent().map_or(false, |d| d == std::path::Path::new("/dev/dri")), Err(_) => false, @@ -223,6 +223,28 @@ impl DrmReader { self.lib.grab_desc.is_some() } + /// Render node (`/dev/dri/renderD*`) of the GPU this reader captures from, to + /// hand to the unprivileged converter so it binds to the device that EXPORTS + /// the scanout. On a multi-GPU host the converter's own auto-selection can + /// land on a different GPU, and importing a scanout across vendors can fail + /// on an incompatible tiling modifier. `None` on a pre-0.4.15 `.so` (the + /// symbol is absent) or on a display-only device with no render node; the + /// converter then auto-selects exactly as before. + pub fn render_node(&mut self) -> Option { + let f = self.lib.render_node?; + // SAFETY: self.ctx is a valid context. The returned pointer is owned by + // the context and stays valid until it is closed, so copying out of it + // here (while &mut self is held) cannot outlive it. + let ptr = unsafe { f(self.ctx) }; + if ptr.is_null() { + return None; + } + unsafe { std::ffi::CStr::from_ptr(ptr) } + .to_str() + .ok() + .map(|s| s.to_owned()) + } + /// Zero-copy EXPORT grab for the split-capture path (root `--service`). Calls /// `drmtap_grab_desc`, which fills a `drmtap_dmabuf_desc` (the scanout dma-buf /// fd + the full plane layout + HDR metadata) WITHOUT mapping, detiling or diff --git a/libs/scrap/src/common/drm_render.rs b/libs/scrap/src/common/drm_render.rs index eb5be020a..605acc3ef 100644 --- a/libs/scrap/src/common/drm_render.rs +++ b/libs/scrap/src/common/drm_render.rs @@ -17,6 +17,7 @@ use super::drmtap_dl::{self, drmtap_ctx, drmtap_dmabuf_desc, drmtap_frame_info, DrmtapLib}; use super::Pixfmt; use hbb_common::log; +use std::ffi::CString; use std::io; use std::os::fd::RawFd; @@ -48,14 +49,16 @@ pub struct RenderConverter { } impl RenderConverter { - /// Open an unprivileged DRM render-node convert context (`drmtap_open_render(NULL)` - /// auto-selects a render node — it opens no KMS card, spawns no helper, and needs - /// no elevated capability). Returns `None` when libdrmtap is unavailable, the split - /// convert symbols are missing (a pre-0.4.9 `.so`), or no render node could be - /// opened (a locked-down seat with no `/dev/dri/renderD*` access) — the caller then - /// degrades to the CPU-mapped / PipeWire path. MUST be called on the thread that - /// will later `convert()` and drop it. - pub fn open_render() -> Option { + /// Open an unprivileged DRM render-node convert context. `node` is the render node + /// of the GPU that exports the scanout (from the service's display list); `None` or + /// an empty/invalid path falls back to libdrmtap auto-selection. It opens no KMS + /// card, spawns no helper, and needs no elevated capability. Returns `None` when + /// libdrmtap is unavailable, the split convert symbols are missing (a pre-0.4.9 + /// `.so`), or no render node could be opened (a locked-down seat with no + /// `/dev/dri/renderD*` access) — the caller then degrades to the service-side CPU + /// convert / PipeWire path. MUST be called on the thread that will later `convert()` + /// and drop it. + pub fn open_render(node: Option<&str>) -> Option { let lib = drmtap_dl::get()?; // The converter needs BOTH split symbols; bail (so the caller degrades) if either // is absent, rather than open a ctx we could never convert with. @@ -67,14 +70,45 @@ impl RenderConverter { ); return None; } - // SAFETY: `open_render` is a resolved C entry point; NULL requests auto-selection - // of a render node. - let ctx = unsafe { open_render(std::ptr::null()) }; + // The service names the render node of the GPU that EXPORTS the scanout, which + // is the only device guaranteed to understand its tiling modifier; auto-select + // (NULL) is the fallback when it cannot. Same /dev/dri gate the capture device + // gets: the path arrives over IPC, and while the peer is root, a converter that + // opens whatever path it is handed is a needless widening. + let node_cstr = match node.filter(|n| !n.is_empty()) { + None => None, + Some(n) => { + if !super::drm_reader::device_under_dev_dri(n) { + log::warn!("drm: render node {n:?} is not under /dev/dri; auto-selecting"); + None + } else { + match CString::new(n) { + Ok(c) => Some(c), + Err(_) => None, // interior NUL + } + } + } + }; + // SAFETY: `open_render` is a resolved C entry point; `node_cstr` outlives the + // call, and NULL requests auto-selection of a render node. + let ctx = unsafe { + open_render(node_cstr.as_ref().map_or(std::ptr::null(), |c| c.as_ptr())) + }; if ctx.is_null() { - log::info!("drmtap_open_render(NULL) failed; no usable DRM render node"); + log::info!( + "drmtap_open_render({}) failed; no usable DRM render node", + node_cstr.as_ref().map_or("NULL".to_owned(), |c| format!("{c:?}")) + ); return None; } - log::info!("drm: opened unprivileged render-node convert context"); + match node_cstr { + Some(c) => log::info!( + "drm: opened unprivileged convert context on the exporting GPU ({c:?})" + ), + None => log::info!( + "drm: opened unprivileged render-node convert context (auto-selected)" + ), + } Some(RenderConverter { lib, ctx }) } diff --git a/libs/scrap/src/common/drmtap_dl.rs b/libs/scrap/src/common/drmtap_dl.rs index c4eb13147..8aa1710bc 100644 --- a/libs/scrap/src/common/drmtap_dl.rs +++ b/libs/scrap/src/common/drmtap_dl.rs @@ -137,6 +137,10 @@ type FnCursorRelease = unsafe extern "C" fn(*mut drmtap_ctx, *mut drmtap_cursor_ type FnGrabDesc = unsafe extern "C" fn(*mut drmtap_ctx, *mut drmtap_dmabuf_desc, *mut drmtap_frame_info) -> c_int; type FnOpenRender = unsafe extern "C" fn(*const c_char) -> *mut drmtap_ctx; +// libdrmtap >= 0.4.15. Names the render node of the device a context is bound to, +// so the exporter can tell the converter which GPU to bind to instead of leaving +// it to auto-selection. Returns a ctx-owned string, or NULL if it has none. +type FnRenderNode = unsafe extern "C" fn(*mut drmtap_ctx) -> *const c_char; type FnConvertDmabuf = unsafe extern "C" fn(*mut drmtap_ctx, *const drmtap_dmabuf_desc, *mut drmtap_frame_info) -> c_int; @@ -159,6 +163,9 @@ pub struct DrmtapLib { pub grab_desc: Option, pub open_render: Option, pub convert_dmabuf: Option, + // libdrmtap >= 0.4.15; `None` on an older .so, where the converter keeps + // relying on `open_render(NULL)` auto-selection exactly as before. + pub render_node: Option, // Parsed (major, minor, patch) from `drmtap_version()`, for feature gating. pub version: (c_int, c_int, c_int), } @@ -225,6 +232,8 @@ impl DrmtapLib { let open_render: Option = lib.get(b"drmtap_open_render").ok().map(|s| *s); let convert_dmabuf: Option = lib.get(b"drmtap_convert_dmabuf").ok().map(|s| *s); + let render_node: Option = + lib.get(b"drmtap_render_node").ok().map(|s| *s); Some(DrmtapLib { _lib: lib, open, @@ -237,6 +246,7 @@ impl DrmtapLib { grab_desc, open_render, convert_dmabuf, + render_node, version: (major, minor, patch), }) } diff --git a/src/ipc.rs b/src/ipc.rs index 85e2e7c2a..a97709285 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -545,6 +545,15 @@ pub struct DrmDisplayInfo { pub width: u32, pub height: u32, pub active: bool, + /// Render node of the GPU that EXPORTS this display's scanout, so the + /// unprivileged converter binds to that device instead of auto-selecting one. + /// On a multi-GPU host auto-selection can land on a different GPU and the + /// cross-vendor import then fails on an incompatible tiling modifier. Empty + /// when the service cannot name it (a pre-0.4.15 libdrmtap, or a display-only + /// device with no render node), which keeps the previous auto-select + /// behaviour; `serde(default)` so an older peer's message still decodes. + #[serde(default)] + pub render_node: String, } /// Serializable metadata descriptor of a scanout dma-buf, shipped over `_drm` as the JSON payload of @@ -1774,6 +1783,9 @@ static DRM_DISPLAY_GENERATION: std::sync::atomic::AtomicU64 = std::sync::atomic: /// device outputs regardless of the reader's target CRTC, so a capture reader can refresh the cache. #[cfg(all(target_os = "linux", feature = "drm"))] fn drm_displays_from_reader(reader: &mut scrap::drm_reader::DrmReader) -> Vec { + // Every display this reader enumerates belongs to the reader's device, so they + // all share its render node. Resolved once here rather than per display. + let render_node = reader.render_node().unwrap_or_default(); reader .displays() .into_iter() @@ -1796,6 +1808,7 @@ fn drm_displays_from_reader(reader: &mut scrap::drm_reader::DrmReader) -> Vec (OwnedFd, OwnedFd) { let mut fds = [0 as libc::c_int; 2]; diff --git a/src/server/drm_capturer.rs b/src/server/drm_capturer.rs index df988bdc9..bdb01fa18 100644 --- a/src/server/drm_capturer.rs +++ b/src/server/drm_capturer.rs @@ -295,11 +295,22 @@ async fn recv_thread( // Skip opening the render-node converter entirely when this display previously failed to convert // (multi-GPU render-node mismatch): request the CPU path so the service does the conversion on the // exporting GPU. Otherwise open it normally and fall back to CPU only if no render node is usable. + // Bind the converter to the GPU that EXPORTS this display's scanout, which the service + // named in the display list. Auto-selection can land on a different GPU on a multi-GPU + // host, and importing a scanout across vendors can fail on an incompatible tiling + // modifier. Empty (an older service or a device with no render node) means auto-select, + // exactly as before. Every display of one device carries the same node, so a display + // index that does not resolve still gets the right answer from the first entry. let force_cpu = drm_prefer_cpu(display); + let render_node = displays + .get(display.max(0) as usize) + .or_else(|| displays.first()) + .map(|d| d.render_node.clone()) + .unwrap_or_default(); let mut converter = if force_cpu { None } else { - RenderConverter::open_render() + RenderConverter::open_render(Some(render_node.as_str())) }; let need_cpu = converter.is_none(); if need_cpu {