Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support

["Gary Guo" <gary@garyguo.net>]

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