diff mbox series

[RFC,3/8] rust: drm: Add Device and Driver abstractions

Message ID 20240520172059.181256-4-dakr@redhat.com (mailing list archive)
State New, archived
Headers show
Series DRM Rust abstractions and Nova | expand

Commit Message

Danilo Krummrich May 20, 2024, 5:20 p.m. UTC
From: Asahi Lina <lina@asahilina.net>

Add abstractions for DRM drivers and devices. These go together in one
commit since both 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>
Co-developed-by: Danilo Krummrich <dakr@redhat.com>
Signed-off-by: Danilo Krummrich <dakr@redhat.com>
---
 rust/bindings/bindings_helper.h |   2 +
 rust/kernel/drm/device.rs       |  87 +++++++++
 rust/kernel/drm/drv.rs          | 318 ++++++++++++++++++++++++++++++++
 rust/kernel/drm/mod.rs          |   2 +
 4 files changed, 409 insertions(+)
 create mode 100644 rust/kernel/drm/device.rs
 create mode 100644 rust/kernel/drm/drv.rs

Comments

Rob Herring May 21, 2024, 9:23 p.m. UTC | #1
On Mon, May 20, 2024 at 07:20:50PM +0200, Danilo Krummrich wrote:
> From: Asahi Lina <lina@asahilina.net>
> 
> Add abstractions for DRM drivers and devices. These go together in one
> commit since both 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>
> Co-developed-by: Danilo Krummrich <dakr@redhat.com>
> Signed-off-by: Danilo Krummrich <dakr@redhat.com>
> ---
>  rust/bindings/bindings_helper.h |   2 +
>  rust/kernel/drm/device.rs       |  87 +++++++++
>  rust/kernel/drm/drv.rs          | 318 ++++++++++++++++++++++++++++++++
>  rust/kernel/drm/mod.rs          |   2 +
>  4 files changed, 409 insertions(+)
>  create mode 100644 rust/kernel/drm/device.rs
>  create mode 100644 rust/kernel/drm/drv.rs

[...]

> diff --git a/rust/kernel/drm/drv.rs b/rust/kernel/drm/drv.rs
> new file mode 100644
> index 000000000000..5dd8f3f8df7c
> --- /dev/null
> +++ b/rust/kernel/drm/drv.rs
> @@ -0,0 +1,318 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +//! DRM driver core.
> +//!
> +//! C header: [`include/linux/drm/drm_drv.h`](../../../../include/linux/drm/drm_drv.h)
> +
> +use crate::{
> +    alloc::flags::*,
> +    bindings, device, drm,
> +    error::code::*,
> +    error::{Error, Result},
> +    prelude::*,
> +    private::Sealed,
> +    str::CStr,
> +    types::{ARef, ForeignOwnable},
> +    ThisModule,
> +};
> +use core::{
> +    marker::{PhantomData, PhantomPinned},
> +    pin::Pin,
> +};
> +use macros::vtable;
> +
> +/// Driver use the GEM memory manager. This should be set for all modern drivers.
> +pub const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
> +/// Driver supports mode setting interfaces (KMS).
> +pub const FEAT_MODESET: u32 = bindings::drm_driver_feature_DRIVER_MODESET;
> +/// Driver supports dedicated render nodes.
> +pub const FEAT_RENDER: u32 = bindings::drm_driver_feature_DRIVER_RENDER;
> +/// Driver supports the full atomic modesetting userspace API.
> +///
> +/// Drivers which only use atomic internally, but do not support the full userspace API (e.g. not
> +/// all properties converted to atomic, or multi-plane updates are not guaranteed to be tear-free)
> +/// should not set this flag.
> +pub const FEAT_ATOMIC: u32 = bindings::drm_driver_feature_DRIVER_ATOMIC;
> +/// Driver supports DRM sync objects for explicit synchronization of command submission.
> +pub const FEAT_SYNCOBJ: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ;
> +/// Driver supports the timeline flavor of DRM sync objects for explicit synchronization of command
> +/// submission.
> +pub const FEAT_SYNCOBJ_TIMELINE: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ_TIMELINE;

This is missing an entry for DRIVER_GEM_GPUVA. And some others perhaps. 
I suppose some are legacy which won't be needed any time soon if ever. 
Not sure if you intend for this to be complete, or you are just adding 
what you are using? Only FEAT_GEM is used by nova ATM.

