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-38351df09...@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.
