Skip to content

revert(amdsmi): remove AMDSMI_SILENCE_GFX_ACTIVITY and its autodetect - #10897

Merged
marifamd merged 1 commit into
developfrom
users/marifamd/remove-silence-gfx-activity
Aug 31, 2026
Merged

marifamd merged 1 commit into
developfrom
users/marifamd/remove-silence-gfx-activity

Conversation

@marifamd

@marifamd marifamd commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

amd-smi should report what the driver reports. The override added in #8674 masked gfx activity behind the AMDSMI_SILENCE_GFX_ACTIVITY environment variable, falling back to a gfx1250 / RLC-firmware autodetect when the variable was unset, so the tool could show N/A for a metric the driver had actually returned. This reverts that change.

It also drops a per-call getenv and a process-wide std::mutex that all four affected getters took on every invocation, which serialized concurrent multi-GPU polling in amd-smi monitor.

Technical Details

This is a straight git revert of 8da40e9 (#8674), plus one line the revert cannot cover.

Reverted verbatim:

  • include/amd_smi/impl/amd_smi_utils.h, src/amd_smi/amd_smi_utils.cc — removed is_gfx_activity_silenced(), apply_gfx_activity_overrides(), the gfx1250 / RLC-firmware 0x18-0x1d detection, and its per-handle unordered_map cache guarded by a static mutex
  • src/amd_smi/amd_smi.cc — amdsmi_get_gpu_metrics_info and amdsmi_get_gpu_partition_metrics_info no longer overwrite average_gfx_activity, gfx_activity_acc, or the per-XCP gfx_busy_inst / gfx_busy_acc fields; amdsmi_get_gpu_busy_percent and amdsmi_get_utilization_count no longer return AMDSMI_STATUS_NOT_SUPPORTED with an all-ones value
  • py-interface/amdsmi_interface.py — the two getters no longer route their values through _validate_if_max_uint
  • tests/python/unit/gpu/test_gfx_activity_silence.py — deleted

The one addition beyond the revert:

To confirm, run the revert yourself and diff it against this branch; the README line is the only difference:

git revert --no-commit 8da40e9703a   # resolve the modify/delete with: git rm

No public header signatures changed, so no wrapper regeneration is needed. No CHANGELOG entry: #8674 landed after the therock-7.14 pin, inside the unreleased 7.15.0 window, and never had one.

Issue Tracking

JIRA ID: ROCM-26000

Test Plan

  • cmake .. -DBUILD_TESTS=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo && make -j$(nproc)
  • Hardware-free Python unit suite: python3 -m unittest discover -s unit -p "test_*.py"
  • pre-commit run on every changed file
  • Diffed the branch against a literal git revert of 8da40e9 on the same base
  • Grepped the tree for any surviving reference to the env var, the removed functions, or the firmware constants

Test Result

  • Build passes, no new warnings
  • Unit suite: 42 tests, no new failures (2 pre-existing CLI-import errors also present on develop)
  • pre-commit passes on all changed files, including the SPDX header hook
  • Diff against the literal revert is exactly the one tests/python/README.md line
  • No surviving references to the removed feature

Note for the PR bot: this is a removal-only change, so it adds no test file. The deleted test_gfx_activity_silence.py covered only the behavior being removed, and the bot's unit-test gate skips files with status removed, so the gate will flag this PR.

Submission Checklist

@marifamd
marifamd requested a review from a team August 28, 2026 16:55
@therock-pr-bot

therock-pr-bot Bot commented Aug 28, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/amdsmi/include/amd_smi/impl/amd_smi_utils.h, projects/amdsmi/py-interface/amdsmi_interface.py, projects/amdsmi/src/amd_smi/amd_smi.cc, projects/amdsmi/src/amd_smi/amd_smi_utils.cc; no test file found
🚫 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?

@marifamd
marifamd force-pushed the users/marifamd/remove-silence-gfx-activity branch from 6fad07e to 160a113 Compare August 28, 2026 19:21

@JeniferC99 JeniferC99 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert looks good to me as long as ci pass.

Reverts #8674.

- Drop the AMDSMI_SILENCE_GFX_ACTIVITY environment variable and the
  gfx1250/RLC-firmware auto-detection it fell back to when unset
- Remove is_gfx_activity_silenced() and apply_gfx_activity_overrides()
  along with the per-handle firmware cache behind them
- Restore amdsmi_get_gpu_metrics_info,
  amdsmi_get_gpu_partition_metrics_info, amdsmi_get_gpu_busy_percent,
  and amdsmi_get_utilization_count to pass driver values through
  unmodified
- Stop rendering the all-ones value as N/A in the Python getters
- Drop the tests/python/README.md line listing the deleted test; that
  line postdates #8674, so the revert alone does not cover it

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
@marifamd
marifamd force-pushed the users/marifamd/remove-silence-gfx-activity branch from 160a113 to e9b915f Compare August 31, 2026 17:36
@marifamd
marifamd merged commit c038fbc into develop Aug 31, 2026
68 of 69 checks passed
@marifamd
marifamd deleted the users/marifamd/remove-silence-gfx-activity branch August 31, 2026 22:05
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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.

5 participants