drm: Require all surfaces to always have a mode set

anvil/src/udev.rs

@@ -319,7 +319,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
                 for crtc in res_handles.filter_crtcs(encoder_info.possible_crtcs()) {
                     if let Entry::Vacant(entry) = backends.entry(crtc) {
                         let renderer = GliumDrawer::init(
-                            device.create_surface(crtc).unwrap(),
+                            device.create_surface(crtc, connector_info.modes()[0], &[connector_info.handle()]).unwrap(),
                             egl_buffer_reader.clone(),
                             logger.clone(),
                         );

examples/raw_atomic_drm.rs

@@ -96,9 +96,7 @@ fn main() {
     let mode = connector_info.modes()[0]; // Use first mode (usually highest resoltion, but in reality you should filter and sort and check and match with other connectors, if you use more then one.)
 
     // Initialize the hardware backend
-    let surface = Rc::new(device.create_surface(crtc).unwrap());
+    let surface = Rc::new(device.create_surface(crtc, mode, &[connector_info.handle()]).unwrap());
-    surface.set_connectors(&[connector_info.handle()]).unwrap();
-    surface.use_mode(Some(mode)).unwrap();
 
     for conn in surface.current_connectors().into_iter() {
         if conn != connector_info.handle() {

examples/raw_legacy_drm.rs

@@ -78,10 +78,8 @@ fn main() {
     let mode = connector_info.modes()[0]; // Use first mode (usually highest resoltion, but in reality you should filter and sort and check and match with other connectors, if you use more then one.)
 
     // Initialize the hardware backend
-    let surface = Rc::new(device.create_surface(crtc).unwrap());
+    let surface = Rc::new(device.create_surface(crtc, mode, &[connector_info.handle()]).unwrap());
 
-    surface.use_mode(Some(mode)).unwrap();
-    surface.set_connectors(&[connector_info.handle()]).unwrap();
     /*
      * Lets create buffers and framebuffers.
      * We use drm-rs DumbBuffers, because they always work and require little to no setup.

src/backend/drm/atomic/mod.rs

@@ -19,7 +19,7 @@ use std::sync::{
 
 use drm::control::{atomic::AtomicModeReq, AtomicCommitFlags, Device as ControlDevice, Event};
 use drm::control::{
-    connector, crtc, encoder, framebuffer, plane, property, PropertyValueSet, ResourceHandle, ResourceHandles,
+    connector, crtc, encoder, framebuffer, plane, property, Mode, PropertyValueSet, ResourceHandle, ResourceHandles,
 };
 use drm::SystemError as DrmError;
 use drm::{ClientCapability, Device as BasicDevice};
@@ -339,7 +339,7 @@ impl<A: AsRawFd + 'static> Device for AtomicDrmDevice<A> {
         let _ = self.handler.take();
     }
 
-    fn create_surface(&mut self, crtc: crtc::Handle) -> Result<AtomicDrmSurface<A>, Error> {
+    fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle]) -> Result<AtomicDrmSurface<A>, Error> {
         if self.backends.borrow().contains_key(&crtc) {
             return Err(Error::CrtcAlreadyInUse(crtc));
         }
@@ -348,9 +348,15 @@ impl<A: AsRawFd + 'static> Device for AtomicDrmDevice<A> {
             return Err(Error::DeviceInactive);
         }
 
+        if connectors.is_empty() {
+            return Err(Error::SurfaceWithoutConnectors(crtc));
+        }
+
         let backend = Rc::new(AtomicDrmSurfaceInternal::new(
             self.dev.clone(),
             crtc,
+            mode,
+            connectors,
             self.logger.new(o!("crtc" => format!("{:?}", crtc))),
         )?);
 

src/backend/drm/atomic/surface.rs

@@ -1,7 +1,7 @@
 use drm::buffer::Buffer;
 use drm::control::atomic::AtomicModeReq;
 use drm::control::Device as ControlDevice;
-use drm::control::{connector, crtc, framebuffer, plane, property, AtomicCommitFlags, Mode, PlaneType};
+use drm::control::{connector, crtc, dumbbuffer::DumbBuffer, framebuffer, plane, property, AtomicCommitFlags, Mode, PlaneType};
 use drm::Device as BasicDevice;
 
 use std::cell::Cell;
@@ -26,8 +26,8 @@ pub struct CursorState {
 
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct State {
-    pub mode: Option<Mode>,
+    pub mode: Mode,
-    pub blob: Option<property::Value<'static>>,
+    pub blob: property::Value<'static>,
     pub connectors: HashSet<connector::Handle>,
 }
 
@@ -45,6 +45,7 @@ pub(super) struct AtomicDrmSurfaceInternal<A: AsRawFd + 'static> {
     pub(super) state: RwLock<State>,
     pub(super) pending: RwLock<State>,
     pub(super) logger: ::slog::Logger,
+    init_buffer: Cell<Option<(DumbBuffer, framebuffer::Handle)>>,
 }
 
 impl<A: AsRawFd + 'static> AsRawFd for AtomicDrmSurfaceInternal<A> {
@@ -57,26 +58,36 @@ impl<A: AsRawFd + 'static> BasicDevice for AtomicDrmSurfaceInternal<A> {}
 impl<A: AsRawFd + 'static> ControlDevice for AtomicDrmSurfaceInternal<A> {}
 
 impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
-    pub(crate) fn new(dev: Rc<Dev<A>>, crtc: crtc::Handle, logger: ::slog::Logger) -> Result<Self, Error> {
+    pub(crate) fn new(dev: Rc<Dev<A>>, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], logger: ::slog::Logger) -> Result<Self, Error> {
         let crtc_info = dev.get_crtc(crtc).compat().map_err(|source| Error::Access {
             errmsg: "Error loading crtc info",
             dev: dev.dev_path(),
             source,
         })?;
-
+        
-        let mode = crtc_info.mode();
+        // If we have no current mode, we create a fake one, which will not match (and thus gets overriden on the commit below).
-        let blob = match mode {
+        // A better fix would probably be making mode an `Option`, but that would mean
-            Some(mode) => Some(
+        // we need to be sure, we require a mode to always be set without relying on the compiler.
-                dev.create_property_blob(mode)
+        // So we cheat, because it works and is easier to handle later.
-                    .compat()
+        let current_mode = crtc_info.mode().unwrap_or_else(|| unsafe { std::mem::zeroed() });
-                    .map_err(|source| Error::Access {
+        let current_blob = match crtc_info.mode() {
-                        errmsg: "Failed to create Property Blob for mode",
+            Some(mode) => dev.create_property_blob(mode)
-                        dev: dev.dev_path(),
+                .compat()
-                        source,
+                .map_err(|source| Error::Access {
-                    })?,
+                    errmsg: "Failed to create Property Blob for mode",
-            ),
+                    dev: dev.dev_path(),
-            None => None,
+                    source,
+                })?,
+            None => property::Value::Unknown(0),
         };
+                
+        let blob = dev.create_property_blob(mode)
+            .compat()
+            .map_err(|source| Error::Access {
+                errmsg: "Failed to create Property Blob for mode",
+                dev: dev.dev_path(),
+                source,
+            })?;
 
         let res_handles = ControlDevice::resource_handles(&*dev)
             .compat()
@@ -86,12 +97,8 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
                 source,
             })?;
 
-        let mut state = State {
-            mode,
-            blob,
-            connectors: HashSet::new(),
-        };
 
+        let mut current_connectors = HashSet::new();
         for conn in res_handles.connectors() {
             let crtc_prop = dev
                 .prop_mapping
@@ -113,7 +120,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
                             crtc_prop_info.value_type().convert_value(val)
                         {
                             if conn_crtc == crtc {
-                                state.connectors.insert(*conn);
+                                current_connectors.insert(*conn);
                             }
                         }
                         break;
@@ -121,13 +128,23 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
                 }
             }
         }
+        let state = State {
+            mode: current_mode,
+            blob: current_blob,
+            connectors: current_connectors,
+        };
+        let pending = State {
+            mode,
+            blob,
+            connectors: connectors.into_iter().copied().collect(),
+        };
 
         let (primary, cursor) =
             AtomicDrmSurfaceInternal::find_planes(&dev, crtc).ok_or(Error::NoSuitablePlanes {
                 crtc,
                 dev: dev.dev_path(),
             })?;
-        Ok(AtomicDrmSurfaceInternal {
+        let surface = AtomicDrmSurfaceInternal {
             dev,
             crtc,
             cursor: CursorState {
@@ -136,10 +153,22 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
                 hotspot: Cell::new((0, 0)),
             },
             planes: Planes { primary, cursor },
-            state: RwLock::new(state.clone()),
+            state: RwLock::new(state),
-            pending: RwLock::new(state),
+            pending: RwLock::new(pending),
             logger,
-        })
+            init_buffer: Cell::new(None),
+        };
+
+        Ok(surface)
+    }
+}
+
+impl<A: AsRawFd + 'static> Drop for AtomicDrmSurfaceInternal<A> {
+    fn drop(&mut self) {
+        if let Some((db, fb)) = self.init_buffer.take() {
+            let _ = self.destroy_framebuffer(fb);
+            let _ = self.destroy_dumb_buffer(db);
+        }
     }
 }
 
@@ -159,11 +188,11 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
         self.pending.read().unwrap().connectors.clone()
     }
 
-    fn current_mode(&self) -> Option<Mode> {
+    fn current_mode(&self) -> Mode {
         self.state.read().unwrap().mode
     }
 
-    fn pending_mode(&self) -> Option<Mode> {
+    fn pending_mode(&self) -> Mode {
         self.pending.read().unwrap().mode
     }
 
@@ -180,15 +209,15 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
         let mut pending = self.pending.write().unwrap();
 
         // check if the connector can handle the current mode
-        if info.modes().contains(pending.mode.as_ref().unwrap()) {
+        if info.modes().contains(&pending.mode) {
             // check if config is supported
             let req = self.build_request(
                 &mut [conn].iter(),
                 &mut [].iter(),
                 &self.planes,
                 None,
-                pending.mode,
+                Some(pending.mode),
-                pending.blob,
+                Some(pending.blob),
             )?;
             self.atomic_commit(
                 &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
@@ -202,7 +231,7 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
 
             Ok(())
         } else {
-            Err(Error::ModeNotSuitable(pending.mode.unwrap()))
+            Err(Error::ModeNotSuitable(pending.mode))
         }
     }
 
@@ -215,8 +244,8 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
             &mut [conn].iter(),
             &self.planes,
             None,
-            pending.mode,
+            Some(pending.mode),
-            pending.blob,
+            Some(pending.blob),
         )?;
         self.atomic_commit(
             &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
@@ -244,8 +273,8 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
             &mut removed,
             &self.planes,
             None,
-            pending.mode,
+            Some(pending.mode),
-            pending.blob,
+            Some(pending.blob),
         )?;
 
         self.atomic_commit(
@@ -259,30 +288,26 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
         Ok(())
     }
 
-    fn use_mode(&self, mode: Option<Mode>) -> Result<(), Error> {
+    fn use_mode(&self, mode: Mode) -> Result<(), Error> {
         let mut pending = self.pending.write().unwrap();
 
         // check if new config is supported
-        let new_blob = Some(match mode {
+        let new_blob = self
-            Some(mode) => self
-                .dev
                 .create_property_blob(mode)
                 .compat()
                 .map_err(|source| Error::Access {
                     errmsg: "Failed to create Property Blob for mode",
                     dev: self.dev_path(),
                     source,
-                })?,
+                })?;
-            None => property::Value::Unknown(0),
-        });
 
         let req = self.build_request(
             &mut pending.connectors.iter(),
             &mut [].iter(),
             &self.planes,
             None,
-            mode,
+            Some(mode),
-            new_blob,
+            Some(new_blob),
         )?;
         if let Err(err) = self
             .atomic_commit(
@@ -292,7 +317,7 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
             .compat()
             .map_err(|_| Error::TestFailed(self.crtc))
         {
-            let _ = self.dev.destroy_property_blob(new_blob.unwrap().into());
+            let _ = self.dev.destroy_property_blob(new_blob.into());
             return Err(err);
         }
 
@@ -343,7 +368,7 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
             info!(
                 self.logger,
                 "Setting new mode: {:?}",
-                pending.mode.as_ref().unwrap().name()
+                pending.mode.name()
             );
         }
 
@@ -355,8 +380,8 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
                 &mut removed,
                 &self.planes,
                 Some(framebuffer),
-                pending.mode,
+                Some(pending.mode),
-                pending.blob,
+                Some(pending.blob),
             )?;
 
             if let Err(err) = self
@@ -380,15 +405,13 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
                     &mut [].iter(),
                     &self.planes,
                     Some(framebuffer),
-                    current.mode,
+                    Some(current.mode),
-                    current.blob,
+                    Some(current.blob),
                 )?
             } else {
                 if current.mode != pending.mode {
-                    if let Some(blob) = current.blob {
+                    if let Err(err) = self.dev.destroy_property_blob(current.blob.into()) {
-                        if let Err(err) = self.dev.destroy_property_blob(blob.into()) {
+                        warn!(self.logger, "Failed to destory old mode property blob: {}", err);
-                            warn!(self.logger, "Failed to destory old mode property blob: {}", err);
-                        }
                     }
                 }
                 *current = pending.clone();
@@ -834,11 +857,11 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurface<A> {
         self.0.pending_connectors()
     }
 
-    fn current_mode(&self) -> Option<Mode> {
+    fn current_mode(&self) -> Mode {
         self.0.current_mode()
     }
 
-    fn pending_mode(&self) -> Option<Mode> {
+    fn pending_mode(&self) -> Mode {
         self.0.pending_mode()
     }
 
@@ -854,7 +877,7 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurface<A> {
         self.0.set_connectors(connectors)
     }
 
-    fn use_mode(&self, mode: Option<Mode>) -> Result<(), Error> {
+    fn use_mode(&self, mode: Mode) -> Result<(), Error> {
         self.0.use_mode(mode)
     }
 }

src/backend/drm/common/fallback.rs

@@ -244,10 +244,10 @@ where
         }
     }
     fallback_device_impl!(clear_handler, &mut Self);
-    fn create_surface(&mut self, crtc: crtc::Handle) -> Result<Self::Surface, E> {
+    fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle]) -> Result<Self::Surface, E> {
         match self {
-            FallbackDevice::Preference(dev) => Ok(FallbackSurface::Preference(dev.create_surface(crtc)?)),
+            FallbackDevice::Preference(dev) => Ok(FallbackSurface::Preference(dev.create_surface(crtc, mode, connectors)?)),
-            FallbackDevice::Fallback(dev) => Ok(FallbackSurface::Fallback(dev.create_surface(crtc)?)),
+            FallbackDevice::Fallback(dev) => Ok(FallbackSurface::Fallback(dev.create_surface(crtc, mode, connectors)?)),
         }
     }
     fallback_device_impl!(process_events, &mut Self);
@@ -297,9 +297,9 @@ where
     fallback_surface_impl!(add_connector, &Self, Result<(), E>, conn: connector::Handle);
     fallback_surface_impl!(remove_connector, &Self, Result<(), E>, conn: connector::Handle);
     fallback_surface_impl!(set_connectors, &Self, Result<(), E>, conns: &[connector::Handle]);
-    fallback_surface_impl!(current_mode, &Self, Option<Mode>);
+    fallback_surface_impl!(current_mode, &Self, Mode);
-    fallback_surface_impl!(pending_mode, &Self, Option<Mode>);
+    fallback_surface_impl!(pending_mode, &Self, Mode);
-    fallback_surface_impl!(use_mode, &Self, Result<(), E>, mode: Option<Mode>);
+    fallback_surface_impl!(use_mode, &Self, Result<(), E>, mode: Mode);
 }
 
 impl<E, C, S1, S2> RawSurface for FallbackSurface<S1, S2>

src/backend/drm/common/mod.rs

@@ -40,6 +40,9 @@ pub enum Error {
     /// The given crtc is already in use by another backend
     #[error("Crtc `{0:?}` is already in use by another backend")]
     CrtcAlreadyInUse(crtc::Handle),
+    /// This operation would result in a surface without connectors.
+    #[error("Surface of crtc `{0:?}` would have no connectors, which is not accepted")]
+    SurfaceWithoutConnectors(crtc::Handle),
     /// No encoder was found for a given connector on the set crtc
     #[error("No encoder found for the given connector '{connector:?}' on crtc `{crtc:?}`")]
     NoSuitableEncoder {

src/backend/drm/egl/mod.rs

@@ -8,7 +8,7 @@
 //! Take a look at `anvil`s source code for an example of this.
 //!
 
-use drm::control::{connector, crtc, encoder, framebuffer, plane, ResourceHandles};
+use drm::control::{connector, crtc, encoder, framebuffer, plane, Mode, ResourceHandles};
 use drm::SystemError as DrmError;
 use nix::libc::dev_t;
 use std::os::unix::io::{AsRawFd, RawFd};
@@ -40,11 +40,13 @@ pub enum Error<U: std::error::Error + std::fmt::Debug + std::fmt::Display + 'sta
     Underlying(#[source] U),
 }
 
+type Arguments = (crtc::Handle, Mode, Vec<connector::Handle>);
+
 /// Representation of an egl device to create egl rendering surfaces
 pub struct EglDevice<B, D>
 where
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + 'static,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     dev: EGLDisplay<B, D>,
@@ -56,7 +58,7 @@ where
 impl<B, D> AsRawFd for EglDevice<B, D>
 where
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + 'static,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     fn as_raw_fd(&self) -> RawFd {
@@ -67,7 +69,7 @@ where
 impl<B, D> EglDevice<B, D>
 where
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + 'static,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     /// Try to create a new [`EglDevice`] from an open device.
@@ -122,7 +124,7 @@ where
 struct InternalDeviceHandler<B, D>
 where
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + 'static,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     handler: Box<dyn DeviceHandler<Device = EglDevice<B, D>> + 'static>,
@@ -131,7 +133,7 @@ where
 impl<B, D> DeviceHandler for InternalDeviceHandler<B, D>
 where
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + 'static,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     type Device = D;
@@ -147,7 +149,7 @@ where
 impl<B, D> Device for EglDevice<B, D>
 where
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + 'static,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     type Surface = EglSurface<<D as Device>::Surface>;
@@ -169,6 +171,8 @@ where
     fn create_surface(
         &mut self,
         crtc: crtc::Handle,
+        mode: Mode,
+        connectors: &[connector::Handle],
     ) -> Result<Self::Surface, <Self::Surface as Surface>::Error> {
         info!(self.logger, "Initializing EglSurface");
 
@@ -182,7 +186,7 @@ where
                 context.get_pixel_format(),
                 self.default_requirements.double_buffer,
                 context.get_config_id(),
-                crtc,
+                (crtc, mode, Vec::from(connectors)),
             )
             .map_err(Error::EGL)?;
 
@@ -221,7 +225,7 @@ where
 impl<B, D> EGLGraphicsBackend for EglDevice<B, D>
 where
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + 'static,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     fn bind_wl_display(&self, display: &Display) -> Result<EGLBufferReader, EGLError> {
@@ -232,7 +236,7 @@ where
 impl<B, D> Drop for EglDevice<B, D>
 where
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + 'static,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     fn drop(&mut self) {

src/backend/drm/egl/session.rs

@@ -3,7 +3,7 @@
 //! to an open [`Session`](::backend::session::Session).
 //!
 
-use drm::control::crtc;
+use drm::control::{crtc, connector, Mode};
 use std::os::unix::io::RawFd;
 
 use super::EglDevice;
@@ -22,7 +22,7 @@ impl<S, B, D> AsSessionObserver<EglDeviceObserver<S>> for EglDevice<B, D>
 where
     S: SessionObserver + 'static,
     B: Backend<Surface = <D as Device>::Surface> + 'static,
-    D: Device + NativeDisplay<B, Arguments = crtc::Handle> + AsSessionObserver<S> + 'static,
+    D: Device + NativeDisplay<B, Arguments = (crtc::Handle, Mode, Vec<connector::Handle>)> + AsSessionObserver<S> + 'static,
     <D as Device>::Surface: NativeSurface,
 {
     fn observer(&mut self) -> EglDeviceObserver<S> {

src/backend/drm/egl/surface.rs

@@ -53,15 +53,15 @@ where
         self.surface.set_connectors(connectors).map_err(Error::Underlying)
     }
 
-    fn current_mode(&self) -> Option<Mode> {
+    fn current_mode(&self) -> Mode {
         self.surface.current_mode()
     }
 
-    fn pending_mode(&self) -> Option<Mode> {
+    fn pending_mode(&self) -> Mode {
         self.surface.pending_mode()
     }
 
-    fn use_mode(&self, mode: Option<Mode>) -> Result<(), Self::Error> {
+    fn use_mode(&self, mode: Mode) -> Result<(), Self::Error> {
         self.surface.use_mode(mode).map_err(Error::Underlying)
     }
 }
@@ -100,7 +100,7 @@ where
     }
 
     fn get_framebuffer_dimensions(&self) -> (u32, u32) {
-        let (w, h) = self.pending_mode().map(|mode| mode.size()).unwrap_or((1, 1));
+        let (w, h) = self.pending_mode().size();
         (w as u32, h as u32)
     }
 

src/backend/drm/gbm/egl.rs

@@ -12,7 +12,7 @@ use crate::backend::graphics::SwapBuffersError;
 
 use super::{Error, GbmDevice, GbmSurface};
 
-use drm::control::{crtc, Device as ControlDevice};
+use drm::control::{crtc, connector, Device as ControlDevice, Mode};
 use gbm::AsRaw;
 use std::marker::PhantomData;
 use std::ptr;
@@ -52,7 +52,7 @@ impl<D: RawDevice + 'static> Backend for Gbm<D> {
 }
 
 unsafe impl<D: RawDevice + ControlDevice + 'static> NativeDisplay<Gbm<D>> for GbmDevice<D> {
-    type Arguments = crtc::Handle;
+    type Arguments = (crtc::Handle, Mode, Vec<connector::Handle>);
     type Error = Error<<<D as Device>::Surface as Surface>::Error>;
 
     fn is_backend(&self) -> bool {
@@ -63,8 +63,8 @@ unsafe impl<D: RawDevice + ControlDevice + 'static> NativeDisplay<Gbm<D>> for Gb
         Ok(self.dev.borrow().as_raw() as *const _)
     }
 
-    fn create_surface(&mut self, crtc: crtc::Handle) -> Result<GbmSurface<D>, Self::Error> {
+    fn create_surface(&mut self, args: Self::Arguments) -> Result<GbmSurface<D>, Self::Error> {
-        Device::create_surface(self, crtc)
+        Device::create_surface(self, args.0, args.1, &args.2)
     }
 }
 

src/backend/drm/gbm/mod.rs

@@ -11,7 +11,7 @@
 
 use super::{Device, DeviceHandler, RawDevice, ResourceHandles, Surface};
 
-use drm::control::{connector, crtc, encoder, framebuffer, plane, Device as ControlDevice};
+use drm::control::{connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Mode};
 use drm::SystemError as DrmError;
 use gbm::{self, BufferObjectFlags, Format as GbmFormat};
 use nix::libc::dev_t;
@@ -33,9 +33,6 @@ pub enum Error<U: std::error::Error + std::fmt::Debug + std::fmt::Display + 'sta
     /// Creation of GBM surface failed
     #[error("Creation of GBM surface failed")]
     SurfaceCreationFailed(#[source] io::Error),
-    /// No mode is set, blocking the current operation
-    #[error("No mode is currently set")]
-    NoModeSet,
     /// Creation of GBM buffer object failed
     #[error("Creation of GBM buffer object failed")]
     BufferCreationFailed(#[source] io::Error),
@@ -159,17 +156,18 @@ impl<D: RawDevice + ControlDevice + 'static> Device for GbmDevice<D> {
     fn create_surface(
         &mut self,
         crtc: crtc::Handle,
+        mode: Mode,
+        connectors: &[connector::Handle],
     ) -> Result<GbmSurface<D>, Error<<<D as Device>::Surface as Surface>::Error>> {
         info!(self.logger, "Initializing GbmSurface");
 
         let drm_surface =
-            Device::create_surface(&mut **self.dev.borrow_mut(), crtc).map_err(Error::Underlying)?;
+            Device::create_surface(&mut **self.dev.borrow_mut(), crtc, mode, connectors).map_err(Error::Underlying)?;
 
         // initialize the surface
         let (w, h) = drm_surface
             .pending_mode()
-            .map(|mode| mode.size())
+            .size();
-            .unwrap_or((1, 1));
         let surface = self
             .dev
             .borrow()

src/backend/drm/gbm/surface.rs

@@ -93,8 +93,6 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
     pub fn recreate(&self) -> Result<(), Error<<<D as Device>::Surface as Surface>::Error>> {
         let (w, h) = self
             .pending_mode()
-            .or_else(|| self.current_mode())
-            .ok_or(Error::NoModeSet)?
             .size();
 
         // Recreate the surface and the related resources to match the new
@@ -167,15 +165,15 @@ impl<D: RawDevice + 'static> Surface for GbmSurfaceInternal<D> {
         self.crtc.set_connectors(connectors).map_err(Error::Underlying)
     }
 
-    fn current_mode(&self) -> Option<Mode> {
+    fn current_mode(&self) -> Mode {
         self.crtc.current_mode()
     }
 
-    fn pending_mode(&self) -> Option<Mode> {
+    fn pending_mode(&self) -> Mode {
         self.crtc.pending_mode()
     }
 
-    fn use_mode(&self, mode: Option<Mode>) -> Result<(), Self::Error> {
+    fn use_mode(&self, mode: Mode) -> Result<(), Self::Error> {
         self.crtc.use_mode(mode).map_err(Error::Underlying)
     }
 }
@@ -325,15 +323,15 @@ impl<D: RawDevice + 'static> Surface for GbmSurface<D> {
         self.0.set_connectors(connectors)
     }
 
-    fn current_mode(&self) -> Option<Mode> {
+    fn current_mode(&self) -> Mode {
         self.0.current_mode()
     }
 
-    fn pending_mode(&self) -> Option<Mode> {
+    fn pending_mode(&self) -> Mode {
         self.0.pending_mode()
     }
 
-    fn use_mode(&self, mode: Option<Mode>) -> Result<(), Self::Error> {
+    fn use_mode(&self, mode: Mode) -> Result<(), Self::Error> {
         self.0.use_mode(mode)
     }
 }

src/backend/drm/legacy/mod.rs

@@ -11,24 +11,24 @@
 use super::{common::Error, DevPath, Device, DeviceHandler, RawDevice};
 
 use drm::control::{
-    connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Event, ResourceHandles,
+    connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Event, Mode, ResourceHandles,
 };
 use drm::{Device as BasicDevice, SystemError as DrmError};
 use nix::libc::dev_t;
 use nix::sys::stat::fstat;
 
 use std::cell::RefCell;
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::rc::{Rc, Weak};
 use std::sync::atomic::{AtomicBool, Ordering};
-use std::sync::{Arc, RwLock};
+use std::sync::Arc;
 
 use failure::{Fail, ResultExt};
 
 mod surface;
 pub use self::surface::LegacyDrmSurface;
-use self::surface::{LegacyDrmSurfaceInternal, State};
+use self::surface::LegacyDrmSurfaceInternal;
 
 #[cfg(feature = "backend_session")]
 pub mod session;
@@ -241,7 +241,7 @@ impl<A: AsRawFd + 'static> Device for LegacyDrmDevice<A> {
         let _ = self.handler.take();
     }
 
-    fn create_surface(&mut self, crtc: crtc::Handle) -> Result<LegacyDrmSurface<A>, Error> {
+    fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle]) -> Result<LegacyDrmSurface<A>, Error> {
         if self.backends.borrow().contains_key(&crtc) {
             return Err(Error::CrtcAlreadyInUse(crtc));
         }
@@ -250,8 +250,12 @@ impl<A: AsRawFd + 'static> Device for LegacyDrmDevice<A> {
             return Err(Error::DeviceInactive);
         }
 
+        if connectors.is_empty() {
+            return Err(Error::SurfaceWithoutConnectors(crtc));
+        }
+
         let backend = Rc::new(LegacyDrmSurfaceInternal::new(
-            self.dev.clone(), crtc,
+            self.dev.clone(), crtc, mode, connectors,
             self.logger.new(o!("crtc" => format!("{:?}", crtc))),
         )?);
 

src/backend/drm/legacy/surface.rs

@@ -5,6 +5,7 @@ use drm::control::{
 };
 use drm::Device as BasicDevice;
 
+use std::cell::Cell;
 use std::collections::HashSet;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::rc::Rc;
@@ -20,7 +21,7 @@ use failure::{Fail, ResultExt};
 
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct State {
-    pub mode: Option<Mode>,
+    pub mode: Mode,
     pub connectors: HashSet<connector::Handle>,
 }
 
@@ -30,6 +31,7 @@ pub(super) struct LegacyDrmSurfaceInternal<A: AsRawFd + 'static> {
     pub(super) state: RwLock<State>,
     pub(super) pending: RwLock<State>,
     pub(super) logger: ::slog::Logger,
+    init_buffer: Cell<Option<(DumbBuffer, framebuffer::Handle)>>,
 }
 
 impl<A: AsRawFd + 'static> AsRawFd for LegacyDrmSurfaceInternal<A> {
@@ -96,18 +98,18 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurfaceInternal<A> {
         self.pending.read().unwrap().connectors.clone()
     }
 
-    fn current_mode(&self) -> Option<Mode> {
+    fn current_mode(&self) -> Mode {
         self.state.read().unwrap().mode
     }
 
-    fn pending_mode(&self) -> Option<Mode> {
+    fn pending_mode(&self) -> Mode {
         self.pending.read().unwrap().mode
     }
 
     fn add_connector(&self, conn: connector::Handle) -> Result<(), Error> {
         let mut pending = self.pending.write().unwrap();
 
-        if self.check_connector(conn, pending.mode.as_ref().unwrap())? {
+        if self.check_connector(conn, &pending.mode)? {
             pending.connectors.insert(conn);
         }
 
@@ -124,7 +126,7 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurfaceInternal<A> {
 
         if connectors
             .iter()
-            .map(|conn| self.check_connector(*conn, pending.mode.as_ref().unwrap()))
+            .map(|conn| self.check_connector(*conn, &pending.mode))
             .collect::<Result<Vec<bool>, _>>()?
             .iter()
             .all(|v| *v)
@@ -135,25 +137,23 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurfaceInternal<A> {
         Ok(())
     }
 
-    fn use_mode(&self, mode: Option<Mode>) -> Result<(), Error> {
+    fn use_mode(&self, mode: Mode) -> Result<(), Error> {
         let mut pending = self.pending.write().unwrap();
 
         // check the connectors to see if this mode is supported
-        if let Some(mode) = mode {
+        for connector in &pending.connectors {
-            for connector in &pending.connectors {
+            if !self
-                if !self
+                .get_connector(*connector)
-                    .get_connector(*connector)
+                .compat()
-                    .compat()
+                .map_err(|source| Error::Access {
-                    .map_err(|source| Error::Access {
+                    errmsg: "Error loading connector info",
-                        errmsg: "Error loading connector info",
+                    dev: self.dev_path(),
-                        dev: self.dev_path(),
+                    source,
-                        source,
+                })?
-                    })?
+                .modes()
-                    .modes()
+                .contains(&mode)
-                    .contains(&mode)
+            {
-                {
+                return Err(Error::ModeNotSuitable(mode));
-                    return Err(Error::ModeNotSuitable(mode));
-                }
             }
         }
 
@@ -211,7 +211,7 @@ impl<A: AsRawFd + 'static> RawSurface for LegacyDrmSurfaceInternal<A> {
                 info!(
                     self.logger,
                     "Setting new mode: {:?}",
-                    pending.mode.as_ref().unwrap().name()
+                    pending.mode.name()
                 );
             }
         }
@@ -226,7 +226,7 @@ impl<A: AsRawFd + 'static> RawSurface for LegacyDrmSurfaceInternal<A> {
                 .iter()
                 .copied()
                 .collect::<Vec<connector::Handle>>(),
-            pending.mode,
+            Some(pending.mode),
         )
         .compat()
         .map_err(|source| Error::Access {
@@ -266,7 +266,7 @@ impl<A: AsRawFd + 'static> RawSurface for LegacyDrmSurfaceInternal<A> {
 }
 
 impl<A: AsRawFd + 'static> LegacyDrmSurfaceInternal<A> {
-    pub(crate) fn new(dev: Rc<Dev<A>>, crtc: crtc::Handle, logger: ::slog::Logger) -> Result<LegacyDrmSurfaceInternal<A>, Error> {
+    pub(crate) fn new(dev: Rc<Dev<A>>, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], logger: ::slog::Logger) -> Result<LegacyDrmSurfaceInternal<A>, Error> {
         // Try to enumarate the current state to set the initial state variable correctly
         let crtc_info = dev.get_crtc(crtc).compat().map_err(|source| Error::Access {
             errmsg: "Error loading crtc info",
@@ -304,14 +304,20 @@ impl<A: AsRawFd + 'static> LegacyDrmSurfaceInternal<A> {
             }
         }
 
-        let state = State { current_mode, current_connectors };
+        // If we have no current mode, we create a fake one, which will not match (and thus gets overriden on the commit below).
+        // 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 { mode: current_mode.unwrap_or_else(|| unsafe { std::mem::zeroed() }), connectors: current_connectors };
+        let pending = State { mode, connectors: connectors.into_iter().copied().collect() };
         
         let surface = LegacyDrmSurfaceInternal {
             dev,
             crtc,
             state: RwLock::new(state),
-            pending: RwLock::new(state.clone()),
+            pending: RwLock::new(pending),
             logger,
+            init_buffer: Cell::new(None),
         };
 
         Ok(surface)
@@ -371,6 +377,10 @@ impl<A: AsRawFd + 'static> Drop for LegacyDrmSurfaceInternal<A> {
     fn drop(&mut self) {
         // ignore failure at this point
         let _ = self.set_cursor(self.crtc, Option::<&DumbBuffer>::None);
+        if let Some((db, fb)) = self.init_buffer.take() {
+            let _ = self.destroy_framebuffer(fb);
+            let _ = self.destroy_dumb_buffer(db);
+        }
     }
 }
 
@@ -419,11 +429,11 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurface<A> {
         self.0.pending_connectors()
     }
 
-    fn current_mode(&self) -> Option<Mode> {
+    fn current_mode(&self) -> Mode {
         self.0.current_mode()
     }
 
-    fn pending_mode(&self) -> Option<Mode> {
+    fn pending_mode(&self) -> Mode {
         self.0.pending_mode()
     }
 
@@ -439,7 +449,7 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurface<A> {
         self.0.set_connectors(connectors)
     }
 
-    fn use_mode(&self, mode: Option<Mode>) -> Result<(), Error> {
+    fn use_mode(&self, mode: Mode) -> Result<(), Error> {
         self.0.use_mode(mode)
     }
 }

src/backend/drm/mod.rs

@@ -94,16 +94,23 @@ pub trait Device: AsRawFd + DevPath {
 
     /// Creates a new rendering surface.
     ///
+    /// # Arguments
+    ///
     /// Initialization of surfaces happens through the types provided by
     /// [`drm-rs`](drm).
     ///
-    /// [`crtc`](drm::control::crtc)s represent scanout engines
+    /// - [`crtc`](drm::control::crtc)s represent scanout engines of the device pointing to one framebuffer. \
-    /// of the device pointer to one framebuffer.
+    ///     Their responsibility is to read the data of the framebuffer and export it into an "Encoder". \
-    /// Their responsibility is to read the data of the framebuffer and export it into an "Encoder".
+    ///     The number of crtc's represent the number of independant output devices the hardware may handle.
-    /// The number of crtc's represent the number of independant output devices the hardware may handle.
+    /// - [`mode`](drm::control::Mode) describes the resolution and rate of images produced by the crtc and \
+    ///     has to be compatible with the provided `connectors`.
+    /// - [`connectors`] - List of connectors driven by the crtc. At least one(!) connector needs to be \
+    ///     attached to a crtc in smithay.
     fn create_surface(
         &mut self,
         crtc: crtc::Handle,
+        mode: Mode,
+        connectors: &[connector::Handle]
     ) -> Result<Self::Surface, <Self::Surface as Surface>::Error>;
 
     /// Processes any open events of the underlying file descriptor.
@@ -181,11 +188,10 @@ pub trait Surface {
     fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error>;
     /// Returns the currently active [`Mode`](drm::control::Mode)
     /// of the underlying [`crtc`](drm::control::crtc)
-    /// if any.
+    fn current_mode(&self) -> Mode;
-    fn current_mode(&self) -> Option<Mode>;
     /// Returns the currently pending [`Mode`](drm::control::Mode)
-    /// to be used after the next commit, if any.
+    /// to be used after the next commit.
-    fn pending_mode(&self) -> Option<Mode>;
+    fn pending_mode(&self) -> Mode;
     /// Tries to set a new [`Mode`](drm::control::Mode)
     /// to be used after the next commit.
     ///
@@ -196,7 +202,7 @@ pub trait Surface {
     /// *Note*: Only on a [`RawSurface`] you may directly trigger
     /// a [`commit`](RawSurface::commit). Other [`Surface`]s provide their
     /// own methods that *may* trigger a commit, you will need to read their docs.
-    fn use_mode(&self, mode: Option<Mode>) -> Result<(), Self::Error>;
+    fn use_mode(&self, mode: Mode) -> Result<(), Self::Error>;
 }
 
 /// An open bare crtc without any rendering abstractions