Re: [PATCH 08/10] gpu: nova-core: use projection for PFALCON and PFALCON2 registers

[Gary Guo]

On Tue Jul 28, 2026 at 7:45 AM BST, Alexandre Courbot wrote:
> On Wed Jul 22, 2026 at 1:54 AM JST, Gary Guo wrote:
>> Add fixed size region types for these and add projection methods that
>> project from `Bar0` into these. Update these registers to be registers on
>> `PFalconRegisters` and `PFalcon2Registers` and not relative registers on
>> `NovaRegisters`.
>>
>> The use sites are updated mechanically; calls to the projection methods are
>> not extracted in this commit.
>>
>> Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>
>> ---
>>  drivers/gpu/nova-core/falcon.rs                    | 158 +++++++++------------
>>  drivers/gpu/nova-core/falcon/fsp.rs                |  53 +++----
>>  drivers/gpu/nova-core/falcon/gsp.rs                |  43 +++---
>>  drivers/gpu/nova-core/falcon/hal/ga102.rs          |  39 ++---
>>  drivers/gpu/nova-core/falcon/hal/tu102.rs          |   8 +-
>>  drivers/gpu/nova-core/falcon/sec2.rs               |  32 +++--
>>  drivers/gpu/nova-core/firmware/fwsec/bootloader.rs |  11 +-
>>  drivers/gpu/nova-core/regs.rs                      |  81 ++++++-----
>>  8 files changed, 193 insertions(+), 232 deletions(-)
>>
> We are calling these methods quite a bit throughout the code. I
> understand they are supposed to be optimized away, but that's still a
> lot of repetition.
>
> Since we now have HRTB and `Falcon` is already referencing the `Bar0`,
> how about storing the projected `Mmio` inside the `Falcon` instance?
>
>     pub(crate) struct Falcon<'a, E: FalconEngine> {
>         ...
>         pfalcon: Mmio<'a, PFalconRegisters>,
>         pfalcon2: Mmio<'a, PFalcon2Registers>,
>     }
>
>     pub(crate) fn new(...) -> Result<Self> {
>         Ok(Self {
>             ...
>             pfalcon: E::pfalcon(bar),
>             pfalcon2: E::pfalcon2(bar),
>         })
>     }
>
> That way all the
>
>     E::pfalcon(self.bar)...
>
> Can become simply
>
>     self.pfalcon

Right, this is the direction that I do think we should head to. As described in
commit message, this commit is intended as a mechanical conversion and refactors
are left to the future.

>
> And I suspect that once this is generalized, `Falcon` won't even need to
> store a reference to the `Bar0` (and potentially poke the I/O of other
> engines) anymore.
>
> Actually I would like to push that even further and replace the
> `pfalcon()` and `pfalcon2()` trait methods by associated constants used
> to construct the projected view in `Falcon::new`, since the projections
> are all constructed the same way, but doing so requires
> `generic_const_exprs`. :/
>
> We could make it work by moving the `OFFSET` generic argument of
> `subregion` into a regular argument and enforcing its invariants using
> `build_assert!`, but that would require `subregion` to be
> `#[inline(always)]`. I don't know if there is another trick we can use,
> if not otherwise I guess the trait methods are ok, especially if they
> are only called once in the constructor.

This was definitely one option that I have considered, however as `NEW_SIZE`
still needs to be generic so we're not entirely turbofish-free. But if making
`offset` become an effective const parameter helps nova impl, it might make
sense to use `build_assert!` for this.

Best,
Gary