Message-Id: <20250930-nova-tryfrom-v1-1-0cc1bb507047@nvidia.com>
Date: Tue, 30 Sep 2025 13:55:00 +0900
From: Alexandre Courbot <acourbot@...dia.com>
To: Miguel Ojeda <ojeda@...nel.org>, Jesung Yang <y.j3ms.n@...il.com>, 
 Danilo Krummrich <dakr@...nel.org>, David Airlie <airlied@...il.com>, 
 Simona Vetter <simona@...ll.ch>, 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>
Cc: rust-for-linux@...r.kernel.org, nouveau@...ts.freedesktop.org, 
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
 Alexandre Courbot <acourbot@...dia.com>
Subject: [PATCH RFC] gpu: nova-core: use the TryFrom/Into derive macros

Use the new Tryfrom and Into derive macros to replace the boilerplate
code used for conversion between register fields types and the primitive
type they are represented in.

This removes a lot of boilerplate, with the only exception being fields
encoded as booleans which still need manual implementations as there is
no `#[repr(bool)]`.

Cc: Jesung Yang <y.j3ms.n@...il.com>
Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
---
As requested on [1], this is how these macros are used in Nova-core.
Note that it requires a tiny fix to the register! macro itself - I
haven't included it because things are still moving and I want to base
the fix on the state of -rc1. In any case, the content of this patch
will be identical.

The only case not covered by the macros is the one of types that can be
converted from/to booleans - but I think this one will be difficult to
handle so we probably need to rely on manual implementations until Rust
supports `#[repr(bool)]` or something.

[1] https://lore.kernel.org/rust-for-linux/CANiq72na_d6JQdyZ1S22mus3oom_jz93rpY+ubr4yOuvMY_fSA@mail.gmail.com/
---
 Documentation/gpu/nova/core/todo.rst |  21 -----
 drivers/gpu/nova-core/falcon.rs      | 151 ++++++-----------------------------
 drivers/gpu/nova-core/gpu.rs         |  33 ++------
 3 files changed, 31 insertions(+), 174 deletions(-)

diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst
index 091a2fb78c6b53037e902f015f504b8819281860..5d6caede2cd5ab9e5e3923770a7cb974b5826670 100644
--- a/Documentation/gpu/nova/core/todo.rst
+++ b/Documentation/gpu/nova/core/todo.rst
@@ -23,27 +23,6 @@ Enablement (Rust)
 Tasks that are not directly related to nova-core, but are preconditions in terms
 of required APIs.
 
