Re: [PATCH v10 08/28] gpu: nova-core: Blackwell: calculate reserved FB heap size

John Hubbard
2026-04-18T01:42:15Z

On 4/17/26 7:23 AM, Alexandre Courbot wrote:
> On Sat Apr 11, 2026 at 11:49 AM JST, John Hubbard wrote:
> <snip>
>> @@ -168,6 +175,8 @@ pub(crate) struct FbLayout {
>> pub(crate) wpr2: FbRange,
>> pub(crate) heap: FbRange,
>> pub(crate) vf_partition_count: u8,
>> + /// PMU reserved memory size, in bytes.
>> + pub(crate) pmu_reserved_size: u32,
>> }
>>
>> impl FbLayout {
>> @@ -268,6 +277,29 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0, gsp_fw: &GspFirmware) -> Result<
>> wpr2,
>> heap,
>> vf_partition_count: 0,
>> + pmu_reserved_size: match chipset.arch() {
>> + Architecture::BlackwellGB10x | Architecture::BlackwellGB20x => PMU_RESERVED_SIZE,
>> + _ => 0,
>> + },
>
> I suspect you can just call `calc_pmu_reserved_size`, which seem to
> perform the exact same computation?

Yes.

>
>> })
>> }
>> }
>> +
>> +/// Returns the PMU reserved memory size for `chipset`.
>> +#[expect(dead_code)]
>> +pub(crate) fn calc_pmu_reserved_size(chipset: Chipset) -> u32 {
>> + match chipset.arch() {
>> + Architecture::BlackwellGB10x | Architecture::BlackwellGB20x => PMU_RESERVED_SIZE,
>> + _ => 0,
>> + }
>
> We should avoid these matches on Architecture in common code as much as
> possible - that's what the HALs are for. Which means that
> `calc_pmu_reserved_size` should call a HAL method to obtain the right
> size (I'd also rename it to just `pmu_reserved_size` as we are not
> computing anything at runtime).
>

Yes.

>> +}
>> +
>> +/// PMU reserved size, aligned to 128KB.
>> +pub(crate) const PMU_RESERVED_SIZE: u32 = usize_into_u32::<
>> + {
>> + match const_align_up(SZ_8M + SZ_16M + SZ_4K, Alignment::new::<SZ_128K>()) {
>> + Some(v) => v,
>> + None => panic!("PMU_RESERVED_SIZE: alignment overflow"),
>> + }
>> + },
>> +>();
>
> Consequently, this should be moved into the Blackwell HAL - which is
> where is belongs, as contrary to what its name implies in the current
> module, this reserved size does not apply to all architectures.

Right. This originated in bringup, when I didn't fully understand where
and when PMU reserved size came into play. Now it's clear, and yes it
needs a HAL.

thanks,
--
John Hubbard