drm: Fixup tty switching

anvil/src/udev.rs

@@ -303,6 +303,7 @@ impl<Data: 'static> UdevHandlerImpl<Data> {
         context: &EGLContext,
         display: &mut Display,
         output_map: &mut Vec<MyOutput>,
+        signaler: &Signaler<SessionSignal>,
         logger: &::slog::Logger,
     ) -> HashMap<crtc::Handle, Rc<RefCell<RenderSurface>>> {
         // Get a set of all modesetting resource handles (excluding planes):
@@ -360,7 +361,7 @@ impl<Data: 'static> UdevHandlerImpl<Data> {
                                 continue;
                             }
                         };
-                        let surface = match device.create_surface(
+                        let mut surface = match device.create_surface(
                             crtc,
                             primary,
                             connector_info.modes()[0],
@@ -372,6 +373,7 @@ impl<Data: 'static> UdevHandlerImpl<Data> {
                                 continue;
                             }
                         };
+                        surface.link(signaler.clone());
                         let renderer =
                             match DrmRenderSurface::new(surface, gbm.clone(), renderer, logger.clone()) {
                                 Ok(renderer) => renderer,
@@ -480,6 +482,7 @@ impl<Data: 'static> UdevHandlerImpl<Data> {
                 &context,
                 &mut *self.display.borrow_mut(),
                 &mut *self.output_map.borrow_mut(),
+                &self.signaler,
                 &self.logger,
             )));
 
@@ -562,6 +565,7 @@ impl<Data: 'static> UdevHandlerImpl<Data> {
             let loop_handle = self.loop_handle.clone();
             let mut display = self.display.borrow_mut();
             let mut output_map = self.output_map.borrow_mut();
+            let signaler = self.signaler.clone();
             output_map.retain(|output| output.device_id != device);
             self.loop_handle
                 .with_source(&backend_data.event_source, |source| {
@@ -573,6 +577,7 @@ impl<Data: 'static> UdevHandlerImpl<Data> {
                         &backend_data.context,
                         &mut *display,
                         &mut *output_map,
+                        &signaler,
                         &logger,
                     );
 

src/backend/drm/device/mod.rs

@@ -509,10 +509,13 @@ impl<A: AsRawFd + 'static> DrmDevice<A> {
         );
 
         Ok(DrmSurface {
+            dev_id: self.dev_id,
             crtc,
             plane,
             internal: Arc::new(internal),
             formats,
+            #[cfg(feature = "backend_session")]
+            links: RefCell::new(Vec::new()),
         })
     }
 
@@ -532,15 +535,6 @@ pub struct Planes {
     pub overlay: Option<Vec<plane::Handle>>,
 }
 
-impl<A: AsRawFd + 'static> DrmDeviceInternal<A> {
-    pub(super) fn reset_state(&self) -> Result<(), Error> {
-        match self {
-            DrmDeviceInternal::Atomic(dev) => dev.reset_state(),
-            DrmDeviceInternal::Legacy(dev) => dev.reset_state(),
-        }
-    }
-}
-
 /// Trait to receive events of a bound [`DrmDevice`]
 ///
 /// See [`device_bind`]

src/backend/drm/session.rs

@@ -4,12 +4,12 @@ use std::sync::{
     Arc, Weak,
 };
 
-use drm::Device as BasicDevice;
+use drm::{Device as BasicDevice, control::{crtc, Device as ControlDevice}};
 use nix::libc::dev_t;
 use nix::sys::stat;
 
 use super::device::{DrmDevice, DrmDeviceInternal};
-use super::error::Error;
+use super::surface::{DrmSurface, DrmSurfaceInternal};
 use crate::{
     backend::session::Signal as SessionSignal,
     signaling::{Linkable, Signaler},
@@ -96,21 +96,74 @@ impl<A: AsRawFd + 'static> DrmDeviceObserver<A> {
             }
         }
 
-        // okay, the previous session/whatever might left the drm devices in any state...
-        // lets fix that
-        if let Err(err) = self.reset_state() {
-            warn!(self.logger, "Unable to reset state after tty switch: {}", err);
-            // TODO call drm-handler::error
-        }
-
         self.active.store(true, Ordering::SeqCst);
     }
+}
 
