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

[Danilo Krummrich <dakr@redhat.com>]

Hi Lina,

Welcome back!

On Sun, Jun 09, 2024 at 02:15:57PM +0900, Asahi Lina wrote:
> 
> 
> On 5/22/24 6:23 AM, Rob Herring wrote:
> > On Mon, May 20, 2024 at 07:20:50PM +0200, Danilo Krummrich wrote:
> >> From: Asahi Lina <lina@asahilina.net>
> > 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.
> > 
> 
> This was developed before DRIVER_GEM_GPUVA existed ^^
> 
> I have this in my branch since I'm using the GPUVA manager now. Danilo,
> what tree are you using for this submission? It would be good to
> coordinate this and try to keep the WIP branches from diverging too much...

Trying to prevent things from diverging was one of my main objectives when I
started those efforts a couple of month ago (I also sent you a mail for that
purpose back then).

I am maintaining a couple of branches:

In the RfL repository [1]:

  - staging/rust-device [2] (Device / driver, devres infrastructure)
  - staging/dev [3] (common branch containing all the Rust infrastructure I'm
    currently upstreaming, including PCI and firmware abstractions)

In the drm-misc repository [4]:
  - topic/rust-drm [5] (all the DRM abstractions)

In the Nova repository [6]
  - nova-next [7] (the Nova stub driver making use of everything)

I regularly rebase those branches and keep them up to date with improvements and
feedback from the mailing list.

The device / driver and PCI abstractions are getting most of my attention
currently, but there are some recent changes (besides some minor ones) on the
DRM abstractions I plan to send in a v2 as well:

- static const allocation of driver and fops structures
- rework of the `Registration` type to use `Devres` and combine the new() and
  register() methods to register in new() already

There is also some more information regarding those branches in the cover
letters of the series and the links included there.

[1] https://github.com/Rust-for-Linux/linux
[2] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device
[3] https://github.com/Rust-for-Linux/linux/tree/staging/dev
[4] https://gitlab.freedesktop.org/drm/misc/kernel
[5] https://gitlab.freedesktop.org/drm/misc/kernel/-/tree/topic/rust-drm
[6] https://gitlab.freedesktop.org/drm/nova
[7] https://gitlab.freedesktop.org/drm/nova/-/tree/nova-next

> 
> That said, I don't think there's reason to support all features unless
> we expect new drivers to actually use them. The goal of the abstractions
> is to serve the drivers that will use them, and to evolve together with
> them and any newer drivers, not to attempt to be feature-complete from
> the get go (it's very difficult to evaluate an abstraction if it has no
> users!). In general my approach when writing them was to abstract what I
> need and add "obvious" extra trivial features that didn't require much
> thought even if I wasn't using them, but otherwise not attempt to
> comprehensively cover everything.

Fully agree, I think the point here only was whether we want to list all the
feature flags in the abstraction already. Which I think is something we can do.
However, I'm also find with only listing the ones we actually use and keep
adding further ones subsequently. It just shouldn't be random.

- Danilo

> 
> ~~ Lina
>