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 <[email protected]> Signed-off-by: Alexandre Courbot <[email protected]> --- 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_...@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 <[email protected]>
