From 524057418e6130af574a39ea0b96222cb4ee5121 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sat, 15 May 2021 22:28:39 +0200 Subject: [PATCH] drm: Fixup tty switching --- anvil/src/udev.rs | 7 ++- src/backend/drm/device/mod.rs | 12 +--- src/backend/drm/session.rs | 83 +++++++++++++++++++++----- src/backend/drm/surface/atomic.rs | 99 ++++++++++++++++++------------- src/backend/drm/surface/legacy.rs | 67 +++++++++++++-------- src/backend/drm/surface/mod.rs | 18 ++++++ 6 files changed, 193 insertions(+), 93 deletions(-) diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index cdad566..a4353a7 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -303,6 +303,7 @@ impl UdevHandlerImpl { context: &EGLContext, display: &mut Display, output_map: &mut Vec, + signaler: &Signaler, logger: &::slog::Logger, ) -> HashMap>> { // Get a set of all modesetting resource handles (excluding planes): @@ -360,7 +361,7 @@ impl UdevHandlerImpl { continue; } }; - let surface = match device.create_surface( + let mut surface = match device.create_surface( crtc, primary, connector_info.modes()[0], @@ -372,6 +373,7 @@ impl UdevHandlerImpl { continue; } }; + surface.link(signaler.clone()); let renderer = match DrmRenderSurface::new(surface, gbm.clone(), renderer, logger.clone()) { Ok(renderer) => renderer, @@ -480,6 +482,7 @@ impl UdevHandlerImpl { &context, &mut *self.display.borrow_mut(), &mut *self.output_map.borrow_mut(), + &self.signaler, &self.logger, ))); @@ -562,6 +565,7 @@ impl UdevHandlerImpl { let loop_handle = self.loop_handle.clone(); let mut display = self.display.borrow_mut(); let mut output_map = self.output_map.borrow_mut(); + let signaler = self.signaler.clone(); output_map.retain(|output| output.device_id != device); self.loop_handle .with_source(&backend_data.event_source, |source| { @@ -573,6 +577,7 @@ impl UdevHandlerImpl { &backend_data.context, &mut *display, &mut *output_map, + &signaler, &logger, ); diff --git a/src/backend/drm/device/mod.rs b/src/backend/drm/device/mod.rs index 5e315ce..7da635a 100644 --- a/src/backend/drm/device/mod.rs +++ b/src/backend/drm/device/mod.rs @@ -509,10 +509,13 @@ impl DrmDevice { ); Ok(DrmSurface { + dev_id: self.dev_id, crtc, plane, internal: Arc::new(internal), formats, + #[cfg(feature = "backend_session")] + links: RefCell::new(Vec::new()), }) } @@ -532,15 +535,6 @@ pub struct Planes { pub overlay: Option>, } -impl DrmDeviceInternal { - pub(super) fn reset_state(&self) -> Result<(), Error> { - match self { - DrmDeviceInternal::Atomic(dev) => dev.reset_state(), - DrmDeviceInternal::Legacy(dev) => dev.reset_state(), - } - } -} - /// Trait to receive events of a bound [`DrmDevice`] /// /// See [`device_bind`] diff --git a/src/backend/drm/session.rs b/src/backend/drm/session.rs index 6529cf1..fd6effa 100644 --- a/src/backend/drm/session.rs +++ b/src/backend/drm/session.rs @@ -4,12 +4,12 @@ use std::sync::{ Arc, Weak, }; -use drm::Device as BasicDevice; +use drm::{Device as BasicDevice, control::{crtc, Device as ControlDevice}}; use nix::libc::dev_t; use nix::sys::stat; use super::device::{DrmDevice, DrmDeviceInternal}; -use super::error::Error; +use super::surface::{DrmSurface, DrmSurfaceInternal}; use crate::{ backend::session::Signal as SessionSignal, signaling::{Linkable, Signaler}, @@ -96,21 +96,74 @@ impl DrmDeviceObserver { } } - // okay, the previous session/whatever might left the drm devices in any state... - // lets fix that - if let Err(err) = self.reset_state() { - warn!(self.logger, "Unable to reset state after tty switch: {}", err); - // TODO call drm-handler::error - } - self.active.store(true, Ordering::SeqCst); } +} - fn reset_state(&mut self) -> Result<(), Error> { - if let Some(dev) = self.dev.upgrade() { - dev.reset_state() - } else { - Ok(()) - } +pub struct DrmSurfaceObserver { + dev_id: dev_t, + crtc: crtc::Handle, + surf: Weak>, + logger: ::slog::Logger, +} + +struct FdHack(RawFd); +impl AsRawFd for FdHack { + fn as_raw_fd(&self) -> RawFd { + self.0 } } +impl BasicDevice for FdHack {} +impl ControlDevice for FdHack {} + +impl Linkable for DrmSurface { + fn link(&mut self, signaler: Signaler) { + let logger = match &*self.internal { + DrmSurfaceInternal::Atomic(surf) => surf.logger.clone(), + DrmSurfaceInternal::Legacy(surf) => surf.logger.clone(), + }; + let mut observer = DrmSurfaceObserver { + dev_id: self.dev_id, + crtc: self.crtc(), + surf: Arc::downgrade(&self.internal), + logger: logger.new(o!("drm_module" => "observer")), + }; + + let token = signaler.register(move |signal| observer.signal(*signal)); + self.links.borrow_mut().push(token); + } +} + +impl DrmSurfaceObserver { + fn signal(&mut self, signal: SessionSignal) { + match signal { + SessionSignal::ActivateSession => self.activate(None), + SessionSignal::ActivateDevice { major, minor, new_fd } => { + self.activate(Some((major, minor, new_fd))) + }, + _ => {}, + } + } + + fn activate(&mut self, devnum: Option<(u32, u32, Option)>) { + if let Some(surf) = self.surf.upgrade() { + // The device will reset the _fd, but the observer order is not deterministic, + // so we might need to use it anyway. + let fd = if let Some((major, minor, fd)) = devnum { + if major as u64 != stat::major(self.dev_id) || minor as u64 != stat::minor(self.dev_id) { + return; + } + fd.map(FdHack) + } else { + None + }; + + if let Err(err) = match &*surf { + DrmSurfaceInternal::Atomic(surf) => surf.reset_state(fd.as_ref()), + DrmSurfaceInternal::Legacy(surf) => surf.reset_state(fd.as_ref()), + } { + warn!(self.logger, "Failed to reset state of surface ({:?}/{:?}): {}", self.dev_id, self.crtc, err); + } + } + } +} \ No newline at end of file diff --git a/src/backend/drm/surface/atomic.rs b/src/backend/drm/surface/atomic.rs index 1ba4f57..4054a33 100644 --- a/src/backend/drm/surface/atomic.rs +++ b/src/backend/drm/surface/atomic.rs @@ -24,40 +24,8 @@ pub struct State { pub connectors: HashSet, } -pub struct AtomicDrmSurface { - pub(super) fd: Arc>, - pub(super) active: Arc, - crtc: crtc::Handle, - plane: plane::Handle, - prop_mapping: Mapping, - state: RwLock, - pending: RwLock, - test_buffer: Mutex>, - pub(crate) logger: ::slog::Logger, -} - -impl AtomicDrmSurface { - #[allow(clippy::too_many_arguments)] - pub fn new( - fd: Arc>, - active: Arc, - crtc: crtc::Handle, - plane: plane::Handle, - prop_mapping: Mapping, - mode: Mode, - connectors: &[connector::Handle], - logger: ::slog::Logger, - ) -> Result { - let logger = logger.new(o!("smithay_module" => "backend_drm_atomic", "drm_module" => "surface")); - info!( - logger, - "Initializing drm surface ({:?}:{:?}) with mode {:?} and connectors {:?}", - crtc, - plane, - mode, - connectors - ); - +impl State { + fn current_state(fd: &A, crtc: crtc::Handle, prop_mapping: &Mapping) -> Result { let crtc_info = fd.get_crtc(crtc).map_err(|source| Error::Access { errmsg: "Error loading crtc info", dev: fd.dev_path(), @@ -78,12 +46,6 @@ impl AtomicDrmSurface { None => property::Value::Unknown(0), }; - let blob = fd.create_property_blob(&mode).map_err(|source| Error::Access { - errmsg: "Failed to create Property Blob for mode", - dev: fd.dev_path(), - source, - })?; - let res_handles = fd.resource_handles().map_err(|source| Error::Access { errmsg: "Error loading drm resources", dev: fd.dev_path(), @@ -122,11 +84,54 @@ impl AtomicDrmSurface { } } } - let state = State { + Ok(State { mode: current_mode, blob: current_blob, connectors: current_connectors, - }; + }) + } +} + +pub struct AtomicDrmSurface { + pub(super) fd: Arc>, + pub(super) active: Arc, + crtc: crtc::Handle, + plane: plane::Handle, + prop_mapping: Mapping, + state: RwLock, + pending: RwLock, + test_buffer: Mutex>, + pub(crate) logger: ::slog::Logger, +} + +impl AtomicDrmSurface { + #[allow(clippy::too_many_arguments)] + pub fn new( + fd: Arc>, + active: Arc, + crtc: crtc::Handle, + plane: plane::Handle, + prop_mapping: Mapping, + mode: Mode, + connectors: &[connector::Handle], + logger: ::slog::Logger, + ) -> Result { + let logger = logger.new(o!("smithay_module" => "backend_drm_atomic", "drm_module" => "surface")); + info!( + logger, + "Initializing drm surface ({:?}:{:?}) with mode {:?} and connectors {:?}", + crtc, + plane, + mode, + connectors + ); + + let state = State::current_state(&*fd, crtc, &prop_mapping)?; + let blob = fd.create_property_blob(&mode).map_err(|source| Error::Access { + errmsg: "Failed to create Property Blob for mode", + dev: fd.dev_path(), + source, + })?; let pending = State { mode, blob, @@ -761,6 +766,16 @@ impl AtomicDrmSurface { source, }) } + + + pub(crate) fn reset_state(&self, fd: Option<&B>) -> Result<(), Error> { + *self.state.write().unwrap() = if let Some(fd) = fd { + State::current_state(fd, self.crtc, &self.prop_mapping)? + } else { + State::current_state(&*self.fd, self.crtc, &self.prop_mapping)? + }; + Ok(()) + } } impl Drop for AtomicDrmSurface { diff --git a/src/backend/drm/surface/legacy.rs b/src/backend/drm/surface/legacy.rs index 11ed92b..0afe54e 100644 --- a/src/backend/drm/surface/legacy.rs +++ b/src/backend/drm/surface/legacy.rs @@ -19,30 +19,8 @@ pub struct State { pub connectors: HashSet, } -pub struct LegacyDrmSurface { - pub(super) fd: Arc>, - pub(super) active: Arc, - crtc: crtc::Handle, - state: RwLock, - pending: RwLock, - pub(crate) logger: ::slog::Logger, -} - -impl LegacyDrmSurface { - pub fn new( - fd: Arc>, - active: Arc, - crtc: crtc::Handle, - mode: Mode, - connectors: &[connector::Handle], - logger: ::slog::Logger, - ) -> Result { - let logger = logger.new(o!("smithay_module" => "backend_drm_legacy", "drm_module" => "surface")); - info!( - logger, - "Initializing drm surface with mode {:?} and connectors {:?}", mode, connectors - ); - +impl State { + fn current_state(fd: &A, crtc: crtc::Handle) -> Result { // Try to enumarate the current state to set the initial state variable correctly. // We need an accurate state to handle `commit_pending`. let crtc_info = fd.get_crtc(crtc).map_err(|source| Error::Access { @@ -83,10 +61,38 @@ impl LegacyDrmSurface { // A better fix would probably be making mode an `Option`, but that would mean // we need to be sure, we require a mode to always be set without relying on the compiler. // So we cheat, because it works and is easier to handle later. - let state = State { + Ok(State { mode: current_mode.unwrap_or_else(|| unsafe { std::mem::zeroed() }), connectors: current_connectors, - }; + }) + } +} + +pub struct LegacyDrmSurface { + pub(super) fd: Arc>, + pub(super) active: Arc, + crtc: crtc::Handle, + state: RwLock, + pending: RwLock, + pub(crate) logger: ::slog::Logger, +} + +impl LegacyDrmSurface { + pub fn new( + fd: Arc>, + active: Arc, + crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], + logger: ::slog::Logger, + ) -> Result { + let logger = logger.new(o!("smithay_module" => "backend_drm_legacy", "drm_module" => "surface")); + info!( + logger, + "Initializing drm surface with mode {:?} and connectors {:?}", mode, connectors + ); + + let state = State::current_state(&*fd, crtc)?; let pending = State { mode, connectors: connectors.iter().copied().collect(), @@ -396,6 +402,15 @@ impl LegacyDrmSurface { Ok(false) } } + + pub(crate) fn reset_state(&self, fd: Option<&B>) -> Result<(), Error> { + *self.state.write().unwrap() = if let Some(fd) = fd { + State::current_state(fd, self.crtc)? + } else { + State::current_state(&*self.fd, self.crtc)? + }; + Ok(()) + } } impl Drop for LegacyDrmSurface { diff --git a/src/backend/drm/surface/mod.rs b/src/backend/drm/surface/mod.rs index 06359a4..6cc6bd9 100644 --- a/src/backend/drm/surface/mod.rs +++ b/src/backend/drm/surface/mod.rs @@ -1,3 +1,4 @@ +use std::cell::RefCell; use std::collections::HashSet; use std::os::unix::io::{AsRawFd, RawFd}; use std::sync::Arc; @@ -5,6 +6,8 @@ use std::sync::Arc; use drm::control::{connector, crtc, framebuffer, plane, Device as ControlDevice, Mode}; use drm::Device as BasicDevice; +use nix::libc::dev_t; + pub(super) mod atomic; pub(super) mod legacy; use super::error::Error; @@ -14,10 +17,13 @@ use legacy::LegacyDrmSurface; /// An open crtc + plane combination that can be used for scan-out pub struct DrmSurface { + pub(super) dev_id: dev_t, pub(super) crtc: crtc::Handle, pub(super) plane: plane::Handle, pub(super) internal: Arc>, pub(super) formats: HashSet, + #[cfg(feature = "backend_session")] + pub(super) links: RefCell>, } pub enum DrmSurfaceInternal { @@ -211,4 +217,16 @@ impl DrmSurface { } // There is no test-commiting with the legacy interface } } + + /// Re-evaluates the current state of the crtc. + /// + /// Usually you do not need to call this, but if the state of + /// the crtc is modified elsewhere and you need to reset the + /// initial state of this surface, you may call this function. + pub fn reset_state(&self) -> Result<(), Error> { + match &*self.internal { + DrmSurfaceInternal::Atomic(surf) => surf.reset_state::(None), + DrmSurfaceInternal::Legacy(surf) => surf.reset_state::(None), + } + } }