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

[Danilo Krummrich <dakr@redhat.com>]

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
>