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 <[email protected]>
> ---
> 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 {
