gpu: nova-core: gpu_name cleanup, PMU debug output cleanup

The first patch, for gpu_name, was suggested by Danilo Krummrich.

The second is from repeating looking at the dmesg output of nova-core lately. This one PMU print item stands out as having a particular poor signal to noise ratio, so deleting it.

John Hubbard (2):
gpu: nova-core: preserve error information in gpu_name()
gpu: nova-core: don't print raw PMU table entries

 drivers/gpu/nova-core/gsp/boot.rs     |  9 ++++-----
 drivers/gpu/nova-core/gsp/commands.rs | 28 +++++++++++++++++++++------
 drivers/gpu/nova-core/vbios.rs        |  5 -----
 3 files changed, 26 insertions(+), 16 deletions(-)

base-commit: 2d7b4a44fb768e1887e7e4cdd8b86817ccd9c3bf --
2.52.0

Change gpu_name() to return a Result instead of an Option. This avoids silently discarding error information when parsing the GPU name string from the GSP.

Update the callsite to log a warning with the error details on failure, rather than just displaying "invalid GPU name".

---
 drivers/gpu/nova-core/gsp/boot.rs     |  9 ++++-----
 drivers/gpu/nova-core/gsp/commands.rs | 28 +++++++++++++++++++++------
 2 files changed, 26 insertions(+), 11 deletions(-)

index a53d80620468..4a5c49a502f7 100644
@@ -238,11 +238,10 @@ pub(crate) fn boot(
 
         // Obtain and display basic GPU information.
         let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
-        dev_info!(
-            pdev.as_ref(),
-            "GPU name: {}\n",
-            info.gpu_name().unwrap_or("invalid GPU name")
-        );
+        match info.gpu_name() {
+            Ok(name) => dev_info!(pdev.as_ref(), "GPU name: {}\n", name),
+            Err(e) => dev_warn!(pdev.as_ref(), "GPU name unavailable: {:?}\n", e),
+        }
 
         Ok(())
     }

index a11fe6018091..c8430a076269 100644
@@ -2,7 +2,9 @@
 
 use core::{
     array,
-    convert::Infallible, //
+    convert::Infallible,
+    ffi::FromBytesUntilNulError,
+    str::Utf8Error, //
 };
 
 use kernel::{

@@ -204,13 +206,27 @@ fn read(
     }
 }
 
+/// Error type for [`GetGspStaticInfoReply::gpu_name`].
+#[derive(Debug)]
+pub(crate) enum GpuNameError {
+    /// The GPU name string does not contain a null terminator.
+    NoNullTerminator(FromBytesUntilNulError),
+
+    /// The GPU name string contains invalid UTF-8.
+    #[expect(dead_code)]
+    InvalidUtf8(Utf8Error),
+}
+
 impl GetGspStaticInfoReply {
-    /// Returns the name of the GPU as a string, or `None` if the string given by the GSP was
-    /// invalid.
-    pub(crate) fn gpu_name(&self) -> Option<&str> {
+    /// Returns the name of the GPU as a string.
+    ///
+    /// Returns an error if the string given by the GSP does not contain a null terminator or
+    /// contains invalid UTF-8.
+    pub(crate) fn gpu_name(&self) -> core::result::Result<&str, GpuNameError> {
         CStr::from_bytes_until_nul(&self.gpu_name)
-            .ok()
-            .and_then(|cstr| cstr.to_str().ok())
+            .map_err(GpuNameError::NoNullTerminator)?
+            .to_str()
+            .map_err(GpuNameError::InvalidUtf8)
     }
 }
 

2.52.0

Remove the (large) raw form of the PMU table entries. The resulting PMULookupTable is still getting printed (in more useful form) later, anyway, so this was redundant, even for debugging. This output (the example is from an Ampere GPU) is what is being removed:

NovaCore 0000:e1:00.0: PMU entry: [01, 01, 54, 54, 01, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00] NovaCore 0000:e1:00.0: PMU entry: [07, 06, e0, b7, 03, 00] NovaCore 0000:e1:00.0: PMU entry: [08, 01, bc, 56, 05, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00] NovaCore 0000:e1:00.0: PMU entry: [45, 07, 88, da, 01, 00] NovaCore 0000:e1:00.0: PMU entry: [85, 07, 34, c9, 02, 00] NovaCore 0000:e1:00.0: PMU entry: [49, 05, 7c, b3, 04, 00] NovaCore 0000:e1:00.0: PMU entry: [89, 05, 1c, 05, 05, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00] NovaCore 0000:e1:00.0: PMU entry: [00, 00, 00, 00, 00, 00]

And it is immediately followed by a more useful, interpreted list of selected PMU table data, which is *not* being removed as part of this commit. That looks like this:

NovaCore 0000:e1:00.0: PmuLookupTableEntry desc: FalconUCodeDescV3 {
    hdr: 78381825,
    stored_size: 59904,
    pkc_data_offset: 1444,
    interface_offset: 28,
    imem_phys_base: 0,
    imem_load_size: 57856,
    imem_virt_base: 0,
    dmem_phys_base: 0,
    dmem_load_size: 2048,
    engine_id_mask: 1024,
    ucode_id: 9,
    signature_count: 3,
    signature_versions: 7,
    _reserved: 37449,
}

---
 drivers/gpu/nova-core/vbios.rs | 5 -----
 1 file changed, 5 deletions(-)

index 7c26e4a2d61c..ac01eb195fb2 100644
@@ -887,11 +887,6 @@ fn new(dev: &device::Device, data: &[u8]) -> Result<Self> {
             ret
         };
 
-        // Debug logging of entries (dumps the table data to dmesg)
-        for i in (header_len..required_bytes).step_by(entry_len) {
-            dev_dbg!(dev, "PMU entry: {:02x?}\n", &data[i..][..entry_len]);
-        }
-
         Ok(PmuLookupTable { header, table_data })
     }
 

2.52.0

thanks,

- Joel

Applied to drm-rust-next, thanks!