metal: preserve mode across reconnects

src/backend.rs

@@ -4,13 +4,16 @@ use {
         drm_feedback::DrmFeedback,
         fixed::Fixed,
         gfx_api::{GfxFramebuffer, SyncFile},
-        ifs::wl_seat::{
+        ifs::{
-            tablet::{
+            wl_output::OutputId,
-                PadButtonState, TabletInit, TabletPadId, TabletPadInit, TabletRingEventSource,
+            wl_seat::{
-                TabletStripEventSource, TabletToolChanges, TabletToolId, TabletToolInit,
+                tablet::{
-                ToolButtonState,
+                    PadButtonState, TabletInit, TabletPadId, TabletPadInit, TabletRingEventSource,
+                    TabletStripEventSource, TabletToolChanges, TabletToolId, TabletToolInit,
+                    ToolButtonState,
+                },
+                wl_pointer::{CONTINUOUS, FINGER, HORIZONTAL_SCROLL, VERTICAL_SCROLL, WHEEL},
             },
-            wl_pointer::{CONTINUOUS, FINGER, HORIZONTAL_SCROLL, VERTICAL_SCROLL, WHEEL},
         },
         libinput::consts::DeviceCapability,
         video::drm::{ConnectorType, DrmConnector, DrmError, DrmVersion},
@@ -64,9 +67,7 @@ pub struct Mode {
 #[derive(Clone, Debug)]
 pub struct MonitorInfo {
     pub modes: Vec<Mode>,
-    pub manufacturer: String,
+    pub output_id: Rc<OutputId>,
-    pub product: String,
-    pub serial_number: String,
     pub initial_mode: Mode,
     pub width_mm: i32,
     pub height_mm: i32,

src/backends/metal.rs

@@ -11,12 +11,16 @@ use {
         },
         backends::metal::video::{
             MetalDrmDeviceData, MetalLeaseData, MetalRenderContext, PendingDrmDevice,
+            PersistentDisplayData,
         },
         dbus::{DbusError, SignalHandler},
         drm_feedback::DrmFeedback,
         gfx_api::GfxError,
-        ifs::wl_seat::tablet::{
+        ifs::{
-            TabletId, TabletInit, TabletPadGroupInit, TabletPadId, TabletPadInit,
+            wl_output::OutputId,
+            wl_seat::tablet::{
+                TabletId, TabletInit, TabletPadGroupInit, TabletPadId, TabletPadInit,
+            },
         },
         libinput::{
             consts::{
@@ -144,6 +148,7 @@ pub struct MetalBackend {
     resume_handler: Cell<Option<SignalHandler>>,
     ctx: CloneCell<Option<Rc<MetalRenderContext>>>,
     default_feedback: CloneCell<Option<Rc<DrmFeedback>>>,
+    persistent_display_data: CopyHashMap<Rc<OutputId>, Rc<PersistentDisplayData>>,
 }
 
 impl Debug for MetalBackend {
@@ -317,6 +322,7 @@ pub async fn create(state: &Rc<State>) -> Result<Rc<MetalBackend>, MetalError> {
         resume_handler: Default::default(),
         ctx: Default::default(),
         default_feedback: Default::default(),
+        persistent_display_data: Default::default(),
     });
     metal.pause_handler.set(Some({
         let mtl = metal.clone();

src/backends/metal/video.rs

@@ -15,7 +15,10 @@ use {
             AcquireSync, BufferResv, GfxApiOpt, GfxContext, GfxFramebuffer, GfxRenderPass,
             GfxTexture, ReleaseSync, SyncFile,
         },
-        ifs::wp_presentation_feedback::{KIND_HW_COMPLETION, KIND_VSYNC},
+        ifs::{
+            wl_output::OutputId,
+            wp_presentation_feedback::{KIND_HW_COMPLETION, KIND_VSYNC},
+        },
         renderer::RenderResult,
         state::State,
         theme::Color,
@@ -23,9 +26,8 @@ use {
         udev::UdevDevice,
         utils::{
             asyncevent::AsyncEvent, bitflags::BitflagsExt, cell_ext::CellExt, clonecell::CloneCell,
-            copyhashmap::CopyHashMap, debug_fn::debug_fn, errorfmt::ErrorFmt, numcell::NumCell,
+            copyhashmap::CopyHashMap, errorfmt::ErrorFmt, numcell::NumCell, on_change::OnChange,
-            on_change::OnChange, opaque_cell::OpaqueCell, oserror::OsError,
+            opaque_cell::OpaqueCell, oserror::OsError, transform_ext::TransformExt,
-            transform_ext::TransformExt,
         },
         video::{
             dmabuf::DmaBufId,
@@ -295,40 +297,36 @@ pub struct MetalDrmDeviceData {
     pub unprocessed_change: Cell<bool>,
 }
 
+#[derive(Debug)]
+pub struct PersistentDisplayData {
+    pub mode: RefCell<Option<DrmModeInfo>>,
+    pub vrr_requested: Cell<bool>,
+}
+
 #[derive(Debug)]
 pub struct ConnectorDisplayData {
     pub crtc_id: MutableProperty<DrmCrtc>,
     pub crtcs: AHashMap<DrmCrtc, Rc<MetalCrtc>>,
     pub modes: Vec<DrmModeInfo>,
     pub mode: Option<DrmModeInfo>,
+    pub persistent: Rc<PersistentDisplayData>,
     pub refresh: u32,
     pub non_desktop: bool,
     pub non_desktop_effective: bool,
     pub vrr_capable: bool,
-    pub vrr_requested: bool,
 
-    pub monitor_manufacturer: String,
+    pub connector_id: ConnectorKernelId,
-    pub monitor_name: String,
+    pub output_id: Rc<OutputId>,
-    pub monitor_serial_number: String,
 
     pub connection: ConnectorStatus,
     pub mm_width: u32,
     pub mm_height: u32,
     pub _subpixel: u32,
-
-    pub connector_type: ConnectorType,
-    pub connector_type_id: u32,
 }
 
 impl ConnectorDisplayData {
-    fn is_same_monitor(&self, other: &Self) -> bool {
-        self.monitor_manufacturer == other.monitor_manufacturer
-            && self.monitor_name == other.monitor_name
-            && self.monitor_serial_number == other.monitor_serial_number
-    }
-
     fn should_enable_vrr(&self) -> bool {
-        self.vrr_requested && self.vrr_capable
+        self.persistent.vrr_requested.get() && self.vrr_capable
     }
 }
 
@@ -1276,11 +1274,7 @@ impl Connector for MetalConnector {
     }
 
     fn kernel_id(&self) -> ConnectorKernelId {
-        let dd = self.display.borrow_mut();
+        self.display.borrow().connector_id
-        ConnectorKernelId {
-            ty: dd.connector_type,
-            idx: dd.connector_type_id,
-        }
     }
 
     fn event(&self) -> Option<ConnectorEvent> {
@@ -1349,6 +1343,8 @@ impl Connector for MetalConnector {
             return;
         };
         log::info!("Trying to change mode from {:?} to {:?}", prev, mode);
+        let persistent = dd.persistent.clone();
+        *persistent.mode.borrow_mut() = Some(mode.clone());
         dd.mode = Some(mode.clone());
         drop(dd);
         let Err(e) = self.backend.handle_drm_change_(&dev, true) else {
@@ -1356,6 +1352,7 @@ impl Connector for MetalConnector {
             return;
         };
         log::warn!("Could not change mode: {}", ErrorFmt(&e));
+        *persistent.mode.borrow_mut() = prev.clone();
         self.display.borrow_mut().mode = prev;
         if let MetalError::Modeset(DrmError::Atomic(OsError(c::EACCES))) = e {
             log::warn!("Failed due to access denied. Resetting in memory only.");
@@ -1396,7 +1393,7 @@ impl Connector for MetalConnector {
         }
         let dd = &mut *self.display.borrow_mut();
         let old_enabled = dd.should_enable_vrr();
-        dd.vrr_requested = enabled;
+        dd.persistent.vrr_requested.set(enabled);
         let new_enabled = dd.should_enable_vrr();
         if old_enabled == new_enabled {
             return;
@@ -1608,13 +1605,10 @@ fn create_connector_display_data(
     let mut name = String::new();
     let mut manufacturer = String::new();
     let mut serial_number = String::new();
-    let mode = info.modes.first().cloned();
+    let connector_id = ConnectorKernelId {
-    let refresh = mode
+        ty: ConnectorType::from_drm(info.connector_type),
-        .as_ref()
+        idx: info.connector_type_id,
-        .map(|m| 1_000_000_000_000u64 / (m.refresh_rate_millihz() as u64))
+    };
-        .unwrap_or(0) as u32;
-    let connector_type = ConnectorType::from_drm(info.connector_type);
-    let connector_name = debug_fn(|f| write!(f, "{}-{}", connector_type, info.connector_type_id));
     'fetch_edid: {
         if connection != ConnectorStatus::Connected {
             break 'fetch_edid;
@@ -1624,7 +1618,7 @@ fn create_connector_display_data(
             _ => {
                 log::warn!(
                     "Connector {} is connected but has no EDID blob",
-                    connector_name,
+                    connector_id,
                 );
                 break 'fetch_edid;
             }
@@ -1634,7 +1628,7 @@ fn create_connector_display_data(
             Err(e) => {
                 log::error!(
                     "Could not fetch edid property of connector {}: {}",
-                    connector_name,
+                    connector_id,
                     ErrorFmt(e)
                 );
                 break 'fetch_edid;
@@ -1645,7 +1639,7 @@ fn create_connector_display_data(
             Err(e) => {
                 log::error!(
                     "Could not parse edid property of connector {}: {}",
-                    connector_name,
+                    connector_id,
                     ErrorFmt(e)
                 );
                 break 'fetch_edid;
@@ -1666,43 +1660,76 @@ fn create_connector_display_data(
         if name.is_empty() {
             log::warn!(
                 "The display attached to connector {} does not have a product name descriptor",
-                connector_name,
+                connector_id,
             );
         }
         if serial_number.is_empty() {
             log::warn!(
                 "The display attached to connector {} does not have a serial number descriptor",
-                connector_name,
+                connector_id,
             );
             serial_number = edid.base_block.id_serial_number.to_string();
         }
     }
-    let props = collect_properties(&dev.master, connector)?;
+    let output_id = Rc::new(OutputId::new(
-    let connector_type = ConnectorType::from_drm(info.connector_type);
+        connector_id.to_string(),
+        manufacturer,
+        name,
+        serial_number,
+    ));
+    let desired_state = match dev.backend.persistent_display_data.get(&output_id) {
+        Some(ds) => {
+            log::info!("Reusing desired state for {:?}", output_id);
+            ds
+        }
+        None => {
+            let ds = Rc::new(PersistentDisplayData {
+                mode: RefCell::new(info.modes.first().cloned()),
+                vrr_requested: Default::default(),
+            });
+            dev.backend
+                .persistent_display_data
+                .set(output_id.clone(), ds.clone());
+            ds
+        }
+    };
+    let mut mode_opt = desired_state.mode.borrow_mut();
+    if let Some(mode) = &*mode_opt {
+        if !info.modes.contains(mode) {
+            log::warn!("Discarding previously desired mode");
+            *mode_opt = None;
+        }
+    }
+    if mode_opt.is_none() {
+        *mode_opt = info.modes.first().cloned();
+    }
+    let refresh = mode_opt
+        .as_ref()
+        .map(|m| 1_000_000_000_000u64 / (m.refresh_rate_millihz() as u64))
+        .unwrap_or(0) as u32;
     let non_desktop = props.get("non-desktop")?.value.get() != 0;
     let vrr_capable = match props.get("vrr_capable") {
         Ok(c) => c.value.get() == 1,
         Err(_) => false,
     };
+    let mode = mode_opt.clone();
+    drop(mode_opt);
     Ok(ConnectorDisplayData {
         crtc_id: props.get("CRTC_ID")?.map(|v| DrmCrtc(v as _)),
         crtcs,
         modes: info.modes,
         mode,
+        persistent: desired_state,
         refresh,
         non_desktop,
         non_desktop_effective: non_desktop_override.unwrap_or(non_desktop),
         vrr_capable,
-        vrr_requested: false,
-        monitor_manufacturer: manufacturer,
-        monitor_name: name,
-        monitor_serial_number: serial_number,
         connection,
         mm_width: info.mm_width,
         mm_height: info.mm_height,
         _subpixel: info.subpixel,
-        connector_type,
+        connector_id,
-        connector_type_id: info.connector_type_id,
+        output_id,
     })
 }
 
@@ -2015,14 +2042,6 @@ impl MetalBackend {
                 }
             };
             let mut old = c.display.borrow_mut();
-            if old.is_same_monitor(&dd) {
-                if let Some(mode) = &old.mode {
-                    if dd.modes.contains(mode) {
-                        dd.mode = Some(mode.clone());
-                    }
-                }
-                dd.vrr_requested = old.vrr_requested;
-            }
             mem::swap(old.deref_mut(), &mut dd);
             match c.frontend_state.get() {
                 FrontState::Removed | FrontState::Disconnected => {}
@@ -2042,7 +2061,7 @@ impl MetalBackend {
                         // Disconnect if the connector is no longer connected.
                         disconnect |= old.connection != ConnectorStatus::Connected;
                         // Disconnect if the connected monitor changed.
-                        disconnect |= !old.is_same_monitor(&dd);
+                        disconnect |= old.output_id != dd.output_id;
                     }
                     if disconnect {
                         c.tearing_requested.set(false);
@@ -2103,9 +2122,7 @@ impl MetalBackend {
         }
         connector.send_event(ConnectorEvent::Connected(MonitorInfo {
             modes,
-            manufacturer: dd.monitor_manufacturer.clone(),
+            output_id: dd.output_id.clone(),
-            product: dd.monitor_name.clone(),
-            serial_number: dd.monitor_serial_number.clone(),
             initial_mode: dd.mode.clone().unwrap().to_backend(),
             width_mm: dd.mm_width as _,
             height_mm: dd.mm_height as _,
@@ -3047,8 +3064,8 @@ impl MetalBackend {
     }
 
     fn start_connector(&self, connector: &Rc<MetalConnector>, log_mode: bool) {
-        let dd = connector.display.borrow_mut();
+        let dd = &*connector.display.borrow();
-        self.send_connected(connector, &dd);
+        self.send_connected(connector, dd);
         match connector.frontend_state.get() {
             FrontState::Connected { non_desktop: false } => {}
             FrontState::Connected { non_desktop: true }
@@ -3058,9 +3075,8 @@ impl MetalBackend {
         }
         if log_mode {
             log::info!(
-                "Initialized connector {}-{} with mode {:?}",
+                "Initialized connector {} with mode {:?}",
-                dd.connector_type,
+                dd.connector_id,
-                dd.connector_type_id,
                 dd.mode.as_ref().unwrap(),
             );
         }

src/backends/x.rs

@@ -11,6 +11,7 @@ use {
         fixed::Fixed,
         format::XRGB8888,
         gfx_api::{GfxContext, GfxError, GfxFramebuffer, GfxTexture},
+        ifs::wl_output::OutputId,
         renderer::RenderResult,
         state::State,
         utils::{
@@ -565,9 +566,12 @@ impl XBackend {
             .push(BackendEvent::NewConnector(output.clone()));
         output.events.push(ConnectorEvent::Connected(MonitorInfo {
             modes: vec![],
-            manufacturer: "X.Org Foundation".to_string(),
+            output_id: Rc::new(OutputId::new(
-            product: format!("X-Window-{}", output.window),
+                String::new(),
-            serial_number: output.window.to_string(),
+                "X.Org Foundation".to_string(),
+                format!("X-Window-{}", output.window),
+                output.window.to_string(),
+            )),
             initial_mode: Mode {
                 width: output.width.get(),
                 height: output.height.get(),

src/compositor.rs

@@ -425,7 +425,7 @@ fn init_fd_limit() {
 
 fn create_dummy_output(state: &Rc<State>) {
     let output_id = Rc::new(OutputId {
-        connector: "jay-dummy-connector".to_string(),
+        connector: Some("jay-dummy-connector".to_string()),
         manufacturer: "jay".to_string(),
         model: "jay-dummy-output".to_string(),
         serial_number: "".to_string(),

src/config/handler.rs

@@ -973,7 +973,7 @@ impl ConfigProxyHandler {
     fn handle_connector_model(&self, connector: Connector) -> Result<(), CphError> {
         let connector = self.get_output(connector)?;
         self.respond(Response::GetConnectorModel {
-            model: connector.monitor_info.product.clone(),
+            model: connector.monitor_info.output_id.model.clone(),
         });
         Ok(())
     }
@@ -981,7 +981,7 @@ impl ConfigProxyHandler {
     fn handle_connector_manufacturer(&self, connector: Connector) -> Result<(), CphError> {
         let connector = self.get_output(connector)?;
         self.respond(Response::GetConnectorManufacturer {
-            manufacturer: connector.monitor_info.manufacturer.clone(),
+            manufacturer: connector.monitor_info.output_id.manufacturer.clone(),
         });
         Ok(())
     }
@@ -989,7 +989,7 @@ impl ConfigProxyHandler {
     fn handle_connector_serial_number(&self, connector: Connector) -> Result<(), CphError> {
         let connector = self.get_output(connector)?;
         self.respond(Response::GetConnectorSerialNumber {
-            serial_number: connector.monitor_info.serial_number.clone(),
+            serial_number: connector.monitor_info.output_id.serial_number.clone(),
         });
         Ok(())
     }

src/ifs/jay_randr.rs

@@ -80,9 +80,9 @@ impl JayRandr {
             None => {
                 self.client.event(NonDesktopOutput {
                     self_id: self.id,
-                    manufacturer: &output.monitor_info.manufacturer,
+                    manufacturer: &output.monitor_info.output_id.manufacturer,
-                    product: &output.monitor_info.product,
+                    product: &output.monitor_info.output_id.model,
-                    serial_number: &output.monitor_info.serial_number,
+                    serial_number: &output.monitor_info.output_id.serial_number,
                     width_mm: output.monitor_info.width_mm,
                     height_mm: output.monitor_info.height_mm,
                 });
@@ -99,9 +99,9 @@ impl JayRandr {
             x: pos.x1(),
             y: pos.y1(),
             transform: global.persistent.transform.get().to_wl(),
-            manufacturer: &output.monitor_info.manufacturer,
+            manufacturer: &output.monitor_info.output_id.manufacturer,
-            product: &output.monitor_info.product,
+            product: &output.monitor_info.output_id.model,
-            serial_number: &output.monitor_info.serial_number,
+            serial_number: &output.monitor_info.output_id.serial_number,
             width_mm: global.width_mm,
             height_mm: global.height_mm,
         });

src/ifs/wl_output.rs

@@ -96,14 +96,30 @@ pub struct PersistentOutputState {
     pub tearing_mode: Cell<&'static TearingMode>,
 }
 
-#[derive(Eq, PartialEq, Hash)]
+#[derive(Eq, PartialEq, Hash, Debug)]
 pub struct OutputId {
-    pub connector: String,
+    pub connector: Option<String>,
     pub manufacturer: String,
     pub model: String,
     pub serial_number: String,
 }
 
+impl OutputId {
+    pub fn new(
+        connector: String,
+        manufacturer: String,
+        model: String,
+        serial_number: String,
+    ) -> Self {
+        Self {
+            connector: serial_number.is_empty().then_some(connector),
+            manufacturer,
+            model,
+            serial_number,
+        }
+    }
+}
+
 impl WlOutputGlobal {
     pub fn clear(&self) {
         self.opt.clear();

src/it/test_backend.rs

@@ -13,6 +13,7 @@ use {
         fixed::Fixed,
         gfx_api::GfxError,
         gfx_apis::create_vulkan_allocator,
+        ifs::wl_output::OutputId,
         it::{
             test_error::TestResult, test_gfx_api::TestGfxCtx, test_utils::test_expected_event::TEEH,
         },
@@ -115,9 +116,12 @@ impl TestBackend {
         };
         let default_monitor_info = MonitorInfo {
             modes: vec![mode],
-            manufacturer: "jay".to_string(),
+            output_id: Rc::new(OutputId {
-            product: "TestConnector".to_string(),
+                connector: None,
-            serial_number: default_connector.id.to_string(),
+                manufacturer: "jay".to_string(),
+                model: "TestConnector".to_string(),
+                serial_number: default_connector.id.to_string(),
+            }),
             initial_mode: mode,
             width_mm: 80,
             height_mm: 60,

src/it/tests/t0034_workspace_restoration.rs

@@ -1,6 +1,7 @@
 use {
     crate::{
         backend::{BackendEvent, ConnectorEvent, ConnectorKernelId, Mode, MonitorInfo},
+        ifs::wl_output::OutputId,
         it::{test_backend::TestConnector, test_error::TestResult, testrun::TestRun},
         video::drm::ConnectorType,
     },
@@ -32,9 +33,12 @@ async fn test(run: Rc<TestRun>) -> TestResult {
     });
     let new_monitor_info = MonitorInfo {
         modes: vec![],
-        manufacturer: "jay".to_string(),
+        output_id: Rc::new(OutputId {
-        product: "jay second connector".to_string(),
+            connector: None,
-        serial_number: "".to_string(),
+            manufacturer: "jay".to_string(),
+            model: "jay second connector".to_string(),
+            serial_number: "".to_string(),
+        }),
         initial_mode: Mode {
             width: 400,
             height: 400,

src/tasks/connector.rs

@@ -2,7 +2,7 @@ use {
     crate::{
         backend::{Connector, ConnectorEvent, ConnectorId, MonitorInfo},
         globals::GlobalName,
-        ifs::wl_output::{OutputId, PersistentOutputState, WlOutputGlobal},
+        ifs::wl_output::{PersistentOutputState, WlOutputGlobal},
         output_schedule::OutputSchedule,
         state::{ConnectorData, OutputData, State},
         tree::{move_ws_to_output, OutputNode, OutputRenderData, WsMoveConfig},
@@ -86,27 +86,17 @@ impl ConnectorHandler {
         log::info!("Connector {} connected", self.data.connector.kernel_id());
         self.data.connected.set(true);
         let name = self.state.globals.name();
-        let output_id = Rc::new(OutputId {
-            connector: self.data.name.clone(),
-            manufacturer: info.manufacturer.clone(),
-            model: info.product.clone(),
-            serial_number: info.serial_number.clone(),
-        });
         if info.non_desktop {
             self.handle_non_desktop_connected(info).await;
         } else {
-            self.handle_desktop_connected(info, name, output_id).await;
+            self.handle_desktop_connected(info, name).await;
         }
         self.data.connected.set(false);
         log::info!("Connector {} disconnected", self.data.connector.kernel_id());
     }
 
-    async fn handle_desktop_connected(
+    async fn handle_desktop_connected(&self, info: MonitorInfo, name: GlobalName) {
-        &self,
+        let output_id = info.output_id.clone();
-        info: MonitorInfo,
-        name: GlobalName,
-        output_id: Rc<OutputId>,
-    ) {
         let desired_state = match self.state.persistent_output_states.get(&output_id) {
             Some(ds) => ds,
             _ => {