Rob
Danilo Krummrich May 27, 2024, 7:26 p.m. UTC | #2
On Tue, May 21, 2024 at 04:23:33PM -0500, Rob Herring wrote:
> On Mon, May 20, 2024 at 07:20:50PM +0200, Danilo Krummrich wrote:
> > From: Asahi Lina <lina@asahilina.net>
> > 
> > Add abstractions for DRM drivers and devices. These go together in one
> > commit since both 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>
> > Co-developed-by: Danilo Krummrich <dakr@redhat.com>
> > Signed-off-by: Danilo Krummrich <dakr@redhat.com>
> > ---
> >  rust/bindings/bindings_helper.h |   2 +
> >  rust/kernel/drm/device.rs       |  87 +++++++++
> >  rust/kernel/drm/drv.rs          | 318 ++++++++++++++++++++++++++++++++
> >  rust/kernel/drm/mod.rs          |   2 +
> >  4 files changed, 409 insertions(+)
> >  create mode 100644 rust/kernel/drm/device.rs
> >  create mode 100644 rust/kernel/drm/drv.rs
> 
> [...]
> 
> > diff --git a/rust/kernel/drm/drv.rs b/rust/kernel/drm/drv.rs
> > new file mode 100644
> > index 000000000000..5dd8f3f8df7c
> > --- /dev/null
> > +++ b/rust/kernel/drm/drv.rs
> > @@ -0,0 +1,318 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > +
> > +//! DRM driver core.
> > +//!
> > +//! C header: [`include/linux/drm/drm_drv.h`](../../../../include/linux/drm/drm_drv.h)
> > +
> > +use crate::{
> > +    alloc::flags::*,
> > +    bindings, device, drm,
> > +    error::code::*,
> > +    error::{Error, Result},
> > +    prelude::*,
> > +    private::Sealed,
> > +    str::CStr,
> > +    types::{ARef, ForeignOwnable},
> > +    ThisModule,
> > +};
> > +use core::{
> > +    marker::{PhantomData, PhantomPinned},
> > +    pin::Pin,
> > +};
> > +use macros::vtable;
> > +
> > +/// Driver use the GEM memory manager. This should be set for all modern drivers.
> > +pub const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
> > +/// Driver supports mode setting interfaces (KMS).
> > +pub const FEAT_MODESET: u32 = bindings::drm_driver_feature_DRIVER_MODESET;
> > +/// Driver supports dedicated render nodes.
> > +pub const FEAT_RENDER: u32 = bindings::drm_driver_feature_DRIVER_RENDER;
> > +/// Driver supports the full atomic modesetting userspace API.
> > +///
> > +/// Drivers which only use atomic internally, but do not support the full userspace API (e.g. not
> > +/// all properties converted to atomic, or multi-plane updates are not guaranteed to be tear-free)
> > +/// should not set this flag.
> > +pub const FEAT_ATOMIC: u32 = bindings::drm_driver_feature_DRIVER_ATOMIC;
> > +/// Driver supports DRM sync objects for explicit synchronization of command submission.
> > +pub const FEAT_SYNCOBJ: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ;
> > +/// Driver supports the timeline flavor of DRM sync objects for explicit synchronization of command
> > +/// submission.
> > +pub const FEAT_SYNCOBJ_TIMELINE: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ_TIMELINE;
> 
> This is missing an entry for DRIVER_GEM_GPUVA. And some others perhaps. 
> I suppose some are legacy which won't be needed any time soon if ever. 
> Not sure if you intend for this to be complete, or you are just adding 
> what you are using? Only FEAT_GEM is used by nova ATM.

Good catch, I think we should add all of them, except the legacy ones. If no one
disagrees, I will fix this in v2.

