On Wed Jun 4, 2025 at 6:14 AM JST, Lyude Paul wrote:
> On Wed, 2025-05-21 at 15:45 +0900, Alexandre Courbot wrote:
>> FWSEC-FRTS is run with the desired address of the FRTS region as
>> parameter, which we need to compute depending on some hardware
>> parameters.
>>
>> Do this in a `FbLayout` structure, that will be later extended to
>> describe more memory regions used to boot the GSP.
>>
>> Signed-off-by: Alexandre Courbot <acour...@nvidia.com>
>> ---
>> drivers/gpu/nova-core/gpu.rs | 4 ++
>> drivers/gpu/nova-core/gsp.rs | 3 ++
>> drivers/gpu/nova-core/gsp/fb.rs | 77
>> +++++++++++++++++++++++++++++++
>> drivers/gpu/nova-core/gsp/fb/hal.rs | 30 ++++++++++++
>> drivers/gpu/nova-core/gsp/fb/hal/ga100.rs | 24 ++++++++++
>> drivers/gpu/nova-core/gsp/fb/hal/ga102.rs | 24 ++++++++++
>> drivers/gpu/nova-core/gsp/fb/hal/tu102.rs | 28 +++++++++++
>> drivers/gpu/nova-core/nova_core.rs | 1 +
>> drivers/gpu/nova-core/regs.rs | 76
>> ++++++++++++++++++++++++++++++
>> 9 files changed, 267 insertions(+)
>>
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index
>> 39b1cd3eaf8dcf95900eb93d43cfb4f085c897f0..7e03a5696011d12814995928b2984cceae6b6756
>> 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -7,6 +7,7 @@
>> use crate::falcon::{gsp::Gsp, sec2::Sec2, Falcon};
>> use crate::firmware::{Firmware, FIRMWARE_VERSION};
>> use crate::gfw;
>> +use crate::gsp::fb::FbLayout;
>> use crate::regs;
>> use crate::util;
>> use crate::vbios::Vbios;
>> @@ -239,6 +240,9 @@ pub(crate) fn new(
>>
>> let _sec2_falcon = Falcon::<Sec2>::new(pdev.as_ref(), spec.chipset,
>> bar, true)?;
>>
>> + let fb_layout = FbLayout::new(spec.chipset, bar)?;
>> + dev_dbg!(pdev.as_ref(), "{:#x?}\n", fb_layout);
>> +
>> // Will be used in a later patch when fwsec firmware is needed.
>> let _bios = Vbios::new(pdev, bar)?;
>>
>> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
>> new file mode 100644
>> index
>> 0000000000000000000000000000000000000000..27616a9d2b7069b18661fc97811fa1cac285b8f8
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/gsp.rs
>> @@ -0,0 +1,3 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +pub(crate) mod fb;
>> diff --git a/drivers/gpu/nova-core/gsp/fb.rs
>> b/drivers/gpu/nova-core/gsp/fb.rs
>> new file mode 100644
>> index
>> 0000000000000000000000000000000000000000..e65f2619b4c03c4fa51bb24f3d60e8e7008e6ca5
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/gsp/fb.rs
>> @@ -0,0 +1,77 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +use core::ops::Range;
>> +
>> +use kernel::num::NumExt;
>> +use kernel::prelude::*;
>> +
>> +use crate::driver::Bar0;
>> +use crate::gpu::Chipset;
>> +use crate::regs;
>> +
>> +mod hal;
>> +
>> +/// Layout of the GPU framebuffer memory.
>> +///
>> +/// Contains ranges of GPU memory reserved for a given purpose during the
>> GSP bootup process.
>> +#[derive(Debug)]
>> +#[expect(dead_code)]
>> +pub(crate) struct FbLayout {
>> + pub fb: Range<u64>,
>> + pub vga_workspace: Range<u64>,
>> + pub frts: Range<u64>,
>> +}
>> +
>> +impl FbLayout {
>> + /// Computes the FB layout.
>> + pub(crate) fn new(chipset: Chipset, bar: &Bar0) -> Result<Self> {
>> + let hal = chipset.get_fb_fal();
>> +
>> + let fb = {
>> + let fb_size = hal.vidmem_size(bar);
>> +
>> + 0..fb_size
>> + };
>> +
>> + let vga_workspace = {
>> + let vga_base = {
>> + const NV_PRAMIN_SIZE: u64 = 0x100000;
>
> Don't leave those size constants out, they're getting lonely :C
Not quite sure where I should put these; they are not used (for now)
anywhere else, so the relevant scope is not obvious to me. Any
suggestion?
>
>> + let base = fb.end - NV_PRAMIN_SIZE;
>> +
>> + if hal.supports_display(bar) {
>> + match
>> regs::NV_PDISP_VGA_WORKSPACE_BASE::read(bar).vga_workspace_addr() {
>
> Considering how long register names are by default, I wonder if we should just
> be doing:
>
> `use crate::regs::*`
>
> Instead, since the NV_* makes it pretty unambiguous already.
We could - I'm just a bit wary of introducing lots of (unrelated)
register names into the file's namespace... Maybe we should split
`regs.rs` into smaller sub-modules, e.g. `pdisp`, `pfb`, `pfalcon`, etc?
>
>> + Some(addr) => {
>> + if addr < base {
>> + const VBIOS_WORKSPACE_SIZE: u64 = 0x20000;
>> +
>> + // Point workspace address to end of
>> framebuffer.
>> + fb.end - VBIOS_WORKSPACE_SIZE
>> + } else {
>> + addr
>> + }
>> + }
>> + None => base,
>> + }
>> + } else {
>> + base
>> + }
>> + };
>> +
>> + vga_base..fb.end
>> + };
>> +
>> + let frts = {
>> + const FRTS_DOWN_ALIGN: u64 = 0x20000;
>> + const FRTS_SIZE: u64 = 0x100000;
>> + let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN)
>> - FRTS_SIZE;
>> +
>> + frts_base..frts_base + FRTS_SIZE
>> + };
>> +
>> + Ok(Self {
>> + fb,
>> + vga_workspace,
>> + frts,
>> + })
>> + }
>> +}
>> diff --git a/drivers/gpu/nova-core/gsp/fb/hal.rs
>> b/drivers/gpu/nova-core/gsp/fb/hal.rs
>> new file mode 100644
>> index
>> 0000000000000000000000000000000000000000..9f8e777e90527026a39061166c6af6257a066aca
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/gsp/fb/hal.rs
>> @@ -0,0 +1,30 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +use crate::driver::Bar0;
>> +use crate::gpu::Chipset;
>> +
>> +mod ga100;
>> +mod ga102;
>> +mod tu102;
>> +
>> +pub(crate) trait FbHal {
>> + /// Returns `true` is display is supported.
>> + fn supports_display(&self, bar: &Bar0) -> bool;
>> + /// Returns the VRAM size, in bytes.
>> + fn vidmem_size(&self, bar: &Bar0) -> u64;
>> +}
>> +
>> +impl Chipset {
>> + /// Returns the HAL corresponding to this chipset.
>> + pub(super) fn get_fb_fal(self) -> &'static dyn FbHal {
>> + use Chipset::*;
>> +
>> + match self {
>> + TU102 | TU104 | TU106 | TU117 | TU116 => tu102::TU102_HAL,
>> + GA100 => ga100::GA100_HAL,
>> + GA102 | GA103 | GA104 | GA106 | GA107 | AD102 | AD103 | AD104 |
>> AD106 | AD107 => {
>
> Hopefully I'm not hallucinating us adding #[derive(Ordering)] or whatever it's
> called now that I'm 17 patches deep but, couldn't we use ranges here w/r/t to
> the model numbers?
I wish we could, but Rust doesn't allow this yet:
error[E0029]: only `char` and numeric types are allowed in range patterns
--> drivers/gpu/nova-core/gsp/fb/hal.rs:23:13
|
23 | TU102..TU116 => tu102::TU102_HAL,
| -----^^-----
| | |
| | this is of type `Chipset` but it should be `char` or
numeric
| this is of type `Chipset` but it should be `char` or numeric
Applying `#[repr(u32)]` on `Chipset` does not enable ranges unfortunately.
>
> Otherwise:
>
> Reviewed-by: Lyude Paul <ly...@redhat.com>
Thank you!
