Lyude Paul
2025-Aug-29 22:35 UTC
[PATCH v3 01/14] 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 at redhat.com>
---
V2:
* Don't refer to Object<T> in callbacks, as this would result in
drivers
getting the wrong gem object type for shmem gem objects once we add
support for those. Instead, we'll just add a type alias to clean this
part up.
V3:
* Fix nova compilation
* Also, add an associated driver type to AllocImpl - as we still need the
current driver accessible from BaseObject so that we can use the driver's
various associated types, like File
V4:
* Add missing Object = Self constraint to type bounds for create_handle,
lookup_handle. I forgot that if drivers can have private gem objects with
a different data layout, we can only guarantee gem objects with handles
are of the same gem object type as the main one in use by the driver.
Signed-off-by: Lyude Paul <lyude at redhat.com>
---
drivers/gpu/drm/nova/gem.rs | 8 ++--
rust/kernel/drm/driver.rs | 3 ++
rust/kernel/drm/gem/mod.rs | 77 ++++++++++++++++---------------------
3 files changed, 40 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index cd82773dab92c..2760ba4f3450b 100644
--- a/drivers/gpu/drm/nova/gem.rs
+++ b/drivers/gpu/drm/nova/gem.rs
@@ -16,16 +16,14 @@
#[pin_data]
pub(crate) struct NovaObject {}
-impl gem::BaseDriverObject<gem::Object<NovaObject>> for NovaObject
{
+impl gem::DriverObject for NovaObject {
+ type Driver = NovaDriver;
+
fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self,
Error> {
try_pin_init!(NovaObject {})
}
}
-impl gem::DriverObject for NovaObject {
- type Driver = NovaDriver;
-}
-
impl NovaObject {
/// Create a new DRM GEM object.
pub(crate) fn new(dev: &NovaDevice, size: usize) ->
Result<ARef<gem::Object<Self>>> {
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index fe7e8d06961aa..dae0f4d1bbe3c 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -86,6 +86,9 @@ pub struct AllocOps {
/// Trait for memory manager implementations. Implemented internally.
pub trait AllocImpl: super::private::Sealed + drm::gem::IntoGEMObject {
+ /// The [`Driver`] implementation for this [`AllocImpl`].
+ type Driver: drm::Driver;
+
/// The C callback operations for this memory manager.
const ALLOC_OPS: AllocOps;
}
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index b71821cfb5eaa..31c5799d995c5 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -15,31 +15,31 @@
use core::{mem, ops::Deref, ptr::NonNull};
/// GEM object functions, which must be implemented by drivers.
-pub trait BaseDriverObject<T: BaseObject>: Sync + Send + Sized {
+pub trait DriverObject: Sync + Send + Sized {
+ /// Parent `Driver` for this object.
+ type Driver: drm::Driver;
+
/// Create a new driver data object for a GEM object of a given size.
- fn new(dev: &drm::Device<T::Driver>, size: usize) -> impl
PinInit<Self, Error>;
+ fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl
PinInit<Self, Error>;
/// Open a new handle to an existing object, associated with a File.
fn open(
- _obj: &<<T as IntoGEMObject>::Driver as
drm::Driver>::Object,
- _file: &drm::File<<<T as IntoGEMObject>::Driver as
drm::Driver>::File>,
+ _obj: &<Self::Driver as drm::Driver>::Object,
+ _file: &drm::File<<Self::Driver as drm::Driver>::File>,
) -> Result {
Ok(())
}
/// Close a handle to an existing object, associated with a File.
fn close(
- _obj: &<<T as IntoGEMObject>::Driver as
drm::Driver>::Object,
- _file: &drm::File<<<T as IntoGEMObject>::Driver as
drm::Driver>::File>,
+ _obj: &<Self::Driver as drm::Driver>::Object,
+ _file: &drm::File<<Self::Driver as drm::Driver>::File>,
) {
}
}
/// Trait that represents a GEM object subtype
pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted {
- /// Owning driver for this type
- type Driver: drm::Driver;
-
/// Returns a reference to the raw `drm_gem_object` structure, which must
be valid as long as
/// this owning object is valid.
fn as_raw(&self) -> *mut bindings::drm_gem_object;
@@ -74,25 +74,15 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
-/// Trait which must be implemented by drivers using base GEM objects.
-pub trait DriverObject: BaseDriverObject<Object<Self>> {
- /// Parent `Driver` for this object.
- type Driver: drm::Driver;
-}
-
-extern "C" fn open_callback<T: BaseDriverObject<U>, U:
BaseObject>(
+extern "C" fn open_callback<T: DriverObject>(
raw_obj: *mut bindings::drm_gem_object,
raw_file: *mut bindings::drm_file,
) -> core::ffi::c_int {
// SAFETY: `open_callback` is only ever called with a valid pointer to a
`struct drm_file`.
- let file = unsafe {
- drm::File::<<<U as IntoGEMObject>::Driver as
drm::Driver>::File>::from_raw(raw_file)
- };
- // SAFETY: `open_callback` is specified in the AllocOps structure for
`Object<T>`, ensuring that
- // `raw_obj` is indeed contained within a `Object<T>`.
- let obj = unsafe {
- <<<U as IntoGEMObject>::Driver as drm::Driver>::Object
as IntoGEMObject>::from_raw(raw_obj)
- };
+ let file = unsafe { drm::File::<<T::Driver as
drm::Driver>::File>::from_raw(raw_file) };
+ // SAFETY: `open_callback` is specified in the AllocOps structure for
`DriverObject<T>`,
+ // ensuring that `raw_obj` is contained within a `DriverObject<T>`
+ let obj = unsafe { <<T::Driver as drm::Driver>::Object as
IntoGEMObject>::from_raw(raw_obj) };
match T::open(obj, file) {
Err(e) => e.to_errno(),
@@ -100,26 +90,21 @@ extern "C" fn open_callback<T:
BaseDriverObject<U>, U: BaseObject>(
}
}
-extern "C" fn close_callback<T: BaseDriverObject<U>, U:
BaseObject>(
+extern "C" fn close_callback<T: DriverObject>(
raw_obj: *mut bindings::drm_gem_object,
raw_file: *mut bindings::drm_file,
) {
// SAFETY: `open_callback` is only ever called with a valid pointer to a
`struct drm_file`.
- let file = unsafe {
- drm::File::<<<U as IntoGEMObject>::Driver as
drm::Driver>::File>::from_raw(raw_file)
- };
+ let file = unsafe { drm::File::<<T::Driver as
drm::Driver>::File>::from_raw(raw_file) };
+
// SAFETY: `close_callback` is specified in the AllocOps structure for
`Object<T>`, ensuring
// that `raw_obj` is indeed contained within a `Object<T>`.
- let obj = unsafe {
- <<<U as IntoGEMObject>::Driver as drm::Driver>::Object
as IntoGEMObject>::from_raw(raw_obj)
- };
+ let obj = unsafe { <<T::Driver as drm::Driver>::Object as
IntoGEMObject>::from_raw(raw_obj) };
T::close(obj, file);
}
impl<T: DriverObject> IntoGEMObject for Object<T> {
- type Driver = T::Driver;
-
fn as_raw(&self) -> *mut bindings::drm_gem_object {
self.obj.get()
}
@@ -141,10 +126,12 @@ fn size(&self) -> usize {
/// Creates a new handle for the object associated with a given `File`
/// (or returns an existing one).
- fn create_handle(
- &self,
- file: &drm::File<<<Self as IntoGEMObject>::Driver as
drm::Driver>::File>,
- ) -> Result<u32> {
+ fn create_handle<D, F>(&self, file: &drm::File<F>)
-> Result<u32>
+ where
+ Self: AllocImpl<Driver = D>,
+ D: drm::Driver<Object = Self, File = F>,
+ F: drm::file::DriverFile,
+ {
let mut handle: u32 = 0;
// SAFETY: The arguments are all valid per the type invariants.
to_result(unsafe {
@@ -154,10 +141,12 @@ fn create_handle(
}
/// Looks up an object by its handle for a given `File`.
- fn lookup_handle(
- file: &drm::File<<<Self as IntoGEMObject>::Driver as
drm::Driver>::File>,
- handle: u32,
- ) -> Result<ARef<Self>> {
+ fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32)
-> Result<ARef<Self>>
+ where
+ Self: AllocImpl<Driver = D>,
+ D: drm::Driver<Object = Self, File = F>,
+ F: drm::file::DriverFile,
+ {
// SAFETY: The arguments are all valid per the type invariants.
let ptr = unsafe {
bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) };
if ptr.is_null() {
@@ -212,8 +201,8 @@ impl<T: DriverObject> Object<T> {
const OBJECT_FUNCS: bindings::drm_gem_object_funcs =
bindings::drm_gem_object_funcs {
free: Some(Self::free_callback),
- open: Some(open_callback::<T, Object<T>>),
- close: Some(close_callback::<T, Object<T>>),
+ open: Some(open_callback::<T>),
+ close: Some(close_callback::<T>),
print_info: None,
export: None,
pin: None,
@@ -296,6 +285,8 @@ fn deref(&self) -> &Self::Target {
}
impl<T: DriverObject> AllocImpl for Object<T> {
+ type Driver = T::Driver;
+
const ALLOC_OPS: AllocOps = AllocOps {
gem_create_object: None,
prime_handle_to_fd: None,
--
2.50.0
Daniel Almeida
2025-Sep-01 15:37 UTC
[PATCH v3 01/14] rust: drm: gem: Simplify use of generics
Hi Lyude, thanks a lot for working on this! :)> On 29 Aug 2025, at 19:35, Lyude Paul <lyude at redhat.com> wrote: > > 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 at redhat.com> > > --- > V2: > * Don't refer to Object<T> in callbacks, as this would result in drivers > getting the wrong gem object type for shmem gem objects once we add > support for those. Instead, we'll just add a type alias to clean this > part up. > V3: > * Fix nova compilation > * Also, add an associated driver type to AllocImpl - as we still need the > current driver accessible from BaseObject so that we can use the driver's > various associated types, like File > V4:? This is v3. Can you clarify this before we go further? :)> * Add missing Object = Self constraint to type bounds for create_handle, > lookup_handle. I forgot that if drivers can have private gem objects with > a different data layout, we can only guarantee gem objects with handles > are of the same gem object type as the main one in use by the driver. > > Signed-off-by: Lyude Paul <lyude at redhat.com>? Daniel
Daniel Almeida
2025-Sep-04 12:11 UTC
[PATCH v3 01/14] rust: drm: gem: Simplify use of generics
Hi Lyude,> On 29 Aug 2025, at 19:35, Lyude Paul <lyude at redhat.com> wrote: > > 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 at redhat.com> > > --- > V2: > * Don't refer to Object<T> in callbacks, as this would result in drivers > getting the wrong gem object type for shmem gem objects once we add > support for those. Instead, we'll just add a type alias to clean this > part up. > V3: > * Fix nova compilation > * Also, add an associated driver type to AllocImpl - as we still need the > current driver accessible from BaseObject so that we can use the driver's > various associated types, like File > V4: > * Add missing Object = Self constraint to type bounds for create_handle, > lookup_handle. I forgot that if drivers can have private gem objects with > a different data layout, we can only guarantee gem objects with handles > are of the same gem object type as the main one in use by the driver. > > Signed-off-by: Lyude Paul <lyude at redhat.com> > --- > drivers/gpu/drm/nova/gem.rs | 8 ++-- > rust/kernel/drm/driver.rs | 3 ++ > rust/kernel/drm/gem/mod.rs | 77 ++++++++++++++++--------------------- > 3 files changed, 40 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs > index cd82773dab92c..2760ba4f3450b 100644 > --- a/drivers/gpu/drm/nova/gem.rs > +++ b/drivers/gpu/drm/nova/gem.rs > @@ -16,16 +16,14 @@ > #[pin_data] > pub(crate) struct NovaObject {} > > -impl gem::BaseDriverObject<gem::Object<NovaObject>> for NovaObject { > +impl gem::DriverObject for NovaObject { > + type Driver = NovaDriver; > + > fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self, Error> { > try_pin_init!(NovaObject {}) > } > } > > -impl gem::DriverObject for NovaObject { > - type Driver = NovaDriver; > -} > - > impl NovaObject { > /// Create a new DRM GEM object. > pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> { > diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs > index fe7e8d06961aa..dae0f4d1bbe3c 100644 > --- a/rust/kernel/drm/driver.rs > +++ b/rust/kernel/drm/driver.rs > @@ -86,6 +86,9 @@ pub struct AllocOps { > > /// Trait for memory manager implementations. Implemented internally. > pub trait AllocImpl: super::private::Sealed + drm::gem::IntoGEMObject { > + /// The [`Driver`] implementation for this [`AllocImpl`]. > + type Driver: drm::Driver; > + > /// The C callback operations for this memory manager. > const ALLOC_OPS: AllocOps; > } > diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs > index b71821cfb5eaa..31c5799d995c5 100644 > --- a/rust/kernel/drm/gem/mod.rs > +++ b/rust/kernel/drm/gem/mod.rs > @@ -15,31 +15,31 @@ > use core::{mem, ops::Deref, ptr::NonNull}; > > /// GEM object functions, which must be implemented by drivers. > -pub trait BaseDriverObject<T: BaseObject>: Sync + Send + Sized { > +pub trait DriverObject: Sync + Send + Sized { > + /// Parent `Driver` for this object. > + type Driver: drm::Driver; > + > /// Create a new driver data object for a GEM object of a given size. > - fn new(dev: &drm::Device<T::Driver>, size: usize) -> impl PinInit<Self, Error>; > + fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl PinInit<Self, Error>; > > /// Open a new handle to an existing object, associated with a File. > fn open( > - _obj: &<<T as IntoGEMObject>::Driver as drm::Driver>::Object, > - _file: &drm::File<<<T as IntoGEMObject>::Driver as drm::Driver>::File>, > + _obj: &<Self::Driver as drm::Driver>::Object, > + _file: &drm::File<<Self::Driver as drm::Driver>::File>, > ) -> Result { > Ok(()) > } > > /// Close a handle to an existing object, associated with a File. > fn close( > - _obj: &<<T as IntoGEMObject>::Driver as drm::Driver>::Object, > - _file: &drm::File<<<T as IntoGEMObject>::Driver as drm::Driver>::File>, > + _obj: &<Self::Driver as drm::Driver>::Object, > + _file: &drm::File<<Self::Driver as drm::Driver>::File>, > ) { > } > } > > /// Trait that represents a GEM object subtype > pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted { > - /// Owning driver for this type > - type Driver: drm::Driver; > - > /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as > /// this owning object is valid. > fn as_raw(&self) -> *mut bindings::drm_gem_object; > @@ -74,25 +74,15 @@ unsafe fn dec_ref(obj: NonNull<Self>) { > } > } > > -/// Trait which must be implemented by drivers using base GEM objects. > -pub trait DriverObject: BaseDriverObject<Object<Self>> { > - /// Parent `Driver` for this object. > - type Driver: drm::Driver; > -} > - > -extern "C" fn open_callback<T: BaseDriverObject<U>, U: BaseObject>( > +extern "C" fn open_callback<T: DriverObject>( > raw_obj: *mut bindings::drm_gem_object, > raw_file: *mut bindings::drm_file, > ) -> core::ffi::c_int { > // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`. > - let file = unsafe { > - drm::File::<<<U as IntoGEMObject>::Driver as drm::Driver>::File>::from_raw(raw_file) > - }; > - // SAFETY: `open_callback` is specified in the AllocOps structure for `Object<T>`, ensuring that > - // `raw_obj` is indeed contained within a `Object<T>`. > - let obj = unsafe { > - <<<U as IntoGEMObject>::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) > - }; > + let file = unsafe { drm::File::<<T::Driver as drm::Driver>::File>::from_raw(raw_file) }; > + // SAFETY: `open_callback` is specified in the AllocOps structure for `DriverObject<T>`, > + // ensuring that `raw_obj` is contained within a `DriverObject<T>` > + let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) }; > > match T::open(obj, file) { > Err(e) => e.to_errno(), > @@ -100,26 +90,21 @@ extern "C" fn open_callback<T: BaseDriverObject<U>, U: BaseObject>( > } > } > > -extern "C" fn close_callback<T: BaseDriverObject<U>, U: BaseObject>( > +extern "C" fn close_callback<T: DriverObject>( > raw_obj: *mut bindings::drm_gem_object, > raw_file: *mut bindings::drm_file, > ) { > // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`. > - let file = unsafe { > - drm::File::<<<U as IntoGEMObject>::Driver as drm::Driver>::File>::from_raw(raw_file) > - }; > + let file = unsafe { drm::File::<<T::Driver as drm::Driver>::File>::from_raw(raw_file) }; > + > // SAFETY: `close_callback` is specified in the AllocOps structure for `Object<T>`, ensuring > // that `raw_obj` is indeed contained within a `Object<T>`. > - let obj = unsafe { > - <<<U as IntoGEMObject>::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) > - }; > + let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) }; > > T::close(obj, file); > } > > impl<T: DriverObject> IntoGEMObject for Object<T> { > - type Driver = T::Driver; > - > fn as_raw(&self) -> *mut bindings::drm_gem_object { > self.obj.get() > } > @@ -141,10 +126,12 @@ fn size(&self) -> usize { > > /// Creates a new handle for the object associated with a given `File` > /// (or returns an existing one). > - fn create_handle( > - &self, > - file: &drm::File<<<Self as IntoGEMObject>::Driver as drm::Driver>::File>, > - ) -> Result<u32> { > + fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32> > + where > + Self: AllocImpl<Driver = D>, > + D: drm::Driver<Object = Self, File = F>, > + F: drm::file::DriverFile,Shouldn?t this be F: drm::file::DriverFile<Driver =D>? As you said in the commit message, I don?t see where exactly we would have two competing drm::Driver types when calling this function, but we should perhaps enforce this bound anyways.> + { > let mut handle: u32 = 0; > // SAFETY: The arguments are all valid per the type invariants. > to_result(unsafe { > @@ -154,10 +141,12 @@ fn create_handle( > } > > /// Looks up an object by its handle for a given `File`. > - fn lookup_handle( > - file: &drm::File<<<Self as IntoGEMObject>::Driver as drm::Driver>::File>, > - handle: u32, > - ) -> Result<ARef<Self>> { > + fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>> > + where > + Self: AllocImpl<Driver = D>, > + D: drm::Driver<Object = Self, File = F>, > + F: drm::file::DriverFile,Same here?> + { > // SAFETY: The arguments are all valid per the type invariants. > let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) }; > if ptr.is_null() { > @@ -212,8 +201,8 @@ impl<T: DriverObject> Object<T> { > > const OBJECT_FUNCS: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs { > free: Some(Self::free_callback), > - open: Some(open_callback::<T, Object<T>>), > - close: Some(close_callback::<T, Object<T>>), > + open: Some(open_callback::<T>), > + close: Some(close_callback::<T>), > print_info: None, > export: None, > pin: None, > @@ -296,6 +285,8 @@ fn deref(&self) -> &Self::Target { > } > > impl<T: DriverObject> AllocImpl for Object<T> { > + type Driver = T::Driver; > + > const ALLOC_OPS: AllocOps = AllocOps { > gem_create_object: None, > prime_handle_to_fd: None, > -- > 2.50.0 >With the DriverFile comment sorted out: Reviewed-by: Daniel Almeida <daniel.almeida at collabora.com>