drivers/gpu/drm/rvkms/rvkms.rs

@@ -18,7 +18,7 @@ use kernel::{
         self,
         drv,
         kms::{
-            Kms,
+            KmsDriver,
             ModeConfigInfo,
             UnregisteredKmsDevice,
             atomic::*,
@@ -72,26 +72,31 @@ impl drv::Driver for RvkmsDriver {
 }
 
 #[vtable]
-impl Kms for RvkmsDriver {
-    type Driver = Self;
+impl KmsDriver for RvkmsDriver {
     type Fbdev = FbdevShmem<Self>;
 
     fn mode_config_info(
         _dev: &device::Device,
-        _drm_data: <<Self::Driver as drv::Driver>::Data as kernel::types::ForeignOwnable>::Borrowed<'_>,
+        _drm_data: <Self::Data as kernel::types::ForeignOwnable>::Borrowed<'_>,
     ) -> Result<ModeConfigInfo> {
         Ok(MODE_CONFIG_INFO)
     }
 
-    fn create_objects(drm: &UnregisteredKmsDevice<'_, Self::Driver>) -> Result {
+    fn create_objects(drm: &UnregisteredKmsDevice<'_, Self>) -> Result
+    where
+        Self: Sized
+    {
         output::create_output(drm, 0)
     }
 
     fn atomic_commit_tail<'a>(
-        mut state: AtomicCommitTail<'a, Self::Driver>,
+        mut state: AtomicCommitTail<'a, Self>,
         modeset_token: ModesetsReadyToken<'_>,
         plane_update_token: PlaneUpdatesReadyToken<'_>,
-    ) -> CommittedAtomicState<'a, Self::Driver> {
+    ) -> CommittedAtomicState<'a, Self>
+    where
+        Self: Sized
+    {
         let modeset_token = state.commit_modeset_disables(modeset_token);
 
         let plane_update_token = state.commit_planes(plane_update_token, Default::default());

rust/kernel/drm/drv.rs

@@ -13,7 +13,7 @@ use crate::{
         kms::{
             KmsImpl,
             private::KmsImpl as KmsImplPrivate,
-            Kms
+            KmsDriver
         }
     },
     device,

rust/kernel/drm/kms.rs

@@ -51,7 +51,7 @@ pub(crate) mod private {
     use super::*;
 
     /// Private callback implemented internally by DRM for setting up KMS on a device, or stubbing
-    /// the KMS setup for devices which don't have KMS support can just use [`PhantomData`].
+    /// the KMS setup for devices which don't have KMS support.
     pub trait KmsImpl {
         /// The parent driver for this KMS implementation
         type Driver: Driver;
@@ -124,10 +124,7 @@ impl<'a, T: Driver> UnregisteredKmsDevice<'a, T> {
 ///
 /// [`PhantomData<Self>`]: PhantomData
 #[vtable]
-pub trait Kms {
-    /// The parent [`Driver`] for this [`Device`].
-    type Driver: KmsDriver;
-
+pub trait KmsDriver: Driver {
     /// The fbdev implementation to use for this [`Device`].
     ///
     /// Which implementation may be used here depends on the GEM implementation specified in
@@ -137,11 +134,13 @@ pub trait Kms {
     /// Return a [`ModeConfigInfo`] structure for this [`device::Device`].
     fn mode_config_info(
         dev: &device::Device,
-        drm_data: <<Self::Driver as Driver>::Data as ForeignOwnable>::Borrowed<'_>,
+        drm_data: <Self::Data as ForeignOwnable>::Borrowed<'_>,
     ) -> Result<ModeConfigInfo>;
 
     /// Create mode objects like [`crtc::Crtc`], [`plane::Plane`], etc. for this device
-    fn create_objects(drm: &UnregisteredKmsDevice<'_, Self::Driver>) -> Result;
+    fn create_objects(drm: &UnregisteredKmsDevice<'_, Self>) -> Result
+    where
+        Self: Sized;
 
     /// The optional [`atomic_commit_tail`] callback for this [`Device`].
     ///
@@ -156,16 +155,19 @@ pub trait Kms {
     /// [`atomic_commit_tail`]: srctree/include/drm/drm_modeset_helper_vtables.h
     /// [`drm_atomic_helper_commit_tail`]: srctree/include/drm/drm_atomic_helpers.h
     fn atomic_commit_tail<'a>(
-        state: atomic::AtomicCommitTail<'a, Self::Driver>,
+        state: atomic::AtomicCommitTail<'a, Self>,
         _modeset_token: atomic::ModesetsReadyToken<'_>,
         _plane_update_token: atomic::PlaneUpdatesReadyToken<'_>
-    ) -> atomic::CommittedAtomicState<'a, Self::Driver> {
+    ) -> atomic::CommittedAtomicState<'a, Self>
+    where
+        Self: Sized
+    {
         build_error::build_error("This function should not be reachable")
     }
 }
 
-impl<T: Kms> private::KmsImpl for T {
-    type Driver = T::Driver;
+impl<T: KmsDriver> private::KmsImpl for T {
+    type Driver = Self;
 
     const MODE_CONFIG_OPS: Option<ModeConfigOps> = Some(ModeConfigOps {
         kms_vtable: bindings::drm_mode_config_funcs {
@@ -238,11 +240,11 @@ impl<T: Kms> private::KmsImpl for T {
     }
 
     unsafe fn setup_fbdev(drm: &Device<Self::Driver>, mode_config_info: &ModeConfigInfo) {
-        <<T as Kms>::Fbdev as fbdev::private::FbdevImpl>::setup_fbdev(drm, mode_config_info)
+        <<T as KmsDriver>::Fbdev as fbdev::private::FbdevImpl>::setup_fbdev(drm, mode_config_info)
     }
 }
 
-impl<T: Kms> KmsImpl for T {}
+impl<T: KmsDriver> KmsImpl for T {}
 
 impl<T: Driver> private::KmsImpl for PhantomData<T> {
     type Driver = T;
@@ -265,17 +267,6 @@ pub struct ModeConfigInfo {
     pub preferred_depth: u32,
 }
 
-/// A [`Driver`] with [`Kms`] implemented.
-///
-/// This is implemented internally by DRM for any [`Device`] whose [`Driver`] type implements
-/// [`Kms`], and provides access to methods which are only safe to use with KMS devices.
-pub trait KmsDriver: Driver {}
-
-impl<T, K> KmsDriver for T
-where
-    T: Driver<Kms = K>,
-    K: Kms<Driver = T> {}
-
 impl<T: KmsDriver> Device<T> {
     /// Retrieve a pointer to the mode_config mutex
     #[inline]
@@ -316,9 +307,19 @@ impl<T: KmsDriver> Device<T> {
 /// A modesetting object in DRM.
 ///
 /// This is any type of object where the underlying C object contains a [`struct drm_mode_object`].
+/// This type requires [`Send`] + [`Sync`] as all modesetting objects in DRM are able to be sent
+/// between threads.
+///
+/// This type is only implemented by the DRM crate itself.
+///
+/// # Safety
+///
+/// [`raw_mode_obj()`] must always return a valid pointer to an initialized
+/// [`struct drm_mode_object`].
 ///
 /// [`struct drm_mode_object`]: srctree/include/drm/drm_mode_object.h
-pub trait ModeObject: Sealed + Send + Sync {
+/// [`raw_mode_obj()`]: ModeObject::raw_mode_obj()
+pub unsafe trait ModeObject: Sealed + Send + Sync {
     /// The parent driver for this [`ModeObject`].
     type Driver: KmsDriver;
 
@@ -409,13 +410,14 @@ pub unsafe trait RcModeObject: ModeObject {}
 
 unsafe impl<T: RcModeObject> AlwaysRefCounted for T {
     fn inc_ref(&self) {
-        // SAFETY: FFI call with no special requirements
+        // SAFETY: We're guaranteed by the safety contract of `ModeObject` that `raw_mode_obj()`
+        // always returns a pointer to an initialized `drm_mode_object`.
         unsafe { bindings::drm_mode_object_get(self.raw_mode_obj()) }
     }
 
     unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
-        // SAFETY: We never expose modesetting objects in our interfaces to users before they're
-        // initialized
+        // SAFETY: We're guaranteed by the safety contract of `ModeObject` that `raw_mode_obj()`
+        // always returns a pointer to an initialized `drm_mode_object`.
         unsafe { bindings::drm_mode_object_put(obj.as_ref().raw_mode_obj()) }
     }
 }

rust/kernel/drm/kms/atomic.rs

@@ -26,7 +26,6 @@ use super::{
     crtc::*,
     connector::*,
     plane::*,
-    Kms,
     KmsDriver,
     ModeObject
 };
@@ -547,14 +546,14 @@ impl<'a, T: KmsDriver> AtomicCommitTail<'a, T> {
 }
 
 // The actual raw C callback for custom atomic commit tail implementations
-pub(crate) unsafe extern "C" fn commit_tail_callback<T: Kms>(
+pub(crate) unsafe extern "C" fn commit_tail_callback<T: KmsDriver>(
     state: *mut bindings::drm_atomic_state
 ) {
     // SAFETY:
     // * We're guaranteed by DRM that `state` always points to a valid instance of
     //   `bindings::drm_atomic_state`
     // * This conversion is safe via the type invariants
-    let state = unsafe { AtomicState::<T::Driver>::from_raw(state.cast_const()) };
+    let state = unsafe { AtomicState::from_raw(state.cast_const()) };
 
     T::atomic_commit_tail(
         AtomicCommitTail(state),

rust/kernel/drm/kms/connector.rs

@@ -160,7 +160,7 @@ pub struct DriverConnectorOps {
 /// # Invariants
 ///
 /// - The DRM C API and our interface guarantees that only the user has mutable access to `state`,
-///   up until [`drm_atomic_helper_commit_hw_done`] is called. Therefore, `plane` follows rust's
+///   up until [`drm_atomic_helper_commit_hw_done`] is called. Therefore, `connector` follows rust's
 ///   data aliasing rules and does not need to be behind an [`Opaque`] type.
 /// - `connector` and `inner` are initialized for as long as this object is made available to users.
 /// - The data layout of this structure begins with [`struct drm_connector`].
@@ -270,8 +270,13 @@ impl<T: DriverConnector> Connector<T> {
 ///
 /// This is implemented internally by DRM.
 ///
+/// # Safety
+///
+/// [`as_raw()`] must always return a pointer to a valid initialized [`struct drm_connector`].
+///
+/// [`as_raw()`]: AsRawConnector::as_raw()
 /// [`struct drm_connector`]: srctree/include/drm/drm_connector.h
-pub trait AsRawConnector: RcModeObject {
+pub unsafe trait AsRawConnector: RcModeObject {
     /// The parent [`Driver`] for this [`Connector`] interface
     ///
     /// TODO: get rid of this
@@ -301,7 +306,9 @@ unsafe impl<T: DriverConnector> Send for Connector<T> {}
 // SAFETY: Our connector interfaces are guaranteed to be thread-safe
 unsafe impl<T: DriverConnector> Sync for Connector<T> {}
 
-impl<T: DriverConnector> ModeObject for Connector<T> {
+// SAFETY: We don't expose Connector<T> to users before `base` is initialized in ::new(), so
+// `raw_mode_obj` always returns a valid pointer to a bindings::drm_mode_object.
+unsafe impl<T: DriverConnector> ModeObject for Connector<T> {
     type Driver = T::Driver;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {
@@ -319,7 +326,11 @@ impl<T: DriverConnector> ModeObject for Connector<T> {
 // SAFETY: DRM connectors are refcounted mode objects
 unsafe impl<T: DriverConnector> RcModeObject for Connector<T> {}
 
-impl<T: DriverConnector> AsRawConnector for Connector<T> {
+// SAFETY:
+// * Via our type variants our data layout starts with `drm_connector`
+// * Since we don't expose `Connector` to users before it has been initialized, this and our data
+//   layout ensure that `as_raw()` always returns a valid pointer to a `drm_connector`.
+unsafe impl<T: DriverConnector> AsRawConnector for Connector<T> {
     type Driver = T::Driver;
     type State = ConnectorState<T::State>;
 
@@ -404,7 +415,11 @@ pub struct OpaqueConnector<T: KmsDriver> {
 
 impl<T: KmsDriver> Sealed for OpaqueConnector<T> {}
 
-impl<T: KmsDriver> AsRawConnector for OpaqueConnector<T> {
+// SAFETY:
+// * Via our type variants our data layout starts is identical to `drm_connector`
+// * Since we don't expose `OpaqueConnector` to users before it has been initialized, this and our
+//   data layout ensure that `as_raw()` always returns a valid pointer to a `drm_connector`.
+unsafe impl<T: KmsDriver> AsRawConnector for OpaqueConnector<T> {
     type Driver = T;
     type State = OpaqueConnectorState<T>;
 
@@ -418,7 +433,9 @@ impl<T: KmsDriver> AsRawConnector for OpaqueConnector<T> {
     }
 }
 
-impl<T: KmsDriver> ModeObject for OpaqueConnector<T> {
+// SAFETY: We don't expose OpaqueConnector<T> to users before `base` is initialized in
+// Connector::new(), so `raw_mode_obj` always returns a valid pointer to a bindings::drm_mode_object.
+unsafe impl<T: KmsDriver> ModeObject for OpaqueConnector<T> {
     type Driver = T;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {

rust/kernel/drm/kms/crtc.rs

@@ -252,7 +252,9 @@ impl<T: DriverCrtc> Deref for Crtc<T> {
     }
 }
 
-impl<T: DriverCrtc> ModeObject for Crtc<T> {
+// SAFETY: We don't expose Crtc<T> to users before `base` is initialized in ::new(), so
+// `raw_mode_obj` always returns a valid pointer to a bindings::drm_mode_object.
+unsafe impl<T: DriverCrtc> ModeObject for Crtc<T> {
     type Driver = T::Driver;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {
@@ -370,8 +372,13 @@ impl<T: DriverCrtc> Crtc<T> {
 ///
 /// This is implemented internally by DRM.
 ///
+/// # Safety
+///
+/// [`as_raw()`] must always return a valid pointer to a [`struct drm_crtc`].
+///
 /// [`struct drm_crtc`]: srctree/include/drm/drm_crtc.h
-pub trait AsRawCrtc: StaticModeObject {
+/// [`as_raw()`]: AsRawCrtc::as_raw()
+pub unsafe trait AsRawCrtc: StaticModeObject {
     /// The type that should be returned for a CRTC state acquired using this CRTC interface
     type State: FromRawCrtcState;
 
@@ -384,7 +391,11 @@ pub trait AsRawCrtc: StaticModeObject {
     unsafe fn from_raw<'a>(ptr: *mut bindings::drm_crtc) -> &'a Self;
 }
 
-impl<T: DriverCrtc> AsRawCrtc for Crtc<T> {
+// SAFETY:
+// * Via our type variants our data layout starts with `drm_crtc`
+// * Since we don't expose `crtc` to users before it has been initialized, this and our data
+//   layout ensure that `as_raw()` always returns a valid pointer to a `drm_crtc`.
+unsafe impl<T: DriverCrtc> AsRawCrtc for Crtc<T> {
     type State = CrtcState<T::State>;
 
     fn as_raw(&self) -> *mut bindings::drm_crtc {
@@ -439,7 +450,11 @@ pub struct OpaqueCrtc<T: KmsDriver> {
 
 impl<T: KmsDriver> Sealed for OpaqueCrtc<T> {}
 
-impl<T: KmsDriver> AsRawCrtc for OpaqueCrtc<T> {
+// SAFETY:
+// * Via our type variants our data layout is identical to `drm_crtc`
+// * Since we don't expose `OpaqueCrtc` to users before it has been initialized, this and our data
+//   layout ensure that `as_raw()` always returns a valid pointer to a `drm_crtc`.
+unsafe impl<T: KmsDriver> AsRawCrtc for OpaqueCrtc<T> {
     type State = OpaqueCrtcState<T>;
 
     fn as_raw(&self) -> *mut bindings::drm_crtc {
@@ -452,7 +467,9 @@ impl<T: KmsDriver> AsRawCrtc for OpaqueCrtc<T> {
     }
 }
 
-impl<T: KmsDriver> ModeObject for OpaqueCrtc<T> {
+// SAFETY: We don't expose OpaqueCrtc<T> to users before `base` is initialized in Crtc::<T>::new(),
+// so `raw_mode_obj` always returns a valid pointer to a bindings::drm_mode_object.
+unsafe impl<T: KmsDriver> ModeObject for OpaqueCrtc<T> {
     type Driver = T;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {

rust/kernel/drm/kms/encoder.rs

@@ -105,8 +105,13 @@ pub struct DriverEncoderOps {
 ///
 /// This is implemented internally by DRM.
 ///
+/// # Safety
+///
+/// [`as_raw()`] must always return a valid pointer to a [`struct drm_encoder`].
+///
 /// [`struct drm_encoder`]: srctree/include/drm/drm_encoder.h
-pub trait AsRawEncoder: StaticModeObject {
+/// [`as_raw()`]: AsRawEncoder::as_raw()
+pub unsafe trait AsRawEncoder: StaticModeObject {
     /// Return the raw `bindings::drm_encoder` for this DRM encoder.
     ///
     /// Drivers should never use this directly
@@ -152,7 +157,9 @@ unsafe impl<T: DriverEncoder> Send for Encoder<T> { }
 // SAFETY: Our interface is thread-safe.
 unsafe impl<T: DriverEncoder> Sync for Encoder<T> { }
 
-impl<T: DriverEncoder> ModeObject for Encoder<T> {
+// SAFETY: We don't expose Encoder<T> to users before `base` is initialized in ::new(), so
+// `raw_mode_obj` always returns a valid pointer to a bindings::drm_mode_object.
+unsafe impl<T: DriverEncoder> ModeObject for Encoder<T> {
     type Driver = T::Driver;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {
@@ -179,7 +186,11 @@ impl<T: DriverEncoder> Deref for Encoder<T> {
     }
 }
 
-impl<T: DriverEncoder> AsRawEncoder for Encoder<T> {
+// SAFETY:
+// * Via our type invariants our data layout starts with `drm_encoder`.
+// * Since we don't expose `Encoder` to users befre it has been initialized, this and our data
+//   layout ensure that `as_raw()` always returns a valid pointer to a `drm_encoder`.
+unsafe impl<T: DriverEncoder> AsRawEncoder for Encoder<T> {
     fn as_raw(&self) -> *mut bindings::drm_encoder {
         self.encoder.get()
     }
@@ -261,7 +272,10 @@ unsafe impl<T: KmsDriver> Send for OpaqueEncoder<T> {}
 // SAFETY: All of our encoder interfaces are thread-safe
 unsafe impl<T: KmsDriver> Sync for OpaqueEncoder<T> {}
 
-impl<T: KmsDriver> ModeObject for OpaqueEncoder<T> {
+// SAFETY: We don't expose OpaqueEncoder<T> to users before `base` is initialized in
+// OpaqueEncoder::new(), so `raw_mode_obj` always returns a valid poiner to a
+// bindings::drm_mode_object.
+unsafe impl<T: KmsDriver> ModeObject for OpaqueEncoder<T> {
     type Driver = T;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {
@@ -280,7 +294,11 @@ impl<T: KmsDriver> ModeObject for OpaqueEncoder<T> {
 // SAFETY: Encoders do not have a refcount
 unsafe impl<T: KmsDriver> StaticModeObject for OpaqueEncoder<T> {}
 
-impl<T: KmsDriver> AsRawEncoder for OpaqueEncoder<T> {
+// SAFETY:
+// * Via our type variants our data layout is identical to  with `drm_encoder`
+// * Since we don't expose `Encoder` to users before it has been initialized, this and our data
+//   layout ensure that `as_raw()` always returns a valid pointer to a `drm_encoder`.
+unsafe impl<T: KmsDriver> AsRawEncoder for OpaqueEncoder<T> {
     fn as_raw(&self) -> *mut bindings::drm_encoder {
         self.encoder.get()
     }

rust/kernel/drm/kms/framebuffer.rs

@@ -27,7 +27,11 @@ use super::{ModeObject, RcModeObject, KmsDriver};
 #[repr(transparent)]
 pub struct Framebuffer<T: KmsDriver>(Opaque<bindings::drm_framebuffer>, PhantomData<T>);
 
-impl<T: KmsDriver> ModeObject for Framebuffer<T> {
+// SAFETY:
+// * `self.0` is initialized for as long as this object is exposed to users
+// * `base` is initialized by DRM when `self.0` is initialized, thus `raw_mode_obj()` always returns
+//   a valid pointer.
+unsafe impl<T: KmsDriver> ModeObject for Framebuffer<T> {
     type Driver = T;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {

rust/kernel/drm/kms/plane.rs

@@ -293,8 +293,13 @@ impl<T: DriverPlane> Plane<T> {
 ///
 /// This is implemented internally by DRM.
 ///
+/// # Safety
+///
+/// [`as_raw()`] must always return a valid pointer to an initialized [`struct drm_plane`].
+///
 /// [`struct drm_plane`]: srctree/include/drm/drm_plane.h
-pub trait AsRawPlane: StaticModeObject {
+/// [`as_raw()`]: AsRawPlane::as_raw()
+pub unsafe trait AsRawPlane: StaticModeObject {
     /// The type that should be used to represent an atomic state for this plane interface.
     type State: FromRawPlaneState;
 
@@ -309,7 +314,11 @@ pub trait AsRawPlane: StaticModeObject {
     unsafe fn from_raw<'a>(ptr: *mut bindings::drm_plane) -> &'a Self;
 }
 
-impl<T: DriverPlane> AsRawPlane for Plane<T> {
+// SAFETY:
+// * Via our type variants our data layout starts with `drm_plane`
+// * Since we don't expose `plane` to users before it has been initialized, this and our data
+//   layout ensure that `as_raw()` always returns a valid pointer to a `drm_plane`.
+unsafe impl<T: DriverPlane> AsRawPlane for Plane<T> {
     type State = PlaneState<T::State>;
 
     fn as_raw(&self) -> *mut bindings::drm_plane {
@@ -322,7 +331,9 @@ impl<T: DriverPlane> AsRawPlane for Plane<T> {
     }
 }
 
-impl<T: DriverPlane> ModeObject for Plane<T> {
+// SAFETY: We don't expose Plane<T> to users before `base` is initialized in ::new(), so
+// `raw_mode_obj` always returns a valid pointer to a bindings::drm_mode_object.
+unsafe impl<T: DriverPlane> ModeObject for Plane<T> {
     type Driver = T::Driver;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {
@@ -389,7 +400,11 @@ pub struct OpaquePlane<T: KmsDriver> {
 
 impl<T: KmsDriver> Sealed for OpaquePlane<T> {}
 
-impl<T: KmsDriver> AsRawPlane for OpaquePlane<T> {
+// SAFETY:
+// * Via our type variants our data layout is identical to `drm_plane`
+// * Since we don't expose `plane` to users before it has been initialized, this and our data
+//   layout ensure that `as_raw()` always returns a valid pointer to a `drm_plane`.
+unsafe impl<T: KmsDriver> AsRawPlane for OpaquePlane<T> {
     type State = OpaquePlaneState<T>;
 
     fn as_raw(&self) -> *mut bindings::drm_plane {
@@ -402,7 +417,10 @@ impl<T: KmsDriver> AsRawPlane for OpaquePlane<T> {
     }
 }
 
-impl<T: KmsDriver> ModeObject for OpaquePlane<T> {
+// SAFETY: We don't expose OpaquePlane<T> to users before `base` is initialized in
+// Plane::<T>::new(), so `raw_mode_obj` always returns a valid pointer to a
+// bindings::drm_mode_object.
+unsafe impl<T: KmsDriver> ModeObject for OpaquePlane<T> {
     type Driver = T;
 
     fn drm_dev(&self) -> &Device<Self::Driver> {