Re: [PATCH 0/6] rust: drm: Higher-Ranked Lifetime private data

[Boris Brezillon <boris.brezillon@collabora.com>]

On Wed, 20 May 2026 12:55:40 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:

> On Wed May 20, 2026 at 8:38 AM CEST, Boris Brezillon wrote:
> > So, GEM allocation and GPUVM are required are required early on, and
> > both want a drm::Device of some sort (Unregistered or Registered). So
> > the question is, are you happy with having [1] applied before/after
> > your patchset to address this chicken-and-egg issue I mentioned,  
> 
> Yes, this is exactly what I had in mind for all of my replies, but I think I
> only implied my key point rather than making it explicit, so let me do that now.
> 
> What I'm saying is that we now have all DRM ioctls behind the SRCU guard, which
> means that the drm::Registration data is always available from DRM ioctls.
> 
> Now, with this precondition, the real question is whether there are any
> resources left that should be bound to the lifetime of the DRM device itself,
> which I don't think is the case.
> 
> After device unbind nothing is able to call back into the driver anymore, so
> there is no reason to keep the corresponding data alive within the driver's
> private data.
> 
> There may be resources that are kept alive through a separate reference count
> while userspace still has open file descriptors, but they are separate reference
> counts and should be independent from the driver's private data.
> 
> IWO, the fact that we guard the ioctls makes the whole class of problems go away
> that we inherit from having to keep the driver's private data alive until after
> remvoe() -- the DRM device private data becomes obsolete and we should probably
> evene remove it.
> 
> Instead, we should use the drm::Registration private data for everything.
> 
> With this, your chicken-and-egg issue is already solved; the DRM device is
> available, you can populate all the resources you need (GEM, GPUVM, etc.) and
> then pass it into drm::Registration::new().

Okay, it starts to click. So ioctls get passed the drm::Registration,
which has both a device and some driver specific data (I guess it's
even hidden behind the File abstraction). During early probe, we'd still
be able to pass an Unregistered variant of the device, and then our
current TyrDrmDeviceData would be attached to the Registration object
when Registration::new_foreign_owned() is called.

This probably imply changing the places where we currently access our
TyrDrmDeviceData through the drm::Device, but hopefully there's not
many of those cases.

> 
> In terms of making any of the resources you store in the drm::Registration data
> available in the bus device private data, it becomes the same pattern as for
> everything else, i.e. you need shared ownership.

Yep, that we can solve.

> 
> So, let's say you need IoMem<'bound> in both the drm::Registration data and in
> the bus device private data, then you currently need Arc<IoMem<'bound>> in both
> of them, which is fine, but I think we can do better.
> 
> Eventually, once we have landed the series I currently have in flight and once
> Gary finished his self-referencial support in pin-init, I think we can even
> avoid the reference count. Here is an example.
> 
> 	struct TyrPlatformDriverData<'bound> {
> 	    _device: ARef<TyrDrmDevice>,
> 	    _reg: drm::Registration<'bound, TyrDrmData<'_>>,
> 	    iomem: IoMem<'bound>,
> 	}
> 
> 	struct TyrDrmData<'bound> {
> 	    iomem: &'bound IoMem<'bound>,
> 	}
> 
> 	fn probe() -> ... {
> 	    try_pin_init!(TyrPlatformDriverData {
> 	        _device: ...,
> 		iomem <- ...,
> 		_reg <- drm::Registration::new(..., TyrDrmData { iomem: &iomem }),
> 	    })
> 	}
> 
> I don't know how the pin-init self-referencial stuff will exactly turn out, but
> this is pretty much what I hope we can evolve this into.

Yep, makes sense.

> 
> (And sorry again, now that I spelled this all out, having that left implicit in
> my first reply was obviously unreasonable. :)

No worries, that's the usual back-and-forth happening during reviews,
and it's all fine.

> 
> > or are you proposing something else (Option<Subcomponent> so that we start
> > with all sub-components set to None and progressively populate those, which I
> > remember Daniel was strongly opposed to)?  
> 
> No, I am strongly opposed to do that (as well).

Got it.