On Wed Feb 25, 2026 at 7:53 AM JST, Joel Fernandes wrote:
<snip>
> @@ -295,18 +309,42 @@ pub(crate) fn new<'a>(
>
> sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset)?,
>
> - // Create GPU memory manager owning memory management resources.
> - // This will be initialized with the usable VRAM region from GSP
> in a later
> - // patch. For now, we use a placeholder of 1MB.
> - mm <- GpuMm::new(devres_bar.clone(), GpuBuddyParams {
> - base_offset_bytes: 0,
> - physical_memory_size_bytes: SZ_1M as u64,
> - chunk_size_bytes: SZ_4K as u64,
> - })?,
> -
> gsp <- Gsp::new(pdev),
>
> - gsp_static_info: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon,
> sec2_falcon)?.0 },
> + // Boot GSP and extract usable VRAM region for buddy allocator.
> + gsp_static_info: {
> + let (info, fb_layout) = gsp.boot(pdev, bar, spec.chipset,
> gsp_falcon, sec2_falcon)?;
> +
> + let usable_vram =
> fb_layout.usable_vram.as_ref().ok_or_else(|| {
> + dev_err!(pdev.as_ref(), "No usable FB regions found from
> GSP\n");
> + ENODEV
> + })?;
The chain through which we obtain `usable_vram` is very inefficient and
much more complex than it needs to be.
`fb_layout` is used as a transport for `usable_vram`, but `usable_vram`
is the only member of it that we ever use. In that case, why not return
`usable_vram` directly?
This is all the better as `usable_vram` is added as an `Option` in
`FbLayout`, not because `None` is a valid state but because `FbLayout`
is already constructed by the time we obtain `usable_vram`. So `None` is
just a value that tells the caller "please return an error". Now we can
remove the option altogether, drop patch 5, and have `boot` return the
error for us if `usable_vram` cannot be obtained.
But let's not stop here. `usable_vram` is first obtained as part of the
`GetGspStaticInfo` reply. It starts its life as `(u64, u64)` before
being morphed into a `Range<u64>` when stored in `FbLayout`. Since its
tuple state is never useful, let's store it as a `Range<u64>` in
`GetGspStaticInfoReply` to begin with and, since we cannot continue
without it anyway, let's make `commands::get_gsp_info()` fail if it
cannot build it. That way we don't even need to store it as an `Option`.
And now, since `boot` already returns the `GetGspStaticInfoReply`, we
can directly access it just by making `usable_vram` public - no need for
`boot` to return a tuple anymore.
Once you have that, something interesting happens: you don't need to
change the `gsp_static_info` arm at all in this patch. All the new code
can be moved to the `mm` arm, which is where it is actually useful
anyway.
The subsequent patches that make use of `boot_params` in other arms can
also access the exact same data using `gsp_static_info`:
- `bar1_pde_base` is also accessible from `gsp_static_info` (let's also
make it public instead of adding an accessor method),
- The other `boot_params` used to build the `GpuMM` can be reconstructed
from `usable_vram`.
Which means `boot_params` and `BootParams` can be removed (and the
`Cell` import), and we have something simpler and more direct that takes
~60 less LoCs.
In other words, this arm remains as it was:
gsp_static_info: gsp.boot(pdev, bar, spec.chipset, gsp_falcon,
sec2_falcon)?,
> + // Create GPU memory manager owning memory management resources.
> + // Uses the usable VRAM region from GSP for buddy allocator.
> + mm <- {
this arm can now be:
let usable_vram = &gsp_static_info.usable_fb_region;
dev_info!(
pdev.as_ref(),
"Using FB region: {:#x}..{:#x}\n",
usable_vram.start,
usable_vram.end
);
GpuMm::new(devres_bar.clone(), GpuBuddyParams {
base_offset_bytes: usable_vram.start,
physical_memory_size_bytes: usable_vram.end - usable_vram.start,
chunk_size_bytes: SZ_4K.into_safe_cast(),
})?
> + let params = boot_params.get();
> + GpuMm::new(devres_bar.clone(), GpuBuddyParams {
> + base_offset_bytes: params.usable_vram_start,
> + physical_memory_size_bytes: params.usable_vram_size,
> + chunk_size_bytes: SZ_4K.into_safe_cast(),
> + })?
> + },
>
> bar: devres_bar,
> })
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs
> b/drivers/gpu/nova-core/gsp/boot.rs
> index 7a4a0c759267..bc4446282613 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -150,7 +150,7 @@ pub(crate) fn boot(
>
> let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset,
> FIRMWARE_VERSION), GFP_KERNEL)?;
>
> - let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
> + let mut fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
> dev_dbg!(dev, "{:#x?}\n", fb_layout);
>
> Self::run_fwsec_frts(dev, gsp_falcon, bar, &bios, &fb_layout)?;
> @@ -252,6 +252,11 @@ pub(crate) fn boot(
> Err(e) => dev_warn!(pdev.as_ref(), "GPU name unavailable:
> {:?}\n", e),
> }
>
> + // Populate usable VRAM from GSP response.
> + if let Some((base, size)) = info.usable_fb_region() {
> + fb_layout.set_usable_vram(base, size);
> + }
> +
And this change is not needed anymore.
