Re: [PATCH v7 7/7] drm/tyr: add Microcontroller Unit (MCU) booting

> On 14 Jul 2026, at 00:18, Deborah Brouwer <[email protected]> wrote:
> 
> Add a firmware module to load, parse, and map the MCU firmware sections
> into shared GEM memory at the required virtual addresses accessible by the
> GPU.
> 
> Create a firmware instance during probe and store it inside the
> TyrDrmRegistrationData to keep it alive after probe. Use the firmware
> instance to boot the MCU.
> 
> Remove the dead-code annotations from the MMU, VM, slot manager, and
> kernel BO code now that these paths are used by the firmware module.
> 
> Update Kconfig to add the RUST_FW_LOADER_ABSTRACTIONS dependency
> required by this module.
> 
> Co-developed-by: Boris Brezillon <[email protected]>
> Signed-off-by: Boris Brezillon <[email protected]>
> Signed-off-by: Deborah Brouwer <[email protected]>
> ---
> drivers/gpu/drm/tyr/Kconfig   |   1 +
> drivers/gpu/drm/tyr/driver.rs |  17 ++-
> drivers/gpu/drm/tyr/fw.rs     | 263 ++++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/tyr/gem.rs    |   3 -
> drivers/gpu/drm/tyr/tyr.rs    |   1 +
> drivers/gpu/drm/tyr/vm.rs     |   1 -
> 6 files changed, 281 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tyr/Kconfig b/drivers/gpu/drm/tyr/Kconfig
> index 79ea4bb214de..8f13e49f11f9 100644
> --- a/drivers/gpu/drm/tyr/Kconfig
> +++ b/drivers/gpu/drm/tyr/Kconfig
> @@ -13,6 +13,7 @@ config DRM_TYR
> select IOMMU_IO_PGTABLE_LPAE
> select RUST_DRM_GEM_SHMEM_HELPER
> select RUST_DRM_GPUVM
> + select RUST_FW_LOADER_ABSTRACTIONS
> help
>  Rust DRM driver for ARM Mali CSF-based GPUs.
> 
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index 9195c8be5203..46ba91af9aca 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -37,6 +37,7 @@
> 
> use crate::{
>     file::TyrDrmFileData,
> +    fw::Firmware,
>     gem::Bo,
>     gpu,
>     gpu::GpuInfo,
> @@ -67,6 +68,9 @@ pub(crate) struct TyrDrmRegistrationData<'bound> {
>     /// Parent platform device.
>     pub(crate) pdev: &'bound platform::Device<Bound>,
> 
> +    /// Firmware sections.
> +    pub(crate) fw: Firmware<'bound>,
> +
>     #[pin]
>     clks: Mutex<Clocks>,
> 
> @@ -144,10 +148,21 @@ fn probe<'bound>(
> 
>         let unreg_dev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, Ok(()))?;
> 
> -        let _mmu = Mmu::new(iomem.as_arc_borrow(), &gpu_info)?;
> +        let mmu = Mmu::new(iomem.as_arc_borrow(), &gpu_info)?;
> +
> +        let firmware = Firmware::new(
> +            pdev.as_ref(),
> +            iomem.clone(),
> +            &unreg_dev,
> +            mmu.as_arc_borrow(),
> +            &gpu_info,
> +        )?;
> +
> +        firmware.boot()?;
> 
>         let reg_data = try_pin_init!(TyrDrmRegistrationData {
>                 pdev,
> +                fw: firmware,
>                 clks <- new_mutex!(Clocks {
>                     core: core_clk,
>                     stacks: stacks_clk,
> diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
> new file mode 100644
> index 000000000000..554808a792aa
> --- /dev/null
> +++ b/drivers/gpu/drm/tyr/fw.rs
> @@ -0,0 +1,263 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +//! Firmware loading and management for Mali CSF GPU.
> +//!
> +//! This module handles loading the Mali GPU firmware binary, parsing it into sections,
> +//! and mapping those sections into the MCU's virtual address space. Each firmware section
> +//! has specific properties (read/write/execute permissions, cache modes) and must be loaded
> +//! at specific virtual addresses expected by the MCU.
> +//!
> +//! See [`Firmware`] for the main firmware management interface and [`Section`] for
> +//! individual firmware sections.
> +//!
> +//! [`Firmware`]: crate::fw::Firmware
> +//! [`Section`]: crate::fw::Section
> +
> +use kernel::{
> +    bits::genmask_u32,
> +    device::{
> +        Bound,
> +        Device, //
> +    },
> +    drm::{
> +        gem::BaseObject, //
> +    },
> +    impl_flags,
> +    io::{
> +        poll,
> +        Io, //
> +    },
> +    prelude::*,
> +    str::CString,
> +    sync::{
> +        Arc,
> +        ArcBorrow, //
> +    },
> +    time, //
> +};
> +
> +use crate::{
> +    driver::{
> +        IoMem,
> +        TyrDrmDevice, //
> +    },
> +    fw::parser::{
> +        FwParser,
> +        ParsedSection, //
> +    },
> +    gem,
> +    gem::{
> +        KernelBo,
> +        KernelBoVaAlloc, //
> +    },
> +    gpu::GpuInfo,
> +
> +    mmu::Mmu,
> +    regs::{
> +        gpu_control::{
> +            McuControlMode,
> +            McuStatus,
> +            GPU_ID,
> +            MCU_CONTROL,
> +            MCU_STATUS, //
> +        }, //
> +        job_control::JOB_IRQ_RAWSTAT, //
> +    },
> +    vm::Vm, //
> +};
> +
> +mod parser;
> +
> +impl_flags!(
> +    #[derive(Debug, Clone, Default, Copy, PartialEq, Eq)]
> +    pub(super) struct SectionFlags(u32);
> +
> +    #[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +    pub(super) enum SectionFlag {
> +        Read = 1 << 0,
> +        Write = 1 << 1,
> +        Exec = 1 << 2,
> +        CacheModeNone = 0 << 3,
> +        CacheModeCached = 1 << 3,
> +        CacheModeUncachedCoherent = 2 << 3,
> +        CacheModeCachedCoherent = 3 << 3,
> +        Prot = 1 << 5,
> +        Shared = 1 << 30,
> +        Zero = 1 << 31,
> +    }
> +);

Can we please rewrite this with the bitfield macro instead? 

I think we can use something like:

4:3 cache_mode => CacheMode


> +
> +pub(super) const CACHE_MODE_MASK: SectionFlags = SectionFlags(genmask_u32(3..=4));
> +
> +pub(super) const CSF_MCU_SHARED_REGION_START: u32 = 0x04000000;
> +
> +impl SectionFlags {
> +    fn cache_mode(&self) -> SectionFlags {
> +        *self & CACHE_MODE_MASK
> +    }
> +}
> +
> +impl TryFrom<u32> for SectionFlags {
> +    type Error = Error;
> +
> +    fn try_from(value: u32) -> Result<Self, Self::Error> {
> +        let valid_flags = SectionFlags::from(SectionFlag::Read)
> +            | SectionFlags::from(SectionFlag::Write)
> +            | SectionFlags::from(SectionFlag::Exec)
> +            | CACHE_MODE_MASK
> +            | SectionFlags::from(SectionFlag::Prot)
> +            | SectionFlags::from(SectionFlag::Shared)
> +            | SectionFlags::from(SectionFlag::Zero);
> +
> +        if value & valid_flags.0 != value {
> +            Err(EINVAL)
> +        } else {
> +            Ok(Self(value))
> +        }
> +    }
> +}
> +
> +/// A parsed section of the firmware binary.
> +struct Section<'bound> {
> +    // Raw firmware section data for reset purposes
> +    #[expect(dead_code)]
> +    data: KVec<u8>,
> +
> +    // Keep the BO backing this firmware section so that both the
> +    // GPU mapping and CPU mapping remain valid until the Section is dropped.
> +    #[expect(dead_code)]
> +    mem: gem::KernelBo<'bound>,
> +}
> +
> +/// Loaded firmware with sections mapped into MCU VM.
> +pub(crate) struct Firmware<'bound> {
> +    /// Iomem need to access registers.
> +    iomem: Arc<IoMem<'bound>>,
> +
> +    /// MCU VM.
> +    vm: Arc<Vm<'bound>>,
> +
> +    /// List of firmware sections.
> +    #[expect(dead_code)]
> +    sections: KVec<Section<'bound>>,
> +}
> +
> +impl<'bound> Drop for Firmware<'bound> {
> +    fn drop(&mut self) {
> +        // AS slots retain a VM ref, we need to kill the circular ref manually.
> +        self.vm.kill();
> +    }
> +}
> +
> +impl<'bound> Firmware<'bound> {
> +    fn init_section_mem(mem: &mut KernelBo<'bound>, data: &KVec<u8>) -> Result {
> +        if data.is_empty() {
> +            return Ok(());
> +        }
> +
> +        let vmap = mem.bo.vmap::<0>()?;
> +        let size = mem.bo.size();
> +
> +        if data.len() > size {
> +            pr_err!("fw section {} bigger than BO {}\n", data.len(), size);
> +            return Err(EINVAL);
> +        }
> +
> +        for (i, &byte) in data.iter().enumerate() {
> +            vmap.try_write8(byte, i)?;
> +        }
> +
> +        Ok(())
> +    }
> +
> +    fn request(ddev: &TyrDrmDevice, gpu_info: &GpuInfo) -> Result<kernel::firmware::Firmware> {
> +        let gpu_id = GPU_ID::from_raw(gpu_info.gpu_id);
> +
> +        let path = CString::try_from_fmt(fmt!(
> +            "arm/mali/arch{}.{}/mali_csffw.bin",
> +            gpu_id.arch_major().get(),
> +            gpu_id.arch_minor().get()
> +        ))?;
> +
> +        kernel::firmware::Firmware::request(&path, ddev.as_ref().as_ref())
> +    }
> +
> +    fn load(
> +        ddev: &TyrDrmDevice,
> +        gpu_info: &GpuInfo,
> +    ) -> Result<(kernel::firmware::Firmware, KVec<ParsedSection>)> {
> +        let fw = Self::request(ddev, gpu_info)?;
> +        let mut parser = FwParser::new(fw.data());
> +
> +        let parsed_sections = parser.parse()?;
> +
> +        Ok((fw, parsed_sections))
> +    }
> +
> +    /// Load firmware and map sections into MCU VM.
> +    pub(crate) fn new(
> +        dev: &'bound Device<Bound>,
> +        iomem: Arc<IoMem<'bound>>,
> +        ddev: &TyrDrmDevice,
> +        mmu: ArcBorrow<'_, Mmu<'bound>>,
> +        gpu_info: &GpuInfo,
> +    ) -> Result<Firmware<'bound>> {
> +        let vm = Vm::new(dev, ddev, mmu, gpu_info)?;
> +        vm.activate()?;
> +
> +        let (fw, parsed_sections) = Self::load(ddev, gpu_info)?;
> +        let mut sections = KVec::new();
> +        for parsed in parsed_sections {
> +            let size = (parsed.va.end - parsed.va.start) as usize;
> +            let va = u64::from(parsed.va.start);
> +
> +            let mut mem = KernelBo::new(
> +                ddev,
> +                vm.clone(),
> +                size.try_into().unwrap(),
> +                KernelBoVaAlloc::Explicit(va),
> +                parsed.vm_map_flags,
> +            )?;
> +
> +            let section_start = parsed.data_range.start as usize;
> +            let section_end = parsed.data_range.end as usize;
> +            let mut data = KVec::new();
> +
> +            // Ensure that the firmware slice is not out of bounds.
> +            let fw_data = fw.data();
> +            let bytes = fw_data.get(section_start..section_end).ok_or(EINVAL)?;
> +            data.extend_from_slice(bytes, GFP_KERNEL)?;
> +
> +            Self::init_section_mem(&mut mem, &data)?;
> +
> +            sections.push(Section { data, mem }, GFP_KERNEL)?;
> +        }
> +
> +        let firmware = Firmware {
> +            iomem,
> +            vm,
> +            sections,
> +        };
> +
> +        Ok(firmware)
> +    }
> +
> +    pub(crate) fn boot(&self) -> Result {
> +        let io = &self.iomem;
> +        io.write_reg(MCU_CONTROL::zeroed().with_req(McuControlMode::Auto));
> +
> +        if let Err(e) = poll::read_poll_timeout(
> +            || Ok((io.read(MCU_STATUS), io.read(JOB_IRQ_RAWSTAT))),
> +            |(mcu_status, irq_rawstat)| {
> +                mcu_status.value() == McuStatus::Enabled && irq_rawstat.glb()
> +            },
> +            time::Delta::from_millis(1),
> +            time::Delta::from_millis(100),
> +        ) {
> +            let status = io.read(MCU_STATUS);
> +            pr_err!("MCU failed to boot, status: {:?}", status.value());
> +            return Err(e);
> +        }
> +        Ok(())
> +    }
> +}
> diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
> index 47a05a33388e..e9bf35682a47 100644
> --- a/drivers/gpu/drm/tyr/gem.rs
> +++ b/drivers/gpu/drm/tyr/gem.rs
> @@ -72,7 +72,6 @@ pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice) -> Result<ARef<Bo>> {
> /// An automatic VA allocation strategy will be added in the future.
> pub(crate) enum KernelBoVaAlloc {
>     /// Explicit VA address specified by the caller.
> -    #[expect(dead_code)]
>     Explicit(u64),
> }
> 
> @@ -85,7 +84,6 @@ pub(crate) enum KernelBoVaAlloc {
> /// When dropped, the buffer is automatically unmapped from the GPU VA space.
> pub(crate) struct KernelBo<'bound> {
>     /// The underlying GEM buffer object.
> -    #[expect(dead_code)]
>     pub(crate) bo: ARef<Bo>,
>     /// The GPU VM this buffer is mapped into.
>     vm: Arc<Vm<'bound>>,
> @@ -99,7 +97,6 @@ impl<'bound> KernelBo<'bound> {
>     /// This function allocates a new shmem-backed GEM object and immediately maps
>     /// it into the specified GPU virtual memory space. The mapping is automatically
>     /// cleaned up when the [`KernelBo`] is dropped.
> -    #[expect(dead_code)]
>     pub(crate) fn new(
>         ddev: &TyrDrmDevice,
>         vm: Arc<Vm<'bound>>,
> diff --git a/drivers/gpu/drm/tyr/tyr.rs b/drivers/gpu/drm/tyr/tyr.rs
> index 92f6885cdaae..e7ec450bdc9c 100644
> --- a/drivers/gpu/drm/tyr/tyr.rs
> +++ b/drivers/gpu/drm/tyr/tyr.rs
> @@ -9,6 +9,7 @@
> 
> mod driver;
> mod file;
> +mod fw;
> mod gem;
> mod gpu;
> mod mmu;
> diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
> index 2358096afc7e..7534e68359bf 100644
> --- a/drivers/gpu/drm/tyr/vm.rs
> +++ b/drivers/gpu/drm/tyr/vm.rs
> @@ -6,7 +6,6 @@
> //! the illusion of owning the entire virtual address (VA) range, similar to CPU virtual memory.
> //! Each virtual memory (VM) area is backed by ARM64 LPAE Stage 1 page tables and can be
> //! mapped into hardware address space (AS) slots for GPU execution.
> -#![expect(dead_code)]
> 
> use core::marker::PhantomData;
> use core::ops::Range;
> 
> -- 
> 2.54.0
> 

Apart from the change I mentioned, this patch looks good to me.