Re: [PATCH v3 12/16] gpu: nova-core: use projection for PFALCON and PFALCON2 registers

On Wed Aug 19, 2026 at 8:09 PM JST, Gary Guo wrote:
> Add fixed size region types `PFalconRegisters` and `PFalcon2Registers` and
> update PFALCON and PFALCON2 registers to be fixed register on them and not
> relative registers on `NovaRegisters`.
>
> Update `Falcon` struct to store projected views when constructing and
> access with `self.pfalcon` and `self.pfalcon2`.
>
> Signed-off-by: Gary Guo <[email protected]>

Reviewed-by: Alexandre Courbot <[email protected]>

> ---
>  drivers/gpu/nova-core/falcon.rs                    | 157 +++++++++------------
>  drivers/gpu/nova-core/falcon/fsp.rs                |  63 +++++----
>  drivers/gpu/nova-core/falcon/gsp.rs                |  51 ++++---
>  drivers/gpu/nova-core/falcon/hal/ga102.rs          |  62 ++++----
>  drivers/gpu/nova-core/falcon/hal/tu102.rs          |   9 +-
>  drivers/gpu/nova-core/falcon/sec2.rs               |  37 +++--
>  drivers/gpu/nova-core/firmware/fwsec/bootloader.rs |  18 +--
>  drivers/gpu/nova-core/gsp/hal/tu102.rs             |   7 +-
>  drivers/gpu/nova-core/regs.rs                      |  92 ++++++------
>  9 files changed, 238 insertions(+), 258 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 65cb12d26e2b..119d305e2ea3 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -14,13 +14,12 @@
>      io::{
>          io_project,
>          poll::read_poll_timeout,
> -        register::{
> -            RegisterBase,
> -            WithBase, //
> -        },
> +        register::Array,
>          Io,
> +        Mmio, //
>      },
>      prelude::*,
> +    sizes::SZ_4K,
>      time::Delta,
>  };
>  
> @@ -165,18 +164,22 @@ pub(crate) enum FalconFbifMemType with From<Bounded<u32, 1>> {
>      }
>  }
>  
> -/// Type used to represent the `PFALCON` registers address base for a given falcon engine.
> -pub(crate) struct PFalconBase(());
> +/// Type used to represent the `PFALCON` registers.
> +#[repr(align(4))]
> +#[derive(FromBytes, IntoBytes)]
> +pub(crate) struct PFalconRegisters([u8; SZ_4K]);
>  
> -/// Type used to represent the `PFALCON2` registers address base for a given falcon engine.
> -pub(crate) struct PFalcon2Base(());
> +/// Type used to represent the `PFALCON2` registers.
> +#[repr(align(4))]
> +#[derive(FromBytes, IntoBytes)]
> +pub(crate) struct PFalcon2Registers([u8; SZ_4K]);
>  
>  /// Trait defining the parameters of a given Falcon engine.
>  ///
>  /// Each engine provides one base for `PFALCON` and `PFALCON2` registers.
> -pub(crate) trait FalconEngine:
> -    Send + Sync + RegisterBase<PFalconBase> + RegisterBase<PFalcon2Base> + Sized
> -{
> +pub(crate) trait FalconEngine: Send + Sync + Sized {
> +    fn pfalcon(io: Bar0<'_>) -> Mmio<'_, PFalconRegisters>;
> +    fn pfalcon2(io: Bar0<'_>) -> Mmio<'_, PFalcon2Registers>;
>  }
>  
>  /// Represents a portion of the firmware to be loaded into a particular memory (e.g. IMEM or DMEM)
> @@ -358,6 +361,8 @@ pub(crate) struct Falcon<'a, E: FalconEngine> {
>      hal: KBox<dyn FalconHal<E>>,
>      dev: &'a device::Device<device::Bound>,
>      bar: Bar0<'a>,
> +    pub(crate) pfalcon: Mmio<'a, PFalconRegisters>,

The only spot that accesses this outside of the `falcon` module is
`FwsecFirmwareWithBl`, and only for the GSP falcon.  I guess we need
some sort of `configure_dma_transfer` method to `Falcon<Gsp>` that
`FwsecFirmwareWithBl` will call, so we can keep this private.

Actually I that reminded me that Antonin ran into the same issue with
his series [1], so that's a logical abstraction to have regardless.
Since we're going to do it, can you just add a `// TODO: make private`
item so we don't lose track of it? Then I'll work with Antonin to add
the method and make this private after this patchset is merged.

[1] https://lore.kernel.org/all/20260826-b4-extract-pfsp-registers-to-falcon-mod-v1-4-38351df09084@nanonej.com/

<...>
> diff --git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
> index 7600ee07ca2e..7880c145f8f7 100644
> --- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
> +++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
> @@ -6,11 +6,9 @@
>      device,
>      io::{
>          poll::read_poll_timeout,
> -        register::{
> -            Array,
> -            WithBase, //
> -        },
> -        Io, //
> +        register::Array,
> +        Io,
> +        Mmio, //
>      },
>      prelude::*,
>      time::Delta, //
> @@ -24,6 +22,7 @@
>          FalconBromParams,
>          FalconEngine,
>          FalconModSelAlgo,
> +        PFalcon2Registers,
>          PeregrineCoreSelect, //
>      },
>      regs,
> @@ -31,17 +30,16 @@
>  
>  use super::FalconHal;
>  
> -fn select_core_ga102<E: FalconEngine>(bar: Bar0<'_>) -> Result {
> -    let bcr_ctrl = bar.read(regs::NV_PRISCV_RISCV_BCR_CTRL::of::<E>());
> +fn select_core_ga102(pfalcon2: Mmio<'_, PFalcon2Registers>) -> Result {
> +    let bcr_ctrl = pfalcon2.read(regs::NV_PRISCV_RISCV_BCR_CTRL);
>      if bcr_ctrl.core_select() != PeregrineCoreSelect::Falcon {
> -        bar.write(
> -            WithBase::of::<E>(),
> +        pfalcon2.write_reg(
>              regs::NV_PRISCV_RISCV_BCR_CTRL::zeroed().with_core_select(PeregrineCoreSelect::Falcon),
>          );
>  
>          // TIMEOUT: falcon core should take less than 10ms to report being enabled.
>          read_poll_timeout(
> -            || Ok(bar.read(regs::NV_PRISCV_RISCV_BCR_CTRL::of::<E>())),
> +            || Ok(pfalcon2.read(regs::NV_PRISCV_RISCV_BCR_CTRL)),
>              |r| r.valid(),
>              Delta::ZERO,
>              Delta::from_millis(10),
> @@ -86,24 +84,23 @@ fn signature_reg_fuse_version_ga102(
>      Ok(u16::BITS - reg_fuse_version.leading_zeros())
>  }
>  
> -fn program_brom_ga102<E: FalconEngine>(bar: Bar0<'_>, params: &FalconBromParams) {
> -    bar.write(
> -        WithBase::of::<E>().at(0),
> +fn program_brom_ga102(
> +    pfalcon2: Mmio<'_, PFalcon2Registers>,
> +    params: &FalconBromParams,
> +) {

rustfmt triggers here.