Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a disable method to DrmSurface/DrmCompositor for implementing DPMS #1561

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/backend/drm/compositor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4216,6 +4216,24 @@ where
Err(Some(RenderingReason::ScanoutFailed))
}
}

/// Disable the surface, setting DPMS state to off, and clear
/// pending frame.
///
/// Calling [`queue_frame`][Self::queue_frame] will re-enable.
pub fn disable(&mut self) -> Result<(), DrmError> {
self.surface.disable()?;

self.current_frame
.planes
.iter_mut()
.for_each(|(_, state)| *state = Default::default());
self.pending_frame = None;
self.queued_frame = None;
self.next_frame = None;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @cmeissl

Does this look fine to you?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends on how it should behave, this should be fine for disabling alone, yes.
We could also reset the element states to drop all framebuffers we imported, this might allow to lower
the memory footprint. Same for the swapchain which could be reset.


Ok(())
}
}

#[inline]
Expand Down
4 changes: 2 additions & 2 deletions src/backend/drm/device/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,9 @@ where
conn,
*handle,
if enabled {
0 /*DRM_MODE_DPMS_ON*/
drm_ffi::DRM_MODE_DPMS_ON.into()
} else {
3 /*DRM_MODE_DPMS_OFF*/
drm_ffi::DRM_MODE_DPMS_OFF.into()
},
)
.map_err(|source| {
Expand Down
4 changes: 4 additions & 0 deletions src/backend/drm/surface/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,10 @@ impl AtomicDrmSurface {
pub(crate) fn device_fd(&self) -> &DrmDeviceFd {
self.fd.device_fd()
}

pub fn disable(&self) -> Result<(), Error> {
self.clear_state()
}
}

struct TestBuffer {
Expand Down
33 changes: 27 additions & 6 deletions src/backend/drm/surface/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use drm::control::{connector, crtc, encoder, framebuffer, Device as ControlDevic
use std::collections::HashSet;
use std::sync::{
atomic::{AtomicBool, Ordering},
Arc, RwLock,
Arc, Mutex, RwLock,
};

use crate::backend::drm::error::AccessError;
Expand All @@ -18,7 +18,6 @@ use tracing::{debug, info, info_span, instrument, trace};

#[derive(Debug, PartialEq, Eq, Clone)]
pub struct State {
pub active: bool,
pub mode: Mode,
pub connectors: HashSet<connector::Handle>,
}
Expand Down Expand Up @@ -74,9 +73,6 @@ impl State {
// 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.
Ok(State {
// On legacy there is not (reliable) way to read-back the dpms state.
// So we just always assume it is off.
active: false,
mode: current_mode.unwrap_or_else(|| unsafe { std::mem::zeroed() }),
connectors: current_connectors,
})
Expand All @@ -90,6 +86,7 @@ pub struct LegacyDrmSurface {
crtc: crtc::Handle,
state: RwLock<State>,
pending: RwLock<State>,
dpms: Mutex<bool>,
pub(super) span: tracing::Span,
}

Expand All @@ -107,7 +104,6 @@ impl LegacyDrmSurface {

let state = State::current_state(&*fd, crtc)?;
let pending = State {
active: true,
mode,
connectors: connectors.iter().copied().collect(),
};
Expand All @@ -119,6 +115,7 @@ impl LegacyDrmSurface {
crtc,
state: RwLock::new(state),
pending: RwLock::new(pending),
dpms: Mutex::new(true),
span,
};

Expand Down Expand Up @@ -238,6 +235,13 @@ impl LegacyDrmSurface {

let mut current = self.state.write().unwrap();
let pending = self.pending.read().unwrap();
let mut dpms = self.dpms.lock().unwrap();

if !*dpms {
let connectors = current.connectors.intersection(&pending.connectors);
set_connector_state(&*self.fd, connectors.copied(), true)?;
*dpms = true;
}

{
let removed = current.connectors.difference(&pending.connectors);
Expand Down Expand Up @@ -336,6 +340,13 @@ impl LegacyDrmSurface {
return Err(Error::DeviceInactive);
}

let current = self.state.read().unwrap();
let mut dpms = self.dpms.lock().unwrap();
if !*dpms {
set_connector_state(&*self.fd, current.connectors.iter().copied(), true)?;
*dpms = true;
}

ControlDevice::page_flip(
&*self.fd,
self.crtc,
Expand Down Expand Up @@ -457,6 +468,16 @@ impl LegacyDrmSurface {
pub(crate) fn device_fd(&self) -> &DrmDeviceFd {
self.fd.device_fd()
}

pub fn disable(&self) -> Result<(), Error> {
let current = self.state.read().unwrap();
let mut dpms = self.dpms.lock().unwrap();
if *dpms {
set_connector_state(&*self.fd, current.connectors.iter().copied(), false)?;
*dpms = false;
}
Ok(())
}
}

impl Drop for LegacyDrmSurface {
Expand Down
11 changes: 11 additions & 0 deletions src/backend/drm/surface/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,17 @@ impl DrmSurface {
DrmSurfaceInternal::Legacy(surf) => &surf.span,
}
}

/// Disable the output, setting DPMS state to off.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo the name disable might be misleading, or at lest the comment might be. it does not only set the dpms state, but also clears the whole state including all planes.

///
/// The surface will be re-enabled on the next [`page_flip`][Self::page_flip] or
/// [`commit`][Self::commit].
pub fn disable(&self) -> Result<(), Error> {
match &*self.internal {
DrmSurfaceInternal::Atomic(surf) => surf.disable(),
DrmSurfaceInternal::Legacy(surf) => surf.disable(),
}
}
}

fn ensure_legacy_planes<'a>(
Expand Down
Loading