> 
> Rob
>
Asahi Lina June 9, 2024, 5:15 a.m. UTC | #3
On 5/22/24 6:23 AM, Rob Herring wrote:
> On Mon, May 20, 2024 at 07:20:50PM +0200, Danilo Krummrich wrote:
>> From: Asahi Lina <lina@asahilina.net>
>>
>> Add abstractions for DRM drivers and devices. These go together in one
>> commit since both 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>
>> Co-developed-by: Danilo Krummrich <dakr@redhat.com>
>> Signed-off-by: Danilo Krummrich <dakr@redhat.com>
>> ---
>>  rust/bindings/bindings_helper.h |   2 +
>>  rust/kernel/drm/device.rs       |  87 +++++++++
>>  rust/kernel/drm/drv.rs          | 318 ++++++++++++++++++++++++++++++++
>>  rust/kernel/drm/mod.rs          |   2 +
>>  4 files changed, 409 insertions(+)
>>  create mode 100644 rust/kernel/drm/device.rs
>>  create mode 100644 rust/kernel/drm/drv.rs
> 
> [...]
> 
>> diff --git a/rust/kernel/drm/drv.rs b/rust/kernel/drm/drv.rs
>> new file mode 100644
>> index 000000000000..5dd8f3f8df7c
>> --- /dev/null
>> +++ b/rust/kernel/drm/drv.rs
>> @@ -0,0 +1,318 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR MIT
>> +
>> +//! DRM driver core.
>> +//!
>> +//! C header: [`include/linux/drm/drm_drv.h`](../../../../include/linux/drm/drm_drv.h)
>> +
>> +use crate::{
>> +    alloc::flags::*,
>> +    bindings, device, drm,
>> +    error::code::*,
>> +    error::{Error, Result},
>> +    prelude::*,
>> +    private::Sealed,
>> +    str::CStr,
>> +    types::{ARef, ForeignOwnable},
>> +    ThisModule,
>> +};
>> +use core::{
>> +    marker::{PhantomData, PhantomPinned},
>> +    pin::Pin,
>> +};
>> +use macros::vtable;
>> +
>> +/// Driver use the GEM memory manager. This should be set for all modern drivers.
>> +pub const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
>> +/// Driver supports mode setting interfaces (KMS).
>> +pub const FEAT_MODESET: u32 = bindings::drm_driver_feature_DRIVER_MODESET;
>> +/// Driver supports dedicated render nodes.
>> +pub const FEAT_RENDER: u32 = bindings::drm_driver_feature_DRIVER_RENDER;
>> +/// Driver supports the full atomic modesetting userspace API.
>> +///
>> +/// Drivers which only use atomic internally, but do not support the full userspace API (e.g. not
>> +/// all properties converted to atomic, or multi-plane updates are not guaranteed to be tear-free)
>> +/// should not set this flag.
>> +pub const FEAT_ATOMIC: u32 = bindings::drm_driver_feature_DRIVER_ATOMIC;
>> +/// Driver supports DRM sync objects for explicit synchronization of command submission.
>> +pub const FEAT_SYNCOBJ: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ;
>> +/// Driver supports the timeline flavor of DRM sync objects for explicit synchronization of command
>> +/// submission.
>> +pub const FEAT_SYNCOBJ_TIMELINE: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ_TIMELINE;
> 
> This is missing an entry for DRIVER_GEM_GPUVA. And some others perhaps. 
> I suppose some are legacy which won't be needed any time soon if ever. 
> Not sure if you intend for this to be complete, or you are just adding 
> what you are using? Only FEAT_GEM is used by nova ATM.
> 

This was developed before DRIVER_GEM_GPUVA existed ^^

I have this in my branch since I'm using the GPUVA manager now. Danilo,
what tree are you using for this submission? It would be good to
coordinate this and try to keep the WIP branches from diverging too much...

That said, I don't think there's reason to support all features unless
we expect new drivers to actually use them. The goal of the abstractions
is to serve the drivers that will use them, and to evolve together with
them and any newer drivers, not to attempt to be feature-complete from
the get go (it's very difficult to evaluate an abstraction if it has no
users!). In general my approach when writing them was to abstract what I
need and add "obvious" extra trivial features that didn't require much
thought even if I wasn't using them, but otherwise not attempt to
comprehensively cover everything.

~~ Lina
Danilo Krummrich June 9, 2024, 2:18 p.m. UTC | #4
Hi Lina,

Welcome back!

On Sun, Jun 09, 2024 at 02:15:57PM +0900, Asahi Lina wrote:
> 
> 
> On 5/22/24 6:23 AM, Rob Herring wrote:
> > On Mon, May 20, 2024 at 07:20:50PM +0200, Danilo Krummrich wrote:
> >> From: Asahi Lina <lina@asahilina.net>
> > This is missing an entry for DRIVER_GEM_GPUVA. And some others perhaps. 
> > I suppose some are legacy which won't be needed any time soon if ever. 
> > Not sure if you intend for this to be complete, or you are just adding 
> > what you are using? Only FEAT_GEM is used by nova ATM.
> > 
> 
> This was developed before DRIVER_GEM_GPUVA existed ^^
> 
> I have this in my branch since I'm using the GPUVA manager now. Danilo,
> what tree are you using for this submission? It would be good to
> coordinate this and try to keep the WIP branches from diverging too much...

Trying to prevent things from diverging was one of my main objectives when I
started those efforts a couple of month ago (I also sent you a mail for that
purpose back then).

I am maintaining a couple of branches:

In the RfL repository [1]:

  - staging/rust-device [2] (Device / driver, devres infrastructure)
  - staging/dev [3] (common branch containing all the Rust infrastructure I'm
    currently upstreaming, including PCI and firmware abstractions)

In the drm-misc repository [4]:
  - topic/rust-drm [5] (all the DRM abstractions)