-    fn reset_state(&mut self) -> Result<(), Error> {
+pub struct DrmSurfaceObserver<A: AsRawFd + 'static> {
-        if let Some(dev) = self.dev.upgrade() {
+    dev_id: dev_t,
-            dev.reset_state()
+    crtc: crtc::Handle,
-        } else {
+    surf: Weak<DrmSurfaceInternal<A>>,
-            Ok(())
+    logger: ::slog::Logger,
+}
+
+struct FdHack(RawFd);
+impl AsRawFd for FdHack {
+    fn as_raw_fd(&self) -> RawFd {
+        self.0
+    }
+}
+impl BasicDevice for FdHack {}
+impl ControlDevice for FdHack {}
+
+impl<A: AsRawFd + 'static> Linkable<SessionSignal> for DrmSurface<A> {
+    fn link(&mut self, signaler: Signaler<SessionSignal>) {
+        let logger = match &*self.internal {
+            DrmSurfaceInternal::Atomic(surf) => surf.logger.clone(),
+            DrmSurfaceInternal::Legacy(surf) => surf.logger.clone(),
+        };
+        let mut observer = DrmSurfaceObserver {
+            dev_id: self.dev_id,
+            crtc: self.crtc(),
+            surf: Arc::downgrade(&self.internal),
+            logger: logger.new(o!("drm_module" => "observer")),
+        };
+
+        let token = signaler.register(move |signal| observer.signal(*signal));
+        self.links.borrow_mut().push(token);
+    }
+}
+
+impl<A: AsRawFd + 'static> DrmSurfaceObserver<A> {
+    fn signal(&mut self, signal: SessionSignal) {
+        match signal {
+            SessionSignal::ActivateSession => self.activate(None),
+            SessionSignal::ActivateDevice { major, minor, new_fd } => {
+                self.activate(Some((major, minor, new_fd)))
+            },
+            _ => {},
+        }
+    }
+
+    fn activate(&mut self, devnum: Option<(u32, u32, Option<RawFd>)>) {
+        if let Some(surf) = self.surf.upgrade() {
+            // The device will reset the _fd, but the observer order is not deterministic,
+            // so we might need to use it anyway.
+            let fd = if let Some((major, minor, fd)) = devnum {
+                if major as u64 != stat::major(self.dev_id) || minor as u64 != stat::minor(self.dev_id) {
+                    return;
+                }
+                fd.map(FdHack)
+            } else {
+                None
+            };
+
+            if let Err(err) = match &*surf {
+                DrmSurfaceInternal::Atomic(surf) => surf.reset_state(fd.as_ref()),
+                DrmSurfaceInternal::Legacy(surf) => surf.reset_state(fd.as_ref()),
+            } {
+                warn!(self.logger, "Failed to reset state of surface ({:?}/{:?}): {}", self.dev_id, self.crtc, err);
+            }
         }
     }
 }

src/backend/drm/surface/atomic.rs

@@ -24,40 +24,8 @@ pub struct State {
     pub connectors: HashSet<connector::Handle>,
 }
 
