Re: [PATCH] gpu: nova-core: Extract FUSE registers definitions

Nanonej Dev <[email protected]>
Newsgroups dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel
Message-ID <1COg2hERAjDijZlXX0uknmkpjlWBxyXWaxdvBX2tGZgcaYyhUhe25d5WGeosMVh_Tri5YELYViaNEl0PHR_Z01L6IdvfuZdjRZtc2TQj_Kw=@nanonej.com>
On Monday, August 24th, 2026 at 04:30, Alexandre Courbot <[email protected]> wrote:
> I am also a bit hesitant to apply this patch as-is. As defined, it only
> adds an indirection through the `gpu` module for FUSE registers that are
> only accessed by `falcon` (actually, the `ga102` HAL of `falcon`).
> 

> `fuse_ucode_version` does some falcon-specific processing (notably with
> the engine ID mask), so it looks out-of-place in `gpu.rs`.
> 

> Also, the patch doesn't move all the FUSE registers -
> NV_FUSE_STATUS_OPT_DISPLAY is still in the root's `regs.rs`. That's
> probably because the destination chosen by this patch is not a good fit
> to contain them all.
> 

> Now I am not quite sure there is a single, good destination for all
> these registers. We could move these to `falcon` (and
> NV_FUSE_STATUS_OPT_DISPLAY to `fb`), but this just happens to match what
> we are doing right now and if another module needs to use them we carry
> the risk that it will redefine them locally. Or we could have a
> dedicated `fuse` module only to carry these registers, and some
> functions to provide the services needed by other modules, including a
> HAL to read the correct NV_FUSE_STATUS_OPT_DISPLAY register depending on
> architecture. But that looks a bit overkill so I'd suggest wait-and-see
> for now. :)

Ok I see, I actually spent the last few days trying to understand better what the different parts of the project code was doing and what part of the NVIDIA GPUs it was interacting with.a
And indeed FUSE, as I understand, doesn't seems to belong tightly to FALCON neither with the future micro-architectures relying more on GSP.

No problem, I learned things and it forced me to finally try to deep-dive more on the driver.

> There is also a more insidious issue below.
> 

> <...>
> > @@ -419,3 +424,27 @@ pub(crate) fn new(
> >  pub(crate) fn boot_0_raw(bar: Bar0<'_>) -> u32 {
> >      bar.read(regs::NV_PMC_BOOT_0).into_raw()
> >  }
> > +
> > +/// Returns the fuse version matching `engine_id_mask`,
> > +/// at the given `ucode_idx`.
> > +/// Returns `None` if no engine matches `engine_id_mask`.
> > +pub(crate) fn fuse_ucode_version(
> > +    bar: Bar0<'_>,
> > +    engine_id_mask: u16,
> > +    ucode_idx: usize,
> > +) -> Option<u16> {
> > +    let version = if engine_id_mask & 0x0001 != 0 {
> > +        bar.read(regs::NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION::at(ucode_idx))
> > +            .data()
> > +    } else if engine_id_mask & 0x0004 != 0 {
> > +        bar.read(regs::NV_FUSE_OPT_FPF_NVDEC_UCODE1_VERSION::at(ucode_idx))
> > +            .data()
> > +    } else if engine_id_mask & 0x0400 != 0 {
> > +        bar.read(regs::NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION::at(ucode_idx))
> > +            .data()
> > +    } else {
> > +        return None;
> > +    };
> > +
> > +    Some(version)
> > +}
> 

> This is moot due to the comments above, but this function should be
> `#[inline(always)]`. The reason is that it uses `at`, which performs a
> `build_assert!` using `ucode_idx`. If this function is not inlined into
> its caller, then the range properties asserted by
> `signature_reg_fuse_version_ga102` won't be visible to the compiler and
> the `build_assert!` will fail.

Ok! I didn't realize that and it make perfect sense now that you point it out!
But I'm surprised: How come it's compiling on my side then?! :O
signature.asc (application/pgp-signature, 343 B)
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wrsEARYKAG0FgmqMDWgJEFKmzSsu6sDwRRQAAAAAABwAIHNhbHRAbm90YXRp
b25zLm9wZW5wZ3Bqcy5vcmdXQDZZA9iDn2Yr0h5iJV5ahk5KrzdbBwGGWUYx
gvgNZhYhBI+99gDJjAW7MoHJ0VKmzSsu6sDwAADgPQEA7PBA+WabNtXtQTSa
9BtqZKaAqcVmu0H6xdqsGSFgi/IBANm+ch2CB8s728PGsaujO4gPZUrqzx0F
HW/qOzDA4WYA
=d9o1
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.