Skip to contentSkip to conversation

gpu: nova: Export parameters from nova-core to nova-drm

PATCH Superseded Alistair Popple 3 participants 10 messages Jul 9

Subsystems (6)
DRM drivers DRM misc Rust DRM Rust +2 more subsystems

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover gpu: nova: Export parameters from nova-core to nova-drm

This patch series adds some basic GPU properties to the existing nova-drm GETPARAM ioctl. It builds on top of the "drm: Higher-Ranked Lifetime private data" series[1] to correctly manage lifetimes of registration data shared between DRM, auxbus and nova-core. It's also based on top of "ForLt/CovariantForLt split, auxiliary closure API and DevresLt"[2] although the functionality of that series isn't actually required.

A tree with this series applied on top of all pre-requisites is available at [3].

Properties are exported via a new NovaCoreApi type which implements methods to read data from the GPU. This has been implemented in a separate module to make the public API implementations obvious and to keep them in one place. Auxiliary bus drivers can obtain a handle to this type using NovaCoreApi::of().

This handle can then be stored as part of the DRM registration data and used to interact with the GPU via the nova-core driver.

Changes since v1:

- Address review comments from Danilo
- Add an API call to read VRAM PCI BAR size using nova-core

[1] - https://lore.kernel.org/rust-for-linux/[email protected]/
[2] - https://lore.kernel.org/driver-core/[email protected]/
[3] - https://github.com/apopple-nvidia/linux/tree/nova-drm

Cc Danilo Krummrich <[email protected]>
Cc Alice Ryhl <[email protected]>
Cc David Airlie <[email protected]>
Cc Alexandre Courbot <[email protected]>
Cc Benno Lossin <[email protected]>
Cc Gary Guo <[email protected]>
Cc Eliot Courtney <[email protected]>
Cc John Hubbard <[email protected]>

Alistair Popple (5):
gpu: nova-core: Add public driver API to nova-core
gpu: nova: Add DRM registration data
drm: nova: Add GETPARAM parameter to read the GPU chipset
drm: nova: Add a GETPARAM parameter to read usable VRAM size
drm: nova: Use nova-core to read VRAM_BAR_SIZE parameter

 drivers/gpu/drm/nova/driver.rs        | 15 +++++-
 drivers/gpu/drm/nova/file.rs          | 24 ++++-----
 drivers/gpu/nova-core/api.rs          | 49 ++++++++++++++++++
 drivers/gpu/nova-core/driver.rs       | 42 +++++++++++-----
 drivers/gpu/nova-core/gpu.rs          | 71 +++++++++++++--------------
 drivers/gpu/nova-core/gsp/commands.rs |  8 +++
 drivers/gpu/nova-core/gsp/hal.rs      |  2 +-
 drivers/gpu/nova-core/nova_core.rs    |  1 +
 include/uapi/drm/nova_drm.h           | 47 ++++++++++++++++++
 9 files changed, 197 insertions(+), 62 deletions(-)
 create mode 100644 drivers/gpu/nova-core/api.rs
Signature · 1 line
2.54.0
#1 lore raw

PATCH v2 1/5 gpu: nova-core: Add public driver API to nova-core

Nova core will be used to export core functionality to other drivers which will bind to it via auxiliary bus devices. Add a NovaCoreApi type which drivers can use to call nova-core methods.

Auxiliary bus drivers can obtain a handle to call nova-core methods on a particular GPU using NovaCoreApi::of(). This takes a reference to a bound auxiliary bus device and returns a handle to NovaCoreApi.

---

Changes since v1:

- Rework unsafe pin-init to make safety comments clearer, suggested by Danilo.
- s/allow(dead_code)/expect(unused)/

---
 drivers/gpu/nova-core/api.rs       | 29 +++++++++++++++++++++
 drivers/gpu/nova-core/driver.rs    | 42 ++++++++++++++++++++++--------
 drivers/gpu/nova-core/gsp/hal.rs   |  2 +-
 drivers/gpu/nova-core/nova_core.rs |  1 +
 4 files changed, 62 insertions(+), 12 deletions(-)
 create mode 100644 drivers/gpu/nova-core/api.rs
