[PATCH v2 0/8] DRM Rust abstractions and Nova

[Simona Vetter]

On Mon, Sep 02, 2024 at 06:40:00PM +0200, Daniel Vetter wrote:
> On Wed, Jun 19, 2024 at 01:31:36AM +0200, Danilo Krummrich wrote:
> > This patch series implements some basic DRM Rust abstractions and a stub
> > implementation of the Nova GPU driver.
> > 
> > Nova is intended to be developed upstream, starting out with just a stub driver
> > to lift some initial required infrastructure upstream. A more detailed
> > explanation can be found in [1].
> > 
> > This patch series is based on the "Device / Driver and PCI Rust abstractions"
> > series [2].
> > 
> > The DRM bits can also be found in [3] and the Nova bits in [4].
> > 
> > Changes in v2:
> > ==============
> > - split up the DRM device / driver abstractions in three separate commits
> > - separate `struct drm_device` abstraction in a separte source file more
> >   cleanly
> > - switch `struct drm_driver` and `struct file_operations` to 'static const'
> >   allocations
> > - implement `Registration::new_foreign_owned` (using `Devres`), such that we
> >   don't need to keep the `Registration` alive on the Rust side, but
> >   automatically revoke it on device unbind
> > - add missing DRM driver features (Rob)
> > - use `module_pci_driver!` macro in Nova
> > - use a const sized `pci::Bar` in Nova
> > - increase the total amount of Documentation, rephrase some safety comments and
> >   commit messages for less ambiguity
> > - fix compilation issues with some documentation example
> > 
> > [1] https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u
> > [2] Reply to this mail titled "Device / Driver and PCI Rust abstractions".
> > [3] https://gitlab.freedesktop.org/drm/misc/kernel/-/tree/topic/rust-drm
> > [4] https://gitlab.freedesktop.org/drm/nova/-/tree/nova-next
> 
> Ok finally stopped dragging my feet on this, went through my old comments,
> looked at stuff again, and wrote some replies.
> 
> I think all but the question around type safety for drm_driver->features
> can be sorted out post-merge, when we have a driver that needs it. The
> feature flags stuff I think essentially makes the current abstraction
> unsafe, and you can blow up when constructing a new drm::Device instance
> if you're creative enough I think.

Oh one thing I've forgotten: I think for the subclassing pattern in rust
there's clear consensus now, at least from my discussion with Lyude on the
modeset side of things. Well maybe aside from the little issue that rust
doesn't guarantee uniqueness for static const ops pointers, but apparently
that's getting addressed. One thing I'd really like us to be consistent at
though is not just the pattern, but the naming (like RawFoo vs whatever
else people came up with) of the different struct/traits, so would be
really good to document that somewhere for drm and make sure we follow it
in all the gpu related rust bindings. Unless upstream has that already
(maybe as part of the device/driver binding stuff).

Cheers, Sima

> > Asahi Lina (4):
> >   rust: drm: ioctl: Add DRM ioctl abstraction
> >   rust: Add a Sealed trait
> >   rust: drm: file: Add File abstraction
> >   rust: drm: gem: Add GEM object abstraction
> > 
> > Danilo Krummrich (4):
> >   rust: drm: add driver abstractions
> >   rust: drm: add device abstraction
> >   rust: drm: add DRM driver registration
> >   nova: add initial driver stub
> > 
> >  MAINTAINERS                     |  10 +
> >  drivers/gpu/drm/Kconfig         |   2 +
> >  drivers/gpu/drm/Makefile        |   1 +
> >  drivers/gpu/drm/nova/Kconfig    |  12 +
> >  drivers/gpu/drm/nova/Makefile   |   3 +
> >  drivers/gpu/drm/nova/driver.rs  |  85 +++++++
> >  drivers/gpu/drm/nova/file.rs    |  70 ++++++
> >  drivers/gpu/drm/nova/gem.rs     |  50 ++++
> >  drivers/gpu/drm/nova/gpu.rs     | 173 ++++++++++++++
> >  drivers/gpu/drm/nova/nova.rs    |  18 ++
> >  include/uapi/drm/nova_drm.h     | 101 ++++++++
> >  rust/bindings/bindings_helper.h |   5 +
> >  rust/helpers.c                  |  23 ++
> >  rust/kernel/drm/device.rs       | 182 ++++++++++++++
> >  rust/kernel/drm/drv.rs          | 199 ++++++++++++++++
> >  rust/kernel/drm/file.rs         | 116 +++++++++
> >  rust/kernel/drm/gem/mod.rs      | 409 ++++++++++++++++++++++++++++++++
> >  rust/kernel/drm/ioctl.rs        | 153 ++++++++++++
> >  rust/kernel/drm/mod.rs          |   9 +
> >  rust/kernel/lib.rs              |   7 +
> >  rust/uapi/uapi_helper.h         |   2 +
> >  21 files changed, 1630 insertions(+)
> >  create mode 100644 drivers/gpu/drm/nova/Kconfig
> >  create mode 100644 drivers/gpu/drm/nova/Makefile
> >  create mode 100644 drivers/gpu/drm/nova/driver.rs
> >  create mode 100644 drivers/gpu/drm/nova/file.rs
> >  create mode 100644 drivers/gpu/drm/nova/gem.rs
> >  create mode 100644 drivers/gpu/drm/nova/gpu.rs
> >  create mode 100644 drivers/gpu/drm/nova/nova.rs
> >  create mode 100644 include/uapi/drm/nova_drm.h
> >  create mode 100644 rust/kernel/drm/device.rs
> >  create mode 100644 rust/kernel/drm/drv.rs
> >  create mode 100644 rust/kernel/drm/file.rs
> >  create mode 100644 rust/kernel/drm/gem/mod.rs
> >  create mode 100644 rust/kernel/drm/ioctl.rs
> >  create mode 100644 rust/kernel/drm/mod.rs
> > 
> > 
> > base-commit: 6646240d29b11de3177f71ff777d0ae683c32623
> > -- 
> > 2.45.1
> > 
> 
> -- 
> Simona Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch