Skip to content

test(amdsmi): fix APU cleanup - #13412

Draft
koushikbillakanti-amd wants to merge 1 commit into
developfrom
users/kbillaka/ROCM-32093
Draft

koushikbillakanti-amd wants to merge 1 commit into
developfrom
users/kbillaka/ROCM-32093

Conversation

@koushikbillakanti-amd

@koushikbillakanti-amd koushikbillakanti-amd commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Prevent CLI set-test cleanup from failing when APU metrics omit dGPU-only sections. Public GPU metrics status handling was already fixed by #11075; this PR changes only the test harness.

Technical Details

  • tests/python/cli/base.py: record the metrics producer's APU flag with the baseline.
  • tests/python/cli/test_set.py: default omitted sections only for confirmed APUs, keep dGPU data strict, require successful power-cap reset when the recorded cap is settable, and validate cleanup before starting the sweep.
  • tests/python/unit/system/test_cli_set_restore.py: add focused cleanup, reset-status and preflight regressions. Existing SOC/PLPD ID handling and fan-reset status handling are already upstream.
  • Remove the previous unreachable native-metrics changes and competing sweep changes. No public API, wrapper, packaging or runtime change remains.

Issue Tracking

JIRA ID: ROCM-32093

Test Plan

  • Fresh RelWithDebInfo build and private staging with BUILD_TESTS=ON, BUILD_CLI=ON, ENABLE_ESMI_LIB=OFF; staged commit verified.
  • Run all Python units with the official runner and only its root check mocked; run native *Unit* tests as non-root.
  • Replay captured MI300X/Raphael and Navi31/Navi48 baselines without executing commands; remove each of four safeguards in memory to confirm a detecting regression.
  • Run the project pre-commit hooks and scoped Ruff checks.

Test Result

  • Python: 510 passed, including 17 focused cleanup tests. Native: 278 passed, 30 skipped for non-root or unavailable features.
  • Captured-data replay: 15 and 26 cleanup commands planned; develop reproduces KeyError('fan'). All four mutations detected. Build and applicable hooks passed.
  • No live setter sweep, partition writes or physical restoration validation. ESMI-off CLI setup limitations are not fixed here. Prior hardware runs and old-head CI are not claimed as final-head validation.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

✅ 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?

@therock-pr-bot

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@koushikbillakanti-amd koushikbillakanti-amd changed the title fix(amdsmi): report unknown gpu_metrics as not supported fix(amdsmi): handle iGPU in metrics APIs and CLI set tests Oct 14, 2026
adam360x added a commit that referenced this pull request Oct 21, 2026
## Motivation

On integrated GPUs with a `gpu_metrics` v2.x table, GPU and multimedia
activity read 100 times too high. A GPU at 45% busy read as 4545 in the
C API; `amd-smi metric --usage` showed N/A, or up to 100% when the GPU
was nearly idle.

The APU firmware reports activity in hundredths of a percent (the SMU
headers mark `GfxActivity`/`UvdActivity` `[centi]`; amdgpu divides by
100 only for `gpu_busy_percent`), and amd-smi passed the value on as
whole percent.

#11075 made these tables readable on v2.1-v2.3 APUs; this follows up on
its post-merge review. v3.0 tables already report whole percent.
Discrete GPUs are not affected.

## Technical Details

- **Behavior change:** v2.x GFX and MM activity are whole percent in
`amdsmi_gpu_metrics_t` and in its APU sub-struct (was hundredths), so
`amdsmi_get_gpu_activity()` follows. The not-applicable value is kept as
is.
- `readDevInfoBinary()`: passes on only the errnos that name a device
state (suspended or resetting, runtime-inactive, access denied,
interrupted, firmware without the metrics command); others report a
failed read (`EINVAL` was reported as a caller error).
- The APU metrics pointer is copied to thread-local storage while the
device lock is held (a concurrent call could replace it while the caller
read it).
- `amdsmi.h`: return statuses of both metrics calls, the activity unit,
and the partition call never returning APU metrics.
- `amd_smi.cc`: three log statements rewrapped (formatting only).
- Tests: `GpuUnit.ApuActivityReachesThePublicFieldsAsWholePercent`,
`NotApplicableApuActivityIsStillReportedAsNotApplicable`,
`DriverRefusedMetricsReadIsNotReportedAsUnsupported`; the
oversized-header test now catches a missing read-size clamp; the Python
APU field test spells out the v2.4 layout.
- CHANGELOG: one Resolved Issues entry; the #11075 entry no longer names
status constants.
- Not changed here: per-metric query statuses, which #13412 changes.

## Issue Tracking

JIRA ID: ROCM-29548

