[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`].
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.