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/[email protected]/
> +
> /// 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.
> + };
