* [PATCH] gpu: nova-core: Extract FUSE registers definitions
@ 2026-08-17 20:04 Antonin Malzieu Ridolfi via B4 Relay
2026-08-24 2:30 ` Alexandre Courbot
0 siblings, 1 reply; 4+ messages in thread
From: Antonin Malzieu Ridolfi via B4 Relay @ 2026-08-17 20:04 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Alexandre Courbot, David Airlie,
Simona Vetter
Cc: nova-gpu, dri-devel, linux-kernel, Antonin Malzieu Ridolfi
From: Antonin Malzieu Ridolfi <dev@nanonej.com>
Move FUSE register definitions from the root regs.rs file into the
gpu module that own them, in the existing gpu/regs.rs file.
This follows the same pattern established by previous commits for
GSP, PDISP, PFB, PBUS and PMC registers: register definitions move to
the module that owns them, visibility changes to pub(super), and
cross-module access is provided via pub(crate) helper functions.
Since gal102.rs (outside the gpu module) also reads fuse registers to
infer fuse version, a pub(crate) helper function fuse_ucode_version()
is added in gpu.rs to provide that information without exposing the
register type directly.
Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Antonin Malzieu Ridolfi <dev@nanonej.com>
---
I got several doubts regarding this patch:
- I didn't touch the comment in falcon/hal/ga102.rs:71 cause I didn't
know if this information should stay there
- I'm not sure if the re-export of NV_FUSE_OPT_FPF_SIZE in gpu.rs is the
right way to keep the read in gal102.rs or if I should also make an
helper to get its value
- Then, as I'm not quite sure to understand the exact purpose of the
code I'm not sure of the `fuse_ucode_version` naming and the comment
explaining what it do
---
| 17 ++++-------------
| 31 ++++++++++++++++++++++++++++++-
| 18 ++++++++++++++++++
| 18 ------------------
4 files changed, 52 insertions(+), 32 deletions(-)
--git a/drivers/gpu/nova-core/falcon/hal/ga102.rs b/drivers/gpu/nova-core/falcon/hal/ga102.rs
index 7600ee07ca2e..590e30218527 100644
--- a/drivers/gpu/nova-core/falcon/hal/ga102.rs
+++ b/drivers/gpu/nova-core/falcon/hal/ga102.rs
@@ -7,7 +7,6 @@
io::{
poll::read_poll_timeout,
register::{
- Array,
WithBase, //
},
Io, //
@@ -26,7 +25,8 @@
FalconModSelAlgo,
PeregrineCoreSelect, //
},
- regs,
+ gpu,
+ regs, //
};
use super::FalconHal;
@@ -59,7 +59,7 @@ fn signature_reg_fuse_version_ga102(
) -> Result<u32> {
// Each engine has 16 ucode version registers numbered from 1 to 16.
let ucode_idx = match usize::from(ucode_id) {
- ucode_id @ 1..=regs::NV_FUSE_OPT_FPF_SIZE => ucode_id - 1,
+ ucode_id @ 1..=gpu::NV_FUSE_OPT_FPF_SIZE => ucode_id - 1,
_ => {
dev_err!(dev, "invalid ucode id {:#x}\n", ucode_id);
return Err(EINVAL);
@@ -68,16 +68,7 @@ fn signature_reg_fuse_version_ga102(
// `ucode_idx` is guaranteed to be in the range [0..15], making the `read` calls provable valid
// at build-time.
- let reg_fuse_version: u16 = 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 {
+ let Some(reg_fuse_version) = gpu::fuse_ucode_version(bar, engine_id_mask, ucode_idx) else {
dev_err!(dev, "unexpected engine_id_mask {:#x}\n", engine_id_mask);
return Err(EINVAL);
};
--git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 9e4232645a7e..746a7dec8906 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -6,7 +6,10 @@
device,
dma::Device,
fmt,
- io::Io,
+ io::{
+ register::Array,
+ Io, //
+ },
num::Bounded,
pci,
prelude::*,
@@ -35,6 +38,8 @@
mod hal;
mod regs;
+pub(crate) use regs::NV_FUSE_OPT_FPF_SIZE;
+
macro_rules! define_chipset {
({ $($variant:ident = $value:expr),* $(,)* }) =>
{
@@ -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)
+}
--git a/drivers/gpu/nova-core/gpu/regs.rs b/drivers/gpu/nova-core/gpu/regs.rs
index 1c4db9625250..a6ce4ff44cd1 100644
--- a/drivers/gpu/nova-core/gpu/regs.rs
+++ b/drivers/gpu/nova-core/gpu/regs.rs
@@ -80,3 +80,21 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
)
}
}
+
+// FUSE
+
+pub(crate) const NV_FUSE_OPT_FPF_SIZE: usize = 16;
+
+register! {
+ pub(super) NV_FUSE_OPT_FPF_NVDEC_UCODE1_VERSION(u32)[NV_FUSE_OPT_FPF_SIZE] @ 0x00824100 {
+ 15:0 data => u16;
+ }
+
+ pub(super) NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION(u32)[NV_FUSE_OPT_FPF_SIZE] @ 0x00824140 {
+ 15:0 data => u16;
+ }
+
+ pub(super) NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION(u32)[NV_FUSE_OPT_FPF_SIZE] @ 0x008241c0 {
+ 15:0 data => u16;
+ }
+}
--git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 3422b49df7a7..562499ff6e08 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -94,24 +94,6 @@ pub(crate) fn usable_fb_size(self) -> u64 {
}
}
-// FUSE
-
-pub(crate) const NV_FUSE_OPT_FPF_SIZE: usize = 16;
-
-register! {
- pub(crate) NV_FUSE_OPT_FPF_NVDEC_UCODE1_VERSION(u32)[NV_FUSE_OPT_FPF_SIZE] @ 0x00824100 {
- 15:0 data => u16;
- }
-
- pub(crate) NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION(u32)[NV_FUSE_OPT_FPF_SIZE] @ 0x00824140 {
- 15:0 data => u16;
- }
-
- pub(crate) NV_FUSE_OPT_FPF_GSP_UCODE1_VERSION(u32)[NV_FUSE_OPT_FPF_SIZE] @ 0x008241c0 {
- 15:0 data => u16;
- }
-}
-
// PFALCON
register! {
---
base-commit: 60b5976d1367cd50314e867bc1169e759ab309b9
change-id: 20260817-b4-extract-fuse-registers-to-gpu-mod-1d30d1f9a370
prerequisite-change-id: 20260804-b4-extract-pmc-registers-to-gpu-mod-793ee2078a23
Best regards,
--
Antonin Malzieu Ridolfi <dev@nanonej.com>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gpu: nova-core: Extract FUSE registers definitions
2026-08-17 20:04 [PATCH] gpu: nova-core: Extract FUSE registers definitions Antonin Malzieu Ridolfi via B4 Relay
@ 2026-08-24 2:30 ` Alexandre Courbot
2026-08-24 9:23 ` Nanonej Dev
0 siblings, 1 reply; 4+ messages in thread
From: Alexandre Courbot @ 2026-08-24 2:30 UTC (permalink / raw)
To: Antonin Malzieu Ridolfi via B4 Relay
Cc: dev, Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
nova-gpu, dri-devel, linux-kernel
On Tue Aug 18, 2026 at 5:04 AM JST, Antonin Malzieu Ridolfi via B4 Relay wrote:
> From: Antonin Malzieu Ridolfi <dev@nanonej.com>
>
> Move FUSE register definitions from the root regs.rs file into the
> gpu module that own them, in the existing gpu/regs.rs file.
>
> This follows the same pattern established by previous commits for
> GSP, PDISP, PFB, PBUS and PMC registers: register definitions move to
> the module that owns them, visibility changes to pub(super), and
> cross-module access is provided via pub(crate) helper functions.
>
> Since gal102.rs (outside the gpu module) also reads fuse registers to
> infer fuse version, a pub(crate) helper function fuse_ucode_version()
> is added in gpu.rs to provide that information without exposing the
> register type directly.
>
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Antonin Malzieu Ridolfi <dev@nanonej.com>
> ---
> I got several doubts regarding this patch:
> - I didn't touch the comment in falcon/hal/ga102.rs:71 cause I didn't
> know if this information should stay there
> - I'm not sure if the re-export of NV_FUSE_OPT_FPF_SIZE in gpu.rs is the
> right way to keep the read in gal102.rs or if I should also make an
> helper to get its value
> - Then, as I'm not quite sure to understand the exact purpose of the
> code I'm not sure of the `fuse_ucode_version` naming and the comment
> explaining what it do
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. :)
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpu: nova-core: Extract FUSE registers definitions
2026-08-24 2:30 ` Alexandre Courbot
@ 2026-08-24 9:23 ` Nanonej Dev
2026-08-24 15:26 ` Alexandre Courbot
0 siblings, 1 reply; 4+ messages in thread
From: Nanonej Dev @ 2026-08-24 9:23 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Antonin Malzieu Ridolfi via B4 Relay, Danilo Krummrich,
Alice Ryhl, David Airlie, Simona Vetter, nova-gpu, dri-devel,
linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 3425 bytes --]
On Monday, August 24th, 2026 at 04:30, Alexandre Courbot <acourbot@nvidia.com> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 343 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpu: nova-core: Extract FUSE registers definitions
2026-08-24 9:23 ` Nanonej Dev
@ 2026-08-24 15:26 ` Alexandre Courbot
0 siblings, 0 replies; 4+ messages in thread
From: Alexandre Courbot @ 2026-08-24 15:26 UTC (permalink / raw)
To: Nanonej Dev
Cc: Antonin Malzieu Ridolfi via B4 Relay, Danilo Krummrich,
Alice Ryhl, David Airlie, Simona Vetter, nova-gpu, dri-devel,
linux-kernel
On Mon Aug 24, 2026 at 6:23 PM JST, Nanonej Dev wrote:
> On Monday, August 24th, 2026 at 04:30, Alexandre Courbot <acourbot@nvidia.com> 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.
Now to be fair, this is a difficult problem: GPU registers were not
exactly designed to be confined to driver sub-modules, we are trying to
do it because it makes the driver cleaner but the case of FUSE is a bit
specific as these are used all over the place. So the answer might also
very well be that there is not good solution for this particular family
of registers.
>
>> 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
It compiles on mine as well, but we had issues with `build_assert!`
failing with `CONFIG_CC_OPTIMIZE_FOR_SIZE` (see for instance [1]). So
while I cannot prove that there is an actual issue, it remains a
theoretical landmine (like anything that uses `build_assert`) that tends
to trigger on the weirdest possible configs and ends up with you
installing an obscure cross-compiler late at night to try and reproduce
the issue. :P
[1] https://lore.kernel.org/all/20251122-bounded_ints_fix-v1-1-1e07589d4955@nvidia.com/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-24 15:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17 20:04 [PATCH] gpu: nova-core: Extract FUSE registers definitions Antonin Malzieu Ridolfi via B4 Relay
2026-08-24 2:30 ` Alexandre Courbot
2026-08-24 9:23 ` Nanonej Dev
2026-08-24 15:26 ` Alexandre Courbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox