From 7095db1befb0871cb24d1c339ffb27df45337dc5 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 7 Nov 2024 20:23:52 +0100 Subject: [PATCH] drm: Add vrr property helpers --- src/backend/drm/compositor/mod.rs | 38 +++++- src/backend/drm/mod.rs | 2 +- src/backend/drm/surface/atomic.rs | 194 ++++++++++++++++++++++++++++-- src/backend/drm/surface/gbm.rs | 18 ++- src/backend/drm/surface/mod.rs | 41 ++++++- 5 files changed, 278 insertions(+), 15 deletions(-) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index 2e7e99fb61e9..ee646bdc81c2 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -172,7 +172,10 @@ use crate::{ wayland::{shm, single_pixel_buffer}, }; -use super::{error::AccessError, DrmDeviceFd, DrmSurface, Framebuffer, PlaneClaim, PlaneInfo, Planes}; +use super::{ + error::AccessError, surface::VrrSupport, DrmDeviceFd, DrmSurface, Framebuffer, PlaneClaim, PlaneInfo, + Planes, +}; pub mod dumb; mod elements; @@ -1582,6 +1585,7 @@ where primary_plane_damage_bag: DamageBag, supports_fencing: bool, direct_scanout: bool, + vrr: bool, reset_pending: bool, signaled_fence: Option>, @@ -1769,6 +1773,7 @@ where primary_plane_damage_bag: DamageBag::new(4), primary_is_opaque: is_opaque, direct_scanout: true, + vrr: false, reset_pending: true, signaled_fence, current_frame, @@ -2357,6 +2362,17 @@ where } } + // If we only have a cursor update and vrr is enabled, + // don't update to not artificially increase the framerate + if self.vrr + && cursor_plane_element.is_some() + && overlay_plane_elements.is_empty() + && primary_plane_scanout_element.is_none() + && primary_plane_elements.is_empty() + { + let _ = cursor_plane_element.take(); + } + // Cleanup old state (e.g. old dmabuffers) for element_state in element_states.values_mut() { element_state.fb_cache.cleanup(); @@ -2930,6 +2946,26 @@ where Ok(()) } + /// Returns if Variable Refresh Rate is advertised as supported by the given connector. + /// + /// See [`DrmSurface::vrr_supported`] for more details. + pub fn vrr_supported(&self, conn: connector::Handle) -> FrameResult { + self.surface.vrr_supported(conn).map_err(FrameError::DrmError) + } + + /// Tries to set variable refresh rate (VRR) for the next frame. + /// + /// Doing so might cause the next frame to trigger a modeset. + /// Check [`DrmCompositor::vrr_supported`], which indicates if VRR can be + /// used without a modeset on the attached connectors. + pub fn use_vrr(&mut self, vrr: bool) -> FrameResult<(), A, F> { + let res = self.surface.use_vrr(vrr).map_err(FrameError::DrmError); + if res.is_ok() { + self.vrr = vrr; + } + res + } + /// Set the [`DebugFlags`] to use /// /// Note: This will reset the primary plane swapchain if diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index 84efda0d54e4..53e657903773 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -95,7 +95,7 @@ pub use error::Error as DrmError; use indexmap::IndexSet; #[cfg(feature = "backend_gbm")] pub use surface::gbm::{Error as GbmBufferedSurfaceError, GbmBufferedSurface}; -pub use surface::{DrmSurface, PlaneConfig, PlaneDamageClips, PlaneState}; +pub use surface::{DrmSurface, PlaneConfig, PlaneDamageClips, PlaneState, VrrSupport}; use drm::{ control::{crtc, framebuffer, plane, Device as ControlDevice, PlaneType}, diff --git a/src/backend/drm/surface/atomic.rs b/src/backend/drm/surface/atomic.rs index 21b6cea8787f..f7fb7589e86e 100644 --- a/src/backend/drm/surface/atomic.rs +++ b/src/backend/drm/surface/atomic.rs @@ -1,4 +1,6 @@ use drm::control::atomic::AtomicModeReq; +use drm::control::connector::Interface; +use drm::control::property::ValueType; use drm::control::Device as ControlDevice; use drm::control::{ connector, crtc, dumbbuffer::DumbBuffer, framebuffer, plane, property, AtomicCommitFlags, Mode, PlaneType, @@ -6,10 +8,9 @@ use drm::control::{ use std::collections::HashSet; use std::os::unix::io::AsRawFd; -use std::sync::Mutex; use std::sync::{ atomic::{AtomicBool, Ordering}, - Arc, RwLock, + Arc, Mutex, RwLock, }; use crate::backend::drm::error::AccessError; @@ -29,20 +30,24 @@ use crate::{ use tracing::{debug, info, info_span, instrument, trace, warn}; -use super::{PlaneConfig, PlaneState}; +use super::{PlaneConfig, PlaneState, VrrSupport}; #[derive(Debug, Clone)] pub struct State { pub active: bool, pub mode: Mode, pub blob: property::Value<'static>, + pub vrr: bool, pub connectors: HashSet, } impl PartialEq for State { #[inline] fn eq(&self, other: &Self) -> bool { - self.active == other.active && self.mode == other.mode && self.connectors == other.connectors + self.active == other.active + && self.mode == other.mode + && self.vrr == other.vrr + && self.connectors == other.connectors } } @@ -110,18 +115,24 @@ impl State { } } - // Get the current active (dpms) state of the CRTC + // Get the current active (dpms) state and vrr state of the CRTC // // Changing a CRTC to active might require a modeset let mut active = None; + let mut vrr = None; if let Ok(props) = fd.get_properties(crtc) { let active_prop = prop_mapping.crtcs.get(&crtc).and_then(|m| m.get("ACTIVE")); + let vrr_prop = prop_mapping.crtcs.get(&crtc).and_then(|m| m.get("VRR_ENABLED")); let (ids, vals) = props.as_props_and_values(); for (&id, &val) in ids.iter().zip(vals.iter()) { if Some(&id) == active_prop { active = property::ValueType::Boolean.convert_value(val).as_boolean(); break; } + if Some(&id) == vrr_prop { + vrr = property::ValueType::Boolean.convert_value(val).as_boolean(); + break; + } } } @@ -131,6 +142,8 @@ impl State { active: active.unwrap_or(false), mode: current_mode, blob: current_blob, + // If we don't know the VRR state, the driver doesn't support the property + vrr: vrr.unwrap_or(false), connectors: current_connectors, }) } @@ -140,6 +153,7 @@ impl State { self.blob = property::Value::Unknown(0); self.connectors.clear(); self.active = false; + self.vrr = false; } } @@ -186,6 +200,7 @@ impl AtomicDrmSurface { active: true, mode, blob, + vrr: false, connectors: connectors.iter().copied().collect(), }; @@ -338,6 +353,7 @@ impl AtomicDrmSurface { }), }], Some(pending.blob), + pending.vrr, )?; self.fd .atomic_commit( @@ -390,6 +406,7 @@ impl AtomicDrmSurface { }), }], Some(pending.blob), + pending.vrr, )?; self.fd .atomic_commit( @@ -444,6 +461,7 @@ impl AtomicDrmSurface { }), }], Some(pending.blob), + pending.vrr, )?; self.fd @@ -496,6 +514,7 @@ impl AtomicDrmSurface { }), }], Some(new_blob), + pending.vrr, )?; if let Err(err) = self .fd @@ -516,6 +535,123 @@ impl AtomicDrmSurface { Ok(()) } + pub fn vrr_supported(&self, conn: connector::Handle) -> Result { + if !self.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + + let props = self.prop_mapping.read().unwrap(); + if self + .prop_mapping + .read() + .unwrap() + .crtc_prop_handle(self.crtc, "VRR_ENABLED") + .is_err() + { + return Ok(VrrSupport::NotSupported); + } + + if let Some(vrr_prop) = props + .connectors + .get(&conn) + .and_then(|props| props.get("vrr_capable")) + { + for (prop, value) in self.fd.get_properties(conn).map_err(|source| { + Error::Access(AccessError { + errmsg: "Error querying properties", + dev: self.fd.dev_path(), + source, + }) + })? { + if prop == *vrr_prop { + let interface = self + .fd + .get_connector(conn, false) + .map_err(|source| { + Error::Access(AccessError { + errmsg: "Error querying connector", + dev: self.fd.dev_path(), + source, + }) + })? + .interface(); + + // see: https://gitlab.freedesktop.org/drm/amd/-/issues/2200#note_2159982 + // Currently setting VRR for HDMI connectors will cause flickering despite not needing `ALLOW_MODESET` + // TODO: Once the kernel is fixed, do actual test commits with and without `ALLOW_MODESET`. + return Ok( + match ValueType::Boolean.convert_value(value).as_boolean().unwrap() { + true if interface == Interface::HDMIA || interface == Interface::HDMIB => { + VrrSupport::RequiresModeset + } + true => VrrSupport::Supported, + false => VrrSupport::NotSupported, + }, + ); + } + } + } + + Ok(VrrSupport::NotSupported) + } + + pub fn use_vrr(&self, value: bool) -> Result<(), Error> { + let mut current = self.state.write().unwrap(); + let mut pending = self.pending.write().unwrap(); + if pending.vrr == value { + return Ok(()); + } + + if value { + if self + .prop_mapping + .read() + .unwrap() + .crtc_prop_handle(self.crtc, "VRR_ENABLED") + .is_err() + { + return Err(Error::UnknownProperty { + handle: self.crtc.into(), + name: "VRR_ENABLED", + }); + } + } + + let test_buffer = self.create_test_buffer(pending.mode.size(), self.plane)?; + let plane_config = PlaneState { + handle: self.plane, + config: Some(PlaneConfig { + src: Rectangle::from_loc_and_size(Point::default(), pending.mode.size()).to_f64(), + dst: Rectangle::from_loc_and_size( + Point::default(), + (pending.mode.size().0 as i32, pending.mode.size().1 as i32), + ), + transform: Transform::Normal, + alpha: 1.0, + damage_clips: None, + fb: test_buffer.fb, + fence: None, + }), + }; + + if &*current == &*pending { + // Try a non modesetting commit + if self + .test_state_internal([plane_config.clone()], false, &*current, &*pending) + .is_ok() + { + current.vrr = value; + pending.vrr = value; + return Ok(()); + } + } + + // Try a modeset commit + self.test_state_internal([plane_config], true, &*current, &*pending)?; + pending.vrr = value; + Ok(()) + } + pub fn commit_pending(&self) -> bool { *self.pending.read().unwrap() != *self.state.read().unwrap() } @@ -531,17 +667,33 @@ impl AtomicDrmSurface { return Err(Error::DeviceInactive); } - let planes = planes.into_iter().collect::>(); - let current = self.state.read().unwrap(); let pending = self.pending.read().unwrap(); + self.test_state_internal(planes, allow_modeset, &*current, &*pending) + } + + fn test_state_internal<'a>( + &self, + planes: impl IntoIterator>, + allow_modeset: bool, + current: &'_ State, + pending: &'_ State, + ) -> Result<(), Error> { + let planes = planes.into_iter().collect::>(); + let current_conns = current.connectors.clone(); let pending_conns = pending.connectors.clone(); let mut removed = current_conns.difference(&pending_conns); let mut added = pending_conns.difference(¤t_conns); - let req = self.build_request(&mut added, &mut removed, &*planes, Some(pending.blob))?; + let req = self.build_request( + &mut added, + &mut removed, + &*planes, + Some(pending.blob), + pending.vrr, + )?; let flags = if allow_modeset { AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY @@ -605,7 +757,13 @@ impl AtomicDrmSurface { // test the new config and return the request if it would be accepted by the driver. let req = { - let req = self.build_request(&mut added, &mut removed, &*planes, Some(pending.blob))?; + let req = self.build_request( + &mut added, + &mut removed, + &*planes, + Some(pending.blob), + pending.vrr, + )?; if let Err(err) = self.fd.atomic_commit( AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY, @@ -683,7 +841,13 @@ impl AtomicDrmSurface { let planes = planes.into_iter().collect::>(); // page flips work just like commits with fewer parameters.. - let req = self.build_request(&mut [].iter(), &mut [].iter(), &*planes, None)?; + let req = self.build_request( + &mut [].iter(), + &mut [].iter(), + &*planes, + None, + self.state.read().unwrap().vrr, + )?; // .. and without `AtomicCommitFlags::AllowModeset`. // If we would set anything here, that would require a modeset, this would fail, @@ -729,6 +893,7 @@ impl AtomicDrmSurface { removed_connectors: &mut dyn Iterator, planes: impl IntoIterator>, blob: Option>, + vrr: bool, ) -> Result { let prop_mapping = self.prop_mapping.read().unwrap(); @@ -775,6 +940,15 @@ impl AtomicDrmSurface { property::Value::Boolean(true), ); + if let Ok(vrr_prop) = prop_mapping.crtc_prop_handle(self.crtc, "VRR_ENABLED") { + req.add_property(self.crtc, vrr_prop, property::Value::Boolean(vrr)); + } else if vrr { + return Err(Error::UnknownProperty { + handle: self.crtc.into(), + name: "VRR_ENABLED", + }); + } + for plane_state in planes.into_iter() { let handle = &plane_state.handle; diff --git a/src/backend/drm/surface/gbm.rs b/src/backend/drm/surface/gbm.rs index 8233e1fc554c..dc3629761374 100644 --- a/src/backend/drm/surface/gbm.rs +++ b/src/backend/drm/surface/gbm.rs @@ -18,7 +18,7 @@ use crate::utils::{DevPath, Physical, Point, Rectangle, Transform}; use tracing::{debug, error, info_span, instrument, trace, warn}; -use super::{PlaneConfig, PlaneDamageClips, PlaneState}; +use super::{PlaneConfig, PlaneDamageClips, PlaneState, VrrSupport}; #[derive(Debug)] struct QueuedFb { @@ -500,6 +500,22 @@ where Ok(()) } + /// Returns if Variable Refresh Rate is advertised as supported by the given connector. + /// + /// See [`DrmSurface::vrr_supported`] for more details. + pub fn vrr_supported(&self, conn: connector::Handle) -> Result> { + self.drm.vrr_supported(conn).map_err(Error::DrmError) + } + + /// Tries to set variable refresh rate (VRR) for the next frame. + // + /// Doing so might cause the next frame to trigger a modeset. + /// Check [`GbmBufferedSurface::vrr_supported`], which indicates if VRR can be + /// used without a modeset on the attached connectors./ + pub fn use_vrr(&self, vrr: bool) -> Result<(), Error> { + self.drm.use_vrr(vrr).map_err(Error::DrmError) + } + /// Returns a reference to the underlying drm surface pub fn surface(&self) -> &DrmSurface { &self.drm diff --git a/src/backend/drm/surface/mod.rs b/src/backend/drm/surface/mod.rs index 57a3540a76d5..065ff2258ae1 100644 --- a/src/backend/drm/surface/mod.rs +++ b/src/backend/drm/surface/mod.rs @@ -128,7 +128,7 @@ impl Clone for PlaneDamageClips { } /// State of a single plane -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct PlaneState<'a> { /// Handle of the plane pub handle: plane::Handle, @@ -139,7 +139,7 @@ pub struct PlaneState<'a> { } /// Configuration for a single plane -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct PlaneConfig<'a> { /// Source [`Rectangle`] of the attached framebuffer pub src: Rectangle, @@ -157,6 +157,17 @@ pub struct PlaneConfig<'a> { pub fence: Option>, } +/// VRR support state +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum VrrSupport { + /// VRR is generally not supported + NotSupported, + /// VRR is supported, but changing it requires a modeset + RequiresModeset, + /// VRR is supported and can be changed without a modeset + Supported, +} + #[derive(Debug)] #[allow(clippy::large_enum_variant)] pub enum DrmSurfaceInternal { @@ -293,6 +304,32 @@ impl DrmSurface { } } + /// Returns if Variable Refresh Rate is advertised as supported by the given connector. + /// + /// Note: This will always return [`VrrSupport::NotSupported`] if the underlying + /// implementation is using the legacy DRM api. + pub fn vrr_supported(&self, conn: connector::Handle) -> Result { + match &*self.internal { + DrmSurfaceInternal::Atomic(surf) => surf.vrr_supported(conn), + DrmSurfaceInternal::Legacy(_) => Ok(VrrSupport::NotSupported), + } + } + + /// Tries to set Variable Refresh Rate (VRR) for the next frame. + /// + /// Doing so might cause [`DrmSurface::commit_pending`] to return `true`. + /// Check [`DrmSurface::vrr_supported`], which indicates if VRR can be + /// used without a modeset on the attached connectors. + pub fn use_vrr(&self, vrr: bool) -> Result<(), Error> { + match &*self.internal { + DrmSurfaceInternal::Atomic(surf) => surf.use_vrr(vrr), + DrmSurfaceInternal::Legacy(_) => Err(Error::UnknownProperty { + handle: self.crtc.into(), + name: "VRR_ENABLED", + }), + } + } + /// Disables the given plane. /// /// Errors if the plane is not supported by this crtc or if the underlying