[PATCH v3 12/23] rust: drm: kms: add owned CRTC and vblank references
Mike Lothian <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
`&Crtc<T>` is only valid for the callback DRM handed it to, and
`VblankRef` borrows one. That does not fit two common shapes:
- a driver that enables vblanks in `atomic_enable` and releases
them in
`atomic_disable` holds the reference across two callbacks,
so it has to `mem::forget` the guard and hand-balance a raw
`drm_crtc_vblank_put`;
- a driver with a software vblank clock must reach its CRTC from
a timer
callback, so it stashes a raw `drm_crtc` pointer and calls
`drm_crtc_handle_vblank` on it.
Both are safe in principle -- mode objects live until their DRM device
is freed -- but neither can be expressed, so drivers reintroduce raw
pointers that the safe KMS API exists to remove.
Add `CrtcRef`, an owned handle holding an `ARef` to the DRM device,
which keeps the CRTC alive and hands back a `&Crtc<T>` from any context,
and `OwnedVblankRef`, a vblank reference built on it and obtained by
converting a `VblankRef` with `into_owned()`. The vblank reference
is still released exactly once, on drop.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Mike Lothian <[email protected]>
---
rust/kernel/drm/kms/crtc.rs | 54 +++++++++++++++++++++++++++++++++++
rust/kernel/drm/kms/vblank.rs | 47 ++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
diff --git a/rust/kernel/drm/kms/crtc.rs b/rust/kernel/drm/kms/crtc.rs
index a7024d8921ca..892f04f04e22 100644
--- a/rust/kernel/drm/kms/crtc.rs
+++ b/rust/kernel/drm/kms/crtc.rs
@@ -14,6 +14,7 @@
drm::device::Device,
error::{from_result, to_result},
prelude::*,
+ sync::aref::ARef,
types::{NotThreadSafe, Opaque},
};
use core::{
@@ -304,6 +305,59 @@ pub(crate) fn get_vblank_ptr(&self) -> *mut bindings::drm_vblank_crtc {
pub(crate) const fn has_vblank() -> bool {
T::OPS.funcs.enable_vblank.is_some()
}
+
+ /// Returns an owned handle to this [`Crtc`].
+ ///
+ /// A `&Crtc<T>` is only valid for the callback that produced it. Drivers that must reach a
+ /// CRTC from a context DRM did not hand one to -- a timer callback driving a software vblank
+ /// clock, for instance -- can keep a [`CrtcRef`] instead of stashing a raw pointer.
+ pub fn to_owned_ref(&self) -> CrtcRef<T> {
+ CrtcRef {
+ dev: self.drm_dev().into(),
+ crtc: NonNull::from(self),
+ }
+ }
+}
+
+/// An owned handle to a [`Crtc`].
+///
+/// Mode objects are owned by their DRM device and live until it is freed, so holding an [`ARef`] to
+/// that device is enough to keep the CRTC valid. [`crtc`](CrtcRef::crtc) then hands back a usable
+/// reference from any context.
+///
+/// [`ARef`]: crate::sync::aref::ARef
+pub struct CrtcRef<T: DriverCrtc> {
+ /// Keeps the DRM device -- and with it every mode object it owns, including `crtc` -- alive.
+ dev: ARef<Device<T::Driver>>,
+ crtc: NonNull<Crtc<T>>,
+}
+
+// SAFETY: This is an owning handle to device state, not to anything thread-local, and the
+// `ARef` it holds is itself `Send`.
+unsafe impl<T: DriverCrtc> Send for CrtcRef<T> {}
+
+// SAFETY: The only shared access offered is `crtc()`, which yields the same `&Crtc<T>` that is
+// already freely shareable between threads.
+unsafe impl<T: DriverCrtc> Sync for CrtcRef<T> {}
+
+impl<T: DriverCrtc> CrtcRef<T> {
+ /// The [`Crtc`] this handle refers to.
+ pub fn crtc(&self) -> &Crtc<T> {
+ // SAFETY: `self.dev` holds a reference to the DRM device that owns this CRTC, and mode
+ // objects live until their device is freed, so the pointer is still valid.
+ unsafe { self.crtc.as_ref() }
+ }
+
+ /// The DRM device that owns the [`Crtc`].
+ pub fn drm_dev(&self) -> &Device<T::Driver> {
+ &self.dev
+ }
+}
+
+impl<T: DriverCrtc> Clone for CrtcRef<T> {
+ fn clone(&self) -> Self {
+ self.crtc().to_owned_ref()
+ }
}
/// A [`Crtc`] that has not yet been registered with userspace.
diff --git a/rust/kernel/drm/kms/vblank.rs b/rust/kernel/drm/kms/vblank.rs
index a725a46110d8..672968c1d3cb 100644
--- a/rust/kernel/drm/kms/vblank.rs
+++ b/rust/kernel/drm/kms/vblank.rs
@@ -359,6 +359,53 @@ fn new(crtc: &'a Crtc<T>) -> Result<Self> {
Ok(Self(crtc))
}
+
+ /// Converts this reference into an [`OwnedVblankRef`], which is not tied to the borrow of the
+ /// [`Crtc`] it came from.
+ pub fn into_owned(self) -> OwnedVblankRef<T> {
+ let crtc = self.0;
+
+ // The new owner takes over the reference this guard was holding.
+ mem::forget(self);
+
+ OwnedVblankRef(crtc.to_owned_ref())
+ }
+}
+
+/// A vblank reference that owns a reference to its DRM device.
+///
+/// [`VblankRef`] borrows the [`Crtc`] it was taken from, so it cannot outlive the callback that
+/// created it. A driver whose vblank interval spans several callbacks -- typically one that enables
+/// vblanks in [`atomic_enable`] and releases them in [`atomic_disable`], or that drives a software
+/// vblank clock from a timer -- needs a reference it can store instead.
+///
+/// It wraps a [`CrtcRef`], which keeps the DRM device -- and so the CRTC -- alive, so [`crtc`]
+/// hands back a usable reference for as long as this object exists. Dropping it releases the
+/// vblank reference exactly once.
+///
+/// [`atomic_enable`]: DriverCrtc::atomic_enable
+/// [`atomic_disable`]: DriverCrtc::atomic_disable
+/// [`crtc`]: OwnedVblankRef::crtc
+pub struct OwnedVblankRef<T: VblankDriverCrtc>(CrtcRef<T>);
+
+impl<T: VblankDriverCrtc> OwnedVblankRef<T> {
+ /// The [`Crtc`] whose vblanks this reference is keeping enabled.
+ pub fn crtc(&self) -> &Crtc<T> {
+ self.0.crtc()
+ }
+
+ /// The DRM device that owns the [`Crtc`].
+ pub fn drm_dev(&self) -> &Device<T::Driver> {
+ self.0.drm_dev()
+ }
+}
+
+impl<T: VblankDriverCrtc> Drop for OwnedVblankRef<T> {
+ fn drop(&mut self) {
+ // SAFETY: `crtc()` returns a valid, initialized `drm_crtc`, and this type holds exactly
+ // one vblank reference -- taken by `VblankRef::new()` and transferred by `into_owned()`.
+ unsafe { bindings::drm_crtc_vblank_put(self.crtc().as_raw()) };
+ }
}
/// The base wrapper for [`drm_vblank_crtc`].