diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index cc744a2..5af0e8a 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -319,7 +319,7 @@ impl UdevHandlerImpl { 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(), ); diff --git a/examples/raw_atomic_drm.rs b/examples/raw_atomic_drm.rs index d3ec01c..47c2f39 100644 --- a/examples/raw_atomic_drm.rs +++ b/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()); - surface.set_connectors(&[connector_info.handle()]).unwrap(); - surface.use_mode(Some(mode)).unwrap(); + let surface = Rc::new(device.create_surface(crtc, mode, &[connector_info.handle()]).unwrap()); for conn in surface.current_connectors().into_iter() { if conn != connector_info.handle() { diff --git a/examples/raw_legacy_drm.rs b/examples/raw_legacy_drm.rs index 7c3c546..f5dc06e 100644 --- a/examples/raw_legacy_drm.rs +++ b/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. diff --git a/src/backend/drm/atomic/mod.rs b/src/backend/drm/atomic/mod.rs index b1fe607..efab65c 100644 --- a/src/backend/drm/atomic/mod.rs +++ b/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 Device for AtomicDrmDevice { let _ = self.handler.take(); } - fn create_surface(&mut self, crtc: crtc::Handle) -> Result, Error> { + fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle]) -> Result, Error> { if self.backends.borrow().contains_key(&crtc) { return Err(Error::CrtcAlreadyInUse(crtc)); } @@ -348,9 +348,15 @@ impl Device for AtomicDrmDevice { 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))), )?); diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index 57c6246..cc01e0f 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/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, - pub blob: Option>, + pub mode: Mode, + pub blob: property::Value<'static>, pub connectors: HashSet, } @@ -45,6 +45,7 @@ pub(super) struct AtomicDrmSurfaceInternal { pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, + init_buffer: Cell>, } impl AsRawFd for AtomicDrmSurfaceInternal { @@ -57,26 +58,36 @@ impl BasicDevice for AtomicDrmSurfaceInternal {} impl ControlDevice for AtomicDrmSurfaceInternal {} impl AtomicDrmSurfaceInternal { - pub(crate) fn new(dev: Rc>, crtc: crtc::Handle, logger: ::slog::Logger) -> Result { + pub(crate) fn new(dev: Rc>, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], logger: ::slog::Logger) -> Result { 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(); - let blob = match mode { - Some(mode) => Some( - dev.create_property_blob(mode) - .compat() - .map_err(|source| Error::Access { - errmsg: "Failed to create Property Blob for mode", - dev: dev.dev_path(), - source, - })?, - ), - None => None, + + // 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 current_mode = crtc_info.mode().unwrap_or_else(|| unsafe { std::mem::zeroed() }); + let current_blob = match crtc_info.mode() { + Some(mode) => dev.create_property_blob(mode) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to create Property Blob for mode", + dev: dev.dev_path(), + 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 AtomicDrmSurfaceInternal { 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 AtomicDrmSurfaceInternal { 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 AtomicDrmSurfaceInternal { } } } + 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 AtomicDrmSurfaceInternal { hotspot: Cell::new((0, 0)), }, planes: Planes { primary, cursor }, - state: RwLock::new(state.clone()), - pending: RwLock::new(state), + state: RwLock::new(state), + pending: RwLock::new(pending), logger, - }) + init_buffer: Cell::new(None), + }; + + Ok(surface) + } +} + +impl Drop for AtomicDrmSurfaceInternal { + 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 Surface for AtomicDrmSurfaceInternal { self.pending.read().unwrap().connectors.clone() } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.state.read().unwrap().mode } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.pending.read().unwrap().mode } @@ -180,15 +209,15 @@ impl Surface for AtomicDrmSurfaceInternal { 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, - pending.blob, + Some(pending.mode), + Some(pending.blob), )?; self.atomic_commit( &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly], @@ -202,7 +231,7 @@ impl Surface for AtomicDrmSurfaceInternal { Ok(()) } else { - Err(Error::ModeNotSuitable(pending.mode.unwrap())) + Err(Error::ModeNotSuitable(pending.mode)) } } @@ -215,8 +244,8 @@ impl Surface for AtomicDrmSurfaceInternal { &mut [conn].iter(), &self.planes, None, - pending.mode, - pending.blob, + Some(pending.mode), + Some(pending.blob), )?; self.atomic_commit( &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly], @@ -244,8 +273,8 @@ impl Surface for AtomicDrmSurfaceInternal { &mut removed, &self.planes, None, - pending.mode, - pending.blob, + Some(pending.mode), + Some(pending.blob), )?; self.atomic_commit( @@ -259,30 +288,26 @@ impl Surface for AtomicDrmSurfaceInternal { Ok(()) } - fn use_mode(&self, mode: Option) -> 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 { - Some(mode) => self - .dev + let new_blob = self .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, - new_blob, + Some(mode), + Some(new_blob), )?; if let Err(err) = self .atomic_commit( @@ -292,7 +317,7 @@ impl Surface for AtomicDrmSurfaceInternal { .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 RawSurface for AtomicDrmSurfaceInternal { info!( self.logger, "Setting new mode: {:?}", - pending.mode.as_ref().unwrap().name() + pending.mode.name() ); } @@ -355,8 +380,8 @@ impl RawSurface for AtomicDrmSurfaceInternal { &mut removed, &self.planes, Some(framebuffer), - pending.mode, - pending.blob, + Some(pending.mode), + Some(pending.blob), )?; if let Err(err) = self @@ -380,15 +405,13 @@ impl RawSurface for AtomicDrmSurfaceInternal { &mut [].iter(), &self.planes, Some(framebuffer), - current.mode, - current.blob, + Some(current.mode), + Some(current.blob), )? } else { if current.mode != pending.mode { - if let Some(blob) = current.blob { - if let Err(err) = self.dev.destroy_property_blob(blob.into()) { - warn!(self.logger, "Failed to destory old mode property blob: {}", err); - } + if let Err(err) = self.dev.destroy_property_blob(current.blob.into()) { + warn!(self.logger, "Failed to destory old mode property blob: {}", err); } } *current = pending.clone(); @@ -834,11 +857,11 @@ impl Surface for AtomicDrmSurface { self.0.pending_connectors() } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.0.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.0.pending_mode() } @@ -854,7 +877,7 @@ impl Surface for AtomicDrmSurface { self.0.set_connectors(connectors) } - fn use_mode(&self, mode: Option) -> Result<(), Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Error> { self.0.use_mode(mode) } } diff --git a/src/backend/drm/common/fallback.rs b/src/backend/drm/common/fallback.rs index eebb38b..3051351 100644 --- a/src/backend/drm/common/fallback.rs +++ b/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 { + fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle]) -> Result { match self { - FallbackDevice::Preference(dev) => Ok(FallbackSurface::Preference(dev.create_surface(crtc)?)), - FallbackDevice::Fallback(dev) => Ok(FallbackSurface::Fallback(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, 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); - fallback_surface_impl!(pending_mode, &Self, Option); - fallback_surface_impl!(use_mode, &Self, Result<(), E>, mode: Option); + fallback_surface_impl!(current_mode, &Self, Mode); + fallback_surface_impl!(pending_mode, &Self, Mode); + fallback_surface_impl!(use_mode, &Self, Result<(), E>, mode: Mode); } impl RawSurface for FallbackSurface diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index 83ef7c2..573d8d6 100644 --- a/src/backend/drm/common/mod.rs +++ b/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 { diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs index 3779cf5..b0927e4 100644 --- a/src/backend/drm/egl/mod.rs +++ b/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); + /// Representation of an egl device to create egl rendering surfaces pub struct EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { dev: EGLDisplay, @@ -56,7 +58,7 @@ where impl AsRawFd for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { fn as_raw_fd(&self) -> RawFd { @@ -67,7 +69,7 @@ where impl EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { /// Try to create a new [`EglDevice`] from an open device. @@ -122,7 +124,7 @@ where struct InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { handler: Box> + 'static>, @@ -131,7 +133,7 @@ where impl DeviceHandler for InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { type Device = D; @@ -147,7 +149,7 @@ where impl Device for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { type Surface = EglSurface<::Surface>; @@ -169,6 +171,8 @@ where fn create_surface( &mut self, crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], ) -> Result::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 EGLGraphicsBackend for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { fn bind_wl_display(&self, display: &Display) -> Result { @@ -232,7 +236,7 @@ where impl Drop for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { fn drop(&mut self) { diff --git a/src/backend/drm/egl/session.rs b/src/backend/drm/egl/session.rs index ae404d0..68006e7 100644 --- a/src/backend/drm/egl/session.rs +++ b/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 AsSessionObserver> for EglDevice where S: SessionObserver + 'static, B: Backend::Surface> + 'static, - D: Device + NativeDisplay + AsSessionObserver + 'static, + D: Device + NativeDisplay)> + AsSessionObserver + 'static, ::Surface: NativeSurface, { fn observer(&mut self) -> EglDeviceObserver { diff --git a/src/backend/drm/egl/surface.rs b/src/backend/drm/egl/surface.rs index e9107a0..0cacac8 100644 --- a/src/backend/drm/egl/surface.rs +++ b/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 { + fn current_mode(&self) -> Mode { self.surface.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.surface.pending_mode() } - fn use_mode(&self, mode: Option) -> 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) } diff --git a/src/backend/drm/gbm/egl.rs b/src/backend/drm/gbm/egl.rs index 201d653..719642f 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/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 Backend for Gbm { } unsafe impl NativeDisplay> for GbmDevice { - type Arguments = crtc::Handle; + type Arguments = (crtc::Handle, Mode, Vec); type Error = Error<<::Surface as Surface>::Error>; fn is_backend(&self) -> bool { @@ -63,8 +63,8 @@ unsafe impl NativeDisplay> for Gb Ok(self.dev.borrow().as_raw() as *const _) } - fn create_surface(&mut self, crtc: crtc::Handle) -> Result, Self::Error> { - Device::create_surface(self, crtc) + fn create_surface(&mut self, args: Self::Arguments) -> Result, Self::Error> { + Device::create_surface(self, args.0, args.1, &args.2) } } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index a292792..91861a9 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/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 Device for GbmDevice { fn create_surface( &mut self, crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], ) -> Result, Error<<::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()) - .unwrap_or((1, 1)); + .size(); let surface = self .dev .borrow() diff --git a/src/backend/drm/gbm/surface.rs b/src/backend/drm/gbm/surface.rs index 30cbbb2..bf5ef9b 100644 --- a/src/backend/drm/gbm/surface.rs +++ b/src/backend/drm/gbm/surface.rs @@ -93,8 +93,6 @@ impl GbmSurfaceInternal { pub fn recreate(&self) -> Result<(), Error<<::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 Surface for GbmSurfaceInternal { self.crtc.set_connectors(connectors).map_err(Error::Underlying) } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.crtc.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.crtc.pending_mode() } - fn use_mode(&self, mode: Option) -> 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 Surface for GbmSurface { self.0.set_connectors(connectors) } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.0.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.0.pending_mode() } - fn use_mode(&self, mode: Option) -> Result<(), Self::Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Self::Error> { self.0.use_mode(mode) } } diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index d1733aa..75b293a 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/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 Device for LegacyDrmDevice { let _ = self.handler.take(); } - fn create_surface(&mut self, crtc: crtc::Handle) -> Result, Error> { + fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle]) -> Result, Error> { if self.backends.borrow().contains_key(&crtc) { return Err(Error::CrtcAlreadyInUse(crtc)); } @@ -250,8 +250,12 @@ impl Device for LegacyDrmDevice { 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))), )?); diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 83ea1f6..adfaf28 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/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, + pub mode: Mode, pub connectors: HashSet, } @@ -30,6 +31,7 @@ pub(super) struct LegacyDrmSurfaceInternal { pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, + init_buffer: Cell>, } impl AsRawFd for LegacyDrmSurfaceInternal { @@ -96,18 +98,18 @@ impl Surface for LegacyDrmSurfaceInternal { self.pending.read().unwrap().connectors.clone() } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.state.read().unwrap().mode } - fn pending_mode(&self) -> Option { + 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 Surface for LegacyDrmSurfaceInternal { if connectors .iter() - .map(|conn| self.check_connector(*conn, pending.mode.as_ref().unwrap())) + .map(|conn| self.check_connector(*conn, &pending.mode)) .collect::, _>>()? .iter() .all(|v| *v) @@ -135,25 +137,23 @@ impl Surface for LegacyDrmSurfaceInternal { Ok(()) } - fn use_mode(&self, mode: Option) -> 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 { - if !self - .get_connector(*connector) - .compat() - .map_err(|source| Error::Access { - errmsg: "Error loading connector info", - dev: self.dev_path(), - source, - })? - .modes() - .contains(&mode) - { - return Err(Error::ModeNotSuitable(mode)); - } + for connector in &pending.connectors { + if !self + .get_connector(*connector) + .compat() + .map_err(|source| Error::Access { + errmsg: "Error loading connector info", + dev: self.dev_path(), + source, + })? + .modes() + .contains(&mode) + { + return Err(Error::ModeNotSuitable(mode)); } } @@ -211,7 +211,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { info!( self.logger, "Setting new mode: {:?}", - pending.mode.as_ref().unwrap().name() + pending.mode.name() ); } } @@ -226,7 +226,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { .iter() .copied() .collect::>(), - pending.mode, + Some(pending.mode), ) .compat() .map_err(|source| Error::Access { @@ -266,7 +266,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { } impl LegacyDrmSurfaceInternal { - pub(crate) fn new(dev: Rc>, crtc: crtc::Handle, logger: ::slog::Logger) -> Result, Error> { + pub(crate) fn new(dev: Rc>, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], logger: ::slog::Logger) -> Result, 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 LegacyDrmSurfaceInternal { } } - 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 Drop for LegacyDrmSurfaceInternal { 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 Surface for LegacyDrmSurface { self.0.pending_connectors() } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.0.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.0.pending_mode() } @@ -439,7 +449,7 @@ impl Surface for LegacyDrmSurface { self.0.set_connectors(connectors) } - fn use_mode(&self, mode: Option) -> Result<(), Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Error> { self.0.use_mode(mode) } } diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index 1e05df0..534ab9d 100644 --- a/src/backend/drm/mod.rs +++ b/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 - /// of the device pointer to one framebuffer. - /// 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. + /// - [`crtc`](drm::control::crtc)s represent scanout engines of the device pointing to one framebuffer. \ + /// 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. + /// - [`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::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) -> Option; + fn current_mode(&self) -> Mode; /// Returns the currently pending [`Mode`](drm::control::Mode) - /// to be used after the next commit, if any. - fn pending_mode(&self) -> Option; + /// to be used after the next commit. + 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) -> Result<(), Self::Error>; + fn use_mode(&self, mode: Mode) -> Result<(), Self::Error>; } /// An open bare crtc without any rendering abstractions