diff options
| -rw-r--r-- | crates/wayland/src/udev/gpu.rs | 72 | ||||
| -rw-r--r-- | crates/wayland/src/udev/platform.rs | 22 | ||||
| -rw-r--r-- | crates/wayland/src/udev/render.rs | 50 | ||||
| -rw-r--r-- | crates/wayland/src/udev/session.rs | 40 |
4 files changed, 108 insertions, 76 deletions
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<GlesRenderer, GpuElement> = DrmOutputRenderElements::default(); match self.output_manager.initialize_output::<GlesRenderer, GpuElement>(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::<CompState>(&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::<CompState>(&display_handle); let xdg_shell_state = XdgShellState::new::<CompState>(&display_handle); let xdg_decoration_state = XdgDecorationState::new::<CompState>(&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<smithay::backend::renderer::gles::GlesRenderer>; 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<smithay::backend::renderer::gles::GlesRenderer>; 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<crtc::Handle> = 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}"); |