Re: [PATCH 3/8] rust: drm: add driver abstractions

[Danilo Krummrich <dakr@kernel.org>]

On Fri, Mar 28, 2025 at 06:00:11PM -0400, Lyude Paul wrote:
> On Wed, 2025-03-26 at 00:54 +0100, Danilo Krummrich wrote:
> > diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> > new file mode 100644
> > index 000000000000..1ac770482ae0
> > --- /dev/null
> > +++ b/rust/kernel/drm/driver.rs
> > @@ -0,0 +1,143 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > +
> > +//! DRM driver core.
> > +//!
> > +//! C header: [`include/linux/drm/drm_drv.h`](srctree/include/linux/drm/drm_drv.h)
> > +
> > +use crate::{bindings, drm, str::CStr};
> > +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;
> > +/// Driver supports compute acceleration devices. This flag is mutually exclusive with `FEAT_RENDER`
> > +/// and `FEAT_MODESET`. Devices that support both graphics and compute acceleration should be
> > +/// handled by two drivers that are connected using auxiliary bus.
> > +pub const FEAT_COMPUTE_ACCEL: u32 = bindings::drm_driver_feature_DRIVER_COMPUTE_ACCEL;
> > +/// Driver supports user defined GPU VA bindings for GEM objects.
> > +pub const FEAT_GEM_GPUVA: u32 = bindings::drm_driver_feature_DRIVER_GEM_GPUVA;
> > +/// Driver supports and requires cursor hotspot information in the cursor plane (e.g. cursor plane
> > +/// has to actually track the mouse cursor and the clients are required to set hotspot in order for
> > +/// the cursor planes to work correctly).
> > +pub const FEAT_CURSOR_HOTSPOT: u32 = bindings::drm_driver_feature_DRIVER_CURSOR_HOTSPOT;
> 
> IMHO I don't think that we should be exposing any of these constants, building
> the feature flag mask should honestly be abstracted away into the rust
> bindings as not doing so means it's very easy to break assumptions that could
> lead to unsoundness.

Yep, that was the plan, just forgot to make them crate private; on the other
hand I should probably drop them entirely for now.

- Danilo