Re: [PATCH 01/31] gpu: nova-core: print FB sizes, along with ranges

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

On Wed Dec 3, 2025 at 5:58 AM GMT, John Hubbard wrote:
> For convenience of the reader: now you can directly see the sizes of
> each range. It is suprising just how much this helps.
>
> Sample output (using an Ampere GA104):
>
> NovaCore 0000:e1:00.0: FbLayout {
>     fb: 0x0..0x3ff800000 (16376 MB),
>     vga_workspace: 0x3ff700000..0x3ff800000 (1 MB),
>     frts: 0x3ff600000..0x3ff700000 (1 MB),
>     boot: 0x3ff5fa000..0x3ff600000 (0 MB),
>     elf: 0x3fb960000..0x3ff5f9000 (60 MB),
>     wpr2_heap: 0x3f3900000..0x3fb900000 (128 MB),
>     wpr2: 0x3f3800000..0x3ff700000 (191 MB),
>     heap: 0x3f3700000..0x3f3800000 (1 MB),
>     vf_partition_count: 0x0,
> }
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  drivers/gpu/nova-core/fb.rs | 66 +++++++++++++++++++++++++++----------
>  1 file changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index 3c9cf151786c..333e952400e6 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
> @@ -1,9 +1,13 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> -use core::ops::Range;
> +use core::ops::{
> +    Deref,
> +    Range, //
> +};
>  
>  use kernel::{
>      device,
> +    fmt,
>      prelude::*,
>      ptr::{
>          Alignable,
> @@ -94,26 +98,54 @@ pub(crate) fn unregister(&self, bar: &Bar0) {
>      }
>  }
>  
> +pub(crate) struct FbRange(Range<u64>);

How useful do you think this is in general? Would it make sense to have a
dedicated PhysAddrRange type in kernel crate that provides this feature?

> +
> +impl From<Range<u64>> for FbRange {
> +    fn from(range: Range<u64>) -> Self {
> +        Self(range)
> +    }
> +}
> +
> +impl Deref for FbRange {
> +    type Target = Range<u64>;
> +
> +    fn deref(&self) -> &Self::Target {
> +        &self.0
> +    }
> +}
> +
> +impl fmt::Debug for FbRange {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

If you want to make this more generic, you can gate the size printing on
`f.alternate()`. So if people use `{:?}` to print a compact form of `FbLayout`,
they get just the range, and if they use `{:#?}` then they also get the sizes.

> +        let size_mb = (self.0.end - self.0.start) >> 20;
> +        f.write_fmt(fmt!(
> +            "{:#x}..{:#x} ({} MB)",

I would either just use IEC prefix "MiB", or alternatively use the coreutils
convention and use "M" to represent MiB.

Best,
Gary

> +            self.0.start,
> +            self.0.end,
> +            size_mb
> +        ))
> +    }
> +}
> +
>  /// Layout of the GPU framebuffer memory.
>  ///
>  /// Contains ranges of GPU memory reserved for a given purpose during the GSP boot process.
>  #[derive(Debug)]
>  pub(crate) struct FbLayout {
>      /// Range of the framebuffer. Starts at `0`.
> -    pub(crate) fb: Range<u64>,
> +    pub(crate) fb: FbRange,
>      /// VGA workspace, small area of reserved memory at the end of the framebuffer.
> -    pub(crate) vga_workspace: Range<u64>,
> +    pub(crate) vga_workspace: FbRange,
>      /// FRTS range.
> -    pub(crate) frts: Range<u64>,
> +    pub(crate) frts: FbRange,
>      /// Memory area containing the GSP bootloader image.
> -    pub(crate) boot: Range<u64>,
> +    pub(crate) boot: FbRange,
>      /// Memory area containing the GSP firmware image.
> -    pub(crate) elf: Range<u64>,
> +    pub(crate) elf: FbRange,
>      /// WPR2 heap.
> -    pub(crate) wpr2_heap: Range<u64>,
> +    pub(crate) wpr2_heap: FbRange,
>      /// WPR2 region range, starting with an instance of `GspFwWprMeta`.
> -    pub(crate) wpr2: Range<u64>,
> -    pub(crate) heap: Range<u64>,
> +    pub(crate) wpr2: FbRange,
> +    pub(crate) heap: FbRange,
>      pub(crate) vf_partition_count: u8,
>  }
>  
> @@ -125,7 +157,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>          let fb = {
>              let fb_size = hal.vidmem_size(bar);
>  
> -            0..fb_size
> +            FbRange(0..fb_size)
>          };
>  
>          let vga_workspace = {
> @@ -152,7 +184,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>                  }
>              };
>  
> -            vga_base..fb.end
> +            FbRange(vga_base..fb.end)
>          };
>  
>          let frts = {
> @@ -160,7 +192,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>              const FRTS_SIZE: u64 = usize_as_u64(SZ_1M);
>              let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - FRTS_SIZE;
>  
> -            frts_base..frts_base + FRTS_SIZE
> +            FbRange(frts_base..frts_base + FRTS_SIZE)
>          };
>  
>          let boot = {
> @@ -168,7 +200,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>              let bootloader_size = u64::from_safe_cast(gsp_fw.bootloader.ucode.size());
>              let bootloader_base = (frts.start - bootloader_size).align_down(BOOTLOADER_DOWN_ALIGN);
>  
> -            bootloader_base..bootloader_base + bootloader_size
> +            FbRange(bootloader_base..bootloader_base + bootloader_size)
>          };
>  
>          let elf = {
> @@ -176,7 +208,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>              let elf_size = u64::from_safe_cast(gsp_fw.size);
>              let elf_addr = (boot.start - elf_size).align_down(ELF_DOWN_ALIGN);
>  
> -            elf_addr..elf_addr + elf_size
> +            FbRange(elf_addr..elf_addr + elf_size)
>          };
>  
>          let wpr2_heap = {
> @@ -185,7 +217,7 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>                  gsp::LibosParams::from_chipset(chipset).wpr_heap_size(chipset, fb.end);
>              let wpr2_heap_addr = (elf.start - wpr2_heap_size).align_down(WPR2_HEAP_DOWN_ALIGN);
>  
> -            wpr2_heap_addr..(elf.start).align_down(WPR2_HEAP_DOWN_ALIGN)
> +            FbRange(wpr2_heap_addr..(elf.start).align_down(WPR2_HEAP_DOWN_ALIGN))
>          };
>  
>          let wpr2 = {
> @@ -193,13 +225,13 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>              let wpr2_addr = (wpr2_heap.start - u64::from_safe_cast(size_of::<gsp::GspFwWprMeta>()))
>                  .align_down(WPR2_DOWN_ALIGN);
>  
> -            wpr2_addr..frts.end
> +            FbRange(wpr2_addr..frts.end)
>          };
>  
>          let heap = {
>              const HEAP_SIZE: u64 = usize_as_u64(SZ_1M);
>  
> -            wpr2.start - HEAP_SIZE..wpr2.start
> +            FbRange(wpr2.start - HEAP_SIZE..wpr2.start)
>          };
>  
>          Ok(Self {