Re: [PATCH] drm/tyr: move probe resources into registration data

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Jun 4, 2026 at 1:19 AM CEST, Deborah Brouwer wrote:
> @@ -59,21 +61,36 @@ pub(crate) struct TyrPlatformDriverData<'bound> {
>  }
>  
>  #[pin_data]
> -pub(crate) struct TyrDrmDeviceData {
> +pub(crate) struct TyrDrmDeviceData;
> +
> +/// Resources kept alive by the DRM registration.
> +#[pin_data]
> +pub(crate) struct TyrDrmRegistrationData<'bound> {
> +    /// Parent platform device.
>      pub(crate) pdev: ARef<platform::Device>,

This can just be &'bound platform::Device, or even &'bound platform::Device<Bound>.

>  
> +    /// Firmware sections.
> +    pub(crate) fw: Arc<Firmware<'bound>>,

This doesn't seem like it needs to be reference counted; at least from the
context I have from this patch.

> +
>      #[pin]
>      clks: Mutex<Clocks>,
>  
>      #[pin]
>      regulators: Mutex<Regulators>,
>  
> +    /// GPU MMIO register mapping.
> +    pub(crate) iomem: Arc<IoMem<'bound>>,

This may not need to be reference counted either; at least eventually.

It seems like this reference count only exists, so you are able to also store it
in struct Firmware.

This goes away once pin-init has the self-referential feature.

If there is no other reason for this reference count, you could temporarily do
what we do in nova-core [1] and obtain a &'bound IoMem<'bound> reference to
store in struct Firmware unsafely.

[1] https://lore.kernel.org/nova-gpu/20260525225838.276108-2-dakr@kernel.org/

> +
>      /// Some information on the GPU.
>      ///
>      /// This is mainly queried by userspace, i.e.: Mesa.
>      pub(crate) gpu_info: GpuInfo,
>  }
>  
> +impl ForLt for TyrDrmRegistrationData<'static> {
> +    type Of<'bound> = TyrDrmRegistrationData<'bound>;
> +}

This isn't wrong, but the intention is to use the ForLt! macro for this.

diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index acf5080a5467..694958509da7 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -87,9 +87,6 @@ pub(crate) struct TyrDrmRegistrationData<'bound> {
     pub(crate) gpu_info: GpuInfo,
 }
 
-impl ForLt for TyrDrmRegistrationData<'static> {
-    type Of<'bound> = TyrDrmRegistrationData<'bound>;
-}
 
 fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
     iomem.write_reg(GPU_COMMAND::reset(ResetMode::SoftReset));
@@ -221,7 +218,7 @@ fn drop(self: Pin<&mut Self>) {}
 #[vtable]
 impl drm::Driver for TyrDrmDriver {
     type Data = TyrDrmDeviceData;
-    type RegistrationData = TyrDrmRegistrationData<'static>;
+    type RegistrationData = ForLt!(TyrDrmRegistrationData<'_>);
     type File = TyrDrmFileData;
     type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
     type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;

> +
>  fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
>      iomem.write_reg(GPU_COMMAND::reset(ResetMode::SoftReset));
>  
> @@ -119,7 +136,7 @@ fn probe<'bound>(
>          let sram_regulator = Regulator::<regulator::Enabled>::get(pdev.as_ref(), c"sram")?;
>  
>          let request = pdev.io_request_by_index(0).ok_or(ENODEV)?;
> -        let iomem = request.iomap_sized::<SZ_2M>()?;
> +        let iomem = Arc::new(request.iomap_sized::<SZ_2M>()?, GFP_KERNEL)?;
>  
>          issue_soft_reset(pdev.as_ref(), &iomem)?;
>          gpu::l2_power_on(pdev.as_ref(), &iomem)?;
> @@ -137,8 +154,23 @@ fn probe<'bound>(
>  
>          let platform: ARef<platform::Device> = pdev.into();
>  
> -        let data = try_pin_init!(TyrDrmDeviceData {
> +        let dev_data = try_pin_init!(TyrDrmDeviceData {});
> +
> +        let unreg_dev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, dev_data)?;

Keeping TyrDrmDeviceData and calling the above try_pin_init! seems unnecessary;
you can just pass Ok(()) to drm::UnregisteredDevice::new().

> +
> +        let mmu = Mmu::new(iomem.as_arc_borrow(), &gpu_info)?;
> +
> +        let firmware = Firmware::new(
> +            pdev,
> +            iomem.clone(),
> +            &unreg_dev,
> +            mmu.as_arc_borrow(),
> +            &gpu_info,
> +        )?;
> +
> +        let reg_data = try_pin_init!(TyrDrmRegistrationData {
>                  pdev: platform.clone(),
> +                fw: firmware,
>                  clks <- new_mutex!(Clocks {
>                      core: core_clk,
>                      stacks: stacks_clk,
> @@ -148,11 +180,16 @@ fn probe<'bound>(
>                      _mali: mali_regulator,
>                      _sram: sram_regulator,
>                  }),
> +                iomem,
>                  gpu_info,
>          });
>  
> -        let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?;
> -        let reg = drm::Registration::new(pdev.as_ref(), tdev, (), 0)?;
> +        // SAFETY: `reg` is stored in the platform driver data and is not leaked or
> +        // forgotten, so it is dropped before the `'bound` registration data can become
> +        // invalid.
> +        let reg = unsafe {
> +            drm::driver::Registration::new_with_lt(pdev.as_ref(), unreg_dev, reg_data, 0)?

This is re-exported as drm::Registration.

> +        };