Messages in this thread | | | Date | Tue, 7 Mar 2023 12:32:39 -0300 | Subject | Re: [PATCH RFC 01/18] rust: drm: ioctl: Add DRM ioctl abstraction | From | Maíra Canal <> |
| |
On 3/7/23 11:25, Asahi Lina wrote: > DRM drivers need to be able to declare which driver-specific ioctls they > support. This abstraction adds the required types and a helper macro to > generate the ioctl definition inside the DRM driver. > > Note that this macro is not usable until further bits of the > abstraction are in place (but it will not fail to compile on its own, if > not called). > > Signed-off-by: Asahi Lina <lina@asahilina.net> > --- > drivers/gpu/drm/Kconfig | 7 ++ > rust/bindings/bindings_helper.h | 2 + > rust/kernel/drm/ioctl.rs | 147 ++++++++++++++++++++++++++++++++++++++++ > rust/kernel/drm/mod.rs | 5 ++ > rust/kernel/lib.rs | 2 + > 5 files changed, 163 insertions(+) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index dc0f94f02a82..dab8f0f9aa96 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -27,6 +27,13 @@ menuconfig DRM > details. You should also select and configure AGP > (/dev/agpgart) support if it is available for your platform. >
[...]
> + > +/// Declare the DRM ioctls for a driver. > +/// > +/// Each entry in the list should have the form: > +/// > +/// `(ioctl_number, argument_type, flags, user_callback),` > +/// > +/// `argument_type` is the type name within the `bindings` crate. > +/// `user_callback` should have the following prototype: > +/// > +/// ``` > +/// fn foo(device: &kernel::drm::device::Device<Self>, > +/// data: &mut bindings::argument_type, > +/// file: &kernel::drm::file::File<Self::File>, > +/// ) > +/// ``` > +/// where `Self` is the drm::drv::Driver implementation these ioctls are being declared within. > +/// > +/// # Examples > +/// > +/// ``` > +/// kernel::declare_drm_ioctls! { > +/// (FOO_GET_PARAM, drm_foo_get_param, ioctl::RENDER_ALLOW, my_get_param_handler), > +/// } > +/// ``` > +/// > +#[macro_export] > +macro_rules! declare_drm_ioctls { > + ( $(($cmd:ident, $struct:ident, $flags:expr, $func:expr)),* $(,)? ) => { > + const IOCTLS: &'static [$crate::drm::ioctl::DrmIoctlDescriptor] = { > + const _:() = { > + let i: u32 = $crate::bindings::DRM_COMMAND_BASE; > + // Assert that all the IOCTLs are in the right order and there are no gaps, > + // and that the sizeof of the specified type is correct.
I believe that not necessarily the IOCTLs need to be in the right order and with no gaps. For example, armada_drm.h has a gap in between 0x00 and 0x02 and exynos_drm.h also have gaps. Moreover, some drivers, like vgem and virtgpu, start their IOCTLs with 0x01.
Best Regards, - Maíra Canal
> + $( > + let cmd: u32 = $crate::macros::concat_idents!($crate::bindings::DRM_IOCTL_, $cmd); > + ::core::assert!(i == $crate::ioctl::_IOC_NR(cmd)); > + ::core::assert!(core::mem::size_of::<$crate::bindings::$struct>() == $crate::ioctl::_IOC_SIZE(cmd)); > + let i: u32 = i + 1; > + )* > + }; > + > + let ioctls = &[$( > + $crate::bindings::drm_ioctl_desc { > + cmd: $crate::macros::concat_idents!($crate::bindings::DRM_IOCTL_, $cmd) as u32, > + func: { > + #[allow(non_snake_case)] > + unsafe extern "C" fn $cmd( > + raw_dev: *mut $crate::bindings::drm_device, > + raw_data: *mut ::core::ffi::c_void, > + raw_file_priv: *mut $crate::bindings::drm_file, > + ) -> core::ffi::c_int { > + // SAFETY: We never drop this, and the DRM core ensures the device lives > + // while callbacks are being called. > + // > + // FIXME: Currently there is nothing enforcing that the types of the > + // dev/file match the current driver these ioctls are being declared > + // for, and it's not clear how to enforce this within the type system. > + let dev = ::core::mem::ManuallyDrop::new(unsafe { > + $crate::drm::device::Device::from_raw(raw_dev) > + }); > + // SAFETY: This is just the ioctl argument, which hopefully has the right type > + // (we've done our best checking the size). > + let data = unsafe { &mut *(raw_data as *mut $crate::bindings::$struct) }; > + // SAFETY: This is just the DRM file structure > + let file = unsafe { $crate::drm::file::File::from_raw(raw_file_priv) }; > + > + match $func(&*dev, data, &file) { > + Err(e) => e.to_kernel_errno(), > + Ok(i) => i.try_into().unwrap_or(ERANGE.to_kernel_errno()), > + } > + } > + Some($cmd) > + }, > + flags: $flags, > + name: $crate::c_str!(::core::stringify!($cmd)).as_char_ptr(), > + } > + ),*]; > + ioctls > + }; > + }; > +} > diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs > new file mode 100644 > index 000000000000..9ec6d7cbcaf3 > --- /dev/null > +++ b/rust/kernel/drm/mod.rs > @@ -0,0 +1,5 @@ > +// SPDX-License-Identifier: GPL-2.0 OR MIT > + > +//! DRM subsystem abstractions. > + > +pub mod ioctl; > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index 7903490816bf..cb23d24c6718 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -37,6 +37,8 @@ mod build_assert; > pub mod delay; > pub mod device; > pub mod driver; > +#[cfg(CONFIG_RUST_DRM)] > +pub mod drm; > pub mod error; > pub mod io_buffer; > pub mod io_mem; >
| |