From e34a289c01c99ab39dac693e5c867f6bdf825b0d Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Wed, 16 Jul 2025 21:13:50 +0200 Subject: [PATCH] kms: lock during screen conf changes --- src/backend/kms/device.rs | 516 +++++++++++++++---- src/backend/kms/mod.rs | 284 +++++----- src/backend/kms/surface/mod.rs | 65 +-- src/backend/winit.rs | 11 +- src/backend/x11.rs | 11 +- src/config/mod.rs | 3 +- src/state.rs | 223 ++++---- src/wayland/handlers/buffer.rs | 5 +- src/wayland/handlers/drm_lease.rs | 6 +- src/wayland/handlers/output_configuration.rs | 5 +- 10 files changed, 747 insertions(+), 382 deletions(-) diff --git a/src/backend/kms/device.rs b/src/backend/kms/device.rs index afe2662b..465f13cc 100644 --- a/src/backend/kms/device.rs +++ b/src/backend/kms/device.rs @@ -1,7 +1,10 @@ // SPDX-License-Identifier: GPL-3.0-only use crate::{ - backend::render::{output_elements, CursorMode, GlMultiRenderer, CLEAR_COLOR}, + backend::{ + kms::render::gles::GbmGlowBackend, + render::{init_shaders, output_elements, CursorMode, GlMultiRenderer, CLEAR_COLOR}, + }, config::{AdaptiveSync, EdidProduct, OutputConfig, OutputState, ScreenFilter}, shell::Shell, utils::{env::dev_list_var, prelude::*}, @@ -20,10 +23,11 @@ use smithay::{ drm::{ compositor::{FrameError, FrameFlags}, exporter::gbm::GbmFramebufferExporter, - output::DrmOutputManager, + output::{DrmOutputManager, LockedDrmOutputManager}, DrmDevice, DrmDeviceFd, DrmEvent, DrmNode, NodeType, }, egl::{context::ContextPriority, EGLContext, EGLDevice, EGLDisplay}, + renderer::glow::GlowRenderer, session::Session, }, desktop::utils::OutputPresentationFeedback, @@ -43,6 +47,7 @@ use smithay::{ use tracing::{error, info, warn}; use std::{ + borrow::BorrowMut, cell::RefCell, collections::{HashMap, HashSet}, fmt, @@ -60,6 +65,18 @@ pub struct EGLInternals { pub context: EGLContext, } +pub type LockedGbmDrmOutputManager<'a> = LockedDrmOutputManager< + 'a, + GbmAllocator, + GbmFramebufferExporter, + Option<( + OutputPresentationFeedback, + Receiver<(Frame, Vec>)>, + Duration, + )>, + DrmDeviceFd, +>; + pub type GbmDrmOutputManager = DrmOutputManager< GbmAllocator, GbmFramebufferExporter, @@ -111,6 +128,22 @@ impl fmt::Debug for Device { } } +pub struct LockedDevice<'a> { + pub dev_node: &'a DrmNode, + pub render_node: &'a DrmNode, + pub egl: &'a mut Option, + + pub outputs: &'a HashMap, + pub surfaces: &'a mut HashMap, + pub drm: LockedGbmDrmOutputManager<'a>, + pub gbm: &'a GbmDevice, + + pub leased_connectors: &'a mut Vec<(connector::Handle, crtc::Handle)>, + pub leasing_global: &'a mut Option, + pub active_leases: &'a mut Vec, + pub active_buffers: &'a HashSet>, +} + pub fn init_egl(gbm: &GbmDevice) -> Result { let path = gbm.dev_path(); @@ -496,13 +529,13 @@ impl State { .write() .unwrap() .take_if(|node| node == &device.render_node); - } + }; self.common .output_configuration_state .remove_heads(outputs_removed.iter()); backend.refresh_used_devices()?; - if self.backend.kms().session.is_active() { + if backend.session.is_active() { for output in outputs_removed { self.common.remove_output(&output); } @@ -570,107 +603,24 @@ impl Device { Ok(OutputChanges { added, removed }) } - pub fn connector_added( - &mut self, - primary_node: Arc>>, - conn: connector::Handle, - maybe_crtc: Option, - position: (u32, u32), - evlh: &LoopHandle<'static, State>, - screen_filter: ScreenFilter, - shell: Arc>, - startup_done: Arc, - ) -> Result<(Output, bool)> { - let output = self - .outputs - .get(&conn) - .cloned() - .map(|output| Ok(output)) - .unwrap_or_else(|| create_output_for_conn(self.drm.device_mut(), conn)) - .context("Failed to create `Output`")?; - - let non_desktop = - match drm_helpers::get_property_val(self.drm.device_mut(), conn, "non-desktop") { - Ok((val_type, value)) => val_type.convert_value(value).as_boolean().unwrap(), - Err(err) => { - warn!( - ?err, - "Failed to determine if connector is meant desktop usage, assuming so." - ); - false - } - }; - - if non_desktop { - if let Some(crtc) = maybe_crtc { - self.leased_connectors.push((conn, crtc)); - info!( - "Connector {} is non-desktop, setting up for leasing", - output.name() - ); - if let Some(lease_state) = self.leasing_global.as_mut() { - let physical = output.physical_properties(); - lease_state.add_connector::( - conn, - output.name(), - format!("{} {}", physical.make, physical.model), - ); - } - } else { - warn!( - "Connector {} is non-desktop, but we don't have a free crtc: not leasing", - output.name() - ); - } - - Ok((output, false)) - } else { - output - .user_data() - .insert_if_missing(|| RefCell::new(OutputConfig::default())); - - populate_modes(self.drm.device_mut(), &output, conn, position) - .with_context(|| "Failed to enumerate connector modes")?; - - let has_surface = if let Some(crtc) = maybe_crtc { - match Surface::new( - &output, - crtc, - conn, - primary_node, - self.dev_node, - self.render_node, - evlh, - screen_filter, - shell, - startup_done, - ) { - Ok(data) => { - self.surfaces.insert(crtc, data); - true - } - Err(err) => { - error!(?crtc, "Failed to initialize surface: {}", err); - false - } - } - } else { - false - }; - - if !has_surface { - output - .user_data() - .get::>() - .unwrap() - .borrow_mut() - .enabled = OutputState::Disabled; - } - - Ok((output, true)) + pub fn lock(&mut self) -> LockedDevice<'_> { + LockedDevice { + dev_node: &self.dev_node, + render_node: &self.render_node, + egl: &mut self.egl, + outputs: &self.outputs, + surfaces: &mut self.surfaces, + drm: self.drm.lock(), + gbm: &self.gbm, + leased_connectors: &mut self.leased_connectors, + leasing_global: &mut self.leasing_global, + active_leases: &mut self.active_leases, + active_buffers: &mut self.active_buffers, } } +} +impl<'a> LockedDevice<'a> { fn allow_frame_flags( &mut self, flag: bool, @@ -692,9 +642,7 @@ impl Device { .map(|(crtc, surface)| (*crtc, surface.output.clone())) .collect::>(); - let mut drm = self.drm.lock(); - let map = drm.compositors(); - for (crtc, compositor) in map.iter() { + for (crtc, compositor) in self.drm.compositors().iter() { let elements = match output_map.get(crtc) { Some(output) => output_elements( Some(&self.render_node), @@ -753,9 +701,7 @@ impl Device { shell, )?; - let mut drm = self.drm.lock(); - let maps = drm.compositors(); - for (crtc, comp) in maps { + for (crtc, comp) in self.drm.compositors() { let Some(surface) = self.surfaces.get_mut(crtc) else { continue; }; @@ -777,12 +723,364 @@ impl Device { Ok(()) } +} - pub fn in_use(&self, primary: Option<&DrmNode>) -> bool { +pub trait MaybeLockedDevice { + fn dev_node(&self) -> DrmNode; + fn render_node(&self) -> DrmNode; + fn output(&self, conn: &connector::Handle) -> Option<&Output>; + fn drm_device_mut(&mut self) -> &mut DrmDevice; + fn gbm(&self) -> &GbmDevice; + fn egl(&mut self) -> &mut Option; + fn insert_surface(&mut self, crtc: crtc::Handle, surface: Surface); + + fn in_use(&self, primary: Option<&DrmNode>) -> bool; + fn add_leased_connector( + &mut self, + crtc: crtc::Handle, + conn: connector::Handle, + output: &Output, + ); + + fn connector_added( + &mut self, + primary_node: Arc>>, + conn: connector::Handle, + maybe_crtc: Option, + position: (u32, u32), + evlh: &LoopHandle<'static, State>, + screen_filter: ScreenFilter, + shell: Arc>, + startup_done: Arc, + ) -> Result<(Output, bool)> { + let output = self + .output(&conn) + .cloned() + .map(|output| Ok(output)) + .unwrap_or_else(|| create_output_for_conn(self.drm_device_mut(), conn)) + .context("Failed to create `Output`")?; + + let non_desktop = + match drm_helpers::get_property_val(self.drm_device_mut(), conn, "non-desktop") { + Ok((val_type, value)) => val_type.convert_value(value).as_boolean().unwrap(), + Err(err) => { + warn!( + ?err, + "Failed to determine if connector is meant desktop usage, assuming so." + ); + false + } + }; + + if non_desktop { + if let Some(crtc) = maybe_crtc { + self.add_leased_connector(crtc, conn, &output); + } else { + warn!( + "Connector {} is non-desktop, but we don't have a free crtc: not leasing", + output.name() + ); + } + + Ok((output, false)) + } else { + output + .user_data() + .insert_if_missing(|| RefCell::new(OutputConfig::default())); + + populate_modes(self.drm_device_mut(), &output, conn, position) + .with_context(|| "Failed to enumerate connector modes")?; + + let has_surface = if let Some(crtc) = maybe_crtc { + match Surface::new( + &output, + crtc, + conn, + primary_node, + self.dev_node(), + self.render_node(), + evlh, + screen_filter, + shell, + startup_done, + ) { + Ok(data) => { + self.insert_surface(crtc, data); + true + } + Err(err) => { + error!(?crtc, "Failed to initialize surface: {}", err); + false + } + } + } else { + false + }; + + if !has_surface { + output + .user_data() + .get::>() + .unwrap() + .borrow_mut() + .enabled = OutputState::Disabled; + } + + Ok((output, true)) + } + } + + fn update_egl( + &mut self, + primary_node: Option<&DrmNode>, + api: &mut GbmGlowBackend, + ) -> Result { + if self.in_use(primary_node) { + if self.egl().is_none() { + let egl = init_egl(self.gbm()).context("Failed to create EGL context")?; + let mut renderer = unsafe { + GlowRenderer::new( + EGLContext::new_shared_with_priority( + &egl.display, + &egl.context, + ContextPriority::High, + ) + .context("Failed to create shared EGL context")?, + ) + .context("Failed to create GL renderer")? + }; + init_shaders(renderer.borrow_mut()).context("Failed to compile shaders")?; + api.add_node( + self.render_node(), + GbmAllocator::new( + self.gbm().clone(), + // SCANOUT because stride bugs + GbmBufferFlags::RENDERING | GbmBufferFlags::SCANOUT, + ), + renderer, + ); + *self.egl() = Some(egl); + } + Ok(true) + } else { + if self.egl().is_some() { + let _ = self.egl().take(); + api.remove_node(&self.render_node()); + } + Ok(false) + } + } + + fn update_surface_nodes<'b>( + &mut self, + used_devices: &HashSet, + others: impl Iterator, + ) -> Result<()> + where + Self: 'b; +} + +impl MaybeLockedDevice for Device { + fn dev_node(&self) -> DrmNode { + self.dev_node + } + + fn render_node(&self) -> DrmNode { + self.render_node + } + + fn output(&self, conn: &connector::Handle) -> Option<&Output> { + self.outputs.get(conn) + } + + fn drm_device_mut(&mut self) -> &mut DrmDevice { + self.drm.device_mut() + } + + fn gbm(&self) -> &GbmDevice { + &self.gbm + } + + fn egl(&mut self) -> &mut Option { + &mut self.egl + } + + fn insert_surface(&mut self, crtc: crtc::Handle, surface: Surface) { + self.surfaces.insert(crtc, surface); + } + + fn in_use(&self, primary: Option<&DrmNode>) -> bool { Some(&self.render_node) == primary || !self.surfaces.is_empty() || !self.active_buffers.is_empty() } + + fn add_leased_connector( + &mut self, + crtc: crtc::Handle, + conn: connector::Handle, + output: &Output, + ) { + self.leased_connectors.push((conn, crtc)); + info!( + "Connector {} is non-desktop, setting up for leasing", + output.name() + ); + if let Some(lease_state) = self.leasing_global.as_mut() { + let physical = output.physical_properties(); + lease_state.add_connector::( + conn, + output.name(), + format!("{} {}", physical.make, physical.model), + ); + } + } + + fn update_surface_nodes<'b>( + &mut self, + used_devices: &HashSet, + mut others: impl Iterator, + ) -> Result<()> { + for surface in self.surfaces.values_mut() { + let known_nodes = surface.known_nodes().clone(); + for gone_device in known_nodes.difference(&used_devices) { + surface.remove_node(*gone_device); + } + for new_device in used_devices.difference(&known_nodes) { + let (render_node, egl, gbm) = if self.render_node == *new_device { + // we need to make sure to do partial borrows here, as device.surfaces is borrowed mutable + ( + self.render_node, + self.egl.as_ref().unwrap(), + self.gbm.clone(), + ) + } else { + let device = others.find(|d| d.render_node == *new_device).unwrap(); + ( + device.render_node, + device.egl.as_ref().unwrap(), + device.gbm.clone(), + ) + }; + + surface.add_node( + render_node, + GbmAllocator::new(gbm, GbmBufferFlags::RENDERING | GbmBufferFlags::SCANOUT), + EGLContext::new_shared_with_priority( + &egl.display, + &egl.context, + ContextPriority::High, + ) + .context("Failed to create shared EGL context")?, + ); + } + } + + Ok(()) + } +} + +impl<'a> MaybeLockedDevice for LockedDevice<'a> { + fn dev_node(&self) -> DrmNode { + *self.dev_node + } + + fn render_node(&self) -> DrmNode { + *self.render_node + } + + fn output(&self, conn: &connector::Handle) -> Option<&Output> { + self.outputs.get(conn) + } + + fn drm_device_mut(&mut self) -> &mut DrmDevice { + self.drm.device_mut() + } + + fn gbm(&self) -> &GbmDevice { + self.gbm + } + + fn egl(&mut self) -> &mut Option { + self.egl + } + + fn insert_surface(&mut self, crtc: crtc::Handle, surface: Surface) { + self.surfaces.insert(crtc, surface); + } + + fn in_use(&self, primary: Option<&DrmNode>) -> bool { + Some(self.render_node) == primary + || !self.surfaces.is_empty() + || !self.active_buffers.is_empty() + } + + fn add_leased_connector( + &mut self, + crtc: crtc::Handle, + conn: connector::Handle, + output: &Output, + ) { + self.leased_connectors.push((conn, crtc)); + info!( + "Connector {} is non-desktop, setting up for leasing", + output.name() + ); + if let Some(lease_state) = self.leasing_global.as_mut() { + let physical = output.physical_properties(); + lease_state.add_connector::( + conn, + output.name(), + format!("{} {}", physical.make, physical.model), + ); + } + } + + fn update_surface_nodes<'b>( + &mut self, + used_devices: &HashSet, + mut others: impl Iterator>, + ) -> Result<()> + where + 'a: 'b, + { + for surface in self.surfaces.values_mut() { + let known_nodes = surface.known_nodes().clone(); + for gone_device in known_nodes.difference(&used_devices) { + surface.remove_node(*gone_device); + } + for new_device in used_devices.difference(&known_nodes) { + let (render_node, egl, gbm) = if *self.render_node == *new_device { + // we need to make sure to do partial borrows here, as device.surfaces is borrowed mutable + ( + self.render_node, + self.egl.as_ref().unwrap(), + self.gbm.clone(), + ) + } else { + let device = others.find(|d| d.render_node == new_device).unwrap(); + ( + device.render_node, + device.egl.as_ref().unwrap(), + device.gbm.clone(), + ) + }; + + surface.add_node( + *render_node, + GbmAllocator::new(gbm, GbmBufferFlags::RENDERING | GbmBufferFlags::SCANOUT), + EGLContext::new_shared_with_priority( + &egl.display, + &egl.context, + ContextPriority::High, + ) + .context("Failed to create shared EGL context")?, + ); + } + } + + Ok(()) + } } fn create_output_for_conn(drm: &mut DrmDevice, conn: connector::Handle) -> Result { diff --git a/src/backend/kms/mod.rs b/src/backend/kms/mod.rs index 5c607442..12988cec 100644 --- a/src/backend/kms/mod.rs +++ b/src/backend/kms/mod.rs @@ -13,13 +13,9 @@ use indexmap::IndexMap; use render::gles::GbmGlowBackend; use smithay::{ backend::{ - allocator::{ - dmabuf::Dmabuf, - gbm::{GbmAllocator, GbmBufferFlags}, - Buffer, - }, - drm::{output::DrmOutputRenderElements, DrmDeviceFd, DrmNode, NodeType}, - egl::{context::ContextPriority, EGLContext, EGLDevice, EGLDisplay}, + allocator::{dmabuf::Dmabuf, format::FormatSet, Buffer}, + drm::{output::DrmOutputRenderElements, DrmDeviceFd, DrmNode, NodeType, VrrSupport}, + egl::{EGLContext, EGLDevice, EGLDisplay}, input::InputEvent, libinput::{LibinputInputBackend, LibinputSessionInterface}, renderer::{glow::GlowRenderer, multigpu::GpuManager}, @@ -47,7 +43,6 @@ use surface::GbmDrmOutput; use tracing::{error, info, trace, warn}; use std::{ - borrow::BorrowMut, collections::{HashMap, HashSet}, path::Path, sync::{atomic::AtomicBool, Arc, RwLock}, @@ -59,11 +54,12 @@ pub mod render; mod socket; mod surface; pub(crate) use surface::Surface; - -use device::*; pub use surface::Timings; -use super::render::{init_shaders, output_elements, CursorMode, CLEAR_COLOR}; +pub use device::MaybeLockedDevice; +use device::*; + +use super::render::{output_elements, CursorMode, CLEAR_COLOR}; #[derive(Debug)] pub struct KmsState { @@ -80,6 +76,13 @@ pub struct KmsState { pub syncobj_state: Option, } +pub struct KmsGuard<'a> { + pub drm_devices: IndexMap>, + pub primary_node: Arc>>, + api: &'a mut GpuManager>, + session: &'a LibSeatSession, +} + pub fn init_backend( dh: &DisplayHandle, event_loop: &mut EventLoop<'static, State>, @@ -515,47 +518,33 @@ impl KmsState { .copied() } + pub fn update_screen_filter(&mut self, screen_filter: &ScreenFilter) -> Result<()> { + for device in self.drm_devices.values_mut() { + for surface in device.surfaces.values_mut() { + surface.set_screen_filter(screen_filter.clone()); + } + } + + // We don't expect this to fail in a meaningful way. + // The shader is already compiled at this point and we don't rely on any features, + // that might not be available for any filters we currently expose. + // + // But we might conditionally fail here in the future. + Ok(()) + } + pub fn refresh_used_devices(&mut self) -> Result<()> { + let primary_node = self.primary_node.read().unwrap(); let mut used_devices = HashSet::new(); for device in self.drm_devices.values_mut() { - if device.in_use(self.primary_node.read().unwrap().as_ref()) { - if device.egl.is_none() { - let egl = init_egl(&device.gbm).context("Failed to create EGL context")?; - let mut renderer = unsafe { - GlowRenderer::new( - EGLContext::new_shared_with_priority( - &egl.display, - &egl.context, - ContextPriority::High, - ) - .context("Failed to create shared EGL context")?, - ) - .context("Failed to create GL renderer")? - }; - init_shaders(renderer.borrow_mut()).context("Failed to compile shaders")?; - self.api.as_mut().add_node( - device.render_node, - GbmAllocator::new( - device.gbm.clone(), - // SCANOUT because stride bugs - GbmBufferFlags::RENDERING | GbmBufferFlags::SCANOUT, - ), - renderer, - ); - device.egl = Some(egl); - } - used_devices.insert(device.render_node); - } else { - if device.egl.is_some() { - let _ = device.egl.take(); - self.api.as_mut().remove_node(&device.render_node); - } + if device.update_egl(primary_node.as_ref(), self.api.as_mut())? { + used_devices.insert(device.render_node()); } } // trigger re-evaluation... urgh - if let Some(primary_node) = self.primary_node.read().unwrap().as_ref() { + if let Some(primary_node) = primary_node.as_ref() { let _ = self.api.single_renderer(primary_node); } @@ -566,54 +555,92 @@ impl KmsState { .map(|d| d.render_node) .collect::>(); for node in all_devices { - let (mut device, mut others) = self + let (mut device, others) = self .drm_devices .values_mut() .partition::, _>(|d| d.render_node == node); - let device = &mut device[0]; - - for surface in device.surfaces.values_mut() { - let known_nodes = surface.known_nodes().clone(); - for gone_device in known_nodes.difference(&used_devices) { - surface.remove_node(*gone_device); - } - for new_device in used_devices.difference(&known_nodes) { - let (render_node, egl, gbm) = if node == *new_device { - // we need to make sure to do partial borrows here, as device.surfaces is borrowed mutable - ( - device.render_node, - device.egl.as_ref().unwrap(), - device.gbm.clone(), - ) - } else { - let device = others - .iter_mut() - .find(|d| d.render_node == *new_device) - .unwrap(); - ( - device.render_node, - device.egl.as_ref().unwrap(), - device.gbm.clone(), - ) - }; - - surface.add_node( - render_node, - GbmAllocator::new(gbm, GbmBufferFlags::RENDERING | GbmBufferFlags::SCANOUT), - EGLContext::new_shared_with_priority( - &egl.display, - &egl.context, - ContextPriority::High, - ) - .context("Failed to create shared EGL context")?, - ); - } - } + device[0].update_surface_nodes(&used_devices, others.iter().map(|device| &**device))?; } Ok(()) } + pub fn lock_devices(&mut self) -> KmsGuard<'_> { + KmsGuard { + drm_devices: self + .drm_devices + .iter_mut() + .map(|(node, device)| (*node, device.lock())) + .collect(), + primary_node: self.primary_node.clone(), + api: &mut self.api, + session: &self.session, + } + } +} + +impl<'a> KmsGuard<'a> { + pub fn schedule_render(&mut self, output: &Output) { + for surface in self + .drm_devices + .values() + .flat_map(|d| d.surfaces.values()) + .filter(|s| s.output == *output || s.output.mirroring().is_some_and(|o| &o == output)) + { + surface.schedule_render(); + } + } + + pub fn refresh_used_devices(&mut self) -> Result<()> { + let primary_node = self.primary_node.read().unwrap(); + let mut used_devices = HashSet::new(); + + for device in self.drm_devices.values_mut() { + if device.update_egl(primary_node.as_ref(), self.api.as_mut())? { + used_devices.insert(device.render_node()); + } + } + + // trigger re-evaluation... urgh + if let Some(primary_node) = primary_node.as_ref() { + let _ = self.api.single_renderer(primary_node); + } + + // I hate this. I want partial borrows of hashmap values + let all_devices = self + .drm_devices + .values() + .map(|d| d.render_node) + .collect::>(); + for node in all_devices { + let (mut device, others) = self + .drm_devices + .values_mut() + .partition::, _>(|d| d.render_node == node); + device[0].update_surface_nodes(&used_devices, others.iter().map(|device| &**device))?; + } + + Ok(()) + } + + pub fn all_outputs(&self) -> Vec { + self.drm_devices + .values() + .flat_map(|device| { + device + .outputs + .iter() + .filter(|(conn, _)| { + !device + .leased_connectors + .iter() + .any(|(leased_conn, _)| *conn == leased_conn) + }) + .map(|(_, output)| output.clone()) + }) + .collect() + } + pub fn apply_config_for_outputs( &mut self, test_only: bool, @@ -622,9 +649,9 @@ impl KmsState { shell: Arc>, startup_done: Arc, clock: &Clock, - ) -> Result, anyhow::Error> { + ) -> Result<(), anyhow::Error> { if !self.session.is_active() { - return Ok(Vec::new()); + return Ok(()); } for device in self.drm_devices.values_mut() { @@ -771,7 +798,7 @@ impl KmsState { for (crtc, surface) in device.surfaces.iter_mut() { let output_config = surface.output.config(); - let drm = &mut device.drm.lock(); + let drm = &mut device.drm; let conn = surface.connector; let conn_info = drm.device().get_connector(conn, false)?; let mode = conn_info @@ -861,34 +888,61 @@ impl KmsState { let vrr = output_config.vrr; std::mem::drop(output_config); - match surface.resume(compositor) { - Ok(_) => { - surface.output.set_adaptive_sync_support( - surface.adaptive_sync_support().ok(), - ); - if surface.use_adaptive_sync(vrr)? { - surface.output.set_adaptive_sync(vrr); - } else { - surface.output.config_mut().vrr = AdaptiveSync::Disabled; - surface.output.set_adaptive_sync(AdaptiveSync::Disabled); - } - } - Err(err) => { - surface.output.config_mut().enabled = OutputState::Disabled; - return Err(err).context("Failed to create surface"); + let compositor_ref = drm.compositors().get(crtc).unwrap().lock().unwrap(); + let vrr_support = compositor_ref + .vrr_supported( + compositor_ref + .pending_connectors() + .into_iter() + .next() + .unwrap(), + ) + .ok(); + surface.resume( + compositor, + compositor_ref.surface().plane_info().formats.clone(), + compositor_ref + .surface() + .planes() + .overlay + .iter() + .flat_map(|p| p.formats.iter().cloned()) + .collect::(), + ); + + surface.output.set_adaptive_sync_support(vrr_support); + if match vrr_support { + Some(VrrSupport::RequiresModeset) if vrr == AdaptiveSync::Enabled => { + false } + Some(VrrSupport::NotSupported) => false, + _ => true, + } { + surface.use_adaptive_sync(vrr); + surface.output.set_adaptive_sync(vrr); + } else { + surface.use_adaptive_sync(AdaptiveSync::Disabled); + surface.output.config_mut().vrr = AdaptiveSync::Disabled; + surface.output.set_adaptive_sync(AdaptiveSync::Disabled); } } else { let vrr = output_config.vrr; std::mem::drop(output_config); if vrr != surface.output.adaptive_sync() { - if surface.use_adaptive_sync(vrr)? { - surface.output.set_adaptive_sync(vrr); - } else if vrr != AdaptiveSync::Disabled { + if match surface.output.adaptive_sync_support() { + Some(VrrSupport::RequiresModeset) + if vrr == AdaptiveSync::Enabled => + { + true + } + Some(VrrSupport::NotSupported) => true, + _ => false, + } { anyhow::bail!("Requested VRR mode unsupported"); - } else { - surface.output.set_adaptive_sync(AdaptiveSync::Disabled); } + + surface.use_adaptive_sync(vrr); + surface.output.set_adaptive_sync(vrr); } let mut renderer = self @@ -956,7 +1010,6 @@ impl KmsState { if let Err(err) = device .drm - .lock() .try_to_restore_modifiers(&mut renderer, &elements) { warn!(?err, "Failed to restore modifiers"); @@ -988,21 +1041,6 @@ impl KmsState { } } - Ok(all_outputs) - } - - pub fn update_screen_filter(&mut self, screen_filter: &ScreenFilter) -> Result<()> { - for device in self.drm_devices.values_mut() { - for surface in device.surfaces.values_mut() { - surface.set_screen_filter(screen_filter.clone()); - } - } - - // We don't expect this to fail in a meaningful way. - // The shader is already compiled at this point and we don't rely on any features, - // that might not be available for any filters we currently expose. - // - // But we might conditionally fail here in the future. Ok(()) } } diff --git a/src/backend/kms/surface/mod.rs b/src/backend/kms/surface/mod.rs index 47cc61ca..f997888f 100644 --- a/src/backend/kms/surface/mod.rs +++ b/src/backend/kms/surface/mod.rs @@ -204,7 +204,6 @@ pub enum ThreadCommand { Suspend(SyncSender<()>), Resume { compositor: GbmDrmOutput, - result: SyncSender>, }, NodeAdded { node: DrmNode, @@ -220,7 +219,7 @@ pub enum ThreadCommand { ScheduleRender, AdaptiveSyncAvailable(SyncSender>), UseAdaptiveSync(AdaptiveSync), - AllowFrameFlags(bool, FrameFlags, SyncSender<()>), + AllowFrameFlags(bool, FrameFlags), End, DpmsOff, } @@ -404,31 +403,16 @@ impl Surface { rx.recv().context("Surface thread died")? } - pub fn use_adaptive_sync(&mut self, vrr: AdaptiveSync) -> Result { - if vrr != AdaptiveSync::Disabled { - let (tx, rx) = std::sync::mpsc::sync_channel(1); - let _ = self - .thread_command - .send(ThreadCommand::AdaptiveSyncAvailable(tx)); - match rx.recv().context("Surface thread died")?? { - VrrSupport::RequiresModeset if vrr == AdaptiveSync::Enabled => return Ok(false), - VrrSupport::NotSupported => return Ok(false), - _ => {} - }; - } - + pub fn use_adaptive_sync(&mut self, vrr: AdaptiveSync) { let _ = self .thread_command .send(ThreadCommand::UseAdaptiveSync(vrr)); - Ok(true) } pub fn allow_frame_flags(&mut self, flag: bool, flags: FrameFlags) { - let (tx, rx) = std::sync::mpsc::sync_channel(1); let _ = self .thread_command - .send(ThreadCommand::AllowFrameFlags(flag, flags, tx)); - let _ = rx.recv(); + .send(ThreadCommand::AllowFrameFlags(flag, flags)); } pub fn suspend(&mut self) { @@ -437,27 +421,19 @@ impl Surface { let _ = rx.recv(); } - pub fn resume(&mut self, compositor: GbmDrmOutput) -> Result<()> { - let (tx, rx) = std::sync::mpsc::sync_channel(1); - (self.primary_plane_formats, self.overlay_plane_formats) = - compositor.with_compositor(|c| { - ( - c.surface().plane_info().formats.clone(), - c.surface() - .planes() - .overlay - .iter() - .flat_map(|p| p.formats.iter().cloned()) - .collect::(), - ) - }); + pub fn resume( + &mut self, + compositor: GbmDrmOutput, + primary_plane_formats: FormatSet, + overlay_plane_formats: FormatSet, + ) { + self.primary_plane_formats = primary_plane_formats; + self.overlay_plane_formats = overlay_plane_formats; + self.active.store(true, Ordering::SeqCst); - let _ = self.thread_command.send(ThreadCommand::Resume { - compositor, - result: tx, - }); - - rx.recv().context("Surface thread died")? + let _ = self + .thread_command + .send(ThreadCommand::Resume { compositor }); } pub fn get_dpms(&mut self) -> bool { @@ -564,8 +540,8 @@ fn surface_thread( .handle() .insert_source(thread_receiver, move |command, _, state| match command { Event::Msg(ThreadCommand::Suspend(tx)) => state.suspend(tx), - Event::Msg(ThreadCommand::Resume { compositor, result }) => { - let _ = result.send(state.resume(compositor)); + Event::Msg(ThreadCommand::Resume { compositor }) => { + state.resume(compositor); } Event::Msg(ThreadCommand::NodeAdded { node, gbm, egl }) => { if let Err(err) = state.node_added(node, gbm, egl) { @@ -631,7 +607,7 @@ fn surface_thread( }; } } - Event::Msg(ThreadCommand::AllowFrameFlags(flag, mut flags, tx)) => { + Event::Msg(ThreadCommand::AllowFrameFlags(flag, mut flags)) => { if crate::utils::env::bool_var("COSMIC_DISABLE_DIRECT_SCANOUT").unwrap_or(false) { flags.remove(FrameFlags::ALLOW_SCANOUT); } @@ -644,7 +620,6 @@ fn surface_thread( } else { state.frame_flags.remove(flags); } - let _ = tx.send(()); } Event::Closed | Event::Msg(ThreadCommand::End) => { signal.stop(); @@ -680,7 +655,7 @@ impl SurfaceThreadState { let _ = tx.send(()); } - fn resume(&mut self, compositor: GbmDrmOutput) -> Result<()> { + fn resume(&mut self, compositor: GbmDrmOutput) { let (mode, min_hz) = compositor.with_compositor(|c| { ( c.surface().pending_mode(), @@ -710,9 +685,7 @@ impl SurfaceThreadState { self.frame_flags .remove(FrameFlags::ALLOW_OVERLAY_PLANE_SCANOUT); } - self.active.store(true, Ordering::SeqCst); self.compositor = Some(compositor); - Ok(()) } fn node_added( diff --git a/src/backend/winit.rs b/src/backend/winit.rs index 9563a6f6..a0af2432 100644 --- a/src/backend/winit.rs +++ b/src/backend/winit.rs @@ -100,10 +100,11 @@ impl WinitState { Ok(()) } - pub fn apply_config_for_outputs( - &mut self, - test_only: bool, - ) -> Result, anyhow::Error> { + pub fn all_outputs(&self) -> Vec { + vec![self.output.clone()] + } + + pub fn apply_config_for_outputs(&mut self, test_only: bool) -> Result<(), anyhow::Error> { // TODO: if we ever have multiple winit outputs, don't ignore config.enabled // reset size let size = self.backend.window_size(); @@ -119,7 +120,7 @@ impl WinitState { } Err(anyhow::anyhow!("Cannot set window size")) } else { - Ok(vec![self.output.clone()]) + Ok(()) } } diff --git a/src/backend/x11.rs b/src/backend/x11.rs index 8b4b4a4b..7bae191e 100644 --- a/src/backend/x11.rs +++ b/src/backend/x11.rs @@ -162,10 +162,11 @@ impl X11State { } } - pub fn apply_config_for_outputs( - &mut self, - test_only: bool, - ) -> Result, anyhow::Error> { + pub fn all_outputs(&self) -> Vec { + self.surfaces.iter().map(|s| s.output.clone()).collect() + } + + pub fn apply_config_for_outputs(&mut self, test_only: bool) -> Result<(), anyhow::Error> { // TODO: if we ever have multiple winit outputs, don't juse use the first and don't ignore OutputState let surface = self.surfaces.first().unwrap(); @@ -184,7 +185,7 @@ impl X11State { } Err(anyhow::anyhow!("Cannot set window size")) } else { - Ok(vec![surface.output.clone()]) + Ok(()) } } diff --git a/src/config/mod.rs b/src/config/mod.rs index d6dba891..4d66547c 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -505,6 +505,7 @@ impl Config { found_outputs.push((output.clone(), enabled)); } + let mut backend = backend.lock(); if let Err(err) = backend.apply_config_for_outputs( false, loop_handle, @@ -580,7 +581,7 @@ impl Config { w += output.geometry().size.w as u32; } - if let Err(err) = backend.apply_config_for_outputs( + if let Err(err) = backend.lock().apply_config_for_outputs( false, loop_handle, self.dynamic_conf.screen_filter(), diff --git a/src/state.rs b/src/state.rs index 9451a17f..e20ac349 100644 --- a/src/state.rs +++ b/src/state.rs @@ -2,7 +2,7 @@ use crate::{ backend::{ - kms::KmsState, + kms::{KmsGuard, KmsState}, render::{GlMultiError, RendererRef}, winit::WinitState, x11::X11State, @@ -261,6 +261,12 @@ pub enum BackendData { Unset, } +pub enum LockedBackend<'a> { + X11(&'a mut X11State), + Winit(&'a mut WinitState), + Kms(KmsGuard<'a>), +} + #[derive(Debug, Clone)] pub struct SurfaceDmabufFeedback { pub render_feedback: DmabufFeedback, @@ -302,93 +308,6 @@ impl BackendData { } } - pub fn apply_config_for_outputs( - &mut self, - test_only: bool, - loop_handle: &LoopHandle<'static, State>, - screen_filter: &ScreenFilter, - shell: Arc>, - workspace_state: &mut WorkspaceUpdateGuard<'_, State>, - xdg_activation_state: &XdgActivationState, - startup_done: Arc, - clock: &Clock, - ) -> Result<(), anyhow::Error> { - let result = match self { - BackendData::Kms(ref mut state) => state.apply_config_for_outputs( - test_only, - loop_handle, - screen_filter, - shell.clone(), - startup_done, - clock, - ), - BackendData::Winit(ref mut state) => state.apply_config_for_outputs(test_only), - BackendData::X11(ref mut state) => state.apply_config_for_outputs(test_only), - _ => unreachable!("No backend set when applying output config"), - }?; - - let mut shell = shell.write(); - for output in result { - // apply to Output - let final_config = output - .user_data() - .get::>() - .unwrap() - .borrow(); - - let mode = Some(final_config.output_mode()).filter(|m| match output.current_mode() { - None => true, - Some(c_m) => m.size != c_m.size || m.refresh != c_m.refresh, - }); - let transform = - Some(final_config.transform.into()).filter(|x| *x != output.current_transform()); - let scale = Some(final_config.scale) - .filter(|x| *x != output.current_scale().fractional_scale()); - let location = Some(Point::from(( - final_config.position.0 as i32, - final_config.position.1 as i32, - ))) - .filter(|x| *x != output.current_location()); - output.change_current_state(mode, transform, scale.map(Scale::Fractional), location); - - output.set_adaptive_sync(final_config.vrr); - output.set_mirroring(match &final_config.enabled { - OutputState::Mirroring(conn) => shell - .outputs() - .find(|output| &output.name() == conn) - .cloned(), - _ => None, - }); - - match final_config.enabled { - OutputState::Enabled => shell.workspaces.add_output(&output, workspace_state), - _ => { - let shell = &mut *shell; - shell.workspaces.remove_output( - &output, - shell.seats.iter(), - workspace_state, - xdg_activation_state, - ) - } - } - - layer_map_for_output(&output).arrange(); - - self.schedule_render(&output); - } - - // Update layout for changes in resolution, scale, orientation - shell.workspaces.recalculate(); - - loop_handle.insert_idle(move |state| { - state.common.update_xwayland_scale(); - state.common.update_xwayland_primary_output(); - }); - - Ok(()) - } - pub fn schedule_render(&mut self, output: &Output) { match self { BackendData::Winit(_) => {} // We cannot do this on the winit backend. @@ -463,6 +382,134 @@ impl BackendData { _ => unreachable!("No backend set when setting screen filters"), } } + + pub fn lock(&mut self) -> LockedBackend<'_> { + match self { + BackendData::Kms(ref mut state) => LockedBackend::Kms(state.lock_devices()), + BackendData::X11(ref mut state) => LockedBackend::X11(state), + BackendData::Winit(ref mut state) => LockedBackend::Winit(state), + _ => unreachable!("Tried to lock unset backend"), + } + } +} + +impl<'a> LockedBackend<'a> { + fn all_outputs(&self) -> Vec { + match self { + LockedBackend::Kms(state) => state.all_outputs(), + LockedBackend::X11(state) => state.all_outputs(), + LockedBackend::Winit(state) => state.all_outputs(), + } + } + + pub fn apply_config_for_outputs( + &mut self, + test_only: bool, + loop_handle: &LoopHandle<'static, State>, + screen_filter: &ScreenFilter, + shell: Arc>, + workspace_state: &mut WorkspaceUpdateGuard<'_, State>, + xdg_activation_state: &XdgActivationState, + startup_done: Arc, + clock: &Clock, + ) -> Result<(), anyhow::Error> { + let all_outputs = self.all_outputs(); + + // update outputs, so that `OutputModeSource`s are correct + for output in &all_outputs { + // apply to Output + let final_config = output + .user_data() + .get::>() + .unwrap() + .borrow(); + + let mode = Some(final_config.output_mode()).filter(|m| match output.current_mode() { + None => true, + Some(c_m) => m.size != c_m.size || m.refresh != c_m.refresh, + }); + let transform = + Some(final_config.transform.into()).filter(|x| *x != output.current_transform()); + let scale = Some(final_config.scale) + .filter(|x| *x != output.current_scale().fractional_scale()); + let location = Some(Point::from(( + final_config.position.0 as i32, + final_config.position.1 as i32, + ))) + .filter(|x| *x != output.current_location()); + output.change_current_state(mode, transform, scale.map(Scale::Fractional), location); + + output.set_adaptive_sync(final_config.vrr); + } + + match self { + LockedBackend::Kms(state) => state.apply_config_for_outputs( + test_only, + loop_handle, + screen_filter, + shell.clone(), + startup_done, + clock, + ), + LockedBackend::Winit(state) => state.apply_config_for_outputs(test_only), + LockedBackend::X11(state) => state.apply_config_for_outputs(test_only), + }?; + + let mut shell_ref = shell.write(); + for output in &all_outputs { + // apply the rest; add / remove outputs + let final_config = output + .user_data() + .get::>() + .unwrap() + .borrow(); + + output.set_mirroring(match &final_config.enabled { + OutputState::Mirroring(conn) => shell_ref + .outputs() + .find(|output| &output.name() == conn) + .cloned(), + _ => None, + }); + + match final_config.enabled { + OutputState::Enabled => shell_ref.workspaces.add_output(&output, workspace_state), + _ => { + let shell = &mut *shell_ref; + shell.workspaces.remove_output( + &output, + shell.seats.iter(), + workspace_state, + xdg_activation_state, + ) + } + } + + layer_map_for_output(&output).arrange(); + } + + // Update layout for changes in resolution, scale, orientation + shell_ref.workspaces.recalculate(); + let active_outputs = shell_ref.outputs().cloned().collect::>(); + std::mem::drop(shell_ref); + + for output in active_outputs { + match self { + LockedBackend::Winit(_) => {} // We cannot do this on the winit backend. + // Winit has a very strict render-loop and skipping frames breaks atleast the wayland winit-backend. + // Swapping with damage (which should be empty on these frames) is likely good enough anyway. + LockedBackend::X11(ref mut state) => state.schedule_render(&output), + LockedBackend::Kms(ref mut state) => state.schedule_render(&output), + } + } + + loop_handle.insert_idle(move |state| { + state.common.update_xwayland_scale(); + state.common.update_xwayland_primary_output(); + }); + + Ok(()) + } } pub struct KmsNodes { diff --git a/src/wayland/handlers/buffer.rs b/src/wayland/handlers/buffer.rs index 8762c22e..4087bd5f 100644 --- a/src/wayland/handlers/buffer.rs +++ b/src/wayland/handlers/buffer.rs @@ -1,6 +1,9 @@ // SPDX-License-Identifier: GPL-3.0-only -use crate::{state::BackendData, utils::prelude::*}; +use crate::{ + backend::kms::MaybeLockedDevice, + state::{BackendData, State}, +}; use smithay::{ reexports::wayland_server::{protocol::wl_buffer::WlBuffer, Resource}, wayland::buffer::BufferHandler, diff --git a/src/wayland/handlers/drm_lease.rs b/src/wayland/handlers/drm_lease.rs index dd1855ae..66561623 100644 --- a/src/wayland/handlers/drm_lease.rs +++ b/src/wayland/handlers/drm_lease.rs @@ -27,10 +27,11 @@ impl DrmLeaseHandler for State { request: DrmLeaseRequest, ) -> Result { let kms = self.backend.kms(); - let backend = kms + let mut backend = kms .drm_devices .get_mut(&node) - .ok_or(LeaseRejected::default())?; + .ok_or(LeaseRejected::default())? + .lock(); let mut renderer = match kms.api.single_renderer(&backend.render_node) { Ok(renderer) => renderer, Err(err) => { @@ -108,6 +109,7 @@ impl DrmLeaseHandler for State { fn lease_destroyed(&mut self, node: DrmNode, lease: u32) { let kms = self.backend.kms(); if let Some(backend) = kms.drm_devices.get_mut(&node) { + let mut backend = backend.lock(); backend.active_leases.retain(|l| l.id() != lease); if backend.active_leases.is_empty() { diff --git a/src/wayland/handlers/output_configuration.rs b/src/wayland/handlers/output_configuration.rs index d97d2009..7e60cfc1 100644 --- a/src/wayland/handlers/output_configuration.rs +++ b/src/wayland/handlers/output_configuration.rs @@ -145,7 +145,8 @@ impl State { } } - let res = self.backend.apply_config_for_outputs( + let mut backend = self.backend.lock(); + let res = backend.apply_config_for_outputs( test_only, &self.common.event_loop_handle, self.common.config.dynamic_conf.screen_filter(), @@ -168,7 +169,7 @@ impl State { } } if !test_only { - if let Err(err) = self.backend.apply_config_for_outputs( + if let Err(err) = backend.apply_config_for_outputs( false, &self.common.event_loop_handle, self.common.config.dynamic_conf.screen_filter(),