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

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

On Thu Nov 6, 2025 at 12:54 PM JST, 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:
>
> 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,
>     rsvd_size: 0x1a00000,
> }
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  drivers/gpu/nova-core/fb.rs       | 33 ++++++++++++++++++++++++++++++-
>  drivers/gpu/nova-core/gsp/boot.rs |  2 +-
>  2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index 10406b6f2e16..004238689f26 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
> @@ -87,7 +87,6 @@ pub(crate) fn unregister(&self, bar: &Bar0) {
>  /// 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>,
> @@ -107,6 +106,38 @@ pub(crate) struct FbLayout {
>      pub(crate) vf_partition_count: u8,
>  }
>  
> +struct RangeWithSize<'a>(&'a Range<u64>);
> +
> +impl core::fmt::Debug for RangeWithSize<'_> {
> +    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> +        if self.0.start == 0 && self.0.end == 0 {
> +            write!(f, "0x0..0x0")
> +        } else {
> +            let size_mb = (self.0.end - self.0.start) >> 20;
> +            write!(f, "{:#x}..{:#x} ({} MB)", self.0.start, self.0.end, size_mb)
> +        }
> +    }
> +}
> +
> +impl core::fmt::Debug for FbLayout {
> +    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> +        f.debug_struct("FbLayout")
> +            .field("fb", &RangeWithSize(&self.fb))
> +            .field("vga_workspace", &RangeWithSize(&self.vga_workspace))
> +            .field("frts", &RangeWithSize(&self.frts))
> +            .field("boot", &RangeWithSize(&self.boot))
> +            .field("elf", &RangeWithSize(&self.elf))
> +            .field("wpr2_heap", &RangeWithSize(&self.wpr2_heap))
> +            .field("wpr2", &RangeWithSize(&self.wpr2))
> +            .field("heap", &RangeWithSize(&self.heap))
> +            .field(
> +                "vf_partition_count",
> +                &fmt!("{:#x}", self.vf_partition_count),
> +            )
> +            .finish()
> +    }
> +}

The only concern I have is that if we add fields to `FbLayout` we will
need (and probably forget) to update its `Debug` implementation.

How about we just use this more intrusively:

    pub(crate) struct FbRange(Range<u64>);

    // Convert easily from a regular `Range`.
    impl From<Range<u64>> for FbRange {
        fn from(range: Range<u64>) -> Self {
            Self(range)
        }
    }

    // Provide transparent access to the members of `Range`.
    impl Deref for FbRange {
        type Target = Range<u64>;

        fn deref(&self) -> &Self::Target {
            &self.0
        }
    }

    impl Debug for FbRange {
        ...
    }

Then we can change the members of `FbLayout` to `FbRange`, and keep its
derived `Debug` implementation.

The initialization code would only need to marginally change, e.g:

    let fb: FbRange = {
        let fb_size = hal.vidmem_size(bar);

        (0..fb_size).into()
    };

And with this new type, we can also address one another shortcoming that
was bugging me! In e.g. `boot.rs` we have this ugly bit:

        frts_size: fb_layout.frts.end - fb_layout.frts.start,

What we want is a `len` method, but since our range uses u64, and `len`
returns a `usize`, standard Rust doesn't provide one for us. But thanks
to this dedicated type we can now implement our own! :)

Not saying this has to be done in this patch though, but it's a nice
side-effect.