Skip to content

Commit 6ea42e9

Browse files
LyudeDarksonn
authored andcommitted
rust: drm: gem: Simplify use of generics
Now that my rust skills have been honed, I noticed that there's a lot of generics in our gem bindings that don't actually need to be here. Currently the hierarchy of traits in our gem bindings looks like this: * Drivers implement: * BaseDriverObject<T: DriverObject> (has the callbacks) * DriverObject (has the drm::Driver type) * Crate implements: * IntoGEMObject for Object<T> where T: DriverObject Handles conversion to/from raw object pointers * BaseObject for T where T: IntoGEMObject Provides methods common to all gem interfaces Also of note, this leaves us with two different drm::Driver associated types: * DriverObject::Driver * IntoGEMObject::Driver I'm not entirely sure of the original intent here unfortunately (if anyone is, please let me know!), but my guess is that the idea would be that some objects can implement IntoGEMObject using a different ::Driver than DriverObject - presumably to enable the usage of gem objects from different drivers. A reasonable usecase of course. However - if I'm not mistaken, I don't think that this is actually how things would go in practice. Driver implementations are of course implemented by their associated drivers, and generally drivers are not linked to each-other when building the kernel. Which is to say that even in a situation where we would theoretically deal with gem objects from another driver, we still wouldn't have access to its drm::driver::Driver implementation. It's more likely we would simply want a variant of gem objects in such a situation that have no association with a drm::driver::Driver type. Taking that into consideration, we can assume the following: * Anything that implements BaseDriverObject will implement DriverObject In other words, all BaseDriverObjects indirectly have an associated ::Driver type - so the two traits can be combined into one with no generics. * Not everything that implements IntoGEMObject will have an associated ::Driver, and that's OK. And with this, we now can do quite a bit of cleanup with the use of generics here. As such, this commit: * Removes the generics on BaseDriverObject * Moves DriverObject::Driver into BaseDriverObject * Removes DriverObject * Removes IntoGEMObject::Driver * Add AllocImpl::Driver, which we can use as a binding to figure out the correct File type for BaseObject Leaving us with a simpler trait hierarchy that now looks like this: * Drivers implement: BaseDriverObject * Crate implements: * IntoGEMObject for Object<T> where T: DriverObject * BaseObject for T where T: IntoGEMObject Which makes the code a lot easier to understand and build on :). Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com> Acked-by: Danilo Krummrich <dakr@kernel.org> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20250908185239.135849-2-lyude@redhat.com Signed-off-by: Alice Ryhl <aliceryhl@google.com>
1 parent e258041 commit 6ea42e9

3 files changed

Lines changed: 40 additions & 48 deletions

File tree

