fix(amdsmi): report LPDDR5 instead of GDDR7 for APU unified memory by adam360x · Pull Request #10775 · ROCm/rocm-systems

adam360x · 2026-08-26T16:23:51Z

Summary

amd-smi static --vram reported TYPE: GDDR7 for the LPDDR5 unified memory on APUs (e.g. gfx117x). AMDSMI_VRAM_TYPE__MAX is defined as the highest real memory type, LPDDR5 (both == 31), so the __MAX special case in the VRAM-type formatting matched a genuine LPDDR5 reading and hardcoded the label to GDDR7. Map the __MAX value to LPDDR5 so unified-memory APUs report the correct type.

Root cause

The library correctly returns vram_type == 31 (LPDDR5) from the DRM AMDGPU_INFO_DEV_INFO path.
amdsmi_vram_type_t__enumvalues[31] resolves to AMDSMI_VRAM_TYPE__MAX (duplicate-key collision with LPDDR5), so the CLI special-cased __MAX — but mapped it to the wrong string GDDR7 (which was the enum max before LPDDR4/LPDDR5 were appended at 30/31).

Fix

amdsmi_cli/subcommands/static.py: map the __MAX value to LPDDR5 instead of GDDR7.

Validation (real gfx117x baremetal)

Before: TYPE: GDDR7; after: TYPE: LPDDR5 (human and --json).
Full amd-smi matrix (list, static, metric, process, firmware, ...) — 31 commands, no tracebacks and no UNKNOWN ASIC; unified memory shown as VRAM carveout (512 MB) + GTT (~13 GB) UMA pool.

Test

New tests/python/unit/gpu/test_cli_vram_type_lpddr5.py (4 cases) drives static_gpu with a stubbed library; it fails on the old code (GDDR7) and passes on the fix (LPDDR5), and confirms LPDDR4/GDDR6 are unaffected.

JIRA ID: ROCM-26476

Copilot

Pull request overview

Fixes incorrect VRAM type labeling in amd-smi static --vram for UMA APUs where LPDDR5 (enum value 31) was being misreported as GDDR7 due to an AMDSMI_VRAM_TYPE__MAX alias/collision in the generated enum-value map.

Changes:

Update the CLI VRAM-type formatting logic to map AMDSMI_VRAM_TYPE__MAX to LPDDR5 instead of GDDR7.
Add a mock-based unit test covering __MAX/LPDDR5 behavior for both human and JSON output formats (plus non-regression cases).
Document the fix in the amdsmi changelog.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
projects/amdsmi/amdsmi_cli/subcommands/static.py	Adjusts VRAM type string mapping so `__MAX` reports as `LPDDR5`.
projects/amdsmi/tests/python/unit/gpu/test_cli_vram_type_lpddr5.py	Adds regression tests for the `__MAX`/LPDDR5 enum collision and expected CLI output.
projects/amdsmi/CHANGELOG.md	Notes the corrected `amd-smi static --vram` memory type reporting for UMA APUs.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

therock-pr-bot · 2026-08-26T16:45:04Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

AMDSMI_VRAM_TYPE__MAX aliases the highest real memory type (LPDDR5; both equal 31), so the __MAX special case in `amd-smi static --vram` matched a genuine LPDDR5 reading and mislabeled it GDDR7. Map the __MAX value to LPDDR5 so APU unified memory (e.g. gfx117x) reports correctly. Add a unit test that drives static_gpu with a stubbed library and asserts the __MAX value renders as LPDDR5.

gabrpham

Minor comment regarding the docstring in the test. Approving anyway since the rest looks good to me and I think it can go in either way.

…leak Snapshot the stubbed amdsmi/CLI module entries in setUpClass and restore them in tearDownClass so the fakes do not leak into sibling suites sharing the interpreter, matching the pattern in test_cli_set_clk_limit.py.

