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

["Alexandre Courbot" <acourbot@nvidia.com>]

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.