drivers/gpu/nova-core/api.rs new file +29 -0
new file mode 100644
index 000000000000..610cfc01111e
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Nova-core auxbus data. Contains all the methods used by the auxbus drivers
+//! to interact with nova-core.
+
+use core::pin::Pin;
+
+use kernel::{
+    auxiliary,
+    device::Bound,
+    prelude::*,
+    types::CovariantForLt, //
+};
+
+use crate::gpu::Gpu;
+
+/// API handle for the auxiliary bus child drivers to interact with nova-core.
+pub struct NovaCoreApi<'bound> {
+    #[expect(unused)]
+    pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
+}
+
+impl NovaCoreApi<'_> {
+    /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
+    /// by nova-core.
+    pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
+        adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
+    }
+}
drivers/gpu/nova-core/driver.rs +31 -11

63 lines not on this page · raw message

drivers/gpu/nova-core/gsp/hal.rs +1 -1
index d3e47ef206de..06647b2b7894 100644
@@ -36,7 +36,7 @@
 /// The GSP unload code might run in a situation where we cannot load firmware dynamically (e.g.
 /// because we are in shutdown and the file system is not accessible anymore). Thus, the firmware
 /// required for unloading is prepared at load time, and stored here until it needs to be run.
-pub(super) trait UnloadBundle: Send {
+pub(super) trait UnloadBundle: Send + Sync {
     /// Performs the steps required to properly reset the GSP after it has been stopped.
     fn run(
         &self,
drivers/gpu/nova-core/nova_core.rs +1 -0
index 735b8e17c6b6..2d48d16a6ee8 100644
@@ -13,6 +13,7 @@
 #[macro_use]
 mod bitfield;
 
+pub mod api;
 mod driver;
 mod falcon;
 mod fb;
Signature · 1 line
2.54.0
#2 lore raw

Re: [PATCH v2 1/5] gpu: nova-core: Add public driver API to nova-core

On Thu Jul 9, 2026 at 5:52 PM JST, Alistair Popple wrote:
<...>

Quoted 12 lines
drivers/gpu/nova-core/driver.rs +0 -8
@@ -86,15 +90,31 @@ fn probe<'bound>(
                 // (`try_pin_init!()` initializes fields in declaration order), lives at a pinned
                 // stable address, and is dropped after `gpu` (struct field drop order).
                 gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }),
-                _reg: auxiliary::Registration::new(
-                    pdev.as_ref(),
-                    c"nova-drm",
-                    // TODO[XARR]: Use XArray or perhaps IDA for proper ID allocation/recycling. For
-                    // now, use a simple atomic counter that never recycles IDs.
-                    AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
-                    crate::MODULE_NAME,
-                    (),

drivers/gpu/nova-core/driver.rs fn probe<'bound>( · line 101

Quoted 10 lines
drivers/gpu/nova-core/driver.rs +9 -1
-                )?,
+
+                _reg: {
+                    // SAFETY: `gpu` is initialized before this expression is evaluated
+                    // (`try_pin_init!()` initializes fields in declaration order), lives at
+                    // a pinned stable address, and is dropped after `_reg` (struct field
+                    // drop order).
+                    let gpu = unsafe {
+                        Pin::new_unchecked(&*core::ptr::from_ref(gpu.as_ref().get_ref()))
+                    };

Your v1 had a

    // TODO: Use `&gpu` self-referential pin-init syntax once available.

Can we preserve it?

#3 lore raw

PATCH v2 2/5 gpu: nova: Add DRM registration data

In reply to Alistair Popple

Currently the nova-drm stub driver doesn't really interact with a real device, so it doesn't have any registration data and has no way to call into nova-core.

To allow for this add a DrmRegData type which will hold a reference to the NovaCoreApi associated with the auxbus device.

---

Changes from v1:

- Drop unrelated device::Bound removal.
- Switch exisiting import to vertical style.
- s/allow(dead_code)/expect(unused)/

---
 drivers/gpu/drm/nova/driver.rs | 16 ++++++++++++++--
 drivers/gpu/drm/nova/file.rs   | 12 ++++++++----
 2 files changed, 22 insertions(+), 6 deletions(-)
drivers/gpu/drm/nova/driver.rs +14 -2
index 739690bc2db5..0204d1cee6a5 100644
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
+use core::pin::Pin;
+
 use kernel::{
     auxiliary,
     device::{
@@ -17,6 +19,7 @@
 
 use crate::file::File;
 use crate::gem::NovaObject;
+use nova_core::api::NovaCoreApi;
 
 pub(crate) struct NovaDriver;
 
@@ -26,6 +29,12 @@ pub(crate) struct Nova<'bound> {
     _reg: drm::Registration<'bound, NovaDriver>,
 }
 
+/// DRM registration data, accessible from ioctl handlers via the registration guard.
+pub(crate) struct DrmRegData<'bound> {
+    #[expect(unused)]
+    pub(crate) api: Pin<&'bound NovaCoreApi<'bound>>,
+}
+
 /// Convienence type alias for the DRM device type for this driver
 pub(crate) type NovaDevice<Ctx = drm::Normal> = drm::Device<NovaDriver, Ctx>;
 
@@ -60,9 +69,12 @@ fn probe<'bound>(
         _info: &'bound Self::IdInfo,
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let drm = drm::UnregisteredDevice::<Self>::new(adev, Ok(()))?;
+        let reg_data = DrmRegData {
+            api: NovaCoreApi::of(adev)?,
+        };
         // SAFETY: `reg` is stored in `Nova` and dropped when the driver is unbound; it is
         // never forgotten.
-        let reg = unsafe { drm::Registration::new(adev.as_ref(), drm, (), 0)? };
+        let reg = unsafe { drm::Registration::new(adev.as_ref(), drm, reg_data, 0)? };
 
         Ok(Nova {
             drm: reg.device().into(),
@@ -74,7 +86,7 @@ fn probe<'bound>(
 #[vtable]
 impl drm::Driver for NovaDriver {
     type Data = ();
-    type RegistrationData<'a> = ();
+    type RegistrationData<'a> = DrmRegData<'a>;
     type File = File;
     type Object = gem::Object<NovaObject>;
     type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
drivers/gpu/drm/nova/file.rs +8 -4
index 298c02bacb4b..1156df51c533 100644
@@ -1,6 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 
-use crate::driver::{NovaDevice, NovaDriver};
+use crate::driver::{
+    DrmRegData,
+    NovaDevice,
+    NovaDriver, //
+};
 use crate::gem::NovaObject;
 use kernel::{
     alloc::flags::*,
@@ -30,7 +34,7 @@ impl File {
     /// IOCTL: get_param: Query GPU / driver metadata.
     pub(crate) fn get_param(
         dev: &NovaDevice<Registered>,
-        _reg_data: &(),
+        _reg_data: &DrmRegData<'_>,
         getparam: &mut uapi::drm_nova_getparam,
         _file: &drm::File<File>,
     ) -> Result<u32> {
@@ -50,7 +54,7 @@ pub(crate) fn get_param(
     /// IOCTL: gem_create: Create a new DRM GEM object.
     pub(crate) fn gem_create(
         dev: &NovaDevice<Registered>,
-        _reg_data: &(),
+        _reg_data: &DrmRegData<'_>,
         req: &mut uapi::drm_nova_gem_create,
         file: &drm::File<File>,
     ) -> Result<u32> {
@@ -64,7 +68,7 @@ pub(crate) fn gem_create(
     /// IOCTL: gem_info: Query GEM metadata.
     pub(crate) fn gem_info(
         _dev: &NovaDevice<Registered>,
-        _reg_data: &(),
+        _reg_data: &DrmRegData<'_>,
         req: &mut uapi::drm_nova_gem_info,
         file: &drm::File<File>,
     ) -> Result<u32> {
Signature · 1 line
2.54.0
#4 lore raw

Re: [PATCH v2 2/5] gpu: nova: Add DRM registration data

Title prefix should be "drm: nova:" I guess?

Quoted 35 lines · Alistair Popple

Currently the nova-drm stub driver doesn't really interact with a real device, so it doesn't have any registration data and has no way to call into nova-core.

To allow for this add a DrmRegData type which will hold a reference to the NovaCoreApi associated with the auxbus device.

---

Changes from v1:

- Drop unrelated device::Bound removal.
- Switch exisiting import to vertical style.
- s/allow(dead_code)/expect(unused)/

---
 drivers/gpu/drm/nova/driver.rs | 16 ++++++++++++++--
 drivers/gpu/drm/nova/file.rs   | 12 ++++++++----
 2 files changed, 22 insertions(+), 6 deletions(-)
drivers/gpu/drm/nova/driver.rs +2 -0
index 739690bc2db5..0204d1cee6a5 100644
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
+use core::pin::Pin;
+
 use kernel::{
     auxiliary,
     device::{

drivers/gpu/drm/nova/driver.rs line 22

Quoted 4 lines
drivers/gpu/drm/nova/driver.rs +1 -0
@@ -17,6 +19,7 @@
 
 use crate::file::File;
 use crate::gem::NovaObject;
+use nova_core::api::NovaCoreApi;

nit: Let's add a blank line between the crate and nova_core imports.

#5 lore raw

PATCH v2 3/5 drm: nova: Add GETPARAM parameter to read the GPU chipset

In reply to Alistair Popple

One of the first things a user needs to know about a GPU is its chipset, so add an ioctl to return that.

---

Changes since v1:

- Vertical import
- Use a single enum for chipset values

---
 drivers/gpu/drm/nova/driver.rs |  1 -
 drivers/gpu/drm/nova/file.rs   |  3 +-
 drivers/gpu/nova-core/api.rs   | 11 ++++--
 drivers/gpu/nova-core/gpu.rs   | 65 +++++++++++++++++-----------------
 include/uapi/drm/nova_drm.h    | 40 +++++++++++++++++++++
 5 files changed, 84 insertions(+), 36 deletions(-)
drivers/gpu/drm/nova/driver.rs +0 -1
index 0204d1cee6a5..42f8845ed6b4 100644
@@ -31,7 +31,6 @@ pub(crate) struct Nova<'bound> {
 
 /// DRM registration data, accessible from ioctl handlers via the registration guard.
 pub(crate) struct DrmRegData<'bound> {
-    #[expect(unused)]
     pub(crate) api: Pin<&'bound NovaCoreApi<'bound>>,
 }
 
drivers/gpu/drm/nova/file.rs +2 -1
index 1156df51c533..1e5364c619ed 100644
@@ -34,7 +34,7 @@ impl File {
     /// IOCTL: get_param: Query GPU / driver metadata.
     pub(crate) fn get_param(
         dev: &NovaDevice<Registered>,
-        _reg_data: &DrmRegData<'_>,
+        reg_data: &DrmRegData<'_>,
         getparam: &mut uapi::drm_nova_getparam,
         _file: &drm::File<File>,
     ) -> Result<u32> {
@@ -43,6 +43,7 @@ pub(crate) fn get_param(
 
         let value = match getparam.param as u32 {
             uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
+            uapi::NOVA_GETPARAM_GPU_CHIPSET => reg_data.api.chipset() as u64,
             _ => return Err(EINVAL),
         };
 
drivers/gpu/nova-core/api.rs +9 -2
index 610cfc01111e..09da0b4e9103 100644
@@ -12,11 +12,13 @@
     types::CovariantForLt, //
 };
 
-use crate::gpu::Gpu;
+use crate::gpu::{
+    Chipset,
+    Gpu, //
+};
 
 /// API handle for the auxiliary bus child drivers to interact with nova-core.
 pub struct NovaCoreApi<'bound> {
-    #[expect(unused)]
     pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
 }
 
@@ -26,4 +28,9 @@ impl NovaCoreApi<'_> {
     pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
         adev.registration_data::<CovariantForLt!(NovaCoreApi<'_>)>()
     }
+
+    /// Returns the chipset of this GPU.
+    pub fn chipset(&self) -> Chipset {
+        self.gpu.spec.chipset
+    }
 }
drivers/gpu/nova-core/gpu.rs +33 -32

112 lines not on this page · raw message

include/uapi/drm/nova_drm.h +40 -0
index 3ca90ed9d2bb..0382a4a70640 100644
@@ -25,6 +25,46 @@ extern "C" {
  */
 #define NOVA_GETPARAM_VRAM_BAR_SIZE	0x1
 
+/*
+ * NOVA_GETPARAM_GPU_CHIPSET
+ *
+ * Query the GPU chipset architecture/implementation.
+ */
+#define NOVA_GETPARAM_GPU_CHIPSET	0x2
+
+enum drm_nova_chipset {
+	/* Turing */
+	NOVA_DRM_CHIPSET_TU102 = 0x162,
+	NOVA_DRM_CHIPSET_TU104 = 0x164,
+	NOVA_DRM_CHIPSET_TU106 = 0x166,
+	NOVA_DRM_CHIPSET_TU117 = 0x167,
+	NOVA_DRM_CHIPSET_TU116 = 0x168,
+	/* Ampere */
+	NOVA_DRM_CHIPSET_GA100 = 0x170,
+	NOVA_DRM_CHIPSET_GA102 = 0x172,
+	NOVA_DRM_CHIPSET_GA103 = 0x173,
+	NOVA_DRM_CHIPSET_GA104 = 0x174,
+	NOVA_DRM_CHIPSET_GA106 = 0x176,
+	NOVA_DRM_CHIPSET_GA107 = 0x177,
+	/* Hopper */
+	NOVA_DRM_CHIPSET_GH100 = 0x180,
+	/* Ada */
+	NOVA_DRM_CHIPSET_AD102 = 0x192,
+	NOVA_DRM_CHIPSET_AD103 = 0x193,
+	NOVA_DRM_CHIPSET_AD104 = 0x194,
+	NOVA_DRM_CHIPSET_AD106 = 0x196,
+	NOVA_DRM_CHIPSET_AD107 = 0x197,
+	/* Blackwell GB10x */
+	NOVA_DRM_CHIPSET_GB100 = 0x1a0,
+	NOVA_DRM_CHIPSET_GB102 = 0x1a2,
+	/* Blackwell GB20x */
+	NOVA_DRM_CHIPSET_GB202 = 0x1b2,
+	NOVA_DRM_CHIPSET_GB203 = 0x1b3,
+	NOVA_DRM_CHIPSET_GB205 = 0x1b5,
+	NOVA_DRM_CHIPSET_GB206 = 0x1b6,
+	NOVA_DRM_CHIPSET_GB207 = 0x1b7,
+};
+
 /**
  * struct drm_nova_getparam - query GPU and driver metadata
  */
Signature · 1 line
2.54.0
#6 lore raw

Re: [PATCH v2 3/5] drm: nova: Add GETPARAM parameter to read the GPU chipset

Quoted 2 lines · Alistair Popple

One of the first things a user needs to know about a GPU is its chipset, so add an ioctl to return that.

We are not adding an ioctl though, just a GETPARAM parameter.

<...>

Quoted 34 lines
drivers/gpu/nova-core/gpu.rs +5 -4
index 43c3f4f8df71..1f4c5f3aa8ea 100644
@@ -10,7 +10,8 @@
     num::Bounded,
     pci,
     prelude::*,
-    sizes::SizeConstants, //
+    sizes::SizeConstants,
+    uapi, //
 };
 
 use crate::{
@@ -34,12 +35,12 @@
 mod hal;
 
 macro_rules! define_chipset {
-    ({ $($variant:ident = $value:expr),* $(,)* }) =>
+    ({ $($variant:ident = $value:path),* $(,)* }) =>
     {
         /// Enum representation of the GPU chipset.
         #[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
-        pub(crate) enum Chipset {
-            $($variant = $value),*,
+        pub enum Chipset {
+            $($variant = $value as isize),*,
         }
 
         impl Chipset {
@@ -82,39 +83,39 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
 
 define_chipset!({
     // Turing

drivers/gpu/nova-core/gpu.rs fn try_from(value: u32) -> Result<Self, Self::Error> { · line 90

Quoted 10 lines
drivers/gpu/nova-core/gpu.rs +5 -5
-    TU102 = 0x162,
-    TU104 = 0x164,
-    TU106 = 0x166,
-    TU117 = 0x167,
-    TU116 = 0x168,
+    TU102 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU102,
+    TU104 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU104,
+    TU106 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU106,
+    TU117 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU117,
+    TU116 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU116,

If we are going to remove the actual values, let's at least generate the right-hand side in the macro as it is just verbose without bringing any new information. The following diff lets you remove the = uapi::... for each chipset declaration.

The UAPI definitions come technically from nova-drm (and are defined as such), which introduces a dependency of sorts from nova-core to nova-drm. I'm not saying this is a problem, but mentioning it as we want to make that decision consciously.

I also think the introduction of UAPI variants and modification of define_chipset should be its own patch, as these are pretty mechanical.

drivers/gpu/nova-core/gpu.rs +8 -5
index 202adfbfa176..5b67f25399de 100644
@@ -35,12 +35,13 @@
 mod hal;
 
 macro_rules! define_chipset {
-    ({ $($variant:ident = $value:path),* $(,)* }) =>
+    ({ $($variant:ident),* $(,)* }) =>
     {
+        ::kernel::macros::paste!(
         /// Enum representation of the GPU chipset.
         #[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
         pub enum Chipset {
-            $($variant = $value as isize),*,
+            $($variant = uapi::[<drm_nova_chipset_NOVA_DRM_CHIPSET_ $variant:upper>] as isize),*,
         }
 
         impl Chipset {
@@ -48,7 +49,6 @@ impl Chipset {
                 $( Chipset::$variant, )*
             ];
 
-            ::kernel::macros::paste!(
             /// Returns the name of this chipset, in lowercase.
             ///
             /// # Examples
@@ -64,7 +64,6 @@ pub(crate) const fn name(&self) -> &'static str {
                 )*
                 }
             }
-            );
         }
 
         // TODO[FPRI]: replace with something like derive(FromPrimitive)
@@ -73,45 +72,49 @@ impl TryFrom<u32> for Chipset {
 
             fn try_from(value: u32) -> Result<Self, Self::Error> {
                 match value {
-                    $( $value => Ok(Chipset::$variant), )*
+                    $(
+                        uapi::[<drm_nova_chipset_NOVA_DRM_CHIPSET_ $variant:upper>]
+                            => Ok(Chipset::$variant),
+                    )*
                     _ => Err(ENODEV),
                 }
             }
         }
+        );
     }
 }
maintainer
#7 lore raw

Re: [PATCH v2 3/5] drm: nova: Add GETPARAM parameter to read the GPU chipset

Quoted 12 lines · Alexandre Courbot
Quoted 11 lines · Alistair Popple
(quoted diff) +2 -5
@@ -82,39 +83,39 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
 
 define_chipset!({
     // Turing
-    TU102 = 0x162,
-    TU104 = 0x164,
-    TU106 = 0x166,
-    TU117 = 0x167,
-    TU116 = 0x168,
+    TU102 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU102,
+    TU104 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU104,
Quoted 8 lines

Review fn try_from(value: u32) -> Result<Self, Self::Error> { · line 90

Quoted 3 lines
(quoted diff) +3 -0
+    TU106 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU106,
+    TU117 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU117,
+    TU116 = uapi::drm_nova_chipset_NOVA_DRM_CHIPSET_TU116,

If we are going to remove the actual values, let's at least generate the right-hand side in the macro as it is just verbose without bringing any new information. The following diff lets you remove the = uapi::... for each chipset declaration.

I was about to propose the same thing, the right-hand side is just noise.

Quoted 4 lines

The UAPI definitions come technically from nova-drm (and are defined as such), which introduces a dependency of sorts from nova-core to nova-drm. I'm not saying this is a problem, but mentioning it as we want to make that decision consciously.

It is the other way around, both nova-core (mostly indirectly) and nova-drm (directly) implement the uAPI contract and hence obviously depend on this contract.

The chipset value that is exposed to userspace is produced by nova-core, not by nova-drm; nova-drm just passes it through.

So, this is not a "we randomly reuse the uAPI enum" kind of situation, it is more about making nova-core commit to the contract that it has to uphold.

Quoted 3 lines

I also think the introduction of UAPI variants and modification of define_chipset should be its own patch, as these are pretty mechanical.

Yes, it should be.

Thanks,
Danilo

#8 lore raw

PATCH v2 4/5 drm: nova: Add a GETPARAM parameter to read usable VRAM size

In reply to Alistair Popple

Add a GET_PARAM ioctl to return the total usable framebuffer size. The usable framebuffer excludes GSP carveouts and other protected regions so may differ in size from BAR size and total physical VRAM.

---

Changes since v1:

- Fix rustfmt

---
 drivers/gpu/drm/nova/file.rs          | 1 +
 drivers/gpu/nova-core/api.rs          | 5 +++++
 drivers/gpu/nova-core/gpu.rs          | 6 ++----
 drivers/gpu/nova-core/gsp/commands.rs | 8 ++++++++
 include/uapi/drm/nova_drm.h           | 7 +++++++
 5 files changed, 23 insertions(+), 4 deletions(-)
drivers/gpu/drm/nova/file.rs +1 -0
index 1e5364c619ed..31aea3283474 100644
@@ -44,6 +44,7 @@ pub(crate) fn get_param(
         let value = match getparam.param as u32 {
             uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
             uapi::NOVA_GETPARAM_GPU_CHIPSET => reg_data.api.chipset() as u64,
+            uapi::NOVA_GETPARAM_VRAM_SIZE => reg_data.api.vram_size(),
             _ => return Err(EINVAL),
         };
 
drivers/gpu/nova-core/api.rs +5 -0
index 09da0b4e9103..f025df90dc5d 100644
@@ -33,4 +33,9 @@ pub fn of(adev: &auxiliary::Device<Bound>) -> Result<Pin<&NovaCoreApi<'_>>> {
     pub fn chipset(&self) -> Chipset {
         self.gpu.spec.chipset
     }
+
+    /// Returns the total usable VRAM size of this GPU in bytes.
+    pub fn vram_size(&self) -> u64 {
+        self.gpu.gsp_static_info.vram_size()
+    }
 }
drivers/gpu/nova-core/gpu.rs +2 -4
index 1f4c5f3aa8ea..202adfbfa176 100644
@@ -289,7 +289,7 @@ struct GspResources<'gpu> {
 pub(crate) struct Gpu<'gpu> {
     pub(crate) spec: Spec,
     /// Static GPU information as provided by the GSP.
-    gsp_static_info: GetGspStaticInfoReply,
+    pub(crate) gsp_static_info: GetGspStaticInfoReply,
     /// GSP and its resources.
     #[pin]
     gsp_resources: GspResources<'gpu>,
@@ -389,9 +389,7 @@ pub(crate) fn new(
                     dev_dbg!(
                         pdev,
                         "Total usable VRAM: {} MiB\n",
-                        info.usable_fb_regions.iter().fold(0u64, |res, region| res
-                            .saturating_add(region.end - region.start))
-                            / u64::SZ_1M
+                        info.vram_size() / u64::SZ_1M
                     );
                 }
 
drivers/gpu/nova-core/gsp/commands.rs +8 -0
index 86a3747cd31c..d1569acb7b92 100644
@@ -242,6 +242,14 @@ pub(crate) fn gpu_name(&self) -> core::result::Result<&str, GpuNameError> {
             .to_str()
             .map_err(GpuNameError::InvalidUtf8)
     }
+
+    /// Returns the total usable VRAM size in bytes, i.e. the summed lengths of all usable FB
+    /// regions.
+    pub(crate) fn vram_size(&self) -> u64 {
+        self.usable_fb_regions.iter().fold(0, |size, region| {
+            size.saturating_add(region.end - region.start)
+        })
+    }
 }
 
 pub(crate) use fw::commands::PowerStateLevel;
include/uapi/drm/nova_drm.h +7 -0
index 0382a4a70640..ffc34dfa1535 100644
@@ -32,6 +32,13 @@ extern "C" {
  */
 #define NOVA_GETPARAM_GPU_CHIPSET	0x2
 
+/*
+ * NOVA_GETPARAM_VRAM_SIZE
+ *
+ * Query the total usable VRAM size in bytes.
+ */
+#define NOVA_GETPARAM_VRAM_SIZE	0x3
+
 enum drm_nova_chipset {
 	/* Turing */
 	NOVA_DRM_CHIPSET_TU102 = 0x162,
Signature · 1 line
2.54.0
#9 lore raw

PATCH v2 5/5 drm: nova: Use nova-core to read VRAM_BAR_SIZE parameter

In reply to Alistair Popple

Currently nova-drm reads the VRAM BAR size directly from the PCIe device which requires trying to cast the parent device into a PCIe device. This obviously requires the parent device to actually be a PCIe bus device. Whilst that is true today it may not always be the case, and there is no reason to make this assumption now that NovaCoreApi can hold a reference to the bound PCIe device.

So convert nova-drm to using nova-core to obtain the VRAM_BAR_SIZE parameter.

---
 drivers/gpu/drm/nova/file.rs    | 10 ++--------
 drivers/gpu/nova-core/api.rs    |  8 ++++++++
 drivers/gpu/nova-core/driver.rs |  2 +-
 3 files changed, 11 insertions(+), 9 deletions(-)
drivers/gpu/drm/nova/file.rs +2 -8
index 31aea3283474..de645f9197ad 100644
@@ -8,14 +8,11 @@
 use crate::gem::NovaObject;
 use kernel::{
     alloc::flags::*,
-    auxiliary,
-    device::Bound,
     drm::{
         self,
         gem::BaseObject,
         Registered, //
     },
-    pci,
     prelude::*,
     uapi,
 };
@@ -33,16 +30,13 @@ fn open(_dev: &NovaDevice) -> Result<Pin<KBox<Self>>> {
 impl File {
     /// IOCTL: get_param: Query GPU / driver metadata.
     pub(crate) fn get_param(
-        dev: &NovaDevice<Registered>,
+        _dev: &NovaDevice<Registered>,
         reg_data: &DrmRegData<'_>,
         getparam: &mut uapi::drm_nova_getparam,
         _file: &drm::File<File>,
     ) -> Result<u32> {
-        let adev: &auxiliary::Device<Bound> = dev.as_ref();
-        let pdev: &pci::Device<Bound> = adev.parent().try_into()?;
-
         let value = match getparam.param as u32 {
-            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
+            uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => reg_data.api.vram_bar_size()?,
             uapi::NOVA_GETPARAM_GPU_CHIPSET => reg_data.api.chipset() as u64,
             uapi::NOVA_GETPARAM_VRAM_SIZE => reg_data.api.vram_size(),
             _ => return Err(EINVAL),
drivers/gpu/nova-core/api.rs +8 -0
index f025df90dc5d..d7b18e33b5f5 100644
@@ -8,6 +8,7 @@
 use kernel::{
     auxiliary,
     device::Bound,
+    pci,
     prelude::*,
     types::CovariantForLt, //
 };
@@ -20,6 +21,7 @@
 /// API handle for the auxiliary bus child drivers to interact with nova-core.
 pub struct NovaCoreApi<'bound> {
     pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
+    pub(crate) pdev: &'bound pci::Device<Bound>,
 }
 
 impl NovaCoreApi<'_> {
@@ -34,6 +36,12 @@ pub fn chipset(&self) -> Chipset {
         self.gpu.spec.chipset
     }
 
+    /// Returns the size of the PCIe BAR used for accessing VRAM, typically
+    /// BAR1.
+    pub fn vram_bar_size(&self) -> Result<u64> {
+        self.pdev.resource_len(1)
+    }
+
     /// Returns the total usable VRAM size of this GPU in bytes.
     pub fn vram_size(&self) -> u64 {
         self.gpu.gsp_static_info.vram_size()
drivers/gpu/nova-core/driver.rs +1 -1
index a5dabc84fbc6..84c713b23c53 100644
@@ -111,7 +111,7 @@ fn probe<'bound>(
                             // never recycles IDs.
                             AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
                             crate::MODULE_NAME,
-                            NovaCoreApi { gpu },
+                            NovaCoreApi { gpu, pdev },
                         )?
                     }
                 },
Signature · 1 line
2.54.0