From 54195f079810410eb9456348e7e44d382d3c4d3f Mon Sep 17 00:00:00 2001 From: srdusr <99972264+srdusr@users.noreply.github.com> Date: Thu, 15 May 2025 20:45:00 +0200 Subject: Extend GPU rendering to every head, not just the first Phase 2 of the GPU-rendering plan deliberately targeted a single head (GpuContext::output: Option<(crtc::Handle, GpuOutput)>) as a narrow proof that GBM+EGL+DrmCompositor rendering works at all on this hardware. DrmOutputManager already supports driving several crtcs at once - initialize_output is a per-crtc call on one shared manager, the same way anvil drives multiple outputs - so this was purely an unexercised restriction, not an architectural limit. GpuContext::output is now GpuContext::outputs: Vec<(crtc::Handle, GpuOutput)>, and udev/platform.rs calls initialize_output for every connected head in its own bring-up loop instead of only the first after that loop finishes. A head this fails for individually (already logged, not fatal) still just has no entry and falls back to the existing legacy Pixman path, unchanged from before. session.rs's VBlank handler and its VT-switch resume path (which excludes GPU-driven crtcs from the legacy set_crtc reassert loop, a different device fd that must never issue mode-set commands against a crtc DrmOutputManager already owns) both now look a crtc up in the Vec instead of comparing against a single stored one. render.rs's own render-loop lookup uses direct field access (gpu.outputs.iter_mut().find(...)) rather than an equivalent &mut self method: the borrow checker treats a method call as borrowing all of GpuContext, including gpu.renderer needed a few lines later for the same head, where direct field access lets it see the two borrows are disjoint. Still gated behind SRDWM_GPU=1 (unset by default) and untested on real multi-monitor hardware with the flag on - this machine has one display, so the actual multi-head path itself only gets exercised whenever it's set on hardware that has more than one. --- crates/wayland/src/udev/gpu.rs | 72 ++++++++++++++++++++++++------------- crates/wayland/src/udev/platform.rs | 22 +++++++----- crates/wayland/src/udev/render.rs | 50 ++++++++++++++------------ crates/wayland/src/udev/session.rs | 40 ++++++++++----------- 4 files changed, 108 insertions(+), 76 deletions(-) (limited to 'crates') diff --git a/crates/wayland/src/udev/gpu.rs b/crates/wayland/src/udev/gpu.rs index 27a73f4..50c42c1 100644 --- a/crates/wayland/src/udev/gpu.rs +++ b/crates/wayland/src/udev/gpu.rs @@ -56,13 +56,36 @@ pub(crate) type GpuOutputManager = DrmOutputManager< pub(crate) struct GpuContext { pub(crate) renderer: GlesRenderer, pub(crate) output_manager: GpuOutputManager, - /// The one head [`GpuContext::initialize_output`] was successfully - /// called for, if any - Phase 2 of the plan this was built from only - /// ever targets a single head, see that call site (`udev/platform.rs`) - /// for which one and why. `render_udev_frame` (`udev/render.rs`) - /// checks this to decide whether a given head renders through here or - /// through the existing legacy Pixman path. - pub(crate) output: Option<(crtc::Handle, GpuOutput)>, + /// Every head [`GpuContext::initialize_output`] was successfully + /// called for - Phase 2 of the plan this was built from only ever + /// targeted a single head; `udev/platform.rs`'s startup path now calls + /// `initialize_output` for every connected head instead of just the + /// first, so a machine with several monitors gets all of them through + /// this same GBM+EGL+`DrmCompositor` pipeline rather than only one. + /// `DrmOutputManager` itself already supports driving multiple crtcs + /// at once (`initialize_output` is a per-crtc call on the one shared + /// manager, same as `anvil`'s own multi-output handling) - Phase 2 + /// simply never exercised that. A head this fails for individually + /// (logged, not fatal) just has no entry here and falls back to the + /// existing legacy Pixman path exactly as before, same as it always + /// could when `SRDWM_GPU` was unset entirely. `render_udev_frame` + /// (`udev/render.rs`) looks a head's own crtc up here to decide + /// whether it renders through this path or through Pixman. + pub(crate) outputs: Vec<(crtc::Handle, GpuOutput)>, +} + +impl GpuContext { + /// The `GpuOutput` driving `crtc`, if [`initialize_output`](Self::initialize_output) + /// was called for it and succeeded. Only an `&self` lookup (`session.rs`'s + /// VBlank handler doesn't need mutable access) - `render.rs`'s own + /// render-frame call site needs `&mut gpu.outputs` *and* `&mut gpu. + /// renderer` at once, which it does via direct field access instead of + /// an equivalent `&mut self` method here, since Rust's disjoint-field- + /// borrow analysis only sees through direct field access, not a method + /// call that borrows all of `self` even when it only touches one field. + pub(crate) fn output_for(&self, crtc: crtc::Handle) -> Option<&GpuOutput> { + self.outputs.iter().find(|(c, _)| *c == crtc).map(|(_, o)| o) + } } /// Reasonable, widely-supported scanout formats to try, most-preferred @@ -189,34 +212,33 @@ pub(crate) fn probe(card: &Card) -> Option<(GpuContext, DrmDeviceNotifier)> { "udev: SRDWM_GPU=1 - GBM+EGL+GLES+DrmOutputManager all initialized successfully on this hardware. \ No output is being driven through it yet (see gpu::probe's own doc comment); every head still renders via Pixman for now." ); - Some((GpuContext { renderer, output_manager, output: None }, notifier)) + Some((GpuContext { renderer, output_manager, outputs: Vec::new() }, notifier)) } impl GpuContext { - /// Drives exactly one head (`crtc`/`mode`/`connector`/`output`, the - /// same values `udev/drm.rs`'s `bring_up_head` already resolved for - /// this head's *existing* legacy `UdevHead`) through this context's - /// `DrmOutputManager`, returning the resulting `GpuOutput` on success. + /// Drives one head (`crtc`/`mode`/`connector`/`output`, the same + /// values `udev/drm.rs`'s `bring_up_head` already resolved for this + /// head's *existing* legacy `UdevHead`) through this context's shared + /// `DrmOutputManager`, appending the resulting `GpuOutput` to + /// `self.outputs` on success - call once per connected head from + /// `udev/platform.rs`'s startup path, same loop that already brings up + /// each head's legacy `UdevHead`. /// - /// `elements` is always empty for this phase (Phase 2 of the plan this - /// was built from renders a plain clear color only, no window content/ - /// decorations/cursor yet) - so the concrete choice of `E` here - /// ([`GpuElement`], a plain `MemoryRenderBufferRenderElement`) is - /// arbitrary; nothing about it is load-bearing until a later phase - /// actually pushes real elements through this same call shape. + /// `elements` is always empty here - initial output setup, not a real + /// frame - so the concrete choice of `E` ([`GpuElement`], a plain + /// `MemoryRenderBufferRenderElement`) is arbitrary. /// - /// Returns `false` and leaves `self.output` untouched on failure - /// (logged) - same fallback contract as [`probe`] itself: a head this - /// fails for simply never gets an entry in `self.output_manager`'s - /// internal map, so it can still be driven through the existing - /// legacy Pixman path exactly as if `SRDWM_GPU` had never been set for - /// that particular head. + /// Returns `false` and leaves `self.outputs` unchanged on failure + /// (logged): that one head simply never gets an entry, so it renders + /// through the existing legacy Pixman path exactly as if `SRDWM_GPU` + /// had never been set for it, while every other head this succeeded + /// for is unaffected. pub(crate) fn initialize_output(&mut self, crtc: crtc::Handle, mode: DrmMode, connector: connector::Handle, output: &Output) -> bool { let elements: DrmOutputRenderElements = DrmOutputRenderElements::default(); match self.output_manager.initialize_output::(crtc, mode, &[connector], output, None, &mut self.renderer, &elements) { Ok(gpu_output) => { log::info!("udev: SRDWM_GPU=1 - output initialized through DrmOutputManager for crtc {crtc:?}"); - self.output = Some((crtc, gpu_output)); + self.outputs.push((crtc, gpu_output)); true } Err(e) => { diff --git a/crates/wayland/src/udev/platform.rs b/crates/wayland/src/udev/platform.rs index 6826cde..19991e8 100644 --- a/crates/wayland/src/udev/platform.rs +++ b/crates/wayland/src/udev/platform.rs @@ -88,6 +88,19 @@ impl UdevPlatform { let resolved_scale = head.output.current_scale().fractional_scale(); x_offset += head.size.0; logical_x += (head.size.0 as f64 / resolved_scale).round() as i32; + // Every connected head gets a chance at the GPU path, not just + // the first - `DrmOutputManager` already supports driving + // several crtcs at once (`GpuContext::outputs`' own doc + // comment), Phase 2 simply never called this more than once. + // A no-op whenever `gpu_context` is `None` (every session that + // doesn't set `SRDWM_GPU=1`, or where `gpu::probe` itself + // failed). A head this fails for individually (logged inside + // `initialize_output`) just falls back to the legacy Pixman + // path below, same as before - this loop doesn't need to know + // which outcome happened. + if let Some(ctx) = gpu_context.as_mut() { + ctx.initialize_output(head.crtc, head.mode, head.connector, &head.output); + } heads.push(head); output_entries.push(entry); } @@ -101,15 +114,6 @@ impl UdevPlatform { // `WaylandPlatform::connect` for why this isn't optional. smithay::wayland::output::OutputManagerState::new_with_xdg_output::(&display_handle); - // Phase 2 of the GPU-rendering plan (`gpu.rs`'s own module doc - // comment) only ever targets one head - the first, same as the - // pointer-centring choice just above - not every head at once. - // A no-op whenever `gpu_context` is `None` (every session that - // doesn't set `SRDWM_GPU=1`, or where `gpu::probe` itself failed). - if let Some(ctx) = gpu_context.as_mut() { - ctx.initialize_output(first.crtc, first.mode, first.connector, &first.output); - } - let compositor_state = CompositorState::new::(&display_handle); let xdg_shell_state = XdgShellState::new::(&display_handle); let xdg_decoration_state = XdgDecorationState::new::(&display_handle); diff --git a/crates/wayland/src/udev/render.rs b/crates/wayland/src/udev/render.rs index 6bf2979..cfc9efd 100644 --- a/crates/wayland/src/udev/render.rs +++ b/crates/wayland/src/udev/render.rs @@ -214,30 +214,36 @@ impl CompState { let Some(udev) = self.udev.as_mut() else { return }; let head_crtc = udev.heads[index].crtc; // Phase 2 of the GPU-rendering plan (`gpu.rs`'s own module doc - // comment): the one head `initialize_output` was called for - // (if `SRDWM_GPU=1` and everything up to that point succeeded) - // renders a plain clear color through the real GBM+EGL+ - // DrmCompositor pipeline instead of anything below - no - // window content, decorations, or cursor yet, deliberately - // (see that module's own doc comment for why). Every other - // head, and this same head whenever `udev.gpu`/its `output` - // is `None`, falls straight through to the existing, - // untouched Pixman path unchanged. + // comment), now extended to every head `initialize_output` + // succeeded for (`GpuContext::outputs`' own doc comment), not + // just the first: each renders a plain clear color through the + // real GBM+EGL+DrmCompositor pipeline instead of anything + // below - no window content, decorations, or cursor yet, + // deliberately (see that module's own doc comment for why). + // Any head `output_for_mut` finds nothing for - either + // `SRDWM_GPU` was never set, or `initialize_output` failed for + // this specific crtc - falls straight through to the + // existing, untouched Pixman path unchanged. if let Some(gpu) = udev.gpu.as_mut() { - if let Some((gpu_crtc, gpu_output)) = gpu.output.as_mut() { - if *gpu_crtc == head_crtc { - let elements: [smithay::backend::renderer::element::memory::MemoryRenderBufferRenderElement; 0] = []; - let clear_color = [0.05, 0.05, 0.08, 1.0]; - match gpu_output.render_frame(&mut gpu.renderer, &elements, clear_color, smithay::backend::drm::compositor::FrameFlags::DEFAULT) { - Ok(res) if !res.is_empty => match gpu_output.queue_frame(None) { - Ok(()) => {} - Err(e) => log::warn!("udev: SRDWM_GPU=1 queue_frame failed for crtc {head_crtc:?}: {e:?}"), - }, - Ok(_) => {} - Err(e) => log::warn!("udev: SRDWM_GPU=1 render_frame failed for crtc {head_crtc:?}: {e:?}"), - } - continue; + // Direct field access (`gpu.outputs`), not `GpuContext:: + // output_for_mut` - that method takes `&mut self`, which + // the borrow checker treats as borrowing all of `gpu`, + // including `gpu.renderer` needed a few lines below. Rust's + // disjoint-field-borrow analysis only sees through *direct* + // field access, not a method call, even one that (like + // this one) only actually touches `self.outputs`. + if let Some(gpu_output) = gpu.outputs.iter_mut().find(|(c, _)| *c == head_crtc).map(|(_, o)| o) { + let elements: [smithay::backend::renderer::element::memory::MemoryRenderBufferRenderElement; 0] = []; + let clear_color = [0.05, 0.05, 0.08, 1.0]; + match gpu_output.render_frame(&mut gpu.renderer, &elements, clear_color, smithay::backend::drm::compositor::FrameFlags::DEFAULT) { + Ok(res) if !res.is_empty => match gpu_output.queue_frame(None) { + Ok(()) => {} + Err(e) => log::warn!("udev: SRDWM_GPU=1 queue_frame failed for crtc {head_crtc:?}: {e:?}"), + }, + Ok(_) => {} + Err(e) => log::warn!("udev: SRDWM_GPU=1 render_frame failed for crtc {head_crtc:?}: {e:?}"), } + continue; } } diff --git a/crates/wayland/src/udev/session.rs b/crates/wayland/src/udev/session.rs index 7423f4a..bf6eb67 100644 --- a/crates/wayland/src/udev/session.rs +++ b/crates/wayland/src/udev/session.rs @@ -67,11 +67,9 @@ pub(crate) fn register_gpu_drm_notifier(handle: &LoopHandle<'static, CompState>, smithay::backend::drm::DrmEvent::VBlank(crtc) => { if let Some(udev) = data.udev.as_mut() { if let Some(gpu) = udev.gpu.as_ref() { - if let Some((gpu_crtc, gpu_output)) = gpu.output.as_ref() { - if *gpu_crtc == crtc { - if let Err(e) = gpu_output.frame_submitted() { - log::warn!("udev: SRDWM_GPU=1 frame_submitted failed for crtc {crtc:?}: {e:?}"); - } + if let Some(gpu_output) = gpu.output_for(crtc) { + if let Err(e) = gpu_output.frame_submitted() { + log::warn!("udev: SRDWM_GPU=1 frame_submitted failed for crtc {crtc:?}: {e:?}"); } } } @@ -217,20 +215,22 @@ pub(crate) fn register_session_notifier(handle: &LoopHandle<'static, CompState>, log::warn!("udev: SRDWM_GPU=1 failed to reactivate DrmOutputManager on resume: {e}"); } } - // The crtc `SRDWM_GPU=1`'s `DrmOutputManager` is driving - // (if any) - excluded from the legacy reassert loop - // below, since that loop's `set_crtc` runs through the - // *legacy* `Card`/fd, a completely different device - // handle than the GPU path's own `DrmDeviceFd`. Two - // separate fds issuing mode-set commands against the - // same physical CRTC is exactly the kind of conflict - // that produced this session's own worst VT-switch - // incidents when it was really one fd racing itself - // (`EBUSY` loops - see `UdevHead::flip_retry_after`'s - // own doc comment) - not a risk worth re-introducing - // here for a head this resume path already just - // reactivated correctly through its own, real API. - let gpu_crtc = udev.gpu.as_ref().and_then(|g| g.output.as_ref()).map(|(crtc, _)| *crtc); + // Every crtc `SRDWM_GPU=1`'s `DrmOutputManager` is + // driving (if any - now possibly several, since it + // targets every head it could, not just one) -- + // excluded from the legacy reassert loop below, since + // that loop's `set_crtc` runs through the *legacy* + // `Card`/fd, a completely different device handle than + // the GPU path's own `DrmDeviceFd`. Two separate fds + // issuing mode-set commands against the same physical + // CRTC is exactly the kind of conflict that produced + // this session's own worst VT-switch incidents when it + // was really one fd racing itself (`EBUSY` loops - see + // `UdevHead::flip_retry_after`'s own doc comment) - not + // a risk worth re-introducing here for a head this + // resume path already just reactivated correctly + // through its own, real API. + let gpu_crtcs: Vec = udev.gpu.as_ref().map(|g| g.outputs.iter().map(|(c, _)| *c).collect()).unwrap_or_default(); // Some drivers reset mode-setting state across a VT // switch; reassert every (non-GPU-driven) head before // rendering again. @@ -242,7 +242,7 @@ pub(crate) fn register_session_notifier(handle: &LoopHandle<'static, CompState>, // switching back with no further VT switch, either // direction, able to recover it. See `UdevHead::mode`'s // own doc comment. - for head in udev.heads.iter_mut().filter(|h| Some(h.crtc) != gpu_crtc) { + for head in udev.heads.iter_mut().filter(|h| !gpu_crtcs.contains(&h.crtc)) { let fb = head.buffers[head.front].fb; if let Err(e) = card.set_crtc(head.crtc, Some(fb), (0, 0), &[head.connector], Some(head.mode)) { log::warn!("udev: failed to reassert crtc on resume: {e}"); -- cgit v1.2.3