drivers/gpu/drm/nova/gem.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,14 @@ use crate::{
1616
#[pin_data]
1717
pub(crate) struct NovaObject {}
1818

19-
impl gem::BaseDriverObject<gem::Object<NovaObject>> for NovaObject {
19+
impl gem::DriverObject for NovaObject {
20+
type Driver = NovaDriver;
21+
2022
fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self, Error> {
2123
try_pin_init!(NovaObject {})
2224
}
2325
}
2426

25-
impl gem::DriverObject for NovaObject {
26-
type Driver = NovaDriver;
27-
}
28-
2927
impl NovaObject {
3028
/// Create a new DRM GEM object.
3129
pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> {

rust/kernel/drm/driver.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ pub struct AllocOps {
8686

8787
/// Trait for memory manager implementations. Implemented internally.
8888
pub trait AllocImpl: super::private::Sealed + drm::gem::IntoGEMObject {
89+
/// The [`Driver`] implementation for this [`AllocImpl`].
90+
type Driver: drm::Driver;
91+
8992
/// The C callback operations for this memory manager.
9093
const ALLOC_OPS: AllocOps;
9194
}

rust/kernel/drm/gem/mod.rs

Lines changed: 34 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -15,31 +15,31 @@ use crate::{
1515
use core::{mem, ops::Deref, ptr::NonNull};
1616

1717
/// GEM object functions, which must be implemented by drivers.
18-
pub trait BaseDriverObject<T: BaseObject>: Sync + Send + Sized {
18+
pub trait DriverObject: Sync + Send + Sized {
19+
/// Parent `Driver` for this object.
20+
type Driver: drm::Driver;
21+
1922
/// Create a new driver data object for a GEM object of a given size.
20-
fn new(dev: &drm::Device<T::Driver>, size: usize) -> impl PinInit<Self, Error>;
23+
fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl PinInit<Self, Error>;
2124

2225
/// Open a new handle to an existing object, associated with a File.
2326
fn open(
24-
_obj: &<<T as IntoGEMObject>::Driver as drm::Driver>::Object,
25-
_file: &drm::File<<<T as IntoGEMObject>::Driver as drm::Driver>::File>,
27+
_obj: &<Self::Driver as drm::Driver>::Object,
28+
_file: &drm::File<<Self::Driver as drm::Driver>::File>,
2629
) -> Result {
2730
Ok(())
2831
}
2932

3033
/// Close a handle to an existing object, associated with a File.
3134
fn close(
32-
_obj: &<<T as IntoGEMObject>::Driver as drm::Driver>::Object,
33-
_file: &drm::File<<<T as IntoGEMObject>::Driver as drm::Driver>::File>,
35+
_obj: &<Self::Driver as drm::Driver>::Object,
36+
_file: &drm::File<<Self::Driver as drm::Driver>::File>,
3437
) {
3538
}
3639
}
3740

3841
/// Trait that represents a GEM object subtype
3942
pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted {
40-
/// Owning driver for this type
41-
type Driver: drm::Driver;
42-
4343
/// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as
4444
/// this owning object is valid.
4545
fn as_raw(&self) -> *mut bindings::drm_gem_object;
@@ -74,52 +74,37 @@ unsafe impl<T: IntoGEMObject> AlwaysRefCounted for T {
7474
}
7575
}
7676

77-
/// Trait which must be implemented by drivers using base GEM objects.
78-
pub trait DriverObject: BaseDriverObject<Object<Self>> {
79-
/// Parent `Driver` for this object.
80-
type Driver: drm::Driver;
81-
}
82-
83-
extern "C" fn open_callback<T: BaseDriverObject<U>, U: BaseObject>(
77+
extern "C" fn open_callback<T: DriverObject>(
8478
raw_obj: *mut bindings::drm_gem_object,
8579
raw_file: *mut bindings::drm_file,
8680
) -> core::ffi::c_int {
8781
// SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
88-
let file = unsafe {
89-
drm::File::<<<U as IntoGEMObject>::Driver as drm::Driver>::File>::from_raw(raw_file)
90-
};
91-
// SAFETY: `open_callback` is specified in the AllocOps structure for `Object<T>`, ensuring that
92-
// `raw_obj` is indeed contained within a `Object<T>`.
93-
let obj = unsafe {
94-
<<<U as IntoGEMObject>::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj)
95-
};
82+
let file = unsafe { drm::File::<<T::Driver as drm::Driver>::File>::from_raw(raw_file) };
83+
// SAFETY: `open_callback` is specified in the AllocOps structure for `DriverObject<T>`,
84+
// ensuring that `raw_obj` is contained within a `DriverObject<T>`
85+
let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) };
9686

9787
match T::open(obj, file) {
9888
Err(e) => e.to_errno(),
9989
Ok(()) => 0,
10090
}
10191
}
10292

103-
extern "C" fn close_callback<T: BaseDriverObject<U>, U: BaseObject>(
93+
extern "C" fn close_callback<T: DriverObject>(
10494
raw_obj: *mut bindings::drm_gem_object,
10595
raw_file: *mut bindings::drm_file,
10696
) {
10797
// SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
108-
let file = unsafe {
109-
drm::File::<<<U as IntoGEMObject>::Driver as drm::Driver>::File>::from_raw(raw_file)
110-
};
98+
let file = unsafe { drm::File::<<T::Driver as drm::Driver>::File>::from_raw(raw_file) };
99+
111100
// SAFETY: `close_callback` is specified in the AllocOps structure for `Object<T>`, ensuring
112101
// that `raw_obj` is indeed contained within a `Object<T>`.
113-
let obj = unsafe {
114-
<<<U as IntoGEMObject>::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj)
115-
};
102+
let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) };
116103

117104
T::close(obj, file);
118105
}
119106

120107
impl<T: DriverObject> IntoGEMObject for Object<T> {
121-
type Driver = T::Driver;
122-
123108
fn as_raw(&self) -> *mut bindings::drm_gem_object {
124109
self.obj.get()
125110
}
@@ -141,10 +126,12 @@ pub trait BaseObject: IntoGEMObject {
141126

142127
/// Creates a new handle for the object associated with a given `File`
143128
/// (or returns an existing one).
144-
fn create_handle(
145-
&self,
146-
file: &drm::File<<<Self as IntoGEMObject>::Driver as drm::Driver>::File>,
147-
) -> Result<u32> {
129+
fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
130+
where
131+
Self: AllocImpl<Driver = D>,
132+
D: drm::Driver<Object = Self, File = F>,
133+
F: drm::file::DriverFile<Driver = D>,
134+
{
148135
let mut handle: u32 = 0;
149136
// SAFETY: The arguments are all valid per the type invariants.
150137
to_result(unsafe {
@@ -154,10 +141,12 @@ pub trait BaseObject: IntoGEMObject {
154141
}
155142

156143
/// Looks up an object by its handle for a given `File`.
157-
fn lookup_handle(
158-
file: &drm::File<<<Self as IntoGEMObject>::Driver as drm::Driver>::File>,
159-
handle: u32,
160-
) -> Result<ARef<Self>> {
144+
fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>>
145+
where
146+
Self: AllocImpl<Driver = D>,
147+
D: drm::Driver<Object = Self, File = F>,
148+
F: drm::file::DriverFile<Driver = D>,
149+
{
161150
// SAFETY: The arguments are all valid per the type invariants.
162151
let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) };
163152
if ptr.is_null() {
@@ -212,8 +201,8 @@ impl<T: DriverObject> Object<T> {
212201

213202
const OBJECT_FUNCS: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs {
214203
free: Some(Self::free_callback),
215-
open: Some(open_callback::<T, Object<T>>),
216-
close: Some(close_callback::<T, Object<T>>),
204+
open: Some(open_callback::<T>),
205+
close: Some(close_callback::<T>),
217206
print_info: None,
218207
export: None,
219208
pin: None,
@@ -296,6 +285,8 @@ impl<T: DriverObject> Deref for Object<T> {
296285
}
297286

298287
impl<T: DriverObject> AllocImpl for Object<T> {
288+
type Driver = T::Driver;
289+
299290
const ALLOC_OPS: AllocOps = AllocOps {
300291
gem_create_object: None,
301292
prime_handle_to_fd: None,

0 commit comments

Comments
 (0)