John Hubbard
2025-Oct-28 02:39 UTC
[PATCH v2 1/2] gpu: nova-core: merge the Revision type into the Spec type
Revision is no longer valuable as a stand-alone type, because we only
ever want the full information that Spec provides. Therefore, enhance
Spec's Display, and remove Revision entirely.
This also simplifies the dev_info!() code for printing banners such as:
NVIDIA (Chipset: GA104, Architecture: Ampere, Revision: a.1)
Signed-off-by: John Hubbard <jhubbard at nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 50 ++++++++++++++----------------------
1 file changed, 19 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 9d182bffe8b4..0ba1cdc42ba3 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -129,31 +129,11 @@ fn try_from(value: u8) -> Result<Self> {
}
}
-pub(crate) struct Revision {
- major: u8,
- minor: u8,
-}
-
-impl Revision {
- fn from_boot0(boot0: regs::NV_PMC_BOOT_0) -> Self {
- Self {
- major: boot0.major_revision(),
- minor: boot0.minor_revision(),
- }
- }
-}
-
-impl fmt::Display for Revision {
- fn fmt(&self, f: &mut fmt::Formatter<'_>) ->
fmt::Result {
- write!(f, "{:x}.{:x}", self.major, self.minor)
- }
-}
-
-/// Structure holding the metadata of the GPU.
+/// Structure holding a basic description of the GPU: Architecture, Chipset and
Revision.
pub(crate) struct Spec {
chipset: Chipset,
- /// The revision of the chipset.
- revision: Revision,
+ major_revision: u8,
+ minor_revision: u8,
}
impl Spec {
@@ -162,11 +142,25 @@ fn new(bar: &Bar0) -> Result<Spec> {
Ok(Self {
chipset: boot0.chipset()?,
- revision: Revision::from_boot0(boot0),
+ major_revision: boot0.major_revision(),
+ minor_revision: boot0.minor_revision(),
})
}
}
+impl fmt::Display for Spec {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) ->
fmt::Result {
+ write!(
+ f,
+ "Chipset: {}, Architecture: {:?}, Revision: {:x}.{:x}",
+ self.chipset,
+ self.chipset.arch(),
+ self.major_revision,
+ self.minor_revision
+ )
+ }
+}
+
/// Structure holding the resources required to operate the GPU.
#[pin_data]
pub(crate) struct Gpu {
@@ -193,13 +187,7 @@ pub(crate) fn new<'a>(
) -> impl PinInit<Self, Error> + 'a {
try_pin_init!(Self {
spec: Spec::new(bar).inspect(|spec| {
- dev_info!(
- pdev.as_ref(),
- "NVIDIA (Chipset: {}, Architecture: {:?}, Revision:
{})\n",
- spec.chipset,
- spec.chipset.arch(),
- spec.revision
- );
+ dev_info!(pdev.as_ref(),"NVIDIA ({})\n", spec);
})?,
// We must wait for GFW_BOOT completion before doing any
significant setup on the GPU.
--
2.51.1
Danilo Krummrich
2025-Oct-28 12:25 UTC
[PATCH v2 1/2] gpu: nova-core: merge the Revision type into the Spec type
On Tue Oct 28, 2025 at 3:39 AM CET, John Hubbard wrote:> -pub(crate) struct Revision { > - major: u8, > - minor: u8, > -} > - > -impl Revision { > - fn from_boot0(boot0: regs::NV_PMC_BOOT_0) -> Self { > - Self { > - major: boot0.major_revision(), > - minor: boot0.minor_revision(), > - } > - } > -} > - > -impl fmt::Display for Revision { > - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { > - write!(f, "{:x}.{:x}", self.major, self.minor) > - } > -} > - > -/// Structure holding the metadata of the GPU. > +/// Structure holding a basic description of the GPU: Architecture, Chipset and Revision. > pub(crate) struct Spec { > chipset: Chipset, > - /// The revision of the chipset. > - revision: Revision, > + major_revision: u8, > + minor_revision: u8, > }Why not keep the Revision type and its Display impl as well?> > impl Spec { > @@ -162,11 +142,25 @@ fn new(bar: &Bar0) -> Result<Spec> { > > Ok(Self { > chipset: boot0.chipset()?, > - revision: Revision::from_boot0(boot0), > + major_revision: boot0.major_revision(), > + minor_revision: boot0.minor_revision(), > }) > } > } > > +impl fmt::Display for Spec { > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { > + write!( > + f, > + "Chipset: {}, Architecture: {:?}, Revision: {:x}.{:x}", > + self.chipset, > + self.chipset.arch(), > + self.major_revision, > + self.minor_revision > + )This could just be: write!( f, "Chipset: {}, Architecture: {:?}, Revision: {}", self.chipset, self.chipset.arch(), self.revision, )> + } > +} > + > /// Structure holding the resources required to operate the GPU. > #[pin_data] > pub(crate) struct Gpu { > @@ -193,13 +187,7 @@ pub(crate) fn new<'a>( > ) -> impl PinInit<Self, Error> + 'a { > try_pin_init!(Self { > spec: Spec::new(bar).inspect(|spec| { > - dev_info!( > - pdev.as_ref(), > - "NVIDIA (Chipset: {}, Architecture: {:?}, Revision: {})\n", > - spec.chipset, > - spec.chipset.arch(), > - spec.revision > - ); > + dev_info!(pdev.as_ref(),"NVIDIA ({})\n", spec); > })?, > > // We must wait for GFW_BOOT completion before doing any significant setup on the GPU. > -- > 2.51.1