[<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