[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c047e11f-33d0-6af4-21c3-adb384b68d8b@igalia.com>
Date: Tue, 7 Mar 2023 12:32:39 -0300
From: Maíra Canal <mcanal@...lia.com>
To: Asahi Lina <lina@...hilina.net>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Wedson Almeida Filho <wedsonaf@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Sumit Semwal <sumit.semwal@...aro.org>,
Christian König <christian.koenig@....com>,
Luben Tuikov <luben.tuikov@....com>,
Jarkko Sakkinen <jarkko@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>
Cc: linaro-mm-sig@...ts.linaro.org, rust-for-linux@...r.kernel.org,
Karol Herbst <kherbst@...hat.com>, asahi@...ts.linux.dev,
linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
Mary <mary@...y.zone>, Alyssa Rosenzweig <alyssa@...enzweig.io>,
linux-sgx@...r.kernel.org, Ella Stanforth <ella@...unix.org>,
Faith Ekstrand <faith.ekstrand@...labora.com>,
linux-media@...r.kernel.org
Subject: Re: [PATCH RFC 01/18] rust: drm: ioctl: Add DRM ioctl abstraction
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@...hilina.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;
>
Powered by blists - more mailing lists