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

John Hubbard posted 6 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH 1/6] gpu: nova-core: print FB sizes, along with ranges
Posted by John Hubbard 1 month, 1 week ago
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()
+    }
+}
+
 impl FbLayout {
     /// Computes the FB layout for `chipset`, for running the `bl` GSP bootloader and `gsp` GSP
     /// firmware.
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index bd3be366526e..c27a90aa782c 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -141,7 +141,7 @@ pub(crate) fn boot(
         )?;
 
         let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
-        dev_dbg!(dev, "{:#x?}\n", fb_layout);
+        dev_dbg!(dev, "{:#?}\n", fb_layout);
 
         Self::run_fwsec_frts(dev, gsp_falcon, bar, &bios, &fb_layout)?;
 
-- 
2.51.2
Re: [PATCH 1/6] gpu: nova-core: print FB sizes, along with ranges
Posted by Alexandre Courbot 1 month ago
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.
Re: [PATCH 1/6] gpu: nova-core: print FB sizes, along with ranges
Posted by John Hubbard 1 month ago
On 11/18/25 11:33 PM, Alexandre Courbot wrote:
> On Thu Nov 6, 2025 at 12:54 PM JST, John Hubbard wrote:
...
> 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.

This all sounds good, I'll do it, thanks for the ideas and most of
the implementation too. :)

thanks,
-- 
John Hubbard