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

[Deborah Brouwer <deborah.brouwer@collabora.com>]

On Tue, May 26, 2026 at 11:43:38AM +0900, Eliot Courtney wrote:
> On Tue May 26, 2026 at 8:01 AM JST, 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.
> >
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >  drivers/gpu/drm/tyr/driver.rs | 19 ++++++-------------
> >  drivers/gpu/drm/tyr/gpu.rs    | 13 +++----------
> >  2 files changed, 9 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> > index 5f4c484f671f..b9a5cc5fc678 100644
> > --- a/drivers/gpu/drm/tyr/driver.rs
> > +++ b/drivers/gpu/drm/tyr/driver.rs
> > @@ -6,11 +6,9 @@
> >          OptionalClk, //
> >      },
> >      device::{
> > -        Bound,
> >          Core,
> >          Device, //
> >      },
> > -    devres::Devres,
> >      dma::{
> >          Device as DmaDevice,
> >          DmaMask, //
> > @@ -30,7 +28,6 @@
> >      sizes::SZ_2M,
> >      sync::{
> >          aref::ARef,
> > -        Arc,
> >          Mutex, //
> >      },
> >      time, //
> > @@ -44,7 +41,7 @@
> >      regs::gpu_control::*, //
> >  };
> >  
> > -pub(crate) type IoMem = kernel::io::mem::IoMem<'static, SZ_2M>;
> > +pub(crate) type IoMem<'a> = kernel::io::mem::IoMem<'a, SZ_2M>;
> >  
> >  pub(crate) struct TyrDrmDriver;
> >  
> > @@ -74,15 +71,11 @@ pub(crate) struct TyrDrmDeviceData {
> >      pub(crate) gpu_info: GpuInfo,
> >  }
> >  
> > -fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
> > -    let io = (*iomem).access(dev)?;
> > -    io.write_reg(GPU_COMMAND::reset(ResetMode::SoftReset));
> > +fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
> > +    iomem.write_reg(GPU_COMMAND::reset(ResetMode::SoftReset));
> >  
> >      poll::read_poll_timeout(
> > -        || {
> > -            let io = (*iomem).access(dev)?;
> > -            Ok(io.read(GPU_IRQ_RAWSTAT))
> > -        },
> > +        || Ok(iomem.read(GPU_IRQ_RAWSTAT)),
> >          |status| status.reset_completed(),
> >          time::Delta::from_millis(1),
> >          time::Delta::from_millis(100),
> > @@ -123,12 +116,12 @@ 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 = 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)?;
> >          gpu_info.log(pdev.as_ref());
> >  
> >          let pa_bits = MMU_FEATURES::from_raw(gpu_info.mmu_features)
> > diff --git a/drivers/gpu/drm/tyr/gpu.rs b/drivers/gpu/drm/tyr/gpu.rs
> > index 652556026f50..3acffefaf210 100644
> > --- a/drivers/gpu/drm/tyr/gpu.rs
> > +++ b/drivers/gpu/drm/tyr/gpu.rs
> > @@ -9,7 +9,6 @@
> >          Bound,
> >          Device, //
> >      },
> > -    devres::Devres,
> >      io::{
> >          poll,
> >          register::Array,
> > @@ -40,9 +39,7 @@
> >  pub(crate) struct GpuInfo(pub(crate) uapi::drm_panthor_gpu_info);
> >  
> >  impl GpuInfo {
> > -    pub(crate) fn new(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result<Self> {
> > -        let io = (*iomem).access(dev)?;
> > -
> > +    pub(crate) fn new(io: &IoMem<'_>) -> Result<Self> {
> 
> Perhaps a future patch can remove this now unnecessary looking Result in
> the return type.

Oh that's right now that we don't have: let io = (*iomem).access(dev)?
we don't need the Result anymore.

Danilo, I can add this fix to our fw-boot series which I'm preparing to sit
on top of these changes.

> 
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
>