Add support for newer GPU metrics versions by superm1 · Pull Request #1961 · ROCm/rocm-systems

superm1 · 2025-11-20T20:00:58Z

This enables output on some newer APUs such as Strix Halo.

superm1 · 2025-11-20T20:01:29Z

The first 5 commits are actually this PR: #1946

Copilot

Pull Request Overview

This PR adds support for GPU metrics versions 2.0-2.4 and 3.0, enabling functionality on newer APUs such as Strix Halo. The implementation includes new data structures, metric types, and supporting functions for the expanded metrics capabilities.

Key Changes:

Added support for GPU Metrics v2.0-2.4 and v3.0 with corresponding data structures
Implemented new metric types for NPU, core temperatures, L3 cache, throttle residency, and power measurements
Extended power info structure with additional power fields (NPU, APU, dGPU, system, etc.)

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
projects/amdsmi/src/amd_smi/amd_smi.cc	Added init_max_uint_types template, expanded power info initialization for new fields
projects/amdsmi/src/amd_smi/amd_smi_utils.cc	Changed error code from API_FAILED to NOT_SUPPORTED for missing power cap
projects/amdsmi/rocm_smi/src/rocm_smi_gpu_metrics.cc	Added v2.0-v3.0 metric structures, populate functions, and translation tables
projects/amdsmi/rocm_smi/include/rocm_smi/rocm_smi_gpu_metrics.h	Defined new metric structures (v20-v30), enum values, and class declarations
projects/amdsmi/rocm_smi/include/rocm_smi/rocm_smi.h	Extended rsmi_gpu_metrics_t with v2.x and v3.0 fields
projects/amdsmi/py-interface/amdsmi_interface.py	Updated power info dict with new fields and improved max value checking
projects/amdsmi/py-interface/amdsmi_wrapper.py	Updated Python bindings for new power info structure fields
projects/amdsmi/amdsmi_cli/amdsmi_logger.py	Improved power usage formatting to handle zero power limits
projects/amdsmi/example/amd_smi_drm_example.cc	Fixed printf format specifier from %ld to %d

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull Request Overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

superm1 · 2025-11-20T23:20:39Z

OK, I've got the right data feeding in. Now I just need to fix the policy for showing GTT vs VRAM to be accurate for an APU like Strix Halo.

❯ amd-smi
+------------------------------------------------------------------------------+
| AMD-SMI 26.1.0+8845b0df64... amdgpu version: Linuxver ROCm version: N/A      |
| VBIOS version: 023.011.000.039.000001                                        |
| Platform: Linux Baremetal                                                    |
|-------------------------------------+----------------------------------------|
| BDF                        GPU-Name | Mem-Uti   Temp   UEC       Power-Usage |
| GPU  HIP-ID  OAM-ID  Partition-Mode | GFX-Uti    Fan               Mem-Usage |
|=====================================+========================================|
| 0000:c2:00.0  Radeon 8050S Graphics | N/A      26 °C   0                 8 W |
|   0       0     N/A             N/A | 0 %        N/A            1536/4096 MB |
+-------------------------------------+----------------------------------------+
+------------------------------------------------------------------------------+
| Processes:                                                                   |
|  GPU        PID  Process Name          GTT_MEM  VRAM_MEM  MEM_USAGE     CU % |
|==============================================================================|
|  No running processes found                                                  |
+------------------------------------------------------------------------------+
❯ amd-smi metric -p
GPU: 0
    POWER:
        SOCKET_POWER: 9 W
        NPU_POWER: 0 W
        APU_POWER: 9 W
        GFX_POWER: 0 W
        DGPU_POWER: 0 W
        ALL_CORE_POWER: 1 W
        SYSTEM_POWER: 63 W
        GFX_VOLTAGE: N/A
        SOC_VOLTAGE: N/A
        MEM_VOLTAGE: N/A
        THROTTLE_STATUS: N/A
        POWER_MANAGEMENT: N/A

❯ amd-smi monitor
GPU  XCP  POWER   GPU_T   MEM_T   GFX_CLK   GFX%   MEM%   ENC%   DEC%      VRAM_USAGE
  0    0    8 W     N/A     N/A       N/A    0 %    N/A    N/A    N/A    1.5/  4.0 GB

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

oliveiradan · 2025-12-18T04:24:15Z

+    uint32_t dGPU_power;            //!< Average dGPU power in mW, A+A only */
+    uint32_t all_core_power;        //!< Average power of all cores in mW, APU only */
+    uint32_t system_power;          //!< Average system power in mW, APU only */
+    uint64_t reserved[20];


Just as a note:
The size of this struct is now 232 bytes (where the expected would be 228) , with an alignment of 8. But due to potential padding, sizes may differ:

Different default packing rules between C and C++ compilers

Different ABI (Application Binary Interface) requirements

Different struct packing on 32-bit vs 64-bit systems