-pub struct AtomicDrmSurface<A: AsRawFd + 'static> {
+impl State {
-    pub(super) fd: Arc<DrmDeviceInternal<A>>,
+    fn current_state<A: AsRawFd + ControlDevice>(fd: &A, crtc: crtc::Handle, prop_mapping: &Mapping) -> Result<Self, Error> {
-    pub(super) active: Arc<AtomicBool>,
-    crtc: crtc::Handle,
-    plane: plane::Handle,
-    prop_mapping: Mapping,
-    state: RwLock<State>,
-    pending: RwLock<State>,
-    test_buffer: Mutex<Option<(DumbBuffer, framebuffer::Handle)>>,
-    pub(crate) logger: ::slog::Logger,
-}
-
-impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
-    #[allow(clippy::too_many_arguments)]
-    pub fn new(
-        fd: Arc<DrmDeviceInternal<A>>,
-        active: Arc<AtomicBool>,
-        crtc: crtc::Handle,
-        plane: plane::Handle,
-        prop_mapping: Mapping,
-        mode: Mode,
-        connectors: &[connector::Handle],
-        logger: ::slog::Logger,
-    ) -> Result<Self, Error> {
-        let logger = logger.new(o!("smithay_module" => "backend_drm_atomic", "drm_module" => "surface"));
-        info!(
-            logger,
-            "Initializing drm surface ({:?}:{:?}) with mode {:?} and connectors {:?}",
-            crtc,
-            plane,
-            mode,
-            connectors
-        );
-
         let crtc_info = fd.get_crtc(crtc).map_err(|source| Error::Access {
             errmsg: "Error loading crtc info",
             dev: fd.dev_path(),
@@ -78,12 +46,6 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
             None => property::Value::Unknown(0),
         };
 
-        let blob = fd.create_property_blob(&mode).map_err(|source| Error::Access {
-            errmsg: "Failed to create Property Blob for mode",
-            dev: fd.dev_path(),
-            source,
-        })?;
-
         let res_handles = fd.resource_handles().map_err(|source| Error::Access {
             errmsg: "Error loading drm resources",
             dev: fd.dev_path(),
@@ -122,11 +84,54 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
                 }
             }
         }
-        let state = State {
+        Ok(State {
             mode: current_mode,
             blob: current_blob,
             connectors: current_connectors,
-        };
+        })
+    }
+}
+
+pub struct AtomicDrmSurface<A: AsRawFd + 'static> {
+    pub(super) fd: Arc<DrmDeviceInternal<A>>,
+    pub(super) active: Arc<AtomicBool>,
+    crtc: crtc::Handle,
+    plane: plane::Handle,
+    prop_mapping: Mapping,
+    state: RwLock<State>,
+    pending: RwLock<State>,
+    test_buffer: Mutex<Option<(DumbBuffer, framebuffer::Handle)>>,
+    pub(crate) logger: ::slog::Logger,
+}
+
+impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
+    #[allow(clippy::too_many_arguments)]
+    pub fn new(
+        fd: Arc<DrmDeviceInternal<A>>,
+        active: Arc<AtomicBool>,
+        crtc: crtc::Handle,
+        plane: plane::Handle,
+        prop_mapping: Mapping,
+        mode: Mode,
+        connectors: &[connector::Handle],
+        logger: ::slog::Logger,
+    ) -> Result<Self, Error> {
+        let logger = logger.new(o!("smithay_module" => "backend_drm_atomic", "drm_module" => "surface"));
+        info!(
+            logger,
+            "Initializing drm surface ({:?}:{:?}) with mode {:?} and connectors {:?}",
+            crtc,
+            plane,
+            mode,
+            connectors
+        );
+
+        let state = State::current_state(&*fd, crtc, &prop_mapping)?;
+        let blob = fd.create_property_blob(&mode).map_err(|source| Error::Access {
+            errmsg: "Failed to create Property Blob for mode",
+            dev: fd.dev_path(),
+            source,
+        })?;        
         let pending = State {
             mode,
             blob,
@@ -761,6 +766,16 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
                 source,
             })
     }
+
+    
+    pub(crate) fn reset_state<B: AsRawFd + ControlDevice + 'static>(&self, fd: Option<&B>) -> Result<(), Error> {
+        *self.state.write().unwrap() = if let Some(fd) = fd {
+            State::current_state(fd, self.crtc, &self.prop_mapping)?
+        } else {
+            State::current_state(&*self.fd, self.crtc, &self.prop_mapping)?
+        };
+        Ok(())
+    }
 }
 
 impl<A: AsRawFd + 'static> Drop for AtomicDrmSurface<A> {

src/backend/drm/surface/legacy.rs

@@ -19,30 +19,8 @@ pub struct State {
     pub connectors: HashSet<connector::Handle>,
 }
 
