drm: Fix drop order

Cargo.toml

@@ -10,7 +10,7 @@ repository = "https://github.com/Smithay/smithay"
 members = [ "anvil" ]
 
 [dependencies]
-wayland-server = "0.21.4"
+wayland-server = "0.21.6"
 wayland-sys = "0.21.3"
 wayland-commons = "0.21.1"
 nix = "0.11"

anvil/src/udev.rs

@@ -385,15 +385,19 @@ impl<S: SessionNotifier, Data: 'static> UdevHandler for UdevHandlerImpl<S, Data>
 
     fn device_removed(&mut self, device: dev_t) {
         // drop the backends on this side
-        if let Some((id, evt_source, _)) = self.backends.remove(&device) {
+        if let Some((id, evt_source, renderers)) = self.backends.remove(&device) {
+            // drop surfaces
+            renderers.borrow_mut().clear();
+            debug!(self.logger, "Surfaces dropped");
+
             // don't use hardware acceleration anymore, if this was the primary gpu
-            let source = evt_source.clone_inner();
+            let device = Rc::try_unwrap(evt_source.remove().unwrap()).map_err(|_| "This should not happend").unwrap().into_inner().0;
-            let evented = source.borrow();
+            if device.dev_path().and_then(|path| path.canonicalize().ok()) == self.primary_gpu {
-            if (*evented).0.dev_path().and_then(|path| path.canonicalize().ok()) == self.primary_gpu {
                 *self.active_egl_context.borrow_mut() = None;
             }
 
             self.notifier.unregister(id);
+            debug!(self.logger, "Dropping device");
         }
     }
 }

src/backend/drm/egl/mod.rs

@@ -25,6 +25,7 @@ pub struct EglDevice<
     B: Backend<Surface = <D as Device>::Surface> + 'static,
     D: Device + NativeDisplay<B> + 'static,
 > where
+    <D as NativeDisplay<B>>::Arguments: From<(crtc::Handle, Mode, Vec<connector::Handle>)>,
     <D as Device>::Surface: NativeSurface,
 {
     dev: Rc<EGLContext<B, D>>,
@@ -34,6 +35,7 @@ pub struct EglDevice<
 impl<B: Backend<Surface = <D as Device>::Surface> + 'static, D: Device + NativeDisplay<B> + 'static> AsRawFd
     for EglDevice<B, D>
 where
+    <D as NativeDisplay<B>>::Arguments: From<(crtc::Handle, Mode, Vec<connector::Handle>)>,
     <D as Device>::Surface: NativeSurface,
 {
     fn as_raw_fd(&self) -> RawFd {
@@ -44,6 +46,7 @@ where
 impl<B: Backend<Surface = <D as Device>::Surface> + 'static, D: Device + NativeDisplay<B> + 'static>
     EglDevice<B, D>
 where
+    <D as NativeDisplay<B>>::Arguments: From<(crtc::Handle, Mode, Vec<connector::Handle>)>,
     <D as Device>::Surface: NativeSurface,
 {
     /// Create a new `EglGbmDrmDevice` from an open drm node
@@ -93,6 +96,7 @@ struct InternalDeviceHandler<
     B: Backend<Surface = <D as Device>::Surface> + 'static,
     D: Device + NativeDisplay<B> + 'static,
 > where
+    <D as NativeDisplay<B>>::Arguments: From<(crtc::Handle, Mode, Vec<connector::Handle>)>,
     <D as Device>::Surface: NativeSurface,
 {
     handler: Box<DeviceHandler<Device = EglDevice<B, D>> + 'static>,
@@ -177,9 +181,21 @@ where
 impl<B: Backend<Surface = <D as Device>::Surface> + 'static, D: Device + NativeDisplay<B> + 'static>
     EGLGraphicsBackend for EglDevice<B, D>
 where
+    <D as NativeDisplay<B>>::Arguments: From<(crtc::Handle, Mode, Vec<connector::Handle>)>,
     <D as Device>::Surface: NativeSurface,
 {
     fn bind_wl_display(&self, display: &Display) -> EGLResult<EGLDisplay> {
         self.dev.bind_wl_display(display)
     }
 }
+
+impl<B: Backend<Surface = <D as Device>::Surface> + 'static, D: Device + NativeDisplay<B> + 'static>
+    Drop for EglDevice<B, D>
+where
+    <D as NativeDisplay<B>>::Arguments: From<(crtc::Handle, Mode, Vec<connector::Handle>)>,
+    <D as Device>::Surface: NativeSurface,
+{
+    fn drop(&mut self) {
+        self.clear_handler();
+    }
+}

src/backend/drm/egl/session.rs

@@ -1,4 +1,5 @@
 use std::os::unix::io::RawFd;
+use drm::control::{crtc, connector, Mode};
 
 use super::EglDevice;
 use backend::drm::Device;
@@ -16,6 +17,7 @@ impl<
         D: Device + NativeDisplay<B> + AsSessionObserver<S> + 'static,
     > AsSessionObserver<EglDeviceObserver<S>> for EglDevice<B, D>
 where
+    <D as NativeDisplay<B>>::Arguments: From<(crtc::Handle, Mode, Vec<connector::Handle>)>,
     <D as Device>::Surface: NativeSurface,
 {
     fn observer(&mut self) -> EglDeviceObserver<S> {

src/backend/drm/gbm/mod.rs

@@ -192,3 +192,9 @@ impl<D: RawDevice + ControlDevice + 'static> AsRawFd for GbmDevice<D> {
         self.dev.borrow().as_raw_fd()
     }
 }
+
+impl<D: RawDevice + ControlDevice + 'static> Drop for GbmDevice<D> {
+    fn drop(&mut self) {
+        self.clear_handler();
+    }
+}

src/backend/drm/legacy/mod.rs

@@ -26,22 +26,51 @@ pub mod session;
 pub struct LegacyDrmDevice<A: AsRawFd + 'static> {
     dev: Rc<Dev<A>>,
     dev_id: dev_t,
-    priviledged: bool,
     active: Arc<AtomicBool>,
-    old_state: HashMap<crtc::Handle, (crtc::Info, Vec<connector::Handle>)>,
     backends: Rc<RefCell<HashMap<crtc::Handle, Weak<LegacyDrmSurfaceInternal<A>>>>>,
     handler: Option<RefCell<Box<DeviceHandler<Device = LegacyDrmDevice<A>>>>>,
     logger: ::slog::Logger,
 }
 
-pub(in crate::backend::drm) struct Dev<A: AsRawFd + 'static>(A);
+pub(in crate::backend::drm) struct Dev<A: AsRawFd + 'static> {
+    fd: A,
+    priviledged: bool,
+    active: Arc<AtomicBool>,
+    old_state: HashMap<crtc::Handle, (crtc::Info, Vec<connector::Handle>)>,
+    logger: ::slog::Logger,
+}
 impl<A: AsRawFd + 'static> AsRawFd for Dev<A> {
     fn as_raw_fd(&self) -> RawFd {
-        self.0.as_raw_fd()
+        self.fd.as_raw_fd()
     }
 }
 impl<A: AsRawFd + 'static> BasicDevice for Dev<A> {}
 impl<A: AsRawFd + 'static> ControlDevice for Dev<A> {}
+impl<A: AsRawFd + 'static>  Drop for Dev<A> {
+    fn drop(&mut self) {
+        info!(self.logger, "Dropping device: {:?}", self.dev_path());
+        if self.active.load(Ordering::SeqCst) {
+            let old_state = self.old_state.clone();
+            for (handle, (info, connectors)) in old_state {
+                if let Err(err) = crtc::set(
+                    &*self,
+                    handle,
+                    info.fb(),
+                    &connectors,
+                    info.position(),
+                    info.mode(),
+                ) {
+                    error!(self.logger, "Failed to reset crtc ({:?}). Error: {}", handle, err);
+                }
+            }
+        }
+        if self.priviledged {
+            if let Err(err) = self.drop_master() {
+                error!(self.logger, "Failed to drop drm master state. Error: {}", err);
+            }
+        }
+    }
+}
 
 impl<A: AsRawFd + 'static> LegacyDrmDevice<A> {
     /// Create a new `LegacyDrmDevice` from an open drm node
@@ -53,47 +82,43 @@ impl<A: AsRawFd + 'static> LegacyDrmDevice<A> {
         L: Into<Option<::slog::Logger>>,
     {
         let log = ::slog_or_stdlog(logger).new(o!("smithay_module" => "backend_drm"));
+        info!(log, "DrmDevice initializing");
 
         let dev_id = fstat(dev.as_raw_fd())
             .chain_err(|| ErrorKind::UnableToGetDeviceId)?
             .st_rdev;
 
-        let mut drm = LegacyDrmDevice {
+        let active = Arc::new(AtomicBool::new(true));
-            // Open the drm device and create a context based on that
+        let mut dev = Dev {
-            dev: Rc::new(Dev(dev)),
+            fd: dev,
-            dev_id,
             priviledged: true,
-            active: Arc::new(AtomicBool::new(true)),
             old_state: HashMap::new(),
-            backends: Rc::new(RefCell::new(HashMap::new())),
+            active: active.clone(),
-            handler: None,
             logger: log.clone(),
         };
 
-        info!(log, "DrmDevice initializing");
-
         // we want to modeset, so we better be the master, if we run via a tty session
-        if drm.set_master().is_err() {
+        if dev.set_master().is_err() {
             warn!(log, "Unable to become drm master, assuming unpriviledged mode");
-            drm.priviledged = false;
+            dev.priviledged = false;
         };
 
-        let res_handles = ControlDevice::resource_handles(&drm).chain_err(|| {
+        let res_handles = ControlDevice::resource_handles(&dev).chain_err(|| {
-            ErrorKind::DrmDev(format!("Error loading drm resources on {:?}", drm.dev_path()))
+            ErrorKind::DrmDev(format!("Error loading drm resources on {:?}", dev.dev_path()))
         })?;
         for &con in res_handles.connectors() {
-            let con_info = connector::Info::load_from_device(&drm, con).chain_err(|| {
+            let con_info = connector::Info::load_from_device(&dev, con).chain_err(|| {
-                ErrorKind::DrmDev(format!("Error loading connector info on {:?}", drm.dev_path()))
+                ErrorKind::DrmDev(format!("Error loading connector info on {:?}", dev.dev_path()))
             })?;
             if let Some(enc) = con_info.current_encoder() {
-                let enc_info = encoder::Info::load_from_device(&drm, enc).chain_err(|| {
+                let enc_info = encoder::Info::load_from_device(&dev, enc).chain_err(|| {
-                    ErrorKind::DrmDev(format!("Error loading encoder info on {:?}", drm.dev_path()))
+                    ErrorKind::DrmDev(format!("Error loading encoder info on {:?}", dev.dev_path()))
                 })?;
                 if let Some(crtc) = enc_info.current_crtc() {
-                    let info = crtc::Info::load_from_device(&drm, crtc).chain_err(|| {
+                    let info = crtc::Info::load_from_device(&dev, crtc).chain_err(|| {
-                        ErrorKind::DrmDev(format!("Error loading crtc info on {:?}", drm.dev_path()))
+                        ErrorKind::DrmDev(format!("Error loading crtc info on {:?}", dev.dev_path()))
                     })?;
-                    drm.old_state
+                    dev.old_state
                         .entry(crtc)
                         .or_insert((info, Vec::new()))
                         .1
@@ -102,13 +127,21 @@ impl<A: AsRawFd + 'static> LegacyDrmDevice<A> {
             }
         }
         
-        Ok(drm)
+        Ok(LegacyDrmDevice {
+            // Open the drm device and create a context based on that
+            dev: Rc::new(dev),
+            dev_id,
+            active,
+            backends: Rc::new(RefCell::new(HashMap::new())),
+            handler: None,
+            logger: log.clone(),
+        })
     }
 }
 
 impl<A: AsRawFd + 'static> AsRawFd for LegacyDrmDevice<A> {
     fn as_raw_fd(&self) -> RawFd {
-        self.dev.0.as_raw_fd()
+        self.dev.as_raw_fd()
     }
 }
 
@@ -247,28 +280,6 @@ impl<A: AsRawFd + 'static> RawDevice for LegacyDrmDevice<A> {
 
 impl<A: AsRawFd + 'static> Drop for LegacyDrmDevice<A> {
     fn drop(&mut self) {
-        self.backends.borrow_mut().clear();
+        self.clear_handler();
-        if Rc::strong_count(&self.dev) > 1 {
-            panic!("Pending DrmBackends. You need to free all backends before the DrmDevice gets destroyed");
-        }
-        if self.active.load(Ordering::SeqCst) {
-            for (handle, (info, connectors)) in self.old_state.drain() {
-                if let Err(err) = crtc::set(
-                    &*self.dev,
-                    handle,
-                    info.fb(),
-                    &connectors,
-                    info.position(),
-                    info.mode(),
-                ) {
-                    error!(self.logger, "Failed to reset crtc ({:?}). Error: {}", handle, err);
-                }
-            }
-        }
-        if self.priviledged {
-            if let Err(err) = self.drop_master() {
-                error!(self.logger, "Failed to drop drm master state. Error: {}", err);
-            }
-        }
     }
 }

src/backend/drm/legacy/session.rs

@@ -18,7 +18,6 @@ pub struct LegacyDrmDeviceObserver<A: AsRawFd + 'static> {
     dev_id: dev_t,
     priviledged: bool,
     active: Arc<AtomicBool>,
-    old_state: HashMap<crtc::Handle, (crtc::Info, Vec<connector::Handle>)>,
     backends: Weak<RefCell<HashMap<crtc::Handle, Weak<LegacyDrmSurfaceInternal<A>>>>>,
     logger: ::slog::Logger,
 }
@@ -28,9 +27,8 @@ impl<A: AsRawFd + 'static> AsSessionObserver<LegacyDrmDeviceObserver<A>> for Leg
         LegacyDrmDeviceObserver {
             dev: Rc::downgrade(&self.dev),
             dev_id: self.dev_id,
-            old_state: self.old_state.clone(),
             active: self.active.clone(),
-            priviledged: self.priviledged,
+            priviledged: self.dev.priviledged,
             backends: Rc::downgrade(&self.backends),
             logger: self.logger.clone(),
         }
@@ -50,18 +48,6 @@ impl<A: AsRawFd + 'static> SessionObserver for LegacyDrmDeviceObserver<A> {
                     let _ = crtc::clear_cursor(&*device, surface.crtc);
                 }
             }
-            for (handle, &(ref info, ref connectors)) in &self.old_state {
-                if let Err(err) = crtc::set(
-                    &*device,
-                    *handle,
-                    info.fb(),
-                    connectors,
-                    info.position(),
-                    info.mode(),
-                ) {
-                    error!(self.logger, "Failed to reset crtc ({:?}). Error: {}", handle, err);
-                }
-            }
         }
         self.active.store(false, Ordering::SeqCst);
         if self.priviledged {