Re: [PATCH REF 24/24] gpu: drm: tyr: use HRT lifetime for IoMem

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

On Tue, Apr 28, 2026 at 12:11:22AM +0200, Danilo Krummrich wrote:
> Take advantage of the lifetime-parameterized IoMem<'a> to use the
> memory mapping directly during probe, eliminating the Arc<Devres<IoMem>>
> indirection.
> 
> Since the IoMem is only used during probe, this also simplifies
> Register::read/write to be infallible -- the Devres access check is no
> longer needed, so reads return u32 directly and writes return ().

Hi Danilo,
Is the intended model that DRM drivers keep lifetime-bound resources such as
IoMem<'bound> only in platform drvdata and access them via Device::drvdata_borrow()?

Or is the expectation that drm::Driver should also have a lifetime-parameterized
Data associated type?

The reason I ask is that Tyr currently stores an MMIO handle in several areas,
(firmware, MMU/address-space management, and IRQ handling)  and it does register
accesses directly. See what we're trying to do:
https://lore.kernel.org/rust-for-linux/20260424-b4-fw-boot-v4-v4-0-a5d91050789d@collabora.com/

Moving IoMem<'bound> only into platform drvdata would require a big refactor
to thread &IoMem<'_> through those paths or fetch it from drvdata at each hardware
access site. So, I wanted to clarify the plan first before I start this work.

Thanks,
Deborah

> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> Not yet updated to Tyr using the register!() macro, but probably good enough for
> reference.
> ---
>  drivers/gpu/drm/tyr/driver.rs | 14 ++++----
>  drivers/gpu/drm/tyr/gpu.rs    | 62 +++++++++++++++++------------------
>  drivers/gpu/drm/tyr/regs.rs   | 21 +++---------
>  3 files changed, 41 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index eaa84efdfdf7..d305ad433e03 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -10,7 +10,6 @@
>          Core,
>          Device, //
>      },
> -    devres::Devres,
>      drm,
>      drm::ioctl,
>      io::poll,
> @@ -23,7 +22,6 @@
>      sizes::SZ_2M,
>      sync::{
>          aref::ARef,
> -        Arc,
>          Mutex, //
>      },
>      time, //
> @@ -37,7 +35,7 @@
>      regs, //
>  };
>  
> -pub(crate) type IoMem = kernel::io::mem::IoMem<'static, SZ_2M>;
> +pub(crate) type IoMem = kernel::io::Mmio<SZ_2M>;
>  
>  pub(crate) struct TyrDrmDriver;
>  
> @@ -65,11 +63,11 @@ pub(crate) struct TyrDrmDeviceData {
>      pub(crate) gpu_info: GpuInfo,
>  }
>  
> -fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
> -    regs::GPU_CMD.write(dev, iomem, regs::GPU_CMD_SOFT_RESET)?;
> +fn issue_soft_reset(dev: &Device<Bound>, iomem: &IoMem) -> Result {
> +    regs::GPU_CMD.write(iomem, regs::GPU_CMD_SOFT_RESET);
>  
>      poll::read_poll_timeout(
> -        || regs::GPU_IRQ_RAWSTAT.read(dev, iomem),
> +        || Ok(regs::GPU_IRQ_RAWSTAT.read(iomem)),
>          |status| *status & regs::GPU_IRQ_RAWSTAT_RESET_COMPLETED != 0,
>          time::Delta::from_millis(1),
>          time::Delta::from_millis(100),
> @@ -109,12 +107,12 @@ fn probe(
>          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);
>  
>          let platform: ARef<platform::Device> = pdev.into();
> diff --git a/drivers/gpu/drm/tyr/gpu.rs b/drivers/gpu/drm/tyr/gpu.rs
> index a88775160f98..bb0473c85bf7 100644
> --- a/drivers/gpu/drm/tyr/gpu.rs
> +++ b/drivers/gpu/drm/tyr/gpu.rs
> @@ -10,7 +10,6 @@
>          Bound,
>          Device, //
>      },
> -    devres::Devres,
>      io::poll,
>      platform,
>      prelude::*,
> @@ -35,37 +34,36 @@
>  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 gpu_id = regs::GPU_ID.read(dev, iomem)?;
> -        let csf_id = regs::GPU_CSF_ID.read(dev, iomem)?;
> -        let gpu_rev = regs::GPU_REVID.read(dev, iomem)?;
> -        let core_features = regs::GPU_CORE_FEATURES.read(dev, iomem)?;
> -        let l2_features = regs::GPU_L2_FEATURES.read(dev, iomem)?;
> -        let tiler_features = regs::GPU_TILER_FEATURES.read(dev, iomem)?;
> -        let mem_features = regs::GPU_MEM_FEATURES.read(dev, iomem)?;
> -        let mmu_features = regs::GPU_MMU_FEATURES.read(dev, iomem)?;
> -        let thread_features = regs::GPU_THREAD_FEATURES.read(dev, iomem)?;
> -        let max_threads = regs::GPU_THREAD_MAX_THREADS.read(dev, iomem)?;
> -        let thread_max_workgroup_size = regs::GPU_THREAD_MAX_WORKGROUP_SIZE.read(dev, iomem)?;
> -        let thread_max_barrier_size = regs::GPU_THREAD_MAX_BARRIER_SIZE.read(dev, iomem)?;
> -        let coherency_features = regs::GPU_COHERENCY_FEATURES.read(dev, iomem)?;
> -
> -        let texture_features = regs::GPU_TEXTURE_FEATURES0.read(dev, iomem)?;
> -
> -        let as_present = regs::GPU_AS_PRESENT.read(dev, iomem)?;
> -
> -        let shader_present = u64::from(regs::GPU_SHADER_PRESENT_LO.read(dev, iomem)?);
> +    pub(crate) fn new(iomem: &IoMem) -> Self {
> +        let gpu_id = regs::GPU_ID.read(iomem);
> +        let csf_id = regs::GPU_CSF_ID.read(iomem);
> +        let gpu_rev = regs::GPU_REVID.read(iomem);
> +        let core_features = regs::GPU_CORE_FEATURES.read(iomem);
> +        let l2_features = regs::GPU_L2_FEATURES.read(iomem);
> +        let tiler_features = regs::GPU_TILER_FEATURES.read(iomem);
> +        let mem_features = regs::GPU_MEM_FEATURES.read(iomem);
> +        let mmu_features = regs::GPU_MMU_FEATURES.read(iomem);
> +        let thread_features = regs::GPU_THREAD_FEATURES.read(iomem);
> +        let max_threads = regs::GPU_THREAD_MAX_THREADS.read(iomem);
> +        let thread_max_workgroup_size = regs::GPU_THREAD_MAX_WORKGROUP_SIZE.read(iomem);
> +        let thread_max_barrier_size = regs::GPU_THREAD_MAX_BARRIER_SIZE.read(iomem);
> +        let coherency_features = regs::GPU_COHERENCY_FEATURES.read(iomem);
> +
> +        let texture_features = regs::GPU_TEXTURE_FEATURES0.read(iomem);
> +
> +        let as_present = regs::GPU_AS_PRESENT.read(iomem);
> +
> +        let shader_present = u64::from(regs::GPU_SHADER_PRESENT_LO.read(iomem));
>          let shader_present =
> -            shader_present | u64::from(regs::GPU_SHADER_PRESENT_HI.read(dev, iomem)?) << 32;
> +            shader_present | u64::from(regs::GPU_SHADER_PRESENT_HI.read(iomem)) << 32;
>  
> -        let tiler_present = u64::from(regs::GPU_TILER_PRESENT_LO.read(dev, iomem)?);
> -        let tiler_present =
> -            tiler_present | u64::from(regs::GPU_TILER_PRESENT_HI.read(dev, iomem)?) << 32;
> +        let tiler_present = u64::from(regs::GPU_TILER_PRESENT_LO.read(iomem));
> +        let tiler_present = tiler_present | u64::from(regs::GPU_TILER_PRESENT_HI.read(iomem)) << 32;
>  
> -        let l2_present = u64::from(regs::GPU_L2_PRESENT_LO.read(dev, iomem)?);
> -        let l2_present = l2_present | u64::from(regs::GPU_L2_PRESENT_HI.read(dev, iomem)?) << 32;
> +        let l2_present = u64::from(regs::GPU_L2_PRESENT_LO.read(iomem));
> +        let l2_present = l2_present | u64::from(regs::GPU_L2_PRESENT_HI.read(iomem)) << 32;
>  
> -        Ok(Self(uapi::drm_panthor_gpu_info {
> +        Self(uapi::drm_panthor_gpu_info {
>              gpu_id,
>              gpu_rev,
>              csf_id,
> @@ -88,7 +86,7 @@ pub(crate) fn new(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result<Self> {
>              core_features,
>              pad: 0,
>              gpu_features: 0,
> -        }))
> +        })
>      }
>  
>      pub(crate) fn log(&self, pdev: &platform::Device) {
> @@ -208,11 +206,11 @@ fn from(value: u32) -> Self {
>  }
>  
>  /// Powers on the l2 block.
> -pub(crate) fn l2_power_on(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
> -    regs::L2_PWRON_LO.write(dev, iomem, 1)?;
> +pub(crate) fn l2_power_on(dev: &Device<Bound>, iomem: &IoMem) -> Result {
> +    regs::L2_PWRON_LO.write(iomem, 1);
>  
>      poll::read_poll_timeout(
> -        || regs::L2_READY_LO.read(dev, iomem),
> +        || Ok(regs::L2_READY_LO.read(iomem)),
>          |status| *status == 1,
>          Delta::from_millis(1),
>          Delta::from_millis(100),
> diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
> index 611870c2e6af..0881b3812afd 100644
> --- a/drivers/gpu/drm/tyr/regs.rs
> +++ b/drivers/gpu/drm/tyr/regs.rs
> @@ -7,16 +7,7 @@
>  // does.
>  #![allow(dead_code)]
>  
> -use kernel::{
> -    bits::bit_u32,
> -    device::{
> -        Bound,
> -        Device, //
> -    },
> -    devres::Devres,
> -    io::Io,
> -    prelude::*, //
> -};
> +use kernel::{bits::bit_u32, io::Io};
>  
>  use crate::driver::IoMem;
>  
> @@ -29,15 +20,13 @@
>  
>  impl<const OFFSET: usize> Register<OFFSET> {
>      #[inline]
> -    pub(crate) fn read(&self, dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result<u32> {
> -        let value = (*iomem).access(dev)?.read32(OFFSET);
> -        Ok(value)
> +    pub(crate) fn read(&self, iomem: &IoMem) -> u32 {
> +        iomem.read32(OFFSET)
>      }
>  
>      #[inline]
> -    pub(crate) fn write(&self, dev: &Device<Bound>, iomem: &Devres<IoMem>, value: u32) -> Result {
> -        (*iomem).access(dev)?.write32(value, OFFSET);
> -        Ok(())
> +    pub(crate) fn write(&self, iomem: &IoMem, value: u32) {
> +        iomem.write32(value, OFFSET);
>      }
>  }
>  
> -- 
> 2.54.0
>