-FromPrimitive API [FPRI]
-------------------------
-
-Sometimes the need arises to convert a number to a value of an enum or a
-structure.
-
-A good example from nova-core would be the ``Chipset`` enum type, which defines
-the value ``AD102``. When probing the GPU the value ``0x192`` can be read from a
-certain register indication the chipset AD102. Hence, the enum value ``AD102``
-should be derived from the number ``0x192``. Currently, nova-core uses a custom
-implementation (``Chipset::from_u32`` for this.
-
-Instead, it would be desirable to have something like the ``FromPrimitive``
-trait [1] from the num crate.
-
-Having this generalization also helps with implementing a generic macro that
-automatically generates the corresponding mappings between a value and a number.
-
-| Complexity: Beginner
-| Link: https://docs.rs/num/latest/num/trait.FromPrimitive.html
-
 Conversion from byte slices for types implementing FromBytes [TRSM]
 -------------------------------------------------------------------
 
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 37e6298195e49a9a29e81226abe16cd410c9adbc..fdc8e512c3f3d4b426d708de17dec0ca522e40bc 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -6,6 +6,7 @@
 use hal::FalconHal;
 use kernel::device;
 use kernel::dma::DmaAddress;
+use kernel::macros::{Into, TryFrom};
 use kernel::prelude::*;
 use kernel::sync::aref::ARef;
 use kernel::time::Delta;
@@ -21,21 +22,10 @@
 mod hal;
 pub(crate) mod sec2;
 
-// TODO[FPRI]: Replace with `ToPrimitive`.
-macro_rules! impl_from_enum_to_u32 {
-    ($enum_type:ty) => {
-        impl From<$enum_type> for u32 {
-            fn from(value: $enum_type) -> Self {
-                value as u32
-            }
-        }
-    };
-}
-
 /// Revision number of a falcon core, used in the [`crate::regs::NV_PFALCON_FALCON_HWCFG1`]
 /// register.
 #[repr(u8)]
-#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, TryFrom, Into)]
 pub(crate) enum FalconCoreRev {
     #[default]
     Rev1 = 1,
@@ -46,34 +36,11 @@ pub(crate) enum FalconCoreRev {
     Rev6 = 6,
     Rev7 = 7,
 }
-impl_from_enum_to_u32!(FalconCoreRev);
-
-// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconCoreRev {
-    type Error = Error;
-
-    fn try_from(value: u8) -> Result<Self> {
-        use FalconCoreRev::*;
-
-        let rev = match value {
-            1 => Rev1,
-            2 => Rev2,
-            3 => Rev3,
-            4 => Rev4,
-            5 => Rev5,
-            6 => Rev6,
-            7 => Rev7,
-            _ => return Err(EINVAL),
-        };
-
-        Ok(rev)
-    }
-}
 
 /// Revision subversion number of a falcon core, used in the
 /// [`crate::regs::NV_PFALCON_FALCON_HWCFG1`] register.
 #[repr(u8)]
-#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, TryFrom, Into)]
 pub(crate) enum FalconCoreRevSubversion {
     #[default]
     Subversion0 = 0,
@@ -81,31 +48,11 @@ pub(crate) enum FalconCoreRevSubversion {
     Subversion2 = 2,
     Subversion3 = 3,
 }
-impl_from_enum_to_u32!(FalconCoreRevSubversion);
-
-// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconCoreRevSubversion {
-    type Error = Error;
-
-    fn try_from(value: u8) -> Result<Self> {
-        use FalconCoreRevSubversion::*;
-
-        let sub_version = match value & 0b11 {
-            0 => Subversion0,
-            1 => Subversion1,
-            2 => Subversion2,
-            3 => Subversion3,
-            _ => return Err(EINVAL),
-        };
-
-        Ok(sub_version)
-    }
-}
 
 /// Security model of a falcon core, used in the [`crate::regs::NV_PFALCON_FALCON_HWCFG1`]
 /// register.
 #[repr(u8)]
-#[derive(Debug, Default, Copy, Clone)]
+#[derive(Debug, Default, Copy, Clone, TryFrom, Into)]
 /// Security mode of the Falcon microprocessor.
 ///
 /// See `falcon.rst` for more details.
@@ -125,73 +72,27 @@ pub(crate) enum FalconSecurityModel {
     /// Also known as High-Secure, Privilege Level 3 or PL3.
     Heavy = 3,
 }
-impl_from_enum_to_u32!(FalconSecurityModel);
-
-// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconSecurityModel {
-    type Error = Error;
-
-    fn try_from(value: u8) -> Result<Self> {
-        use FalconSecurityModel::*;
-
-        let sec_model = match value {
-            0 => None,
-            2 => Light,
-            3 => Heavy,
-            _ => return Err(EINVAL),
-        };
-
-        Ok(sec_model)
-    }
-}
 
 /// Signing algorithm for a given firmware, used in the [`crate::regs::NV_PFALCON2_FALCON_MOD_SEL`]
 /// register. It is passed to the Falcon Boot ROM (BROM) as a parameter.
 #[repr(u8)]
-#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)]
+#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, TryFrom, Into)]
 pub(crate) enum FalconModSelAlgo {
     /// AES.
-    #[expect(dead_code)]
     Aes = 0,
     /// RSA3K.
     #[default]
     Rsa3k = 1,
 }
-impl_from_enum_to_u32!(FalconModSelAlgo);
-
-// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconModSelAlgo {
-    type Error = Error;
-
-    fn try_from(value: u8) -> Result<Self> {
-        match value {
-            1 => Ok(FalconModSelAlgo::Rsa3k),
-            _ => Err(EINVAL),
-        }
-    }
-}
 
 /// Valid values for the `size` field of the [`crate::regs::NV_PFALCON_FALCON_DMATRFCMD`] register.
 #[repr(u8)]
-#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)]
+#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, TryFrom, Into)]
 pub(crate) enum DmaTrfCmdSize {
     /// 256 bytes transfer.
     #[default]
     Size256B = 0x6,
 }
-impl_from_enum_to_u32!(DmaTrfCmdSize);
-
-// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for DmaTrfCmdSize {
-    type Error = Error;
-
-    fn try_from(value: u8) -> Result<Self> {
-        match value {
-            0x6 => Ok(Self::Size256B),
-            _ => Err(EINVAL),
-        }
-    }
-}
 
 /// Currently active core on a dual falcon/riscv (Peregrine) controller.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
@@ -202,7 +103,15 @@ pub(crate) enum PeregrineCoreSelect {
     /// RISC-V core is active.
     Riscv = 1,
 }