For that (reserved[]) to really make a difference, we would need to force 1-byte packing, no padding and still static assert size checking, for example: ((14 member fields + 20 reserved) * alignment) + versioning.

As we don't do that, it will not help much due to our C header and C++ implementation.

FWIW I was running into data that didn't make sense and it turned out to be caused by padding sizes which is how I got here. Are you suggesting totally dropping the uint64_t reserved[20]??

oliveiradan · 2025-12-18T04:37:04Z

@@ -154,6 +153,12 @@ const AMDGpuMetricVersionTranslationTbl_t amdgpu_metric_version_translation_tabl
  {join_metrics_version(1, 7), AMDGpuMetricVersionFlags_t::kGpuMetricV17},
  {join_metrics_version(1, 8), AMDGpuMetricVersionFlags_t::kGpuMetricV18},
  {join_metrics_version(1, 9), AMDGpuMetricVersionFlags_t::kGpuMetricDynV19Plus},  // Dynamic GPU Metrics
+  {join_metrics_version(2, 0), AMDGpuMetricVersionFlags_t::kGpuMetricV20},


I don't think metrics 2.0 and newer are using the "dynamic metrics" format yet. We will need to double-check on this as we consider anything with gpu metrics >= 1.9 or xcp metrics >= 1.1 to be dynamic metrics, which, if that is not the case, will break the readings.

Can you explain what dynamic metrics actually means? I'm guessing it's structs that are not fixed and can have variable sizes. And thus yes - these are fixed size structs and not dynamic so there will be some places that could break I'll have to look for.

oliveiradan

This is a large set of changes, and some things need to be double and triple checked.
Where are these metric versions (2.0...3.0) coming from? Are they using dynamic metrics format?
Which system has gpu_metric files which these versions in the header?
If all that checks, we need to review a few more points, and then:

Update unit tests for gpu metrics, and partition metrics
Update examples
Make sure Golang and Rust shims/bindings will work properly
Then check for changes:

py-interface/amdsmi_interface.py
py-interface/init.py
amdsmi_cli/amdsmi_parser.py
py-interface/README.md
docs/how-to/using-amdsmi-for-python.md
py-interface/amdsmi_wrapper.py
./CHANGELOG.md

Let's chat about the initial questions, before we get to #4.

superm1 · 2025-12-18T05:13:32Z

Where are these metric versions (2.0...3.0) coming from?

The structs come from amdgpu headers.

Are they using dynamic metrics format?

I don't believe so.

Which system has gpu_metric files which these versions in the header?

3.0 is Strix/Strix Halo.
2.x Is Phoenix/Hawk Point

Update unit tests for gpu metrics, and partition metrics
Update examples
Make sure Golang and Rust shims/bindings will work properly

OK, will hold off on getting started for any of this until we're clear on the other review points.

…lback

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>

superm1 · 2026-03-31T19:08:18Z

re-opened as #4596

Copilot AI review requested due to automatic review settings November 20, 2025 20:00

github-actions Bot added the project: amdsmi label Nov 20, 2025

Copilot started reviewing on behalf of superm1 November 20, 2025 20:01

View session

Copilot finished reviewing on behalf of superm1 November 20, 2025 20:03

Copilot AI reviewed Nov 20, 2025

View reviewed changes

systems-assistant Bot added the external contribution label Nov 20, 2025

superm1 force-pushed the apu-support branch 2 times, most recently from 4d1ecc0 to edda5ff Compare November 20, 2025 21:10

superm1 requested a review from Copilot November 20, 2025 21:11

Copilot started reviewing on behalf of superm1 November 20, 2025 21:12

View session

Copilot finished reviewing on behalf of superm1 November 20, 2025 21:12

Copilot AI reviewed Nov 20, 2025

View reviewed changes

superm1 force-pushed the apu-support branch from edda5ff to 8845b0d Compare November 20, 2025 23:03

superm1 force-pushed the apu-support branch from c51e3ce to f661678 Compare November 21, 2025 14:26

superm1 force-pushed the apu-support branch from 2cca315 to c3f4303 Compare November 30, 2025 14:58

dmitrii-galantsev requested changes Dec 5, 2025

View reviewed changes

Comment thread projects/amdsmi/include/amd_smi/amdsmi.h Resolved

superm1 requested a review from dmitrii-galantsev December 5, 2025 15:27

superm1 force-pushed the apu-support branch 2 times, most recently from 5faca4c to 1909a0b Compare December 8, 2025 17:24

superm1 added organization: ROCm and removed external contribution labels Dec 8, 2025

superm1 force-pushed the apu-support branch from 1909a0b to 214b126 Compare December 9, 2025 03:39

superm1 requested review from Copilot and marifamd December 9, 2025 03:39

Copilot started reviewing on behalf of superm1 December 9, 2025 03:41

View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes