Re: [PATCH v2 1/2] gpu: nova-core: merge the Revision type into the Spec type

Danilo Krummrich
2025-10-28T12:25:36Z

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