-impl_from_enum_to_u32!(PeregrineCoreSelect);
+
+impl From<PeregrineCoreSelect> for bool {
+    fn from(value: PeregrineCoreSelect) -> Self {
+        match value {
+            PeregrineCoreSelect::Falcon => false,
+            PeregrineCoreSelect::Riscv => true,
+        }
+    }
+}
 
 impl From<bool> for PeregrineCoreSelect {
     fn from(value: bool) -> Self {
@@ -225,7 +134,8 @@ pub(crate) enum FalconMem {
 /// Defines the Framebuffer Interface (FBIF) aperture type.
 /// This determines the memory type for external memory access during a DMA transfer, which is
 /// performed by the Falcon's Framebuffer DMA (FBDMA) engine. See falcon.rst for more details.
-#[derive(Debug, Clone, Default)]
+#[repr(u8)]
+#[derive(Debug, Clone, Default, TryFrom, Into)]
 pub(crate) enum FalconFbifTarget {
     /// VRAM.
     #[default]
@@ -236,23 +146,6 @@ pub(crate) enum FalconFbifTarget {
     /// Non-coherent system memory (System DRAM).
     NoncoherentSysmem = 2,
 }
-impl_from_enum_to_u32!(FalconFbifTarget);
-
-// TODO[FPRI]: replace with `FromPrimitive`.
-impl TryFrom<u8> for FalconFbifTarget {
-    type Error = Error;
-
-    fn try_from(value: u8) -> Result<Self> {
-        let res = match value {
-            0 => Self::LocalFb,
-            1 => Self::CoherentSysmem,
-            2 => Self::NoncoherentSysmem,
-            _ => return Err(EINVAL),
-        };
-
-        Ok(res)
-    }
-}
 
 /// Type of memory addresses to use.
 #[derive(Debug, Clone, Default)]
@@ -263,7 +156,15 @@ pub(crate) enum FalconFbifMemType {
     /// Physical memory addresses.
     Physical = 1,
 }
-impl_from_enum_to_u32!(FalconFbifMemType);
+
+impl From<FalconFbifMemType> for bool {
+    fn from(value: FalconFbifMemType) -> Self {
+        match value {
+            FalconFbifMemType::Virtual => false,
+            FalconFbifMemType::Physical => true,
+        }
+    }
+}
 
 /// Conversion from a single-bit register field.
 impl From<bool> for FalconFbifMemType {
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 5da9ad72648340ed988184737219b14771f31b7a..4d90c821c5954fe4cf9b1562d853fddf5cedbce3 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 
-use kernel::{device, devres::Devres, error::code::*, pci, prelude::*, sync::Arc};
+use kernel::{device, devres::Devres, pci, prelude::*, sync::Arc};
 
 use crate::driver::Bar0;
 use crate::falcon::{gsp::Gsp as GspFalcon, sec2::Sec2 as Sec2Falcon, Falcon};
@@ -14,7 +14,8 @@ macro_rules! define_chipset {
     ({ $($variant:ident = $value:expr),* $(,)* }) =>
     {
         /// Enum representation of the GPU chipset.
-        #[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
+        #[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq, kernel::macros::TryFrom)]
+        #[try_from(u32)]
         pub(crate) enum Chipset {
             $($variant = $value),*,
         }
@@ -42,18 +43,6 @@ pub(crate) const fn name(&self) -> &'static str {
             }
             );
         }
-
-        // TODO[FPRI]: replace with something like derive(FromPrimitive)
-        impl TryFrom<u32> for Chipset {
-            type Error = kernel::error::Error;
-
-            fn try_from(value: u32) -> Result<Self, Self::Error> {
-                match value {
-                    $( $value => Ok(Chipset::$variant), )*
-                    _ => Err(ENODEV),
-                }
-            }
-        }
     }
 }
 
@@ -110,26 +99,14 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 }
 
 /// Enum representation of the GPU generation.
-#[derive(fmt::Debug)]
+#[derive(fmt::Debug, kernel::macros::TryFrom)]
+#[try_from(u8)]
 pub(crate) enum Architecture {
     Turing = 0x16,
     Ampere = 0x17,
     Ada = 0x19,
 }
 
-impl TryFrom<u8> for Architecture {
-    type Error = Error;
-
-    fn try_from(value: u8) -> Result<Self> {
-        match value {
-            0x16 => Ok(Self::Turing),
-            0x17 => Ok(Self::Ampere),
-            0x19 => Ok(Self::Ada),
-            _ => Err(ENODEV),
-        }
-    }
-}
-
 pub(crate) struct Revision {
     major: u8,
     minor: u8,

---
base-commit: 299eb32863e584cfff7c6b667c3e92ae7d4d2bf9
change-id: 20250930-nova-tryfrom-a2cfa169dfd2

Best regards,
-- 
Alexandre Courbot <acourbot@...dia.com>

Powered by blists - more mailing lists