Re: [PATCH v2 2/2] drm/tyr: use IoMem directly instead of Devres

["Gary Guo" <gary@garyguo.net>]

On Tue Jun 2, 2026 at 2:12 AM BST, Deborah Brouwer wrote:
> On Mon, Jun 01, 2026 at 09:35:04AM +0000, Alice Ryhl wrote:
>> On Fri, May 29, 2026 at 02:00:54AM +0200, Danilo Krummrich wrote:
>> > Now that IoMem is lifetime-parameterized, use it directly in probe
>> > rather than wrapping it in Devres and Arc. The I/O memory mapping is
>> > only used during probe and not stored in driver data, so device-managed
>> > revocation is unnecessary.
>> > 
>> > This removes the Devres access(dev) pattern from issue_soft_reset(),
>> > GpuInfo::new(), and l2_power_on(), simplifying register access.
>> > 
>> > Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
>> > Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> 
>> > -pub(crate) type IoMem = kernel::io::mem::IoMem<'static, SZ_2M>;
>> > +pub(crate) type IoMem<'a> = kernel::io::mem::IoMem<'a, SZ_2M>;
>> 
>> It'd make more sense for me to put 'b or 'bound here.
>> 
>> >          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 = Arc::new(request.iomap_sized::<SZ_2M>()?.into_devres()?, GFP_KERNEL)?;
>> > +        let iomem = request.iomap_sized::<SZ_2M>()?;
>> >  
>> >          issue_soft_reset(pdev.as_ref(), &iomem)?;
>> >          gpu::l2_power_on(pdev.as_ref(), &iomem)?;
>> >  
>> > -        let gpu_info = GpuInfo::new(pdev.as_ref(), &iomem)?;
>> > +        let gpu_info = GpuInfo::new(&iomem);
>> 
>> 
>> While this change is fine, I notice that we don't actually keep the
>> iomem alive past the probe method. I assume we're going to need that,
>> which leads to the question of whether we can store the iomem in the
>> places we need it.
>> 
>> As far as I can tell, we can store it in TyrPlatformDriverData but not
>> in TyrDrmDeviceData, is that right?
>
> I'm still getting my head around how this applies to tyr's firmware
> series, but yes we stop storing iomem in TyrDrmDeviceData, but we won't
> store it in TyrPlatformDriverData.
> Instead there is a new struct "RegistrationData" that will store the iomem
> like this:
>
> #[vtable]
> impl drm::Driver for TyrDrmDriver {
>     type Data = TyrDrmDeviceData;
>     type RegistrationData = TyrDrmRegistrationData<'static>;
>

I am not sure what the distinction even mean for a class device?

There's 1 device per registration, so they're equal. Am I missing something?

Best,
Gary

> And then in probe something like:
>
>  let reg_data = try_pin_init!(TyrDrmRegistrationData {
>   pdev: platform.clone(),
>   fw: firmware,
>   clks <- new_mutex!(Clocks {
>     core: core_clk,
>     stacks: stacks_clk,
>     coregroup: coregroup_clk,
>   }),
>   regulators <- new_mutex!(Regulators {
>     _mali: mali_regulator,
>     _sram: sram_regulator,
>   }),
>   iomem,
>   gpu_info,
>  });
>
>  drm::driver::Registration::new_foreign_owned(ddev, pdev.as_ref(), reg_data, 0)?;
>
>
>> 
>> I guess it does make sense because the io memory goes away if the
>> underlying platform device (the bus) goes away, even if the drm device
>> still exists due to open fds from userspace.
>> 
>> Alice