Re: [PATCH 4/4] gpu: nova-core: Extract PFALCON register

[Nanonej Dev]

On Friday, August 28th, 2026 at 03:12, Alexandre Courbot <acourbot@xxxxxxxxxx> wrote:

> On Wed Aug 26, 2026 at 8:44 AM JST, Antonin Malzieu Ridolfi via B4 Relay wrote:
> <...>
> > +    // Configure DMA index for the bootloader to fetch the FWSEC firmware from system memory.
> > +    pub(crate) fn set_fbif_transcfg_regs_at(&self, ctx_dma: u32) -> Result {
> > +        self.bar.update(
> > +            regs::NV_PFALCON_FBIF_TRANSCFG::of::<E>()
> > +                .try_at(usize::from_safe_cast(ctx_dma))
> > +                .ok_or(EINVAL)?,
> > +            |v| {
> > +                v.with_target(FalconFbifTarget::CoherentSysmem)
> > +                    .with_mem_type(FalconFbifMemType::Physical)
> > +            },
> > +        );
> > +        Ok(())
> > +    }
>
> Having this will be nice in that it will also enable us to keep
> `pfalcon` private in [1].
>
> [1] https://lore.kernel.org/all/DL068LFDXPM1.7A1482C1UJF@xxxxxxxxxx/

Happy to read that!

> But I'd like the method to look at bit less
> ad-hoc and more confined. So would you:
>
> - Move it to `falcon/gsp.rs` and implement it only to `Falcon<Gsp>`, as
>   that's the only case where we need it so far,
> - Make `ctx_dma` a `usize` (it's the caller's problem to provide the
>   correct type),
> - Add the `FalconFbifTarget` and `FalconFbifMemType` as arguments of the
>   method,
> - Maybe rename it to `set_fbif_transcfg`, because the fact we touch
>   registers is an abstraction detail.
>
> As a side-note, the series is going to clash heavily with Gary's, and
> will need to be rebased on top of it after Gary's series is merged
> (which as far as I'm concerned should be the next version).

Thanks for the review I'll refacto on a v2 like that then and wait for
Gary's series to rebase and send it.

> Apologies
> for the inconvenience, but thankfully this should be mostly mechanical.

No problem that's part of SDLC, so
it's expected to happen from time to time :D

Attachment: signature.asc
Description: OpenPGP digital signature