Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs

[John Hubbard <jhubbard@nvidia.com>]

On 10/29/25 4:26 AM, Danilo Krummrich wrote:
> On Wed Oct 29, 2025 at 4:03 AM CET, John Hubbard wrote:
...
>> +    fn try_from(boot0: regs::NV_PMC_BOOT_0) -> Result<Self> {
>> +        Ok(Self {
>> +            chipset: boot0.chipset()?,
>> +            revision: Revision {
>> +                major: boot0.major_revision(),
>> +                minor: boot0.minor_revision(),
>> +            },
> 
> Actually, would be nice to handle this just like chipset for consistency, i.e.
> 
> 	revision: boot0.revision()

Good idea, done.

...
>> +    fn try_from(boot42: regs::NV_PMC_BOOT_42) -> Result<Self> {
>> +        Ok(Self {
>> +            chipset: boot42.chipset()?,
>> +            revision: Revision {
>> +                major: boot42.major_revision(),
>> +                minor: boot42.minor_revision(),
>> +            },
> 
> Same here, could be
> 
> 	revision: boot42.revision()

Done. 

...> Without the comments this currently is:
> 
> 	let boot42 = if boot0.is_next_gen() {
> 	    Some(regs::NV_PMC_BOOT_42::read(bar))
> 	} else {
> 	    None
> 	};
> 	
> 	if boot0.is_nv04() {
> 	    Err(ENODEV)?
> 	}
> 	
> 	boot42
> 	    .map(Spec::try_from)
> 	    .unwrap_or_else(|| Spec::try_from(boot0))
> 
> Which I think is a bit heavy-handed. Let's simplify this a bit:
> 
> 	let boot0 = regs::NV_PMC_BOOT_0::read(bar);
> 
> 	if boot0.is_nv04() {
> 	    return Err(ENODEV);
> 	}
> 
> 	Spec::try_from(
> 	    if boot0.is_next_gen() {
> 	        regs::NV_PMC_BOOT_42::read(bar)
> 	    } else {
> 	        boot0
> 	    }
> 	)

That, combined with Timur's comment, made me realize that .is_next_gen()
can be made reliable enough that we don't even need is_nv04() at all.

I've shortened and fixed it up accordingly.

(I think I got a little too involved in the Nouveau-related discussions,
there: Nouveau has a different set of things to support, and to check
for.)


thanks,
-- 
John Hubbard