In the Nova repository [6]
  - nova-next [7] (the Nova stub driver making use of everything)

I regularly rebase those branches and keep them up to date with improvements and
feedback from the mailing list.

The device / driver and PCI abstractions are getting most of my attention
currently, but there are some recent changes (besides some minor ones) on the
DRM abstractions I plan to send in a v2 as well:

- static const allocation of driver and fops structures
- rework of the `Registration` type to use `Devres` and combine the new() and
  register() methods to register in new() already

There is also some more information regarding those branches in the cover
letters of the series and the links included there.

[1] https://github.com/Rust-for-Linux/linux
[2] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device
[3] https://github.com/Rust-for-Linux/linux/tree/staging/dev
[4] https://gitlab.freedesktop.org/drm/misc/kernel
[5] https://gitlab.freedesktop.org/drm/misc/kernel/-/tree/topic/rust-drm
[6] https://gitlab.freedesktop.org/drm/nova
[7] https://gitlab.freedesktop.org/drm/nova/-/tree/nova-next

> 
> That said, I don't think there's reason to support all features unless
> we expect new drivers to actually use them. The goal of the abstractions
> is to serve the drivers that will use them, and to evolve together with
> them and any newer drivers, not to attempt to be feature-complete from
> the get go (it's very difficult to evaluate an abstraction if it has no
> users!). In general my approach when writing them was to abstract what I
> need and add "obvious" extra trivial features that didn't require much
> thought even if I wasn't using them, but otherwise not attempt to
> comprehensively cover everything.

Fully agree, I think the point here only was whether we want to list all the
feature flags in the abstraction already. Which I think is something we can do.
However, I'm also find with only listing the ones we actually use and keep
adding further ones subsequently. It just shouldn't be random.

- Danilo

> 
> ~~ Lina
>
Rob Herring June 11, 2024, 3:46 p.m. UTC | #5
On Sat, Jun 8, 2024 at 11:16 PM Asahi Lina <lina@asahilina.net> wrote:
>
>
>
> On 5/22/24 6:23 AM, Rob Herring wrote:
> > On Mon, May 20, 2024 at 07:20:50PM +0200, Danilo Krummrich wrote:
> >> From: Asahi Lina <lina@asahilina.net>
> >>
> >> Add abstractions for DRM drivers and devices. These go together in one
> >> commit since both 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>
> >> Co-developed-by: Danilo Krummrich <dakr@redhat.com>
> >> Signed-off-by: Danilo Krummrich <dakr@redhat.com>
> >> ---
> >>  rust/bindings/bindings_helper.h |   2 +
> >>  rust/kernel/drm/device.rs       |  87 +++++++++
> >>  rust/kernel/drm/drv.rs          | 318 ++++++++++++++++++++++++++++++++
> >>  rust/kernel/drm/mod.rs          |   2 +
> >>  4 files changed, 409 insertions(+)
> >>  create mode 100644 rust/kernel/drm/device.rs
> >>  create mode 100644 rust/kernel/drm/drv.rs
> >
> > [...]
> >
> >> diff --git a/rust/kernel/drm/drv.rs b/rust/kernel/drm/drv.rs
> >> new file mode 100644
> >> index 000000000000..5dd8f3f8df7c
> >> --- /dev/null
> >> +++ b/rust/kernel/drm/drv.rs
> >> @@ -0,0 +1,318 @@
> >> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> >> +
> >> +//! DRM driver core.
> >> +//!
> >> +//! C header: [`include/linux/drm/drm_drv.h`](../../../../include/linux/drm/drm_drv.h)
> >> +
> >> +use crate::{
> >> +    alloc::flags::*,
> >> +    bindings, device, drm,
> >> +    error::code::*,
> >> +    error::{Error, Result},
> >> +    prelude::*,
> >> +    private::Sealed,
> >> +    str::CStr,
> >> +    types::{ARef, ForeignOwnable},
> >> +    ThisModule,
> >> +};
> >> +use core::{
> >> +    marker::{PhantomData, PhantomPinned},
> >> +    pin::Pin,
> >> +};
> >> +use macros::vtable;
> >> +
> >> +/// Driver use the GEM memory manager. This should be set for all modern drivers.
> >> +pub const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
> >> +/// Driver supports mode setting interfaces (KMS).
> >> +pub const FEAT_MODESET: u32 = bindings::drm_driver_feature_DRIVER_MODESET;
> >> +/// Driver supports dedicated render nodes.
> >> +pub const FEAT_RENDER: u32 = bindings::drm_driver_feature_DRIVER_RENDER;
> >> +/// Driver supports the full atomic modesetting userspace API.
> >> +///
> >> +/// Drivers which only use atomic internally, but do not support the full userspace API (e.g. not
> >> +/// all properties converted to atomic, or multi-plane updates are not guaranteed to be tear-free)
> >> +/// should not set this flag.
> >> +pub const FEAT_ATOMIC: u32 = bindings::drm_driver_feature_DRIVER_ATOMIC;
> >> +/// Driver supports DRM sync objects for explicit synchronization of command submission.
> >> +pub const FEAT_SYNCOBJ: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ;
> >> +/// Driver supports the timeline flavor of DRM sync objects for explicit synchronization of command
> >> +/// submission.
> >> +pub const FEAT_SYNCOBJ_TIMELINE: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ_TIMELINE;
> >
> > This is missing an entry for DRIVER_GEM_GPUVA. And some others perhaps.
> > I suppose some are legacy which won't be needed any time soon if ever.
> > Not sure if you intend for this to be complete, or you are just adding
> > what you are using? Only FEAT_GEM is used by nova ATM.
> >
>
> This was developed before DRIVER_GEM_GPUVA existed ^^
>
> I have this in my branch since I'm using the GPUVA manager now.

