[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a24876cd-1a41-488f-968f-38d2ebebdd39@nvidia.com>
Date: Wed, 29 Oct 2025 17:29:18 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Alexandre Courbot <acourbot@...dia.com>,
 Joel Fernandes <joelagnelf@...dia.com>, Timur Tabi <ttabi@...dia.com>,
 Alistair Popple <apopple@...dia.com>, Edwin Peer <epeer@...dia.com>,
 Zhi Wang <zhiw@...dia.com>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Bjorn Helgaas <bhelgaas@...gle.com>,
 Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
 nouveau@...ts.freedesktop.org, rust-for-linux@...r.kernel.org,
 LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen
 GPUs
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
Powered by blists - more mailing lists
 
