fix(rog-platform): select the dGPU by PCI vendor in telemetry fallbacks - #274
Closed
NB-Group wants to merge 1 commit into
Closed
fix(rog-platform): select the dGPU by PCI vendor in telemetry fallbacks#274NB-Group wants to merge 1 commit into
NB-Group wants to merge 1 commit into
Conversation
The two dGPU readouts (get_gpu_usage_pct, get_gpu_temp) go through NVML first and only fall back to sysfs when NVML is unavailable, which is exactly when the wrong card gets reported. The fallbacks scanned /sys/class/drm and /sys/class/hwmon and grabbed the first matching node, with no vendor or device pinning. On a hybrid laptop that first node is the iGPU: amdgpu sits on card0 and exposes gpu_busy_percent and temp1_input, so an NVML failure (a common driver/library mismatch on rolling-release distros) silently turned the home page GPU tile into an iGPU readout. Pin the dGPU with the existing Device::find() helper, which already does the PCI vendor plus connected-panel check, and read these nodes from the dGPU's own DRM card and hwmon directory. The iGPU counterparts (get_igpu_usage_pct, get_igpu_temp) already filtered by vendor; this brings the dGPU side in line with them.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details🔇 Additional comments (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe dGPU PCI device now identifies its dedicated DRM card and hwmon directories. GPU temperature and utilization fallbacks use those directories instead of the first matching system entries. ChangesdGPU metric fallback
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
7 tasks
Member
|
Superseded by #294 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When NVML isn't available, get_gpu_usage_pct and get_gpu_temp scanned sysfs for the first node that matched, without checking which GPU it came from. On a hybrid laptop with an AMD APU alongside the NVIDIA dGPU, that first match is the APU, so the values shown as dGPU were really the integrated GPU.
This adds two helpers on top of the existing Device::find() (which already picks the dGPU by PCI vendor and whether it drives a display) and points both fallbacks at the dGPU's own drm card and hwmon directories. The iGPU helpers already filtered by vendor; the dGPU ones now match that.
On a box where NVML fails and the closed NVIDIA driver exposes no sysfs nodes, these still read nothing. That's a driver or userspace mismatch, not something the code can work around.