TBC, I'm using it as well, not just providing drive-by comments. I'm
starting to look at converting panthor to rust.

> Danilo,
> what tree are you using for this submission? It would be good to
> coordinate this and try to keep the WIP branches from diverging too much...

Yes, please!

Besides asahi of course, there's several people[1][2][3] using the
platform and DT abstractions and we're all rebasing those. I'd
volunteer to maintain, but I don't know rust well enough yet to even
be dangerous... :)

Rob

[1] https://lore.kernel.org/all/cover.1717750631.git.viresh.kumar@linaro.org/
[2] https://lore.kernel.org/all/20240322221305.1403600-1-lyude@redhat.com/
[3] https://lore.kernel.org/all/CAPFo5VLoYGq_OgC5dqcueTyymuSCsLpjasLZnKO1jpY6gV7s2g@mail.gmail.com/
diff mbox series

Patch

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 14188034285a..831fbfe03a47 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -6,6 +6,8 @@ 
  * Sorted alphabetically.
  */
 
+#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_ioctl.h>
 #include <kunit/test.h>
 #include <linux/errname.h>
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
new file mode 100644
index 000000000000..f72bab8dd42d
--- /dev/null
+++ b/rust/kernel/drm/device.rs
@@ -0,0 +1,87 @@ 
+// 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,
+    error::code::*,
+    error::from_err_ptr,
+    error::Result,
+    types::{ARef, AlwaysRefCounted, ForeignOwnable, Opaque},
+};
+use alloc::boxed::Box;
+use core::{ffi::c_void, marker::PhantomData, pin::Pin, ptr::NonNull};
+
+/// A typed DRM device with a specific driver. The device is always reference-counted.
+#[repr(transparent)]
+pub struct Device<T: drm::drv::Driver>(Opaque<bindings::drm_device>, PhantomData<T>);
+
+impl<T: drm::drv::Driver> Device<T> {
+    pub(crate) fn new(
+        dev: &device::Device,
+        vtable: &Pin<Box<bindings::drm_driver>>,
+    ) -> Result<ARef<Self>> {
+        let raw_drm = unsafe { bindings::drm_dev_alloc(&**vtable, dev.as_raw()) };
+        let raw_drm = NonNull::new(from_err_ptr(raw_drm)? as *mut _).ok_or(ENOMEM)?;
+
+        // SAFETY: The reference count is one, and now we take ownership of that reference as a
+        // drm::device::Device.
+        Ok(unsafe { ARef::from_raw(raw_drm) })
+    }
+
+    pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
+        self.0.get()
+    }
+
+    // Not intended to be called externally, except via declare_drm_ioctls!()
+    #[doc(hidden)]
+    pub unsafe fn borrow<'a>(raw: *const bindings::drm_device) -> &'a Self {
+        unsafe { &*(raw as *const Self) }
+    }
+
+    pub(crate) fn raw_data(&self) -> *const c_void {
+        // SAFETY: `self` is guaranteed to hold a valid `bindings::drm_device` pointer.
+        unsafe { *self.as_raw() }.dev_private
+    }
+
+    // SAFETY: Must be called only once after device creation.
+    pub(crate) unsafe fn set_raw_data(&self, ptr: *const c_void) {
+        // SAFETY: Safe as by the safety precondition.
+        unsafe { &mut *self.as_raw() }.dev_private = ptr as _;
+    }
+
+    /// Returns a borrowed reference to the user data associated with this Device.
+    pub fn data(&self) -> Option<<T::Data as ForeignOwnable>::Borrowed<'_>> {
+        let dev_private = self.raw_data();
+
+        if dev_private.is_null() {
+            None
+        } else {
+            // SAFETY: `dev_private` is NULL before the DRM device is registered; after the DRM
+            // device has been registered dev_private is guaranteed to be valid.
+            Some(unsafe { T::Data::borrow(dev_private) })
+        }
+    }
+}
+
+// SAFETY: DRM device objects are always reference counted and the get/put functions
+// satisfy the requirements.
+unsafe impl<T: drm::drv::Driver> AlwaysRefCounted for Device<T> {
+    fn inc_ref(&self) {
+        unsafe { bindings::drm_dev_get(&self.as_raw() as *const _ as *mut _) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The Device<T> type has the same layout as drm_device, so we can just cast.
+        unsafe { bindings::drm_dev_put(obj.as_ptr() as *mut _) };
+    }
+}
+
+// SAFETY: `Device` only holds a pointer to a C device, which is safe to be used from any thread.
+unsafe impl<T: drm::drv::Driver> Send for Device<T> {}
+
+// SAFETY: `Device` only holds a pointer to a C device, references to which are safe to be used
+// from any thread.
+unsafe impl<T: drm::drv::Driver> Sync for Device<T> {}
diff --git a/rust/kernel/drm/drv.rs b/rust/kernel/drm/drv.rs
new file mode 100644
index 000000000000..5dd8f3f8df7c
--- /dev/null
+++ b/rust/kernel/drm/drv.rs
@@ -0,0 +1,318 @@ 
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+
+//! DRM driver core.
+//!
+//! C header: [`include/linux/drm/drm_drv.h`](../../../../include/linux/drm/drm_drv.h)
+
+use crate::{
+    alloc::flags::*,
+    bindings, device, drm,
+    error::code::*,
+    error::{Error, Result},
+    prelude::*,
+    private::Sealed,
+    str::CStr,
+    types::{ARef, ForeignOwnable},
+    ThisModule,
+};
+use core::{
+    marker::{PhantomData, PhantomPinned},
+    pin::Pin,
+};
+use macros::vtable;
+
+/// Driver use the GEM memory manager. This should be set for all modern drivers.
+pub const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
+/// Driver supports mode setting interfaces (KMS).
+pub const FEAT_MODESET: u32 = bindings::drm_driver_feature_DRIVER_MODESET;
+/// Driver supports dedicated render nodes.
+pub const FEAT_RENDER: u32 = bindings::drm_driver_feature_DRIVER_RENDER;
+/// Driver supports the full atomic modesetting userspace API.
+///
+/// Drivers which only use atomic internally, but do not support the full userspace API (e.g. not
+/// all properties converted to atomic, or multi-plane updates are not guaranteed to be tear-free)
+/// should not set this flag.
+pub const FEAT_ATOMIC: u32 = bindings::drm_driver_feature_DRIVER_ATOMIC;
+/// Driver supports DRM sync objects for explicit synchronization of command submission.
+pub const FEAT_SYNCOBJ: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ;
+/// Driver supports the timeline flavor of DRM sync objects for explicit synchronization of command
+/// submission.
+pub const FEAT_SYNCOBJ_TIMELINE: u32 = bindings::drm_driver_feature_DRIVER_SYNCOBJ_TIMELINE;
+
+/// Information data for a DRM Driver.
+pub struct DriverInfo {
+    /// Driver major version.
+    pub major: i32,
+    /// Driver minor version.
+    pub minor: i32,
+    /// Driver patchlevel version.
+    pub patchlevel: i32,
+    /// Driver name.
+    pub name: &'static CStr,
+    /// Driver description.
+    pub desc: &'static CStr,
+    /// Driver date.
+    pub date: &'static CStr,
+}
+
+/// Internal memory management operation set, normally created by memory managers (e.g. GEM).
+///
+/// See `kernel::drm::gem` and `kernel::drm::gem::shmem`.
+pub struct AllocOps {
+    pub(crate) gem_create_object: Option<
+        unsafe extern "C" fn(
+            dev: *mut bindings::drm_device,
+            size: usize,
+        ) -> *mut bindings::drm_gem_object,
+    >,
+    pub(crate) prime_handle_to_fd: Option<
+        unsafe extern "C" fn(
+            dev: *mut bindings::drm_device,
+            file_priv: *mut bindings::drm_file,
+            handle: u32,
+            flags: u32,
+            prime_fd: *mut core::ffi::c_int,
+        ) -> core::ffi::c_int,
+    >,
+    pub(crate) prime_fd_to_handle: Option<
+        unsafe extern "C" fn(
+            dev: *mut bindings::drm_device,
+            file_priv: *mut bindings::drm_file,
+            prime_fd: core::ffi::c_int,
+            handle: *mut u32,
+        ) -> core::ffi::c_int,
+    >,
+    pub(crate) gem_prime_import: Option<
+        unsafe extern "C" fn(
+            dev: *mut bindings::drm_device,
+            dma_buf: *mut bindings::dma_buf,
+        ) -> *mut bindings::drm_gem_object,
+    >,
+    pub(crate) gem_prime_import_sg_table: Option<
+        unsafe extern "C" fn(
+            dev: *mut bindings::drm_device,
+            attach: *mut bindings::dma_buf_attachment,
+            sgt: *mut bindings::sg_table,
+        ) -> *mut bindings::drm_gem_object,
+    >,
+    pub(crate) dumb_create: Option<
+        unsafe extern "C" fn(
+            file_priv: *mut bindings::drm_file,
+            dev: *mut bindings::drm_device,
+            args: *mut bindings::drm_mode_create_dumb,
+        ) -> core::ffi::c_int,
+    >,
+    pub(crate) dumb_map_offset: Option<
+        unsafe extern "C" fn(
+            file_priv: *mut bindings::drm_file,
+            dev: *mut bindings::drm_device,
+            handle: u32,
+            offset: *mut u64,
+        ) -> core::ffi::c_int,
+    >,
+}
+
+/// Trait for memory manager implementations. Implemented internally.
+pub trait AllocImpl: Sealed {
+    /// The C callback operations for this memory manager.
+    const ALLOC_OPS: AllocOps;
+}
+
+/// A DRM driver implementation.
+#[vtable]
+pub trait Driver {
+    /// Context data associated with the DRM driver
+    ///
+    /// Determines the type of the context data passed to each of the methods of the trait.
+    type Data: ForeignOwnable + Sync + Send;
+
+    /// The type used to manage memory for this driver.
+    ///
+    /// Should be either `drm::gem::Object<T>` or `drm::gem::shmem::Object<T>`.
+    type Object: AllocImpl;
+
+    /// Driver metadata
+    const INFO: DriverInfo;
+
+    /// Feature flags
+    const FEATURES: u32;
+
+    /// IOCTL list. See `kernel::drm::ioctl::declare_drm_ioctls!{}`.
+    const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor];
+}
+
+/// A registration of a DRM device
+///
+/// # Invariants:
+///
+/// drm is always a valid pointer to an allocated drm_device
+pub struct Registration<T: Driver> {
+    drm: ARef<drm::device::Device<T>>,
+    registered: bool,
+    fops: bindings::file_operations,
+    vtable: Pin<Box<bindings::drm_driver>>,
+    _p: PhantomData<T>,
+    _pin: PhantomPinned,
+}
+
+#[cfg(CONFIG_DRM_LEGACY)]
+macro_rules! drm_legacy_fields {
+    ( $($field:ident: $val:expr),* $(,)? ) => {
+        bindings::drm_driver {
+            $( $field: $val ),*,
+            firstopen: None,
+            preclose: None,
+            dma_ioctl: None,
+            dma_quiescent: None,
+            context_dtor: None,
+            irq_handler: None,
+            irq_preinstall: None,
+            irq_postinstall: None,
+            irq_uninstall: None,
+            get_vblank_counter: None,
+            enable_vblank: None,
+            disable_vblank: None,
+            dev_priv_size: 0,
+        }
+    }
+}
+
+#[cfg(not(CONFIG_DRM_LEGACY))]
+macro_rules! drm_legacy_fields {
+    ( $($field:ident: $val:expr),* $(,)? ) => {
+        bindings::drm_driver {
+            $( $field: $val ),*
+        }
+    }
+}
+
+/// Registers a DRM device with the rest of the kernel.
+///
+/// It automatically picks up THIS_MODULE.
+#[allow(clippy::crate_in_macro_def)]
+#[macro_export]
+macro_rules! drm_device_register {
+    ($reg:expr, $data:expr, $flags:expr $(,)?) => {{
+        $crate::drm::drv::Registration::register($reg, $data, $flags, &crate::THIS_MODULE)
+    }};
+}
+
+impl<T: Driver> Registration<T> {
+    const VTABLE: bindings::drm_driver = drm_legacy_fields! {
+        load: None,
+        open: None, // TODO: File abstraction
+        postclose: None, // TODO: File abstraction
+        lastclose: None,
+        unload: None,
+        release: None,
+        master_set: None,
+        master_drop: None,
+        debugfs_init: None,
+        gem_create_object: T::Object::ALLOC_OPS.gem_create_object,
+        prime_handle_to_fd: T::Object::ALLOC_OPS.prime_handle_to_fd,
+        prime_fd_to_handle: T::Object::ALLOC_OPS.prime_fd_to_handle,
+        gem_prime_import: T::Object::ALLOC_OPS.gem_prime_import,
+        gem_prime_import_sg_table: T::Object::ALLOC_OPS.gem_prime_import_sg_table,
+        dumb_create: T::Object::ALLOC_OPS.dumb_create,
+        dumb_map_offset: T::Object::ALLOC_OPS.dumb_map_offset,
+        show_fdinfo: None,
+
+        major: T::INFO.major,
+        minor: T::INFO.minor,
+        patchlevel: T::INFO.patchlevel,
+        name: T::INFO.name.as_char_ptr() as *mut _,
+        desc: T::INFO.desc.as_char_ptr() as *mut _,
+        date: T::INFO.date.as_char_ptr() as *mut _,
+
+        driver_features: T::FEATURES,
+        ioctls: T::IOCTLS.as_ptr(),
+        num_ioctls: T::IOCTLS.len() as i32,
+        fops: core::ptr::null_mut(),
+    };
+
+    /// Creates a new [`Registration`] but does not register it yet.
+    ///
+    /// It is allowed to move.
+    pub fn new(parent: &device::Device) -> Result<Self> {
+        let vtable = Pin::new(Box::new(Self::VTABLE, GFP_KERNEL)?);
+
+        Ok(Self {
+            drm: drm::device::Device::new(parent, &vtable)?,
+            registered: false,
+            vtable,
+            fops: Default::default(), // TODO: GEM abstraction
+            _pin: PhantomPinned,
+            _p: PhantomData,
+        })
+    }
+
+    /// Registers a DRM device with the rest of the kernel.
+    ///
+    /// Users are encouraged to use the [`drm_device_register!()`] macro because it automatically
+    /// picks up the current module.
+    pub fn register(
+        self: Pin<&mut Self>,
+        data: T::Data,
+        flags: usize,
+        module: &'static ThisModule,
+    ) -> Result {
+        if self.registered {
+            // Already registered.
+            return Err(EINVAL);
+        }
+
+        // SAFETY: We never move out of `this`.
+        let this = unsafe { self.get_unchecked_mut() };
+        let data_pointer = <T::Data as ForeignOwnable>::into_foreign(data);
+
+        // SAFETY: This is the only code touching a device' private data pointer.
+        unsafe { this.drm.set_raw_data(data_pointer) };
+
+        this.fops.owner = module.0;
+        this.vtable.fops = &this.fops;
+
+        // SAFETY: The device is now initialized and ready to be registered.
+        let ret = unsafe { bindings::drm_dev_register(this.drm.as_raw(), flags as u64) };
+        if ret < 0 {
+            // SAFETY: `data_pointer` was returned by `into_foreign` above.
+            unsafe { T::Data::from_foreign(data_pointer) };
+            return Err(Error::from_errno(ret));
+        }
+
+        this.registered = true;
+        Ok(())
+    }
+
+    /// Returns a reference to the `Device` instance for this registration.
+    pub fn device(&self) -> &drm::device::Device<T> {
+        &self.drm
+    }
+}
+
+// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between threads
+// or CPUs, so it is safe to share it.
+unsafe impl<T: Driver> Sync for Registration<T> {}
+
+// SAFETY: Registration with and unregistration from the drm subsystem can happen from any thread.
+// Additionally, `T::Data` (which is dropped during unregistration) is `Send`, so it is ok to move
+// `Registration` to different threads.
+#[allow(clippy::non_send_fields_in_send_ty)]
+unsafe impl<T: Driver> Send for Registration<T> {}
+
+impl<T: Driver> Drop for Registration<T> {
+    /// Removes the registration from the kernel if it has completed successfully before.
+    fn drop(&mut self) {
+        if self.registered {
+            // Get a pointer to the data stored in device before destroying it.
+            // SAFETY: `drm` is valid per the type invariant
+            let data_pointer = self.drm.raw_data();
+
+            // SAFETY: Since `registered` is true, `self.drm` is both valid and registered.
+            unsafe { bindings::drm_dev_unregister(self.drm.as_raw()) };
+
+            // Free data as well.
+            // SAFETY: `data_pointer` was returned by `into_foreign` during registration.
+            unsafe { <T::Data as ForeignOwnable>::from_foreign(data_pointer) };
+        }
+    }
+}
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index 9ec6d7cbcaf3..69376b3c6db9 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -2,4 +2,6 @@ 
 
 //! DRM subsystem abstractions.
 
+pub mod device;
+pub mod drv;
 pub mod ioctl;