drm: Let swap_buffers return real error

src/backend/drm/atomic/surface.rs

@@ -15,7 +15,6 @@ use failure::ResultExt as FailureResultExt;
 use super::Dev;
 use crate::backend::drm::{common::Error, DevPath, RawSurface, Surface};
 use crate::backend::graphics::CursorBackend;
-use crate::backend::graphics::SwapBuffersError;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct CursorState {
@@ -508,9 +507,9 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
         Ok(())
     }
 
-    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> {
+    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> {
         if !self.dev.active.load(Ordering::SeqCst) {
-            return Err(SwapBuffersError::AlreadySwapped);
+            return Err(Error::DeviceInactive);
         }
 
         let req = self
@@ -521,14 +520,18 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
                 Some(framebuffer),
                 None,
                 None,
-            ) //current.mode)
+            )?;
-            .map_err(|_| SwapBuffersError::ContextLost)?;
+
         trace!(self.logger, "Queueing page flip: {:#?}", req);
         self.atomic_commit(
             &[AtomicCommitFlags::PageFlipEvent, AtomicCommitFlags::Nonblock],
             req,
         )
-        .map_err(|_| SwapBuffersError::ContextLost)?;
+        .compat().map_err(|source| Error::Access {
+            errmsg: "Page flip commit failed",
+            dev: self.dev_path(),
+            source
+        })?;
 
         Ok(())
     }
@@ -971,7 +974,7 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurface<A> {
         self.0.commit(framebuffer)
     }
 
-    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> {
+    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> {
         RawSurface::page_flip(&*self.0, framebuffer)
     }
 }

src/backend/drm/common/fallback.rs