-pub struct LegacyDrmSurface<A: AsRawFd + 'static> {
+impl State {
-    pub(super) fd: Arc<DrmDeviceInternal<A>>,
+    fn current_state<A: AsRawFd + ControlDevice>(fd: &A, crtc: crtc::Handle) -> Result<Self, Error> {
-    pub(super) active: Arc<AtomicBool>,
-    crtc: crtc::Handle,
-    state: RwLock<State>,
-    pending: RwLock<State>,
-    pub(crate) logger: ::slog::Logger,
-}
-
-impl<A: AsRawFd + 'static> LegacyDrmSurface<A> {
-    pub fn new(
-        fd: Arc<DrmDeviceInternal<A>>,
-        active: Arc<AtomicBool>,
-        crtc: crtc::Handle,
-        mode: Mode,
-        connectors: &[connector::Handle],
-        logger: ::slog::Logger,
-    ) -> Result<Self, Error> {
-        let logger = logger.new(o!("smithay_module" => "backend_drm_legacy", "drm_module" => "surface"));
-        info!(
-            logger,
-            "Initializing drm surface with mode {:?} and connectors {:?}", mode, connectors
-        );
-
         // Try to enumarate the current state to set the initial state variable correctly.
         // We need an accurate state to handle `commit_pending`.
         let crtc_info = fd.get_crtc(crtc).map_err(|source| Error::Access {
@@ -83,10 +61,38 @@ impl<A: AsRawFd + 'static> LegacyDrmSurface<A> {
         // A better fix would probably be making mode an `Option`, but that would mean
         // we need to be sure, we require a mode to always be set without relying on the compiler.
         // So we cheat, because it works and is easier to handle later.
-        let state = State {
+        Ok(State {
             mode: current_mode.unwrap_or_else(|| unsafe { std::mem::zeroed() }),
             connectors: current_connectors,
-        };
+        })
+    }
+}
+
+pub struct LegacyDrmSurface<A: AsRawFd + 'static> {
+    pub(super) fd: Arc<DrmDeviceInternal<A>>,
+    pub(super) active: Arc<AtomicBool>,
+    crtc: crtc::Handle,
+    state: RwLock<State>,
+    pending: RwLock<State>,
+    pub(crate) logger: ::slog::Logger,
+}
+
+impl<A: AsRawFd + 'static> LegacyDrmSurface<A> {
+    pub fn new(
+        fd: Arc<DrmDeviceInternal<A>>,
+        active: Arc<AtomicBool>,
+        crtc: crtc::Handle,
+        mode: Mode,
+        connectors: &[connector::Handle],
+        logger: ::slog::Logger,
+    ) -> Result<Self, Error> {
+        let logger = logger.new(o!("smithay_module" => "backend_drm_legacy", "drm_module" => "surface"));
+        info!(
+            logger,
+            "Initializing drm surface with mode {:?} and connectors {:?}", mode, connectors
+        );
+
+        let state = State::current_state(&*fd, crtc)?; 
         let pending = State {
             mode,
             connectors: connectors.iter().copied().collect(),
@@ -396,6 +402,15 @@ impl<A: AsRawFd + 'static> LegacyDrmSurface<A> {
             Ok(false)
         }
     }
+    
+    pub(crate) fn reset_state<B: AsRawFd + ControlDevice + 'static>(&self, fd: Option<&B>) -> Result<(), Error> {
+        *self.state.write().unwrap() = if let Some(fd) = fd {
+            State::current_state(fd, self.crtc)?
+        } else {
+            State::current_state(&*self.fd, self.crtc)?
+        };
+        Ok(())
+    }
 }
 
 impl<A: AsRawFd + 'static> Drop for LegacyDrmSurface<A> {

src/backend/drm/surface/mod.rs

@@ -1,3 +1,4 @@
+use std::cell::RefCell;
 use std::collections::HashSet;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::sync::Arc;
@@ -5,6 +6,8 @@ use std::sync::Arc;
 use drm::control::{connector, crtc, framebuffer, plane, Device as ControlDevice, Mode};
 use drm::Device as BasicDevice;
 
+use nix::libc::dev_t;
+
 pub(super) mod atomic;
 pub(super) mod legacy;
 use super::error::Error;
@@ -14,10 +17,13 @@ use legacy::LegacyDrmSurface;
 
 /// An open crtc + plane combination that can be used for scan-out
 pub struct DrmSurface<A: AsRawFd + 'static> {
+    pub(super) dev_id: dev_t,
     pub(super) crtc: crtc::Handle,
     pub(super) plane: plane::Handle,
     pub(super) internal: Arc<DrmSurfaceInternal<A>>,
     pub(super) formats: HashSet<Format>,
+    #[cfg(feature = "backend_session")]
+    pub(super) links: RefCell<Vec<crate::signaling::SignalToken>>,
 }
 
 pub enum DrmSurfaceInternal<A: AsRawFd + 'static> {
@@ -211,4 +217,16 @@ impl<A: AsRawFd + 'static> DrmSurface<A> {
             } // There is no test-commiting with the legacy interface
         }
     }
+    
+    /// Re-evaluates the current state of the crtc.
+    ///
+    /// Usually you do not need to call this, but if the state of
+    /// the crtc is modified elsewhere and you need to reset the
+    /// initial state of this surface, you may call this function.
+    pub fn reset_state(&self) -> Result<(), Error> {
+        match &*self.internal {
+            DrmSurfaceInternal::Atomic(surf) => surf.reset_state::<Self>(None),
+            DrmSurfaceInternal::Legacy(surf) => surf.reset_state::<Self>(None),
+        }
+    }
 }