…0845) ## Why #10402 converted every amd-smi source to the two-line SPDX MIT header and added the `amdsmi-license-headers` pre-commit hook to hold the line. **The hook currently fails on `develop` itself**, so every open amd-smi PR is red — including ones that touch none of the offending files. ``` $ git checkout origin/develop $ python3 projects/amdsmi/tests/check_license_headers.py Missing or malformed AMD SPDX license header: src/ualoe_lib/ualoe_nl.c src/ualoe_lib/ualoe_telem.c tests/amd_smi_test/unit/system/status_string_test.cc tests/python/unit/gpu/test_cli_vram_type_lpddr5.py tests/python/unit/system/test_expect_status.py ``` The blast radius is wide because the hook is `pass_filenames: false` and rescans the whole tracked tree. It fires for any PR touching a single amd-smi `.py`/`.cc`/`.txt` and then reports files that PR never went near. #10083 trips it by editing one line of `src/CMakeLists.txt`. ## What The five files fall into two groups with different causes. **Three are amd-smi's own**, and simply landed while #10402 was in review, so its branch never saw them and they arrived with the old twenty-line MIT block. Converted here: | File | Landed via | |---|---| | `tests/amd_smi_test/unit/system/status_string_test.cc` | #10177 | | `tests/python/unit/gpu/test_cli_vram_type_lpddr5.py` | #10775 | | `tests/python/unit/system/test_expect_status.py` | `88dbbe3b93c` | **The other two are not ours to edit.** `src/ualoe_lib/` is the vendored UALoE library, replaced wholesale by the periodic *"sync UALoE lib"* drops — which is exactly why #9688 added `ualoe_lib/` to the pre-commit `exclude:` block, keeping clang-format and the rest off that code. #10402 carried only the `include/ualoe_lib/` half of that exclusion into the checker and left `src/` behind, so the checker now polices code the next sync will overwrite. Rewriting those headers would be reverted by the next drop and the hook would break again, so the missing prefix is added to `EXCLUDE_PREFIX` instead. The three files #10402 did rewrite under `src/ualoe_lib/` are left alone; they now fall outside the check and will drift back on their own at the next sync. Header text and check scope only — no functional change. ## Verification ``` $ python3 projects/amdsmi/tests/check_license_headers.py # exit 0 $ python3 projects/amdsmi/tests/run_amdsmi_license_header_test.py # 6/6 cases $ pre-commit run --config projects/amdsmi/.pre-commit-config.yaml --files <changed> codespell / trailing-whitespace / end-of-file-fixer / merge-conflicts / large-files / mixed-line-ending / clang-format / ruff format / amd-smi C++ test conventions / amd-smi SPDX license headers ....... all Passed ``` Confirmed the two vendored files are byte-identical to `develop` (`git diff origin/develop -- projects/amdsmi/src/ualoe_lib/` is empty), and that the checker skips them when named explicitly. Fixes the `amdsmi-license-headers` hook regression introduced by #10402.

Drops the 'if __name__ == "__main__"' block. The runner-hygiene guard rejects it, since running a leaf module directly bypasses the suite runner's root check, path resolution, GTest summary and sys.modules isolation guard. The file arrived via #10775, after that guard was written. Resolves the CLI directory with common.find_cli_dir instead of walking four levels up from the test file. The old derivation only landed on the CLI in a source checkout; installed it pointed at <rocm>/share/amd_smi/amdsmi_cli, so the class skipped every time the suite ran from /opt/rocm. Matches test_cli_cache_labels.py, which loads the same static.py.

ROCm#10402 converted every amd-smi source to the two-line SPDX MIT header and added the `amdsmi-license-headers` pre-commit hook to hold the line. The hook currently fails on develop itself, so every open amd-smi PR is red. The hook is `pass_filenames: false` and rescans the whole tracked tree, so it fires for any PR touching a single amd-smi `.py`/`.cc`/`.txt` and then reports files that PR never went near. Five files were flagged, in two groups with different causes. Three are amd-smi's own, and simply landed while ROCm#10402 was in review, so its branch never saw them and they arrived with the old twenty-line MIT block. They are converted here: tests/amd_smi_test/unit/system/status_string_test.cc (ROCm#10177) tests/python/unit/gpu/test_cli_vram_type_lpddr5.py (ROCm#10775) tests/python/unit/system/test_expect_status.py The other two, src/ualoe_lib/ualoe_nl.c and ualoe_telem.c, are not ours to edit. src/ualoe_lib is the vendored UALoE library, replaced wholesale by the periodic "sync UALoE lib" drops (ROCm#9688), which is why that PR excluded `ualoe_lib/` from the rest of the pre-commit suite. ROCm#10402 carried only the `include/ualoe_lib/` half of that exclusion into the checker and left `src/`, so the checker polices code the next sync will overwrite - a header rewritten here reverts, and the hook breaks again. The missing prefix is added instead. The three files ROCm#10402 did rewrite under src/ualoe_lib are left alone; they now fall outside the check and will drift back on their own. Header text and check scope only - no functional change.