@@ -311,7 +311,7 @@ where
 {
     fallback_surface_impl!(commit_pending, &Self, bool);
     fallback_surface_impl!(commit, &Self, Result<(), E>, fb: framebuffer::Handle);
-    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> {
+    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), E> {
         match self {
             FallbackSurface::Preference(dev) => RawSurface::page_flip(dev, framebuffer),
             FallbackSurface::Fallback(dev) => RawSurface::page_flip(dev, framebuffer),

src/backend/drm/gbm/egl.rs

@@ -89,6 +89,13 @@ unsafe impl<D: RawDevice + 'static> NativeSurface for GbmSurface<D> {
     fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> {
         // this is safe since `eglSwapBuffers` will have been called exactly once
         // if this is used by our egl module, which is why this trait is unsafe.
-        unsafe { self.page_flip() }
+        match unsafe { self.page_flip() } {
+            Ok(()) => Ok(()),
+            Err(Error::FrontBuffersExhausted) => Err(SwapBuffersError::AlreadySwapped),
+            Err(err) => {
+                warn!(self.0.logger, "Page flip failed: {}", err);
+                Err(SwapBuffersError::Unknown(0))
+            }
+        }
     }
 }

src/backend/drm/gbm/mod.rs

@@ -39,9 +39,18 @@ pub enum Error<U: std::error::Error + std::fmt::Debug + std::fmt::Display + 'sta
     /// Writing to GBM buffer failed
     #[error("Writing to GBM buffer failed")]
     BufferWriteFailed(#[source] io::Error),
+    /// Creation of drm framebuffer failed
+    #[error("Creation of drm framebuffer failed")]
+    FramebufferCreationFailed(#[source] failure::Compat<drm::SystemError>),
     /// Lock of GBM surface front buffer failed
     #[error("Lock of GBM surface font buffer failed")]
     FrontBufferLockFailed,
+    /// No additional buffers are available
+    #[error("No additional buffers are available. Did you swap twice?")]
+    FrontBuffersExhausted,
+    /// Internal state was modified
+    #[error("Internal state was modified. Did you change gbm userdata?")]
+    InvalidInternalState,
     /// The GBM device was destroyed
     #[error("The GBM device was destroyed")]
     DeviceDestroyed,

src/backend/drm/gbm/surface.rs

@@ -4,12 +4,12 @@ use super::Error;
 use drm::control::{connector, crtc, framebuffer, Device as ControlDevice, Mode};
 use gbm::{self, BufferObject, BufferObjectFlags, Format as GbmFormat, SurfaceBufferHandle};
 use image::{ImageBuffer, Rgba};
+use failure::ResultExt;
 
 use std::cell::{Cell, RefCell};
 use std::rc::Rc;
 
 use crate::backend::graphics::CursorBackend;
-use crate::backend::graphics::SwapBuffersError;
 
 pub(super) struct GbmSurfaceInternal<D: RawDevice + 'static> {
     pub(super) dev: Rc<RefCell<gbm::Device<D>>>,
@@ -32,7 +32,7 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
         // drop and release the old buffer
     }
 
-    pub unsafe fn page_flip(&self) -> ::std::result::Result<(), SwapBuffersError> {
+    pub unsafe fn page_flip(&self) -> Result<(), Error<<<D as Device>::Surface as Surface>::Error>> {
         let res = {
             let nb = self.next_buffer.take();
             let res = nb.is_some();
@@ -42,7 +42,7 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
         if res {
             // We cannot call lock_front_buffer anymore without releasing the previous buffer, which will happen when the page flip is done
             warn!(self.logger, "Tried to swap with an already queued flip");
-            return Err(SwapBuffersError::AlreadySwapped);
+            return Err(Error::FrontBuffersExhausted);
         }
 
         // supporting only one buffer would cause a lot of inconvinience and
@@ -53,13 +53,13 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
             .surface
             .borrow()
             .lock_front_buffer()
-            .expect("Surface only has one front buffer. Not supported by smithay");
+            .map_err(|_| Error::FrontBufferLockFailed)?;
 
         // create a framebuffer if the front buffer does not have one already
         // (they are reused by gbm)
         let maybe_fb = next_bo
             .userdata()
-            .map_err(|_| SwapBuffersError::ContextLost)?
+            .map_err(|_| Error::InvalidInternalState)?
             .cloned();
         let fb = if let Some(info) = maybe_fb {
             info
@@ -67,7 +67,8 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
             let fb = self
                 .crtc
                 .add_planar_framebuffer(&*next_bo, &[0; 4], 0)
-                .map_err(|_| SwapBuffersError::ContextLost)?;
+                .compat()
+                .map_err(Error::FramebufferCreationFailed)?;
             next_bo.set_userdata(fb).unwrap();
             fb
         };
@@ -75,14 +76,11 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
 
         if self.recreated.get() {
             debug!(self.logger, "Commiting new state");
-            if let Err(err) = self.crtc.commit(fb) {
+            self.crtc.commit(fb).map_err(Error::Underlying)?;
-                error!(self.logger, "Error commiting crtc: {}", err);
-                return Err(SwapBuffersError::ContextLost);
-            }
             self.recreated.set(false);
         } else {
             trace!(self.logger, "Queueing Page flip");
-            RawSurface::page_flip(&self.crtc, fb)?;
+            RawSurface::page_flip(&self.crtc, fb).map_err(Error::Underlying)?;
         }
 
         self.current_frame_buffer.set(Some(fb));
@@ -275,7 +273,7 @@ impl<D: RawDevice + 'static> GbmSurface<D> {
     ///
     /// When used in conjunction with an EGL context, this must be called exactly once
     /// after page-flipping the associated context.
-    pub unsafe fn page_flip(&self) -> ::std::result::Result<(), SwapBuffersError> {
+    pub unsafe fn page_flip(&self) -> ::std::result::Result<(), <Self as Surface>::Error> {
         self.0.page_flip()
     }
 

src/backend/drm/legacy/surface.rs

@@ -12,7 +12,6 @@ use std::sync::{RwLock, atomic::Ordering};
 
 use crate::backend::drm::{common::Error, DevPath, RawSurface, Surface};
 use crate::backend::graphics::CursorBackend;
-use crate::backend::graphics::SwapBuffersError;
 
 use super::Dev;
 
@@ -285,11 +284,11 @@ impl<A: AsRawFd + 'static> RawSurface for LegacyDrmSurfaceInternal<A> {
         })
     }
 
-    fn page_flip(&self, framebuffer: framebuffer::Handle) -> ::std::result::Result<(), SwapBuffersError> {
+    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> {
         trace!(self.logger, "Queueing Page flip");
         
         if !self.dev.active.load(Ordering::SeqCst) {
-            return Err(SwapBuffersError::AlreadySwapped);
+            return Err(Error::DeviceInactive);
         }
 
         ControlDevice::page_flip(
@@ -298,8 +297,12 @@ impl<A: AsRawFd + 'static> RawSurface for LegacyDrmSurfaceInternal<A> {
             framebuffer,
             &[PageFlipFlags::PageFlipEvent],
             None,
-        )
+        ).compat()
-        .map_err(|_| SwapBuffersError::ContextLost)
+        .map_err(|source| Error::Access {
+            errmsg: "Failed to page flip",
+            dev: self.dev_path(),
+            source,
+        })
     }
 }
 
@@ -511,7 +514,7 @@ impl<A: AsRawFd + 'static> RawSurface for LegacyDrmSurface<A> {
         self.0.commit(framebuffer)
     }
 
-    fn page_flip(&self, framebuffer: framebuffer::Handle) -> ::std::result::Result<(), SwapBuffersError> {
+    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> {
         RawSurface::page_flip(&*self.0, framebuffer)
     }
 }

src/backend/drm/mod.rs

@@ -51,8 +51,6 @@ use calloop::mio::Interest;
 use calloop::InsertError;
 use calloop::{LoopHandle, Source};
 
-use super::graphics::SwapBuffersError;
-
 #[cfg(feature = "backend_drm_atomic")]
 pub mod atomic;
 #[cfg(feature = "backend_drm")]
@@ -233,7 +231,7 @@ pub trait RawSurface: Surface + ControlDevice + BasicDevice {
     /// This operation is not blocking and will produce a `vblank` event once swapping is done.
     /// Make sure to [set a `DeviceHandler`](Device::set_handler) and
     /// [register the belonging `Device`](device_bind) before to receive the event in time.
-    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError>;
+    fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), <Self as Surface>::Error>;
 }
 
 /// Trait representing open devices that *may* return a `Path`