From 7e8f6b2955afb585fc4d484460a211a5b70e812c Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Fri, 5 Jun 2020 20:31:14 +0200 Subject: [PATCH] docs: Add more explanations to the legacy-drm-code --- src/backend/drm/legacy/mod.rs | 39 +++++++++++++++++++++++++++---- src/backend/drm/legacy/session.rs | 7 ++++++ src/backend/drm/legacy/surface.rs | 21 +++++++++++++++-- 3 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 1897222..0b9c0c3 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -2,11 +2,20 @@ //! [`RawDevice`](RawDevice) and [`RawSurface`](RawSurface) //! implementations using the legacy mode-setting infrastructure. //! +//! Legacy mode-setting refers to the now outdated, but still supported direct manager api +//! of the linux kernel. Adaptations of this api can be found in BSD kernels. +//! +//! The newer and objectively better api is known as atomic-modesetting (or nuclear-page-flip), +//! however this api is not supported by every driver, so this is provided for backwards compatibility. +//! Currenly there are no features in smithay, that are exclusive to the atomic api. +//! //! Usually this implementation will be wrapped into a [`GbmDevice`](::backend::drm::gbm::GbmDevice). //! Take a look at `anvil`s source code for an example of this. //! //! For an example how to use this standalone, take a look at the `raw_legacy_drm` example. //! +//! For detailed overview of these abstractions take a look at the module documentation of backend::drm. +//! use super::{common::Error, DevPath, Device, DeviceHandler, RawDevice}; @@ -66,6 +75,8 @@ impl Drop for Dev { // Here we restore the tty to it's previous state. // In case e.g. getty was running on the tty sets the correct framebuffer again, // so that getty will be visible. + // We do exit correctly, if this fails, but the user will be presented with + // a black screen, if no display handler takes control again. let old_state = self.old_state.clone(); for (handle, (info, connectors)) in old_state { if let Err(err) = self.set_crtc( @@ -88,7 +99,7 @@ impl Drop for Dev { } impl LegacyDrmDevice { - /// Create a new [`LegacyDrmDevice`] from an open drm node + /// Create a new [`LegacyDrmDevice`] from an open drm node. /// /// # Arguments /// @@ -115,6 +126,8 @@ impl LegacyDrmDevice { .map_err(Error::UnableToGetDeviceId)? .st_rdev; + // we wrap some of the internal state in another struct to share with + // the surfaces and event loop handlers. let active = Arc::new(AtomicBool::new(true)); let mut dev = Dev { fd: dev, @@ -124,13 +137,17 @@ impl LegacyDrmDevice { logger: log.clone(), }; - // we want to modeset, so we better be the master, if we run via a tty session + // We want to modeset, so we better be the master, if we run via a tty session. + // This is only needed on older kernels. Newer kernels grant this permission, + // if no other process is already the *master*. So we skip over this error. if dev.acquire_master_lock().is_err() { warn!(log, "Unable to become drm master, assuming unprivileged mode"); dev.privileged = false; }; - // enumerate (and save) the current device state + // Enumerate (and save) the current device state. + // We need to keep the previous device configuration to restore the state later, + // so we query everything, that we can set. let res_handles = ControlDevice::resource_handles(&dev) .compat() .map_err(|source| Error::Access { @@ -165,11 +182,18 @@ impl LegacyDrmDevice { } } + // If the user does not explicitly requests us to skip this, + // we clear out the complete connector<->crtc mapping on device creation. + // + // The reason is, that certain operations may be racy otherwise, as surfaces can + // exist on different threads. As a result, we cannot enumerate the current state + // on surface creation (it might be changed on another thread during the enumeration). + // An easy workaround is to set a known state on device creation. if disable_connectors { dev.set_connector_state(res_handles.connectors().iter().copied(), false)?; for crtc in res_handles.crtcs() { - // null commit + // null commit (necessary to trigger removal on the kernel side with the legacy api.) dev.set_crtc(*crtc, None, (0, 0), &[], None) .compat() .map_err(|source| Error::Access { @@ -199,6 +223,7 @@ impl Dev { connectors: impl Iterator, enabled: bool, ) -> Result<(), Error> { + // for every connector... for conn in connectors { let info = self .get_connector(conn) @@ -208,7 +233,9 @@ impl Dev { dev: self.dev_path(), source, })?; + // that is currently connected ... if info.state() == connector::State::Connected { + // get a list of it's properties. let props = self .get_properties(conn) .compat() @@ -218,7 +245,9 @@ impl Dev { source, })?; let (handles, _) = props.as_props_and_values(); + // for every handle ... for handle in handles { + // get information of that property let info = self .get_property(*handle) .compat() @@ -227,7 +256,9 @@ impl Dev { dev: self.dev_path(), source, })?; + // to find out, if we got the handle of the "DPMS" property ... if info.name().to_str().map(|x| x == "DPMS").unwrap_or(false) { + // so we can use that to turn on / off the connector self.set_property( conn, *handle, diff --git a/src/backend/drm/legacy/session.rs b/src/backend/drm/legacy/session.rs index 5ceda28..8c254c1 100644 --- a/src/backend/drm/legacy/session.rs +++ b/src/backend/drm/legacy/session.rs @@ -72,6 +72,8 @@ impl LegacyDrmDeviceObserver { for surface in backends.borrow().values().filter_map(Weak::upgrade) { // other ttys that use no cursor, might not clear it themselves. // This makes sure our cursor won't stay visible. + // + // This usually happens with getty, and a cursor on top of a kernel console looks very weird. let _ = (*device).set_cursor( surface.crtc, Option::<&drm::control::dumbbuffer::DumbBuffer>::None, @@ -119,6 +121,11 @@ impl LegacyDrmDeviceObserver { } impl LegacyDrmDeviceObserver { + // reset state enumerates the actual current state of the drm device + // and applies that to the `current_state` as saved in the surfaces. + // + // This re-sync is necessary after a tty swap, as the pipeline might + // be left in a different state. fn reset_state(&mut self) -> Result<(), Error> { // lets enumerate it the current state if let Some(dev) = self.dev.upgrade() { diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 035a88c..9e6917c 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -70,10 +70,13 @@ impl CursorBackend for LegacyDrmSurfaceInternal { trace!(self.logger, "Setting the new imported cursor"); + // set_cursor2 allows us to set the hotspot, but is not supported by every implementation. if self .set_cursor2(self.crtc, Some(buffer), (hotspot.0 as i32, hotspot.1 as i32)) .is_err() { + // the cursor will be slightly misplaced, when using the function for hotspots other then (0, 0), + // but that is still better then no cursor. self.set_cursor(self.crtc, Some(buffer)) .compat() .map_err(|source| Error::Access { @@ -221,7 +224,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { self.dev.set_connector_state(removed.copied(), false)?; if conn_removed { - // We need to do a null commit to free graphics pipelines + // null commit (necessary to trigger removal on the kernel side with the legacy api.) self.set_crtc(self.crtc, None, (0, 0), &[], None) .compat() .map_err(|source| Error::Access { @@ -246,6 +249,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { } debug!(self.logger, "Setting screen"); + // do a modeset and attach the given framebuffer self.set_crtc( self.crtc, Some(framebuffer), @@ -266,6 +270,11 @@ impl RawSurface for LegacyDrmSurfaceInternal { *current = pending.clone(); + // set crtc does not trigger page_flip events, so we immediately queue a flip + // with the same framebuffer. + // this will result in wasting a frame, because this flip will need to wait + // for `set_crtc`, but is necessary to drive the event loop and thus provide + // a more consistent api. ControlDevice::page_flip( self, self.crtc, @@ -316,7 +325,8 @@ impl LegacyDrmSurfaceInternal { "Initializing drm surface with mode {:?} and connectors {:?}", mode, connectors ); - // Try to enumarate the current state to set the initial state variable correctly + // 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 = dev.get_crtc(crtc).compat().map_err(|source| Error::Access { errmsg: "Error loading crtc info", dev: dev.dev_path(), @@ -377,6 +387,13 @@ impl LegacyDrmSurfaceInternal { Ok(surface) } + // we use this function to verify, if a certain connector/mode combination + // is valid on our crtc. We do this with the most basic information we have: + // - is there a matching encoder + // - does the connector support the provided Mode. + // + // Better would be some kind of test commit to ask the driver, + // but that only exists for the atomic api. fn check_connector(&self, conn: connector::Handle, mode: &Mode) -> Result { let info = self .get_connector(conn)