## Test Plan

- `amdsmitst --gtest_filter='*Unit*'` and the Python unit suite; each
new test with its fix reverted.
- Hawk Point APU (gfx1103, `gpu_metrics` v2.1) under a GPU load at a
measured 45.9% duty. One captured table was bind-mounted over the sysfs
node, so develop, the previous push and this branch read the same bytes:
C API, Python and `amd-smi metric --usage`.

## Test Result

- Unit: 298 passed (18 skipped); Python: 485 passed. Each new test fails
with its fix reverted.
- Table holding 4545 hundredths (45.45%):

| | C API (top / APU) | `GFX_ACTIVITY` | APU GFX row |
|---|---|---|---|
| develop | 4545 / 4545 | N/A | missing |
| previous push | 45 / 4545 | 45 % | missing |
| this branch | 45 / 45 | 45 % | 45 % |

- Nearly idle table (18 hundredths): develop showed 18 % in both rows;
this branch shows 0 %.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

---------

Signed-off-by: Adam360x <Adam.pryor@amd.com>
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
dgaliffiAMD pushed a commit that referenced this pull request Oct 21, 2026
## Motivation

On integrated GPUs with a `gpu_metrics` v2.x table, GPU and multimedia
activity read 100 times too high. A GPU at 45% busy read as 4545 in the
C API; `amd-smi metric --usage` showed N/A, or up to 100% when the GPU
was nearly idle.

The APU firmware reports activity in hundredths of a percent (the SMU
headers mark `GfxActivity`/`UvdActivity` `[centi]`; amdgpu divides by
100 only for `gpu_busy_percent`), and amd-smi passed the value on as
whole percent.

#11075 made these tables readable on v2.1-v2.3 APUs; this follows up on
its post-merge review. v3.0 tables already report whole percent.
Discrete GPUs are not affected.

## Technical Details

- **Behavior change:** v2.x GFX and MM activity are whole percent in
`amdsmi_gpu_metrics_t` and in its APU sub-struct (was hundredths), so
`amdsmi_get_gpu_activity()` follows. The not-applicable value is kept as
is.
- `readDevInfoBinary()`: passes on only the errnos that name a device
state (suspended or resetting, runtime-inactive, access denied,
interrupted, firmware without the metrics command); others report a
failed read (`EINVAL` was reported as a caller error).
- The APU metrics pointer is copied to thread-local storage while the
device lock is held (a concurrent call could replace it while the caller
read it).
- `amdsmi.h`: return statuses of both metrics calls, the activity unit,
and the partition call never returning APU metrics.
- `amd_smi.cc`: three log statements rewrapped (formatting only).
- Tests: `GpuUnit.ApuActivityReachesThePublicFieldsAsWholePercent`,
`NotApplicableApuActivityIsStillReportedAsNotApplicable`,
`DriverRefusedMetricsReadIsNotReportedAsUnsupported`; the
oversized-header test now catches a missing read-size clamp; the Python
APU field test spells out the v2.4 layout.
- CHANGELOG: one Resolved Issues entry; the #11075 entry no longer names
status constants.
- Not changed here: per-metric query statuses, which #13412 changes.

## Issue Tracking

JIRA ID: ROCM-29548

## Test Plan

- `amdsmitst --gtest_filter='*Unit*'` and the Python unit suite; each
new test with its fix reverted.
- Hawk Point APU (gfx1103, `gpu_metrics` v2.1) under a GPU load at a
measured 45.9% duty. One captured table was bind-mounted over the sysfs
node, so develop, the previous push and this branch read the same bytes:
C API, Python and `amd-smi metric --usage`.

## Test Result

- Unit: 298 passed (18 skipped); Python: 485 passed. Each new test fails
with its fix reverted.
- Table holding 4545 hundredths (45.45%):

| | C API (top / APU) | `GFX_ACTIVITY` | APU GFX row |
|---|---|---|---|
| develop | 4545 / 4545 | N/A | missing |
| previous push | 45 / 4545 | 45 % | missing |
| this branch | 45 / 45 | 45 % | 45 % |

- Nearly idle table (18 hundredths): develop showed 18 % in both rows;
this branch shows 0 %.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

---------

Signed-off-by: Adam360x <Adam.pryor@amd.com>
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
@marifamd marifamd self-assigned this Oct 26, 2026
@marifamd
marifamd marked this pull request as draft October 26, 2026 17:28
@marifamd marifamd removed their assignment Oct 26, 2026
Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>
@koushikbillakanti-amd koushikbillakanti-amd changed the title fix(amdsmi): handle iGPU in metrics APIs and CLI set tests test(amdsmi): fix APU cleanup Nov 2, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants