In the fullness of time, it has become clear that these two types are
not actually needed for anything, after all. Remove them both, and use
boot0.chipset directly, instead.
This deletes a net total of 33 lines of code and simplifies the code as
well.
Signed-off-by: John Hubbard <jhubbard at nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 67 +++++++++---------------------------
1 file changed, 17 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index af20e2daea24..a8a993424771 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -129,48 +129,10 @@ 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.
-pub(crate) struct Spec {
- chipset: Chipset,
- /// The revision of the chipset.
- revision: Revision,
-}
-
-impl Spec {
- fn new(bar: &Bar0) -> Result<Spec> {
- let boot0 = regs::NV_PMC_BOOT_0::read(bar);
-
- Ok(Self {
- chipset: boot0.chipset()?,
- revision: Revision::from_boot0(boot0),
- })
- }
-}
-
/// Structure holding the resources required to operate the GPU.
#[pin_data]
pub(crate) struct Gpu {
- spec: Spec,
+ chipset: Chipset,
/// MMIO mapping of PCI BAR 0
bar: Arc<Devres<Bar0>>,
/// System memory page required for flushing all pending GPU-side memory
writes done through
@@ -191,16 +153,21 @@ pub(crate) fn new<'a>(
devres_bar: Arc<Devres<Bar0>>,
bar: &'a Bar0,
) -> impl PinInit<Self, Error> + 'a {
+ let boot0 = regs::NV_PMC_BOOT_0::read(bar);
+
try_pin_init!(Self {
- spec: Spec::new(bar).inspect(|spec| {
+ chipset: {
+ let chipset = boot0.chipset()?;
dev_info!(
pdev.as_ref(),
- "NVIDIA (Chipset: {}, Architecture: {:?}, Revision:
{})\n",
- spec.chipset,
- spec.chipset.arch(),
- spec.revision
+ "NVIDIA (Chipset: {}, Architecture: {:?}, Revision:
{:x}.{:x})\n",
+ chipset,
+ chipset.arch(),
+ boot0.major_revision(),
+ boot0.minor_revision()
);
- })?,
+ chipset
+ },
// We must wait for GFW_BOOT completion before doing any
significant setup on the GPU.
_: {
@@ -208,21 +175,21 @@ pub(crate) fn new<'a>(
.inspect_err(|_| dev_err!(pdev.as_ref(), "GFW boot did
not complete"))?;
},
- sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar,
spec.chipset)?,
+ sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar,
boot0.chipset()?)?,
gsp_falcon: Falcon::new(
pdev.as_ref(),
- spec.chipset,
+ boot0.chipset()?,
bar,
- spec.chipset > Chipset::GA100,
+ boot0.chipset()? > Chipset::GA100,
)
.inspect(|falcon| falcon.clear_swgen0_intr(bar))?,
- sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset, bar, true)?,
+ sec2_falcon: Falcon::new(pdev.as_ref(), boot0.chipset()?, bar,
true)?,
gsp <- Gsp::new(),
- _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? },
+ _: { gsp.boot(pdev, bar, boot0.chipset()?, gsp_falcon,
sec2_falcon)? },
bar: devres_bar,
})
--
2.51.1
Danilo Krummrich
2025-Oct-25 10:01 UTC
[PATCH 1/2] gpu: nova: remove Spec and Revision types
On Sat Oct 25, 2025 at 2:14 AM CEST, John Hubbard wrote:> In the fullness of time, it has become clear that these two types are > not actually needed for anything, after all. Remove them both, and use > boot0.chipset directly, instead.What has become clear is that we don't use the spec field in struct Gpu so far, which is not a surprise given that all efforts focus on the initialization path. But even if we forsee that we do not have a reason to store the spec field in struct Gpu, it doesn't mean that the structure itself is useless, see more below.> /// Structure holding the resources required to operate the GPU. > #[pin_data] > pub(crate) struct Gpu { > - spec: Spec, > + chipset: Chipset, > /// MMIO mapping of PCI BAR 0 > bar: Arc<Devres<Bar0>>, > /// System memory page required for flushing all pending GPU-side memory writes done through > @@ -191,16 +153,21 @@ pub(crate) fn new<'a>( > devres_bar: Arc<Devres<Bar0>>, > bar: &'a Bar0, > ) -> impl PinInit<Self, Error> + 'a { > + let boot0 = regs::NV_PMC_BOOT_0::read(bar); > + > try_pin_init!(Self { > - spec: Spec::new(bar).inspect(|spec| { > + chipset: { > + let chipset = boot0.chipset()?; > dev_info!( > pdev.as_ref(), > - "NVIDIA (Chipset: {}, Architecture: {:?}, Revision: {})\n", > - spec.chipset, > - spec.chipset.arch(), > - spec.revision > + "NVIDIA (Chipset: {}, Architecture: {:?}, Revision: {:x}.{:x})\n", > + chipset, > + chipset.arch(), > + boot0.major_revision(), > + boot0.minor_revision()Now you have to open code reading the register, get the Chipset instance and manually format the revision, which was previously done through a Display impl of Revision. And the subsequent patch introduces even more open coded logic in the constructor of struct Gpu. Instead of removing Spec, we should improve it by giving it its own Display impl, such that this code can become: dev_info!(pdev.as_ref(), "{}\n", spec);