systems-assistant · 2026-08-28T13:18:11Z

TheRock Submodule Bump Activity

Newest first

2026-08-28 — Included in TheRock via ROCm/TheRock#7710.

Drops the 'if __name__ == "__main__"' block. The runner-hygiene guard rejects it, since running a leaf module directly bypasses the suite runner's root check, path resolution, GTest summary and sys.modules isolation guard. The file arrived via #10775, after that guard was written. Resolves the CLI directory with common.find_cli_dir instead of walking four levels up from the test file. The old derivation only landed on the CLI in a source checkout; installed it pointed at <rocm>/share/amd_smi/amdsmi_cli, so the class skipped every time the suite ran from /opt/rocm. Matches test_cli_cache_labels.py, which loads the same static.py.

…10775) ## Summary `amd-smi static --vram` reported `TYPE: GDDR7` for the LPDDR5 unified memory on APUs (e.g. gfx117x). `AMDSMI_VRAM_TYPE__MAX` is defined as the highest real memory type, `LPDDR5` (both `== 31`), so the `__MAX` special case in the VRAM-type formatting matched a genuine LPDDR5 reading and hardcoded the label to `GDDR7`. Map the `__MAX` value to `LPDDR5` so unified-memory APUs report the correct type. ## Root cause - The library correctly returns `vram_type == 31` (LPDDR5) from the DRM `AMDGPU_INFO_DEV_INFO` path. - `amdsmi_vram_type_t__enumvalues[31]` resolves to `AMDSMI_VRAM_TYPE__MAX` (duplicate-key collision with `LPDDR5`), so the CLI special-cased `__MAX` — but mapped it to the wrong string `GDDR7` (which was the enum max before `LPDDR4`/`LPDDR5` were appended at 30/31). ## Fix - `amdsmi_cli/subcommands/static.py`: map the `__MAX` value to `LPDDR5` instead of `GDDR7`. ## Validation (real gfx117x baremetal) - Before: `TYPE: GDDR7`; after: `TYPE: LPDDR5` (human and `--json`). - Full `amd-smi` matrix (`list`, `static`, `metric`, `process`, `firmware`, ...) — 31 commands, no tracebacks and no `UNKNOWN ASIC`; unified memory shown as VRAM carveout (512 MB) + GTT (~13 GB) UMA pool. ## Test - New `tests/python/unit/gpu/test_cli_vram_type_lpddr5.py` (4 cases) drives `static_gpu` with a stubbed library; it fails on the old code (`GDDR7`) and passes on the fix (`LPDDR5`), and confirms `LPDDR4`/`GDDR6` are unaffected. JIRA ID: ROCM-26476

#10402 converted every amd-smi source to the two-line SPDX MIT header and added the `amdsmi-license-headers` pre-commit hook to hold the line. The hook currently fails on develop itself, so every open amd-smi PR is red. The hook is `pass_filenames: false` and rescans the whole tracked tree, so it fires for any PR touching a single amd-smi `.py`/`.cc`/`.txt` and then reports files that PR never went near. Five files were flagged, in two groups with different causes. Three are amd-smi's own, and simply landed while #10402 was in review, so its branch never saw them and they arrived with the old twenty-line MIT block. They are converted here: tests/amd_smi_test/unit/system/status_string_test.cc (#10177) tests/python/unit/gpu/test_cli_vram_type_lpddr5.py (#10775) tests/python/unit/system/test_expect_status.py The other two, src/ualoe_lib/ualoe_nl.c and ualoe_telem.c, are not ours to edit. src/ualoe_lib is the vendored UALoE library, replaced wholesale by the periodic "sync UALoE lib" drops (#9688), which is why that PR excluded `ualoe_lib/` from the rest of the pre-commit suite. #10402 carried only the `include/ualoe_lib/` half of that exclusion into the checker and left `src/`, so the checker polices code the next sync will overwrite - a header rewritten here reverts, and the hook breaks again. The missing prefix is added instead. The three files #10402 did rewrite under src/ualoe_lib are left alone; they now fall outside the check and will drift back on their own. Header text and check scope only - no functional change.