Re: [PATCH RFC 02/18] rust: drm: Add Device and Driver abstractions

Boqun Feng
2023-03-11T05:41:20Z

On Tue, Mar 07, 2023 at 11:25:27PM +0900, Asahi Lina wrote:
> Add the initial abstractions for DRM drivers and devices. These go
> together in one commit since they are fairly tightly coupled types.
>
> A few things have been stubbed out, to be implemented as further bits of
> the DRM subsystem are introduced.
>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
> rust/bindings/bindings_helper.h | 3 +
> rust/kernel/drm/device.rs | 76 +++++++++
> rust/kernel/drm/drv.rs | 339 ++++++++++++++++++++++++++++++++++++++++
> rust/kernel/drm/mod.rs | 2 +
> 4 files changed, 420 insertions(+)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 2687bef1676f..2a999138c4ae 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -6,10 +6,13 @@
> * Sorted alphabetically.
> */
>
> +#include <drm/drm_device.h>
> +#include <drm/drm_drv.h>
> #include <drm/drm_ioctl.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/dma-mapping.h>
> +#include <linux/fs.h>
> #include <linux/ioctl.h>
> #include <linux/io-pgtable.h>
> #include <linux/ktime.h>
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> new file mode 100644
> index 000000000000..6007f941137a
> --- /dev/null
> +++ b/rust/kernel/drm/device.rs
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +//! DRM device.
> +//!
> +//! C header: [`include/linux/drm/drm_device.h`](../../../../include/linux/drm/drm_device.h)
> +
> +use crate::{bindings, device, drm, types::ForeignOwnable};
> +use core::marker::PhantomData;
> +
> +/// Represents a reference to a DRM device. The device is reference-counted and is guaranteed to
> +/// not be dropped while this object is alive.
> +pub struct Device<T: drm::drv::Driver> {
> + // Type invariant: ptr must be a valid and initialized drm_device,
> + // and this value must either own a reference to it or the caller
> + // must ensure that it is never dropped if the reference is borrowed.
> + pub(super) ptr: *mut bindings::drm_device,
> + _p: PhantomData<T>,
> +}
> +
> +impl<T: drm::drv::Driver> Device<T> {
> + // Not intended to be called externally, except via declare_drm_ioctls!()
> + #[doc(hidden)]
> + pub unsafe fn from_raw(raw: *mut bindings::drm_device) -> Device<T> {
> + Device {
> + ptr: raw,
> + _p: PhantomData,
> + }
> + }
> +
> + #[allow(dead_code)]
> + pub(crate) fn raw(&self) -> *const bindings::drm_device {
> + self.ptr
> + }
> +
> + pub(crate) fn raw_mut(&mut self) -> *mut bindings::drm_device {
> + self.ptr
> + }

Since you can always get a *mut bindings::drm_device safely from

a.raw() as *mut _

, this mutable version seems unnecesarry to me. In other words, no way
to prevent getting a *mut bindings::drm_device from only &Device.

Regards,
Boqun

> +
> + /// Returns a borrowed reference to the user data associated with this Device.
> + pub fn data(&self) -> <T::Data as ForeignOwnable>::Borrowed<'_> {
> + unsafe { T::Data::borrow((*self.ptr).dev_private) }
> + }
> +}
> + …