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 <[email protected]>
>> > Reviewed-by: Alexandre Courbot <[email protected]>
>> > Signed-off-by: Danilo Krummrich <[email protected]>
>>
>> > -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
