* [PATCH v5 0/6] gpu: nova-core: add GA100 support
@ 2026-04-17 19:13 Timur Tabi
2026-04-17 19:13 ` [PATCH v5 1/6] gpu: nova-core: use correct fwsignature for GA100 Timur Tabi
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Timur Tabi @ 2026-04-17 19:13 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, Gary Guo,
Joel Fernandes, Eliot Courtney, rust-for-linux
GA100 is an odd GPU. Architecturally, it's an Ampere. However, it
uses the same GSP-RM boot process as a Turing. Its VBIOS, for whatever
reason, has an IFR header that precedes the PCI ROM header and must
be skipped. In addition, the FRTS window size must be set to 0 just
because it's a GA100.
So add all the missing code that GA100 needs to be supported. There was
some infrastructure already for GA100 (e.g. the FbHal), but it was
incomplete.
Changes in v5:
1) Moved IFR registers into vbios.rs
2) Renamed current_offset to vbios_rom_offset
3) Moved IFR constants into vbios_rom_offset
4) Misc other improvements
Timur Tabi (6):
gpu: nova-core: use correct fwsignature for GA100
gpu: nova-core: do not consider 0xBB77 as a valid PCI ROM header
signature
gpu: nova-core: only boot FRTS if its region is allocated
gpu: nova-core: add FbHal::frts_size() for GA100 support
gpu: nova-core: skip the IFR header if present
gpu: nova-core: enable GA100
drivers/gpu/nova-core/falcon/hal.rs | 2 +-
drivers/gpu/nova-core/fb.rs | 6 +-
drivers/gpu/nova-core/fb/hal.rs | 3 +
drivers/gpu/nova-core/fb/hal/ga100.rs | 6 ++
drivers/gpu/nova-core/fb/hal/ga102.rs | 4 ++
drivers/gpu/nova-core/fb/hal/tu102.rs | 11 +++-
drivers/gpu/nova-core/firmware/gsp.rs | 3 +-
drivers/gpu/nova-core/gsp/boot.rs | 5 +-
drivers/gpu/nova-core/vbios.rs | 87 ++++++++++++++++++++++++++-
9 files changed, 117 insertions(+), 10 deletions(-)
base-commit: 8884dbc1fd174315b406027375bb7a4b211a8fae
--
2.53.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 1/6] gpu: nova-core: use correct fwsignature for GA100
2026-04-17 19:13 [PATCH v5 0/6] gpu: nova-core: add GA100 support Timur Tabi
@ 2026-04-17 19:13 ` Timur Tabi
2026-04-19 19:34 ` Gary Guo
2026-04-17 19:13 ` [PATCH v5 2/6] gpu: nova-core: do not consider 0xBB77 as a valid PCI ROM header signature Timur Tabi
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Timur Tabi @ 2026-04-17 19:13 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, Gary Guo,
Joel Fernandes, Eliot Courtney, rust-for-linux
Although GA100 uses the same GSP-RM firmware as Turing, it has a different
signature specifically for it.
Fixes: 121ea04cd9f2 ("gpu: nova-core: add support for Turing/GA100 fwsignature")
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/firmware/gsp.rs | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 2fcc255c3bc8..c423191b21f0 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -138,8 +138,7 @@ pub(crate) fn new<'a>(
".fwsignature_tu11x"
}
Architecture::Turing => ".fwsignature_tu10x",
- // GA100 uses the same firmware as Turing
- Architecture::Ampere if chipset == Chipset::GA100 => ".fwsignature_tu10x",
+ Architecture::Ampere if chipset == Chipset::GA100 => ".fwsignature_ga100",
Architecture::Ampere => ".fwsignature_ga10x",
Architecture::Ada => ".fwsignature_ad10x",
};
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 2/6] gpu: nova-core: do not consider 0xBB77 as a valid PCI ROM header signature
2026-04-17 19:13 [PATCH v5 0/6] gpu: nova-core: add GA100 support Timur Tabi
2026-04-17 19:13 ` [PATCH v5 1/6] gpu: nova-core: use correct fwsignature for GA100 Timur Tabi
@ 2026-04-17 19:13 ` Timur Tabi
2026-04-19 19:36 ` Gary Guo
2026-04-17 19:13 ` [PATCH v5 3/6] gpu: nova-core: only boot FRTS if its region is allocated Timur Tabi
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Timur Tabi @ 2026-04-17 19:13 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, Gary Guo,
Joel Fernandes, Eliot Courtney, rust-for-linux
Nvidia GPUs have some PCI expansion ROM sections that have an Nvidia-
specific signature instead of 0xAA55. Signature 0xBB77 is actually an
internal-only value that has been deprecated for over a decade.
Nova-core will never encounter a GPU with that signature, so don't look
for it.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/vbios.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index ebda28e596c5..e726594eb130 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -491,7 +491,7 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
// Check for valid ROM signatures.
match signature {
- 0xAA55 | 0xBB77 | 0x4E56 => {}
+ 0xAA55 | 0x4E56 => {}
_ => {
dev_err!(dev, "ROM signature unknown {:#x}\n", signature);
return Err(EINVAL);
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 3/6] gpu: nova-core: only boot FRTS if its region is allocated
2026-04-17 19:13 [PATCH v5 0/6] gpu: nova-core: add GA100 support Timur Tabi
2026-04-17 19:13 ` [PATCH v5 1/6] gpu: nova-core: use correct fwsignature for GA100 Timur Tabi
2026-04-17 19:13 ` [PATCH v5 2/6] gpu: nova-core: do not consider 0xBB77 as a valid PCI ROM header signature Timur Tabi
@ 2026-04-17 19:13 ` Timur Tabi
2026-04-19 19:36 ` Gary Guo
2026-04-17 19:13 ` [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support Timur Tabi
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Timur Tabi @ 2026-04-17 19:13 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, Gary Guo,
Joel Fernandes, Eliot Courtney, rust-for-linux
On some Nvidia GPUs (i.e. GA100), the FRTS region is not allocated
(its size is set to 0). In such cases, FWSEC-FRTS should not be run.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/boot.rs | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 18f356c9178e..5c56f0539dd7 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -155,7 +155,10 @@ pub(crate) fn boot(
let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
dev_dbg!(dev, "{:#x?}\n", fb_layout);
- Self::run_fwsec_frts(dev, chipset, gsp_falcon, bar, &bios, &fb_layout)?;
+ // FWSEC-FRTS is not executed on chips where the FRTS region size is 0 (e.g. GA100).
+ if !fb_layout.frts.is_empty() {
+ Self::run_fwsec_frts(dev, chipset, gsp_falcon, bar, &bios, &fb_layout)?;
+ }
let booter_loader = BooterFirmware::new(
dev,
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-17 19:13 [PATCH v5 0/6] gpu: nova-core: add GA100 support Timur Tabi
` (2 preceding siblings ...)
2026-04-17 19:13 ` [PATCH v5 3/6] gpu: nova-core: only boot FRTS if its region is allocated Timur Tabi
@ 2026-04-17 19:13 ` Timur Tabi
2026-04-19 19:47 ` Gary Guo
2026-04-17 19:13 ` [PATCH v5 5/6] gpu: nova-core: skip the IFR header if present Timur Tabi
2026-04-17 19:13 ` [PATCH v5 6/6] gpu: nova-core: enable GA100 Timur Tabi
5 siblings, 1 reply; 20+ messages in thread
From: Timur Tabi @ 2026-04-17 19:13 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, Gary Guo,
Joel Fernandes, Eliot Courtney, rust-for-linux
Introduce FbHal method frts_size() to return the size of the FRTS
window. GA100 is a special case in that there is no FRTS, and so
the size must be set to 0.
Note that we cannot use supports_display() to determine the FRTS
size because there are other GPUs (e.g. GA102GL) that have display
disabled (and so supports_display() returns False), but the FRTS
window size still needs to be 1MB.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/fb.rs | 6 +++---
drivers/gpu/nova-core/fb/hal.rs | 3 +++
drivers/gpu/nova-core/fb/hal/ga100.rs | 6 ++++++
drivers/gpu/nova-core/fb/hal/ga102.rs | 4 ++++
drivers/gpu/nova-core/fb/hal/tu102.rs | 11 ++++++++++-
5 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index f357fb28b22c..a305a6dac758 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -216,10 +216,10 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
let frts = {
const FRTS_DOWN_ALIGN: Alignment = Alignment::new::<SZ_128K>();
- const FRTS_SIZE: u64 = usize_as_u64(SZ_1M);
- let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - FRTS_SIZE;
+ let frts_size: u64 = hal.frts_size();
+ let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - frts_size;
- FbRange(frts_base..frts_base + FRTS_SIZE)
+ FbRange(frts_base..frts_base + frts_size)
};
let boot = {
diff --git a/drivers/gpu/nova-core/fb/hal.rs b/drivers/gpu/nova-core/fb/hal.rs
index aba0abd8ee00..1c01a6cbed65 100644
--- a/drivers/gpu/nova-core/fb/hal.rs
+++ b/drivers/gpu/nova-core/fb/hal.rs
@@ -25,6 +25,9 @@ pub(crate) trait FbHal {
/// Returns the VRAM size, in bytes.
fn vidmem_size(&self, bar: &Bar0) -> u64;
+
+ /// Returns the FRTS size, in bytes.
+ fn frts_size(&self) -> u64;
}
/// Returns the HAL corresponding to `chipset`.
diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
index 1c03783cddef..2f5871d915c3 100644
--- a/drivers/gpu/nova-core/fb/hal/ga100.rs
+++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
@@ -66,6 +66,12 @@ fn supports_display(&self, bar: &Bar0) -> bool {
fn vidmem_size(&self, bar: &Bar0) -> u64 {
super::tu102::vidmem_size_gp102(bar)
}
+
+ // GA100 is a special case where its FRTS region exists, but is empty. We
+ // return a size of 0 because we still need to record where the region starts.
+ fn frts_size(&self) -> u64 {
+ 0
+ }
}
const GA100: Ga100 = Ga100;
diff --git a/drivers/gpu/nova-core/fb/hal/ga102.rs b/drivers/gpu/nova-core/fb/hal/ga102.rs
index 4b9f0f74d0e7..3bb66f64bef7 100644
--- a/drivers/gpu/nova-core/fb/hal/ga102.rs
+++ b/drivers/gpu/nova-core/fb/hal/ga102.rs
@@ -35,6 +35,10 @@ fn supports_display(&self, bar: &Bar0) -> bool {
fn vidmem_size(&self, bar: &Bar0) -> u64 {
vidmem_size_ga102(bar)
}
+
+ fn frts_size(&self) -> u64 {
+ super::tu102::frts_size_tu102()
+ }
}
const GA102: Ga102 = Ga102;
diff --git a/drivers/gpu/nova-core/fb/hal/tu102.rs b/drivers/gpu/nova-core/fb/hal/tu102.rs
index 281bb796e198..22c174bf1472 100644
--- a/drivers/gpu/nova-core/fb/hal/tu102.rs
+++ b/drivers/gpu/nova-core/fb/hal/tu102.rs
@@ -2,7 +2,8 @@
use kernel::{
io::Io,
- prelude::*, //
+ prelude::*,
+ sizes::*, //
};
use crate::{
@@ -38,6 +39,10 @@ pub(super) fn vidmem_size_gp102(bar: &Bar0) -> u64 {
.usable_fb_size()
}
+pub(super) const fn frts_size_tu102() -> u64 {
+ u64::SZ_1M
+}
+
struct Tu102;
impl FbHal for Tu102 {
@@ -56,6 +61,10 @@ fn supports_display(&self, bar: &Bar0) -> bool {
fn vidmem_size(&self, bar: &Bar0) -> u64 {
vidmem_size_gp102(bar)
}
+
+ fn frts_size(&self) -> u64 {
+ frts_size_tu102()
+ }
}
const TU102: Tu102 = Tu102;
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 5/6] gpu: nova-core: skip the IFR header if present
2026-04-17 19:13 [PATCH v5 0/6] gpu: nova-core: add GA100 support Timur Tabi
` (3 preceding siblings ...)
2026-04-17 19:13 ` [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support Timur Tabi
@ 2026-04-17 19:13 ` Timur Tabi
2026-04-20 6:37 ` Eliot Courtney
2026-04-17 19:13 ` [PATCH v5 6/6] gpu: nova-core: enable GA100 Timur Tabi
5 siblings, 1 reply; 20+ messages in thread
From: Timur Tabi @ 2026-04-17 19:13 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, Gary Guo,
Joel Fernandes, Eliot Courtney, rust-for-linux
The GPU's ROM may begin with an Init-from-ROM (IFR) header that precedes
the PCI Expansion ROM images (VBIOS). When present, the PROM shadow
method must parse this header to determine the offset where the PCI ROM
images actually begin, and adjust all subsequent reads accordingly.
On most GPUs this is not needed because the IFR microcode has already
applied the ROM offset so that PROM reads transparently skip the header.
On GA100, for whatever reason, the IFR offset is not applied to PROM
reads. Therefore, the search for the PCI expansion must skip the IFR
header, if found.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/vbios.rs | 85 +++++++++++++++++++++++++++++++++-
1 file changed, 84 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index e726594eb130..6bcfb6c5cf44 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -12,6 +12,8 @@
Alignable,
Alignment, //
},
+ register,
+ sizes::SZ_4K,
sync::aref::ARef,
transmute::FromBytes,
};
@@ -89,13 +91,94 @@ struct VbiosIterator<'a> {
last_found: bool,
}
+// IFR Header in VBIOS.
+
+register! {
+ pub(crate) NV_PBUS_IFR_FMT_FIXED0(u32) @ 0x300000 {
+ 31:0 signature;
+ }
+}
+
+register! {
+ pub(crate) NV_PBUS_IFR_FMT_FIXED1(u32) @ 0x300004 {
+ 30:16 fixed_data_size;
+ 15:8 version => u8;
+ }
+}
+
+register! {
+ pub(crate) NV_PBUS_IFR_FMT_FIXED2(u32) @ 0x300008 {
+ 19:0 total_data_size;
+ }
+}
+
+/// Return the byte offset where the PCI Expansion ROM images begin in the GPU's ROM.
+///
+/// The GPU's ROM may begin with an Init-from-ROM (IFR) header that precedes
+/// the PCI Expansion ROM images (VBIOS). When present, the PROM shadow
+/// method must parse this header to determine the offset where the PCI ROM
+/// images actually begin, and adjust all subsequent reads accordingly.
+///
+/// On most GPUs this is not needed because the IFR microcode has already
+/// applied the ROM offset so that PROM reads transparently skip the header.
+/// On GA100, for some reason, the IFR offset is not applied to PROM
+/// reads. Therefore, the search for the PCI expansion must skip the IFR
+/// header, if found.
+fn vbios_rom_offset(dev: &device::Device, bar0: &Bar0) -> Result<usize> {
+ /// IFR signature.
+ const NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE: u32 = u32::from_le_bytes(*b"NVGI");
+ /// ROM directory signature.
+ const NV_ROM_DIRECTORY_IDENTIFIER: u32 = u32::from_le_bytes(*b"RFRD");
+ /// Offset of the NV_PMGR_ROM_ADDR_OFFSET register in IFR Extended section.
+ const IFR_SW_EXT_ROM_ADDR_OFFSET: usize = 4;
+ /// Size of Redundant Firmware Flash Status section.
+ const RFW_FLASH_STATUS_SIZE: usize = SZ_4K;
+ /// Offset in the ROM Directory of the PCI Option ROM offset
+ const PCI_OPTION_ROM_OFFSET: usize = 8;
+
+ let signature = bar0.read(NV_PBUS_IFR_FMT_FIXED0).signature();
+
+ if signature == NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE {
+ let fixed1 = bar0.read(NV_PBUS_IFR_FMT_FIXED1);
+
+ match fixed1.version() {
+ 1 | 2 => {
+ let fixed_data_size = usize::from(fixed1.fixed_data_size());
+ let pmgr_rom_addr_offset = fixed_data_size + IFR_SW_EXT_ROM_ADDR_OFFSET;
+ bar0.try_read32(ROM_OFFSET + pmgr_rom_addr_offset)
+ .map(usize::from_safe_cast)
+ }
+ 3 => {
+ let fixed2 = bar0.read(NV_PBUS_IFR_FMT_FIXED2);
+ let total_data_size = usize::from(fixed2.total_data_size());
+ let flash_status_offset =
+ usize::from_safe_cast(bar0.try_read32(ROM_OFFSET + total_data_size)?);
+ let dir_offset = flash_status_offset + RFW_FLASH_STATUS_SIZE;
+ let dir_sig = bar0.try_read32(ROM_OFFSET + dir_offset)?;
+ if dir_sig != NV_ROM_DIRECTORY_IDENTIFIER {
+ dev_err!(dev, "could not find IFR ROM directory\n");
+ return Err(EINVAL);
+ }
+ bar0.try_read32(ROM_OFFSET + dir_offset + PCI_OPTION_ROM_OFFSET)
+ .map(usize::from_safe_cast)
+ }
+ _ => {
+ dev_err!(dev, "unsupported IFR header version {}\n", fixed1.version());
+ Err(EINVAL)
+ }
+ }
+ } else {
+ Ok(0)
+ }
+}
+
impl<'a> VbiosIterator<'a> {
fn new(dev: &'a device::Device, bar0: &'a Bar0) -> Result<Self> {
Ok(Self {
dev,
bar0,
data: KVec::new(),
- current_offset: 0,
+ current_offset: vbios_rom_offset(dev, bar0)?,
last_found: false,
})
}
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 6/6] gpu: nova-core: enable GA100
2026-04-17 19:13 [PATCH v5 0/6] gpu: nova-core: add GA100 support Timur Tabi
` (4 preceding siblings ...)
2026-04-17 19:13 ` [PATCH v5 5/6] gpu: nova-core: skip the IFR header if present Timur Tabi
@ 2026-04-17 19:13 ` Timur Tabi
2026-04-19 19:36 ` Gary Guo
5 siblings, 1 reply; 20+ messages in thread
From: Timur Tabi @ 2026-04-17 19:13 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, Gary Guo,
Joel Fernandes, Eliot Courtney, rust-for-linux
GA100 is a compute-only variant of GA102 that boots GSP-RM like a
Turing, although it also has its own unique requirements.
Now that all the pieces are in place, we can enable GA100 support.
Although architecturally like an Ampere, GA100 uses the same GSP-RM
firmware files as Turing, and therefore must boot it like Turing does.
However, as a compute-only part, GA100 has no display engine.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/falcon/hal.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/falcon/hal.rs b/drivers/gpu/nova-core/falcon/hal.rs
index 71df33c79884..f82087df4936 100644
--- a/drivers/gpu/nova-core/falcon/hal.rs
+++ b/drivers/gpu/nova-core/falcon/hal.rs
@@ -77,13 +77,13 @@ pub(super) fn falcon_hal<E: FalconEngine + 'static>(
use Chipset::*;
let hal = match chipset {
+ GA100 | // GA100 boots like Turing so use Turing HAL
TU102 | TU104 | TU106 | TU116 | TU117 => {
KBox::new(tu102::Tu102::<E>::new(), GFP_KERNEL)? as KBox<dyn FalconHal<E>>
}
GA102 | GA103 | GA104 | GA106 | GA107 | AD102 | AD103 | AD104 | AD106 | AD107 => {
KBox::new(ga102::Ga102::<E>::new(), GFP_KERNEL)? as KBox<dyn FalconHal<E>>
}
- _ => return Err(ENOTSUPP),
};
Ok(hal)
--
2.53.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v5 1/6] gpu: nova-core: use correct fwsignature for GA100
2026-04-17 19:13 ` [PATCH v5 1/6] gpu: nova-core: use correct fwsignature for GA100 Timur Tabi
@ 2026-04-19 19:34 ` Gary Guo
0 siblings, 0 replies; 20+ messages in thread
From: Gary Guo @ 2026-04-19 19:34 UTC (permalink / raw)
To: Timur Tabi, Danilo Krummrich, Alexandre Courbot, John Hubbard,
Gary Guo, Joel Fernandes, Eliot Courtney, rust-for-linux
On Fri Apr 17, 2026 at 8:13 PM BST, Timur Tabi wrote:
> Although GA100 uses the same GSP-RM firmware as Turing, it has a different
> signature specifically for it.
>
> Fixes: 121ea04cd9f2 ("gpu: nova-core: add support for Turing/GA100 fwsignature")
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/firmware/gsp.rs | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 6/6] gpu: nova-core: enable GA100
2026-04-17 19:13 ` [PATCH v5 6/6] gpu: nova-core: enable GA100 Timur Tabi
@ 2026-04-19 19:36 ` Gary Guo
0 siblings, 0 replies; 20+ messages in thread
From: Gary Guo @ 2026-04-19 19:36 UTC (permalink / raw)
To: Timur Tabi, Danilo Krummrich, Alexandre Courbot, John Hubbard,
Gary Guo, Joel Fernandes, Eliot Courtney, rust-for-linux
On Fri Apr 17, 2026 at 8:13 PM BST, Timur Tabi wrote:
> GA100 is a compute-only variant of GA102 that boots GSP-RM like a
> Turing, although it also has its own unique requirements.
>
> Now that all the pieces are in place, we can enable GA100 support.
> Although architecturally like an Ampere, GA100 uses the same GSP-RM
> firmware files as Turing, and therefore must boot it like Turing does.
> However, as a compute-only part, GA100 has no display engine.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/falcon/hal.rs | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/6] gpu: nova-core: do not consider 0xBB77 as a valid PCI ROM header signature
2026-04-17 19:13 ` [PATCH v5 2/6] gpu: nova-core: do not consider 0xBB77 as a valid PCI ROM header signature Timur Tabi
@ 2026-04-19 19:36 ` Gary Guo
0 siblings, 0 replies; 20+ messages in thread
From: Gary Guo @ 2026-04-19 19:36 UTC (permalink / raw)
To: Timur Tabi, Danilo Krummrich, Alexandre Courbot, John Hubbard,
Gary Guo, Joel Fernandes, Eliot Courtney, rust-for-linux
On Fri Apr 17, 2026 at 8:13 PM BST, Timur Tabi wrote:
> Nvidia GPUs have some PCI expansion ROM sections that have an Nvidia-
> specific signature instead of 0xAA55. Signature 0xBB77 is actually an
> internal-only value that has been deprecated for over a decade.
> Nova-core will never encounter a GPU with that signature, so don't look
> for it.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/vbios.rs | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 3/6] gpu: nova-core: only boot FRTS if its region is allocated
2026-04-17 19:13 ` [PATCH v5 3/6] gpu: nova-core: only boot FRTS if its region is allocated Timur Tabi
@ 2026-04-19 19:36 ` Gary Guo
0 siblings, 0 replies; 20+ messages in thread
From: Gary Guo @ 2026-04-19 19:36 UTC (permalink / raw)
To: Timur Tabi, Danilo Krummrich, Alexandre Courbot, John Hubbard,
Gary Guo, Joel Fernandes, Eliot Courtney, rust-for-linux
On Fri Apr 17, 2026 at 8:13 PM BST, Timur Tabi wrote:
> On some Nvidia GPUs (i.e. GA100), the FRTS region is not allocated
> (its size is set to 0). In such cases, FWSEC-FRTS should not be run.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/gsp/boot.rs | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-17 19:13 ` [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support Timur Tabi
@ 2026-04-19 19:47 ` Gary Guo
2026-04-20 1:28 ` Alexandre Courbot
0 siblings, 1 reply; 20+ messages in thread
From: Gary Guo @ 2026-04-19 19:47 UTC (permalink / raw)
To: Timur Tabi, Danilo Krummrich, Alexandre Courbot, John Hubbard,
Gary Guo, Joel Fernandes, Eliot Courtney, rust-for-linux
On Fri Apr 17, 2026 at 8:13 PM BST, Timur Tabi wrote:
> Introduce FbHal method frts_size() to return the size of the FRTS
> window. GA100 is a special case in that there is no FRTS, and so
> the size must be set to 0.
>
> Note that we cannot use supports_display() to determine the FRTS
> size because there are other GPUs (e.g. GA102GL) that have display
> disabled (and so supports_display() returns False), but the FRTS
> window size still needs to be 1MB.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> drivers/gpu/nova-core/fb.rs | 6 +++---
> drivers/gpu/nova-core/fb/hal.rs | 3 +++
> drivers/gpu/nova-core/fb/hal/ga100.rs | 6 ++++++
> drivers/gpu/nova-core/fb/hal/ga102.rs | 4 ++++
> drivers/gpu/nova-core/fb/hal/tu102.rs | 11 ++++++++++-
> 5 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index f357fb28b22c..a305a6dac758 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
> @@ -216,10 +216,10 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>
> let frts = {
> const FRTS_DOWN_ALIGN: Alignment = Alignment::new::<SZ_128K>();
> - const FRTS_SIZE: u64 = usize_as_u64(SZ_1M);
> - let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - FRTS_SIZE;
> + let frts_size: u64 = hal.frts_size();
> + let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - frts_size;
>
> - FbRange(frts_base..frts_base + FRTS_SIZE)
> + FbRange(frts_base..frts_base + frts_size)
> };
>
> let boot = {
> diff --git a/drivers/gpu/nova-core/fb/hal.rs b/drivers/gpu/nova-core/fb/hal.rs
> index aba0abd8ee00..1c01a6cbed65 100644
> --- a/drivers/gpu/nova-core/fb/hal.rs
> +++ b/drivers/gpu/nova-core/fb/hal.rs
> @@ -25,6 +25,9 @@ pub(crate) trait FbHal {
>
> /// Returns the VRAM size, in bytes.
> fn vidmem_size(&self, bar: &Bar0) -> u64;
> +
> + /// Returns the FRTS size, in bytes.
> + fn frts_size(&self) -> u64;
> }
>
> /// Returns the HAL corresponding to `chipset`.
> diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
> index 1c03783cddef..2f5871d915c3 100644
> --- a/drivers/gpu/nova-core/fb/hal/ga100.rs
> +++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
> @@ -66,6 +66,12 @@ fn supports_display(&self, bar: &Bar0) -> bool {
> fn vidmem_size(&self, bar: &Bar0) -> u64 {
> super::tu102::vidmem_size_gp102(bar)
> }
> +
> + // GA100 is a special case where its FRTS region exists, but is empty. We
> + // return a size of 0 because we still need to record where the region starts.
> + fn frts_size(&self) -> u64 {
> + 0
> + }
> }
>
> const GA100: Ga100 = Ga100;
> diff --git a/drivers/gpu/nova-core/fb/hal/ga102.rs b/drivers/gpu/nova-core/fb/hal/ga102.rs
> index 4b9f0f74d0e7..3bb66f64bef7 100644
> --- a/drivers/gpu/nova-core/fb/hal/ga102.rs
> +++ b/drivers/gpu/nova-core/fb/hal/ga102.rs
> @@ -35,6 +35,10 @@ fn supports_display(&self, bar: &Bar0) -> bool {
> fn vidmem_size(&self, bar: &Bar0) -> u64 {
> vidmem_size_ga102(bar)
> }
> +
> + fn frts_size(&self) -> u64 {
> + super::tu102::frts_size_tu102()
I checked the patch history and it looks like this changes from v1 to v2. If the
argument that "both have the same size" then I think this just makes thing more
confusing then the purpose of deduplicating.
All the methods above are either delegating to GA100 code or custom, and now
this delegates to TU102 without any explanation.
So this should either be
// FRTS on GA102 behaves identically to that of Turing series.
super::tu102::frts_size_tu102()
Or, it should just be simply
u64::SZ_1M
Best,
Gary
> + }
> }
>
> const GA102: Ga102 = Ga102;
> diff --git a/drivers/gpu/nova-core/fb/hal/tu102.rs b/drivers/gpu/nova-core/fb/hal/tu102.rs
> index 281bb796e198..22c174bf1472 100644
> --- a/drivers/gpu/nova-core/fb/hal/tu102.rs
> +++ b/drivers/gpu/nova-core/fb/hal/tu102.rs
> @@ -2,7 +2,8 @@
>
> use kernel::{
> io::Io,
> - prelude::*, //
> + prelude::*,
> + sizes::*, //
> };
>
> use crate::{
> @@ -38,6 +39,10 @@ pub(super) fn vidmem_size_gp102(bar: &Bar0) -> u64 {
> .usable_fb_size()
> }
>
> +pub(super) const fn frts_size_tu102() -> u64 {
> + u64::SZ_1M
> +}
> +
> struct Tu102;
>
> impl FbHal for Tu102 {
> @@ -56,6 +61,10 @@ fn supports_display(&self, bar: &Bar0) -> bool {
> fn vidmem_size(&self, bar: &Bar0) -> u64 {
> vidmem_size_gp102(bar)
> }
> +
> + fn frts_size(&self) -> u64 {
> + frts_size_tu102()
> + }
> }
>
> const TU102: Tu102 = Tu102;
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-19 19:47 ` Gary Guo
@ 2026-04-20 1:28 ` Alexandre Courbot
2026-04-20 10:23 ` Gary Guo
0 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-04-20 1:28 UTC (permalink / raw)
To: Gary Guo
Cc: Timur Tabi, Danilo Krummrich, John Hubbard, Joel Fernandes,
Eliot Courtney, rust-for-linux
On Mon Apr 20, 2026 at 4:47 AM JST, Gary Guo wrote:
> On Fri Apr 17, 2026 at 8:13 PM BST, Timur Tabi wrote:
>> Introduce FbHal method frts_size() to return the size of the FRTS
>> window. GA100 is a special case in that there is no FRTS, and so
>> the size must be set to 0.
>>
>> Note that we cannot use supports_display() to determine the FRTS
>> size because there are other GPUs (e.g. GA102GL) that have display
>> disabled (and so supports_display() returns False), but the FRTS
>> window size still needs to be 1MB.
>>
>> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
>> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>> drivers/gpu/nova-core/fb.rs | 6 +++---
>> drivers/gpu/nova-core/fb/hal.rs | 3 +++
>> drivers/gpu/nova-core/fb/hal/ga100.rs | 6 ++++++
>> drivers/gpu/nova-core/fb/hal/ga102.rs | 4 ++++
>> drivers/gpu/nova-core/fb/hal/tu102.rs | 11 ++++++++++-
>> 5 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
>> index f357fb28b22c..a305a6dac758 100644
>> --- a/drivers/gpu/nova-core/fb.rs
>> +++ b/drivers/gpu/nova-core/fb.rs
>> @@ -216,10 +216,10 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>>
>> let frts = {
>> const FRTS_DOWN_ALIGN: Alignment = Alignment::new::<SZ_128K>();
>> - const FRTS_SIZE: u64 = usize_as_u64(SZ_1M);
>> - let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - FRTS_SIZE;
>> + let frts_size: u64 = hal.frts_size();
>> + let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - frts_size;
>>
>> - FbRange(frts_base..frts_base + FRTS_SIZE)
>> + FbRange(frts_base..frts_base + frts_size)
>> };
>>
>> let boot = {
>> diff --git a/drivers/gpu/nova-core/fb/hal.rs b/drivers/gpu/nova-core/fb/hal.rs
>> index aba0abd8ee00..1c01a6cbed65 100644
>> --- a/drivers/gpu/nova-core/fb/hal.rs
>> +++ b/drivers/gpu/nova-core/fb/hal.rs
>> @@ -25,6 +25,9 @@ pub(crate) trait FbHal {
>>
>> /// Returns the VRAM size, in bytes.
>> fn vidmem_size(&self, bar: &Bar0) -> u64;
>> +
>> + /// Returns the FRTS size, in bytes.
>> + fn frts_size(&self) -> u64;
>> }
>>
>> /// Returns the HAL corresponding to `chipset`.
>> diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
>> index 1c03783cddef..2f5871d915c3 100644
>> --- a/drivers/gpu/nova-core/fb/hal/ga100.rs
>> +++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
>> @@ -66,6 +66,12 @@ fn supports_display(&self, bar: &Bar0) -> bool {
>> fn vidmem_size(&self, bar: &Bar0) -> u64 {
>> super::tu102::vidmem_size_gp102(bar)
>> }
>> +
>> + // GA100 is a special case where its FRTS region exists, but is empty. We
>> + // return a size of 0 because we still need to record where the region starts.
>> + fn frts_size(&self) -> u64 {
>> + 0
>> + }
>> }
>>
>> const GA100: Ga100 = Ga100;
>> diff --git a/drivers/gpu/nova-core/fb/hal/ga102.rs b/drivers/gpu/nova-core/fb/hal/ga102.rs
>> index 4b9f0f74d0e7..3bb66f64bef7 100644
>> --- a/drivers/gpu/nova-core/fb/hal/ga102.rs
>> +++ b/drivers/gpu/nova-core/fb/hal/ga102.rs
>> @@ -35,6 +35,10 @@ fn supports_display(&self, bar: &Bar0) -> bool {
>> fn vidmem_size(&self, bar: &Bar0) -> u64 {
>> vidmem_size_ga102(bar)
>> }
>> +
>> + fn frts_size(&self) -> u64 {
>> + super::tu102::frts_size_tu102()
>
> I checked the patch history and it looks like this changes from v1 to v2. If the
> argument that "both have the same size" then I think this just makes thing more
> confusing then the purpose of deduplicating.
>
> All the methods above are either delegating to GA100 code or custom, and now
> this delegates to TU102 without any explanation.
The reason for this is that all chips have the same FRTS region size,
*except* GA100 which is 0. GA100 is the exception (it is also documented
as you can see in its snippet above) and should be treated as such. Thus
I'd say the current approach is correct.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 5/6] gpu: nova-core: skip the IFR header if present
2026-04-17 19:13 ` [PATCH v5 5/6] gpu: nova-core: skip the IFR header if present Timur Tabi
@ 2026-04-20 6:37 ` Eliot Courtney
0 siblings, 0 replies; 20+ messages in thread
From: Eliot Courtney @ 2026-04-20 6:37 UTC (permalink / raw)
To: Timur Tabi, Danilo Krummrich, Alexandre Courbot, John Hubbard,
Gary Guo, Joel Fernandes, Eliot Courtney, rust-for-linux
On Sat Apr 18, 2026 at 4:13 AM JST, Timur Tabi wrote:
> The GPU's ROM may begin with an Init-from-ROM (IFR) header that precedes
> the PCI Expansion ROM images (VBIOS). When present, the PROM shadow
> method must parse this header to determine the offset where the PCI ROM
> images actually begin, and adjust all subsequent reads accordingly.
>
> On most GPUs this is not needed because the IFR microcode has already
> applied the ROM offset so that PROM reads transparently skip the header.
> On GA100, for whatever reason, the IFR offset is not applied to PROM
> reads. Therefore, the search for the PCI expansion must skip the IFR
> header, if found.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> drivers/gpu/nova-core/vbios.rs | 85 +++++++++++++++++++++++++++++++++-
> 1 file changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index e726594eb130..6bcfb6c5cf44 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -12,6 +12,8 @@
> Alignable,
> Alignment, //
> },
> + register,
> + sizes::SZ_4K,
> sync::aref::ARef,
> transmute::FromBytes,
> };
> @@ -89,13 +91,94 @@ struct VbiosIterator<'a> {
> last_found: bool,
> }
>
> +// IFR Header in VBIOS.
> +
> +register! {
> + pub(crate) NV_PBUS_IFR_FMT_FIXED0(u32) @ 0x300000 {
> + 31:0 signature;
> + }
> +}
> +
> +register! {
> + pub(crate) NV_PBUS_IFR_FMT_FIXED1(u32) @ 0x300004 {
> + 30:16 fixed_data_size;
> + 15:8 version => u8;
> + }
> +}
> +
> +register! {
> + pub(crate) NV_PBUS_IFR_FMT_FIXED2(u32) @ 0x300008 {
> + 19:0 total_data_size;
> + }
> +}
> +
> +/// Return the byte offset where the PCI Expansion ROM images begin in the GPU's ROM.
> +///
> +/// The GPU's ROM may begin with an Init-from-ROM (IFR) header that precedes
> +/// the PCI Expansion ROM images (VBIOS). When present, the PROM shadow
> +/// method must parse this header to determine the offset where the PCI ROM
> +/// images actually begin, and adjust all subsequent reads accordingly.
> +///
> +/// On most GPUs this is not needed because the IFR microcode has already
> +/// applied the ROM offset so that PROM reads transparently skip the header.
> +/// On GA100, for some reason, the IFR offset is not applied to PROM
> +/// reads. Therefore, the search for the PCI expansion must skip the IFR
> +/// header, if found.
> +fn vbios_rom_offset(dev: &device::Device, bar0: &Bar0) -> Result<usize> {
> + /// IFR signature.
> + const NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE: u32 = u32::from_le_bytes(*b"NVGI");
> + /// ROM directory signature.
> + const NV_ROM_DIRECTORY_IDENTIFIER: u32 = u32::from_le_bytes(*b"RFRD");
> + /// Offset of the NV_PMGR_ROM_ADDR_OFFSET register in IFR Extended section.
> + const IFR_SW_EXT_ROM_ADDR_OFFSET: usize = 4;
> + /// Size of Redundant Firmware Flash Status section.
> + const RFW_FLASH_STATUS_SIZE: usize = SZ_4K;
> + /// Offset in the ROM Directory of the PCI Option ROM offset
> + const PCI_OPTION_ROM_OFFSET: usize = 8;
> +
> + let signature = bar0.read(NV_PBUS_IFR_FMT_FIXED0).signature();
> +
> + if signature == NV_PBUS_IFR_FMT_FIXED0_SIGNATURE_VALUE {
> + let fixed1 = bar0.read(NV_PBUS_IFR_FMT_FIXED1);
> +
> + match fixed1.version() {
> + 1 | 2 => {
> + let fixed_data_size = usize::from(fixed1.fixed_data_size());
> + let pmgr_rom_addr_offset = fixed_data_size + IFR_SW_EXT_ROM_ADDR_OFFSET;
> + bar0.try_read32(ROM_OFFSET + pmgr_rom_addr_offset)
> + .map(usize::from_safe_cast)
> + }
> + 3 => {
> + let fixed2 = bar0.read(NV_PBUS_IFR_FMT_FIXED2);
> + let total_data_size = usize::from(fixed2.total_data_size());
> + let flash_status_offset =
> + usize::from_safe_cast(bar0.try_read32(ROM_OFFSET + total_data_size)?);
> + let dir_offset = flash_status_offset + RFW_FLASH_STATUS_SIZE;
> + let dir_sig = bar0.try_read32(ROM_OFFSET + dir_offset)?;
> + if dir_sig != NV_ROM_DIRECTORY_IDENTIFIER {
> + dev_err!(dev, "could not find IFR ROM directory\n");
> + return Err(EINVAL);
> + }
> + bar0.try_read32(ROM_OFFSET + dir_offset + PCI_OPTION_ROM_OFFSET)
> + .map(usize::from_safe_cast)
> + }
> + _ => {
> + dev_err!(dev, "unsupported IFR header version {}\n", fixed1.version());
> + Err(EINVAL)
> + }
> + }
> + } else {
> + Ok(0)
> + }
> +}
nit: register! type definitions can be put inside the `vbios_rom_offset`
function since they are only used there. And the function itself might
be better inside `VbiosIterator`. Alex, if you agree, maybe you could do
this on apply but not blocking at all.
Thanks Timur!
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-20 1:28 ` Alexandre Courbot
@ 2026-04-20 10:23 ` Gary Guo
2026-04-20 12:50 ` Alexandre Courbot
0 siblings, 1 reply; 20+ messages in thread
From: Gary Guo @ 2026-04-20 10:23 UTC (permalink / raw)
To: Alexandre Courbot, Gary Guo
Cc: Timur Tabi, Danilo Krummrich, John Hubbard, Joel Fernandes,
Eliot Courtney, rust-for-linux
On Mon Apr 20, 2026 at 2:28 AM BST, Alexandre Courbot wrote:
> On Mon Apr 20, 2026 at 4:47 AM JST, Gary Guo wrote:
>> On Fri Apr 17, 2026 at 8:13 PM BST, Timur Tabi wrote:
>>> Introduce FbHal method frts_size() to return the size of the FRTS
>>> window. GA100 is a special case in that there is no FRTS, and so
>>> the size must be set to 0.
>>>
>>> Note that we cannot use supports_display() to determine the FRTS
>>> size because there are other GPUs (e.g. GA102GL) that have display
>>> disabled (and so supports_display() returns False), but the FRTS
>>> window size still needs to be 1MB.
>>>
>>> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
>>> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
>>> ---
>>> drivers/gpu/nova-core/fb.rs | 6 +++---
>>> drivers/gpu/nova-core/fb/hal.rs | 3 +++
>>> drivers/gpu/nova-core/fb/hal/ga100.rs | 6 ++++++
>>> drivers/gpu/nova-core/fb/hal/ga102.rs | 4 ++++
>>> drivers/gpu/nova-core/fb/hal/tu102.rs | 11 ++++++++++-
>>> 5 files changed, 26 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
>>> index f357fb28b22c..a305a6dac758 100644
>>> --- a/drivers/gpu/nova-core/fb.rs
>>> +++ b/drivers/gpu/nova-core/fb.rs
>>> @@ -216,10 +216,10 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>>>
>>> let frts = {
>>> const FRTS_DOWN_ALIGN: Alignment = Alignment::new::<SZ_128K>();
>>> - const FRTS_SIZE: u64 = usize_as_u64(SZ_1M);
>>> - let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - FRTS_SIZE;
>>> + let frts_size: u64 = hal.frts_size();
>>> + let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - frts_size;
>>>
>>> - FbRange(frts_base..frts_base + FRTS_SIZE)
>>> + FbRange(frts_base..frts_base + frts_size)
>>> };
>>>
>>> let boot = {
>>> diff --git a/drivers/gpu/nova-core/fb/hal.rs b/drivers/gpu/nova-core/fb/hal.rs
>>> index aba0abd8ee00..1c01a6cbed65 100644
>>> --- a/drivers/gpu/nova-core/fb/hal.rs
>>> +++ b/drivers/gpu/nova-core/fb/hal.rs
>>> @@ -25,6 +25,9 @@ pub(crate) trait FbHal {
>>>
>>> /// Returns the VRAM size, in bytes.
>>> fn vidmem_size(&self, bar: &Bar0) -> u64;
>>> +
>>> + /// Returns the FRTS size, in bytes.
>>> + fn frts_size(&self) -> u64;
>>> }
>>>
>>> /// Returns the HAL corresponding to `chipset`.
>>> diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
>>> index 1c03783cddef..2f5871d915c3 100644
>>> --- a/drivers/gpu/nova-core/fb/hal/ga100.rs
>>> +++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
>>> @@ -66,6 +66,12 @@ fn supports_display(&self, bar: &Bar0) -> bool {
>>> fn vidmem_size(&self, bar: &Bar0) -> u64 {
>>> super::tu102::vidmem_size_gp102(bar)
>>> }
>>> +
>>> + // GA100 is a special case where its FRTS region exists, but is empty. We
>>> + // return a size of 0 because we still need to record where the region starts.
>>> + fn frts_size(&self) -> u64 {
>>> + 0
>>> + }
>>> }
>>>
>>> const GA100: Ga100 = Ga100;
>>> diff --git a/drivers/gpu/nova-core/fb/hal/ga102.rs b/drivers/gpu/nova-core/fb/hal/ga102.rs
>>> index 4b9f0f74d0e7..3bb66f64bef7 100644
>>> --- a/drivers/gpu/nova-core/fb/hal/ga102.rs
>>> +++ b/drivers/gpu/nova-core/fb/hal/ga102.rs
>>> @@ -35,6 +35,10 @@ fn supports_display(&self, bar: &Bar0) -> bool {
>>> fn vidmem_size(&self, bar: &Bar0) -> u64 {
>>> vidmem_size_ga102(bar)
>>> }
>>> +
>>> + fn frts_size(&self) -> u64 {
>>> + super::tu102::frts_size_tu102()
>>
>> I checked the patch history and it looks like this changes from v1 to v2. If the
>> argument that "both have the same size" then I think this just makes thing more
>> confusing then the purpose of deduplicating.
>>
>> All the methods above are either delegating to GA100 code or custom, and now
>> this delegates to TU102 without any explanation.
>
> The reason for this is that all chips have the same FRTS region size,
> *except* GA100 which is 0. GA100 is the exception (it is also documented
> as you can see in its snippet above) and should be treated as such. Thus
> I'd say the current approach is correct.
Then it should be:
trait FbHal {
fn frts_size(&self) -> u64 {
u64::SZ_1M
}
}
and only have it overriden on GA100.
Or better, use a boolean, I find Nouveau's C code clearer in this regard:
/*
* Calculate FB layout. FRTS is a memory region created by the FWSEC-FRTS firmware.
* FWSEC comes from VBIOS. So on systems with no VBIOS (e.g. GA100), the FRTS does
* not exist. Therefore, use the existence of VBIOS to determine whether to reserve
* an FRTS region.
*/
gsp->fb.wpr2.frts.size = device->bios ? 0x100000 : 0;
so we could simiarly do
fn has_frts(&self) -> bool;
Or
fn has_vbios(&self) -> bool;
(BTW, I suppose the FRTS region really does not exist, but the start address
matters because firmware does some checks on the start address to ensure it's
within a specific region regardless whether the size is 0?)
Best,
Gary
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-20 10:23 ` Gary Guo
@ 2026-04-20 12:50 ` Alexandre Courbot
2026-04-20 17:51 ` Timur Tabi
0 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-04-20 12:50 UTC (permalink / raw)
To: Gary Guo
Cc: Timur Tabi, Danilo Krummrich, John Hubbard, Joel Fernandes,
Eliot Courtney, rust-for-linux
On Mon Apr 20, 2026 at 7:23 PM JST, Gary Guo wrote:
> On Mon Apr 20, 2026 at 2:28 AM BST, Alexandre Courbot wrote:
>> On Mon Apr 20, 2026 at 4:47 AM JST, Gary Guo wrote:
>>> On Fri Apr 17, 2026 at 8:13 PM BST, Timur Tabi wrote:
>>>> Introduce FbHal method frts_size() to return the size of the FRTS
>>>> window. GA100 is a special case in that there is no FRTS, and so
>>>> the size must be set to 0.
>>>>
>>>> Note that we cannot use supports_display() to determine the FRTS
>>>> size because there are other GPUs (e.g. GA102GL) that have display
>>>> disabled (and so supports_display() returns False), but the FRTS
>>>> window size still needs to be 1MB.
>>>>
>>>> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
>>>> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
>>>> ---
>>>> drivers/gpu/nova-core/fb.rs | 6 +++---
>>>> drivers/gpu/nova-core/fb/hal.rs | 3 +++
>>>> drivers/gpu/nova-core/fb/hal/ga100.rs | 6 ++++++
>>>> drivers/gpu/nova-core/fb/hal/ga102.rs | 4 ++++
>>>> drivers/gpu/nova-core/fb/hal/tu102.rs | 11 ++++++++++-
>>>> 5 files changed, 26 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
>>>> index f357fb28b22c..a305a6dac758 100644
>>>> --- a/drivers/gpu/nova-core/fb.rs
>>>> +++ b/drivers/gpu/nova-core/fb.rs
>>>> @@ -216,10 +216,10 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>>>>
>>>> let frts = {
>>>> const FRTS_DOWN_ALIGN: Alignment = Alignment::new::<SZ_128K>();
>>>> - const FRTS_SIZE: u64 = usize_as_u64(SZ_1M);
>>>> - let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - FRTS_SIZE;
>>>> + let frts_size: u64 = hal.frts_size();
>>>> + let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - frts_size;
>>>>
>>>> - FbRange(frts_base..frts_base + FRTS_SIZE)
>>>> + FbRange(frts_base..frts_base + frts_size)
>>>> };
>>>>
>>>> let boot = {
>>>> diff --git a/drivers/gpu/nova-core/fb/hal.rs b/drivers/gpu/nova-core/fb/hal.rs
>>>> index aba0abd8ee00..1c01a6cbed65 100644
>>>> --- a/drivers/gpu/nova-core/fb/hal.rs
>>>> +++ b/drivers/gpu/nova-core/fb/hal.rs
>>>> @@ -25,6 +25,9 @@ pub(crate) trait FbHal {
>>>>
>>>> /// Returns the VRAM size, in bytes.
>>>> fn vidmem_size(&self, bar: &Bar0) -> u64;
>>>> +
>>>> + /// Returns the FRTS size, in bytes.
>>>> + fn frts_size(&self) -> u64;
>>>> }
>>>>
>>>> /// Returns the HAL corresponding to `chipset`.
>>>> diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
>>>> index 1c03783cddef..2f5871d915c3 100644
>>>> --- a/drivers/gpu/nova-core/fb/hal/ga100.rs
>>>> +++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
>>>> @@ -66,6 +66,12 @@ fn supports_display(&self, bar: &Bar0) -> bool {
>>>> fn vidmem_size(&self, bar: &Bar0) -> u64 {
>>>> super::tu102::vidmem_size_gp102(bar)
>>>> }
>>>> +
>>>> + // GA100 is a special case where its FRTS region exists, but is empty. We
>>>> + // return a size of 0 because we still need to record where the region starts.
>>>> + fn frts_size(&self) -> u64 {
>>>> + 0
>>>> + }
>>>> }
>>>>
>>>> const GA100: Ga100 = Ga100;
>>>> diff --git a/drivers/gpu/nova-core/fb/hal/ga102.rs b/drivers/gpu/nova-core/fb/hal/ga102.rs
>>>> index 4b9f0f74d0e7..3bb66f64bef7 100644
>>>> --- a/drivers/gpu/nova-core/fb/hal/ga102.rs
>>>> +++ b/drivers/gpu/nova-core/fb/hal/ga102.rs
>>>> @@ -35,6 +35,10 @@ fn supports_display(&self, bar: &Bar0) -> bool {
>>>> fn vidmem_size(&self, bar: &Bar0) -> u64 {
>>>> vidmem_size_ga102(bar)
>>>> }
>>>> +
>>>> + fn frts_size(&self) -> u64 {
>>>> + super::tu102::frts_size_tu102()
>>>
>>> I checked the patch history and it looks like this changes from v1 to v2. If the
>>> argument that "both have the same size" then I think this just makes thing more
>>> confusing then the purpose of deduplicating.
>>>
>>> All the methods above are either delegating to GA100 code or custom, and now
>>> this delegates to TU102 without any explanation.
>>
>> The reason for this is that all chips have the same FRTS region size,
>> *except* GA100 which is 0. GA100 is the exception (it is also documented
>> as you can see in its snippet above) and should be treated as such. Thus
>> I'd say the current approach is correct.
>
> Then it should be:
>
> trait FbHal {
> fn frts_size(&self) -> u64 {
> u64::SZ_1M
> }
> }
>
> and only have it overriden on GA100.
Mmm we can discuss the design of the HAL, but this is beyond the scope
of this series which just follows what other modules do.
>
> Or better, use a boolean, I find Nouveau's C code clearer in this regard:
>
> /*
> * Calculate FB layout. FRTS is a memory region created by the FWSEC-FRTS firmware.
> * FWSEC comes from VBIOS. So on systems with no VBIOS (e.g. GA100), the FRTS does
> * not exist. Therefore, use the existence of VBIOS to determine whether to reserve
> * an FRTS region.
> */
> gsp->fb.wpr2.frts.size = device->bios ? 0x100000 : 0;
>
> so we could simiarly do
>
> fn has_frts(&self) -> bool;
>
> Or
>
> fn has_vbios(&self) -> bool;
>
The boolean-based approach might make more sense if there is really no
other possible size for the FRTS (which the Nouveau code seems to
suggest). I'll let Timur answer that one.
> (BTW, I suppose the FRTS region really does not exist, but the start address
> matters because firmware does some checks on the start address to ensure it's
> within a specific region regardless whether the size is 0?)
And that one as well. :)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-20 12:50 ` Alexandre Courbot
@ 2026-04-20 17:51 ` Timur Tabi
2026-04-20 20:27 ` Gary Guo
0 siblings, 1 reply; 20+ messages in thread
From: Timur Tabi @ 2026-04-20 17:51 UTC (permalink / raw)
To: gary@garyguo.net, Alexandre Courbot
Cc: Joel Fernandes, dakr@kernel.org, Eliot Courtney, John Hubbard,
rust-for-linux@vger.kernel.org
On Mon, 2026-04-20 at 21:50 +0900, Alexandre Courbot wrote:
> > Or better, use a boolean, I find Nouveau's C code clearer in this regard:
> >
> > /*
> > * Calculate FB layout. FRTS is a memory region created by the FWSEC-FRTS firmware.
> > * FWSEC comes from VBIOS. So on systems with no VBIOS (e.g. GA100), the FRTS does
> > * not exist. Therefore, use the existence of VBIOS to determine whether to reserve
> > * an FRTS region.
> > */
> > gsp->fb.wpr2.frts.size = device->bios ? 0x100000 : 0;
So first, this code in Nouveau is actually wrong, and I have a patchset pending upstream that fixes
it. This is the new version:
gsp->fb.wpr2.frts.size = device->chipset == 0x170 ? 0 : 0x100000;
So this really needs to be a HAL-specific implementation. For better or worse, we're actively
trying to avoid code like this in Nova:
if chipset is X or Y
{
// do this
} else {
// do that
}
> >
> > so we could simiarly do
> >
> > fn has_frts(&self) -> bool;
> >
> > Or
> >
> > fn has_vbios(&self) -> bool;
> >
>
> The boolean-based approach might make more sense if there is really no
> other possible size for the FRTS (which the Nouveau code seems to
> suggest). I'll let Timur answer that one.
I think there is something that everyone needs to understand. 99% of this stuff is completely
arbitrary. Someone, somewhere decided that FRTS is normally 1MB, but for some reason on GA100 it's
0. It's not because GA100 doesn't have display hardware, because there are other GPUs that also
don't have it but the FRTS is still 1MB.
So there's no reason to believe that in some future version of hardware and/or software, there might
be a bigger or smaller FRTS. If it really were an either-or thing, then it would have been defined
as a boolean in our firmware data structures.
The other thing to keep in mind is that a lot of this code is based on OpenRM. One thing you need
to understand is that RM is GPU HAL-happy. We have this weird in-house object-oriented C variant
that makes it trivial to HAL-ify any code. So as soon as there is even the slightest difference
between any two GPUs -- boom! make it a HAL! There are a bunch of places in RM that are implemented
as HALs that could be implemented more abstractly, but just aren't. So if that's how RM does it,
then that's how Nova is going to do it.
> > (BTW, I suppose the FRTS region really does not exist, but the start address
> > matters because firmware does some checks on the start address to ensure it's
> > within a specific region regardless whether the size is 0?)
>
> And that one as well. :)
The FRTS region *does* exist, but its size is zero. This is because the start address is still
needed.
Some parts of GSP-RM will look at frts_size, see that it's zero, and then bail early. For example,
here's one code snippet from GSP-RM:
if (size == 0)
{
return NV_TRUE;
}
But elsewhere in the same file is this:
rmLibosConfigInfo.gfwSrMem.pa = wprApertureStart + pWprMeta->frtsOffset + pWprMeta->frtsSize;
So I guess what I'm really trying to say is that I've done a lot of research into how this code
works, and it's written the way I wrote it for a reason.
As for using a Trait to define a default value that is just overridden by GA100, that would be okay,
but it wouldn't be any clearer or more accurate.
I personally don't like `super::tu102::frts_size_tu102()` and I think both tu102::frts_size() and
ga102::frts_size() should just return u64::SZ_1M, but I don't care enough to make a stink about it.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-20 17:51 ` Timur Tabi
@ 2026-04-20 20:27 ` Gary Guo
2026-04-20 21:15 ` Timur Tabi
0 siblings, 1 reply; 20+ messages in thread
From: Gary Guo @ 2026-04-20 20:27 UTC (permalink / raw)
To: Timur Tabi, gary@garyguo.net, Alexandre Courbot
Cc: Joel Fernandes, dakr@kernel.org, Eliot Courtney, John Hubbard,
rust-for-linux@vger.kernel.org
On Mon Apr 20, 2026 at 6:51 PM BST, Timur Tabi wrote:
> On Mon, 2026-04-20 at 21:50 +0900, Alexandre Courbot wrote:
>> > Or better, use a boolean, I find Nouveau's C code clearer in this regard:
>> >
>> > /*
>> > * Calculate FB layout. FRTS is a memory region created by the FWSEC-FRTS firmware.
>> > * FWSEC comes from VBIOS. So on systems with no VBIOS (e.g. GA100), the FRTS does
>> > * not exist. Therefore, use the existence of VBIOS to determine whether to reserve
>> > * an FRTS region.
>> > */
>> > gsp->fb.wpr2.frts.size = device->bios ? 0x100000 : 0;
>
> So first, this code in Nouveau is actually wrong, and I have a patchset pending upstream that fixes
> it. This is the new version:
Ah, I see, so this is what the "supports_display" refers to in the commit
message?
It's not instinctively the same concept in my mind, but it makes sense now you
explained it.
>
> gsp->fb.wpr2.frts.size = device->chipset == 0x170 ? 0 : 0x100000;
>
> So this really needs to be a HAL-specific implementation. For better or worse, we're actively
> trying to avoid code like this in Nova:
>
> if chipset is X or Y
> {
> // do this
> } else {
> // do that
> }
>
>> >
>> > so we could simiarly do
>> >
>> > fn has_frts(&self) -> bool;
>> >
>> > Or
>> >
>> > fn has_vbios(&self) -> bool;
>> >
>>
>> The boolean-based approach might make more sense if there is really no
>> other possible size for the FRTS (which the Nouveau code seems to
>> suggest). I'll let Timur answer that one.
>
> I think there is something that everyone needs to understand. 99% of this stuff is completely
> arbitrary. Someone, somewhere decided that FRTS is normally 1MB, but for some reason on GA100 it's
> 0. It's not because GA100 doesn't have display hardware, because there are other GPUs that also
> don't have it but the FRTS is still 1MB.
>
> So there's no reason to believe that in some future version of hardware and/or software, there might
> be a bigger or smaller FRTS. If it really were an either-or thing, then it would have been defined
> as a boolean in our firmware data structures.
>
> The other thing to keep in mind is that a lot of this code is based on OpenRM. One thing you need
> to understand is that RM is GPU HAL-happy. We have this weird in-house object-oriented C variant
> that makes it trivial to HAL-ify any code. So as soon as there is even the slightest difference
> between any two GPUs -- boom! make it a HAL! There are a bunch of places in RM that are implemented
> as HALs that could be implemented more abstractly, but just aren't. So if that's how RM does it,
> then that's how Nova is going to do it.
>
>> > (BTW, I suppose the FRTS region really does not exist, but the start address
>> > matters because firmware does some checks on the start address to ensure it's
>> > within a specific region regardless whether the size is 0?)
>>
>> And that one as well. :)
>
> The FRTS region *does* exist, but its size is zero. This is because the start address is still
> needed.
>
> Some parts of GSP-RM will look at frts_size, see that it's zero, and then bail early. For example,
> here's one code snippet from GSP-RM:
>
> if (size == 0)
> {
> return NV_TRUE;
> }
>
> But elsewhere in the same file is this:
>
> rmLibosConfigInfo.gfwSrMem.pa = wprApertureStart + pWprMeta->frtsOffset + pWprMeta->frtsSize;
>
> So I guess what I'm really trying to say is that I've done a lot of research into how this code
> works, and it's written the way I wrote it for a reason.
>
> As for using a Trait to define a default value that is just overridden by GA100, that would be okay,
> but it wouldn't be any clearer or more accurate.
>
> I personally don't like `super::tu102::frts_size_tu102()` and I think both tu102::frts_size() and
> ga102::frts_size() should just return u64::SZ_1M, but I don't care enough to make a stink about it.
Thanks, that's a very good explanation how the reason the code is written as is.
With the information above, I'm happy with current approach.
Acked-by: Gary Guo <gary@garyguo.net>
I think it's important to note that not all of us have knowledge (or even
access) to some of the context that you have, so something that may seem obvious
to you could require some additional background or reasons to be added in commit
message or comment.
Also, in the commit message you mentioned
> Introduce FbHal method frts_size() to return the size of the FRTS
> window. GA100 is a special case in that there is no FRTS, and so
> the size must be set to 0.
and this probably should be corrected.
Thanks,
Gary
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-20 20:27 ` Gary Guo
@ 2026-04-20 21:15 ` Timur Tabi
2026-04-21 1:22 ` Alexandre Courbot
0 siblings, 1 reply; 20+ messages in thread
From: Timur Tabi @ 2026-04-20 21:15 UTC (permalink / raw)
To: gary@garyguo.net, Alexandre Courbot
Cc: Joel Fernandes, dakr@kernel.org, Eliot Courtney, John Hubbard,
rust-for-linux@vger.kernel.org
On Mon, 2026-04-20 at 21:27 +0100, Gary Guo wrote:
>
> Ah, I see, so this is what the "supports_display" refers to in the commit
> message?
Kinda. It refers to this FbHal function:
/// Returns `true` is display is supported.
fn supports_display(&self, bar: &Bar0) -> bool;
It reads an arch-specific register to determine whether display is "supported". There is no
register that tells us "needs FRTS", and so that's why frts_size() exists.
> > So I guess what I'm really trying to say is that I've done a lot of research into how this code
> > works, and it's written the way I wrote it for a reason.
> >
> > As for using a Trait to define a default value that is just overridden by GA100, that would be
> > okay,
> > but it wouldn't be any clearer or more accurate.
> >
> > I personally don't like `super::tu102::frts_size_tu102()` and I think both tu102::frts_size()
> > and
> > ga102::frts_size() should just return u64::SZ_1M, but I don't care enough to make a stink about
> > it.
>
> Thanks, that's a very good explanation how the reason the code is written as is.
> With the information above, I'm happy with current approach.
>
> Acked-by: Gary Guo <gary@garyguo.net>
Thanks.
> I think it's important to note that not all of us have knowledge (or even
> access) to some of the context that you have, so something that may seem obvious
> to you could require some additional background or reasons to be added in commit
> message or comment.
Unfortunately, the process to boot GSP-RM is so convoluted that it's not really possible to document
everything publicly. A lot of it just porting OpenRM (and to some degree Nouveau) to Rust and
saying "Well, this is how it is". In fact, it's much easier to mimic OpenRM than it is to actually
read the internal documentation to figure out what to do.
> Also, in the commit message you mentioned
>
> > Introduce FbHal method frts_size() to return the size of the FRTS
> > window. GA100 is a special case in that there is no FRTS, and so
> > the size must be set to 0.
>
> and this probably should be corrected.
Ugh, I thought I caught all of these.
Alex, if you wouldn't mind, change that last sentence to:
"GA100 is a special case in that although there is an FRTS, its size must arbitrarily be set to 0."
If you update to the latest git, you can use the new "git history" command:
https://github.blog/open-source/git/highlights-from-git-2-54/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support
2026-04-20 21:15 ` Timur Tabi
@ 2026-04-21 1:22 ` Alexandre Courbot
0 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-04-21 1:22 UTC (permalink / raw)
To: Timur Tabi
Cc: gary@garyguo.net, Joel Fernandes, dakr@kernel.org, Eliot Courtney,
John Hubbard, rust-for-linux@vger.kernel.org
On Tue Apr 21, 2026 at 6:15 AM JST, Timur Tabi wrote:
> On Mon, 2026-04-20 at 21:27 +0100, Gary Guo wrote:
>>
>> Ah, I see, so this is what the "supports_display" refers to in the commit
>> message?
>
> Kinda. It refers to this FbHal function:
>
> /// Returns `true` is display is supported.
> fn supports_display(&self, bar: &Bar0) -> bool;
>
> It reads an arch-specific register to determine whether display is "supported". There is no
> register that tells us "needs FRTS", and so that's why frts_size() exists.
>
>> > So I guess what I'm really trying to say is that I've done a lot of research into how this code
>> > works, and it's written the way I wrote it for a reason.
>> >
>> > As for using a Trait to define a default value that is just overridden by GA100, that would be
>> > okay,
>> > but it wouldn't be any clearer or more accurate.
>> >
>> > I personally don't like `super::tu102::frts_size_tu102()` and I think both tu102::frts_size()
>> > and
>> > ga102::frts_size() should just return u64::SZ_1M, but I don't care enough to make a stink about
>> > it.
>>
>> Thanks, that's a very good explanation how the reason the code is written as is.
>> With the information above, I'm happy with current approach.
>>
>> Acked-by: Gary Guo <gary@garyguo.net>
>
> Thanks.
>
>> I think it's important to note that not all of us have knowledge (or even
>> access) to some of the context that you have, so something that may seem obvious
>> to you could require some additional background or reasons to be added in commit
>> message or comment.
>
> Unfortunately, the process to boot GSP-RM is so convoluted that it's not really possible to document
> everything publicly. A lot of it just porting OpenRM (and to some degree Nouveau) to Rust and
> saying "Well, this is how it is". In fact, it's much easier to mimic OpenRM than it is to actually
> read the internal documentation to figure out what to do.
>
>> Also, in the commit message you mentioned
>>
>> > Introduce FbHal method frts_size() to return the size of the FRTS
>> > window. GA100 is a special case in that there is no FRTS, and so
>> > the size must be set to 0.
>>
>> and this probably should be corrected.
>
> Ugh, I thought I caught all of these.
>
> Alex, if you wouldn't mind, change that last sentence to:
>
> "GA100 is a special case in that although there is an FRTS, its size must arbitrarily be set to 0."
Sure, I've fixed it on my staging branch.
With this last point cleared, you can consider this series merged as I
will push it as soon as drm-rust-next reopens. It also makes the
Blackwell series a bit smoother, so I'm happy we can push it first!
>
>
> If you update to the latest git, you can use the new "git history" command:
> https://github.blog/open-source/git/highlights-from-git-2-54/
Thanks for the pointer, I do commit log edits *a lot* and this is
definitely going to help!
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-04-21 1:22 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 19:13 [PATCH v5 0/6] gpu: nova-core: add GA100 support Timur Tabi
2026-04-17 19:13 ` [PATCH v5 1/6] gpu: nova-core: use correct fwsignature for GA100 Timur Tabi
2026-04-19 19:34 ` Gary Guo
2026-04-17 19:13 ` [PATCH v5 2/6] gpu: nova-core: do not consider 0xBB77 as a valid PCI ROM header signature Timur Tabi
2026-04-19 19:36 ` Gary Guo
2026-04-17 19:13 ` [PATCH v5 3/6] gpu: nova-core: only boot FRTS if its region is allocated Timur Tabi
2026-04-19 19:36 ` Gary Guo
2026-04-17 19:13 ` [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support Timur Tabi
2026-04-19 19:47 ` Gary Guo
2026-04-20 1:28 ` Alexandre Courbot
2026-04-20 10:23 ` Gary Guo
2026-04-20 12:50 ` Alexandre Courbot
2026-04-20 17:51 ` Timur Tabi
2026-04-20 20:27 ` Gary Guo
2026-04-20 21:15 ` Timur Tabi
2026-04-21 1:22 ` Alexandre Courbot
2026-04-17 19:13 ` [PATCH v5 5/6] gpu: nova-core: skip the IFR header if present Timur Tabi
2026-04-20 6:37 ` Eliot Courtney
2026-04-17 19:13 ` [PATCH v5 6/6] gpu: nova-core: enable GA100 Timur Tabi
2026-04-19 19:36 ` Gary Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox