Repository navigation
fix: read GPU VRAM via DRM sysfs vram_size fallback - #964
Conversation
Reviewer's GuideWhen hwinfo/amdgpu detection leaves VRAM unset, the GPU device code now identifies the corresponding DRM card by PCI BDF and reads its sysfs vram_size, while preserving existing reporting paths and formatting the fallback value in GB or MB. Sequence diagram for DRM sysfs VRAM fallbacksequenceDiagram
participant GPU as DeviceGpu
participant Hwinfo as hwinfo/amdgpu
participant DRM as DRM sysfs
participant File as vram_size
GPU->>Hwinfo: setHwinfoInfo(mapInfo)
alt m_GraphicsMemory is not empty
Hwinfo-->>GPU: Existing VRAM value
else m_GraphicsMemory is empty
GPU->>GPU: Extract PCI BDF from m_SysPath
GPU->>DRM: Enumerate /sys/class/drm/card[0-9]*
DRM-->>GPU: Card directories
GPU->>DRM: Resolve card/device symlink
alt PCI BDF matches
GPU->>File: Read card/device/vram_size
File-->>GPU: VRAM size in MB
GPU->>GPU: Format value as GB or MB
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="deepin-devicemanager/src/DeviceManager/DeviceGpu.cpp" line_range="247-249" />
<code_context>
+ memFile.close();
+ bool ok;
+ quint64 vramMB = memStr.toULongLong(&ok);
+ if (ok && vramMB >= 1024) {
+ m_GraphicsMemory = QString::number(vramMB / 1024) + "GB";
+ } else {
+ m_GraphicsMemory = QString::number(vramMB) + "MB";
+ }
</code_context>
<issue_to_address>
**VRAM capacities are vastly overstated**
When the DRM fallback reads a valid `vram_size` value, `vram_size` reports bytes, but the fallback treats the value as megabytes and divides by only 1024 for GB (or labels the raw bytes as MB), so a multi-GiB GPU is displayed with a vastly overstated capacity.
Convert the byte count to the intended unit before formatting the capacity.
</issue_to_address>
### Comment 2
<location path="deepin-devicemanager/src/DeviceManager/DeviceGpu.cpp" line_range="242-254" />
<code_context>
+ if (!devicePath.contains(pciBdf))
+ continue;
+ QFile memFile(QString("/sys/class/drm/%1/device/vram_size").arg(card));
+ if (memFile.open(QIODevice::ReadOnly)) {
+ QString memStr = memFile.readAll().trimmed();
+ memFile.close();
+ bool ok;
+ quint64 vramMB = memStr.toULongLong(&ok);
+ if (ok && vramMB >= 1024) {
+ m_GraphicsMemory = QString::number(vramMB / 1024) + "GB";
+ } else {
+ m_GraphicsMemory = QString::number(vramMB) + "MB";
+ }
+ break;
+ }
</code_context>
<issue_to_address>
**Fallback failures are invisible**
When the DRM card cannot be matched to the PCI BDF or its `vram_size` file cannot be opened, a failed file open or unmatched DRM card leaves `m_GraphicsMemory` empty without a diagnostic. `setHwinfoInfo` still returns true, so the caller adds the GPU and operators see missing memory without knowing why the fallback failed.
Log when the fallback cannot find a matching card or read its VRAM file.
</issue_to_address>| if (ok && vramMB >= 1024) { | ||
| m_GraphicsMemory = QString::number(vramMB / 1024) + "GB"; | ||
| } else { |
There was a problem hiding this comment.
🟠 High · VRAM capacities are vastly overstated
When the DRM fallback reads a valid vram_size value, vram_size reports bytes, but the fallback treats the value as megabytes and divides by only 1024 for GB (or labels the raw bytes as MB), so a multi-GiB GPU is displayed with a vastly overstated capacity.
Convert the byte count to the intended unit before formatting the capacity.
Prompt for AI agents
In `deepin-devicemanager/src/DeviceManager/DeviceGpu.cpp` at lines 247-249:
**VRAM capacities are vastly overstated**
When the DRM fallback reads a valid `vram_size` value, `vram_size` reports bytes, but the fallback treats the value as megabytes and divides by only 1024 for GB (or labels the raw bytes as MB), so a multi-GiB GPU is displayed with a vastly overstated capacity.
Convert the byte count to the intended unit before formatting the capacity.| if (memFile.open(QIODevice::ReadOnly)) { | ||
| QString memStr = memFile.readAll().trimmed(); | ||
| memFile.close(); | ||
| bool ok; | ||
| quint64 vramMB = memStr.toULongLong(&ok); | ||
| if (ok && vramMB >= 1024) { | ||
| m_GraphicsMemory = QString::number(vramMB / 1024) + "GB"; | ||
| } else { | ||
| m_GraphicsMemory = QString::number(vramMB) + "MB"; | ||
| } | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
⚪ Low · Fallback failures are invisible
When the DRM card cannot be matched to the PCI BDF or its vram_size file cannot be opened, a failed file open or unmatched DRM card leaves m_GraphicsMemory empty without a diagnostic. setHwinfoInfo still returns true, so the caller adds the GPU and operators see missing memory without knowing why the fallback failed.
Log when the fallback cannot find a matching card or read its VRAM file.
Prompt for AI agents
In `deepin-devicemanager/src/DeviceManager/DeviceGpu.cpp` at lines 242-254:
**Fallback failures are invisible**
When the DRM card cannot be matched to the PCI BDF or its `vram_size` file cannot be opened, a failed file open or unmatched DRM card leaves `m_GraphicsMemory` empty without a diagnostic. `setHwinfoInfo` still returns true, so the caller adds the GPU and operators see missing memory without knowing why the fallback failed.
Log when the fallback cannot find a matching card or read its VRAM file.22a1154 to
370c5af
Compare
1. Root cause: radeon driver does not expose VRAM size through hwinfo, leaving m_GraphicsMemory empty for Turks HD 7600 GPU 2. Fix: when m_GraphicsMemory is empty, match PCI BDF against /sys/class/drm/card*/device symlinks and read vram_size file, convert to GB (>=1024 MB) or MB 3. Impact: only triggers when existing amdgpu/hwinfo paths yield empty memory, no regression for GPUs already reporting VRAM Log: GPU VRAM size now displayed for radeon GPUs via DRM sysfs Influence: 1. Verify VRAM displays for radeon GPU (Turks HD 7600) 2. Verify amdgpu GPUs still show VRAM correctly 3. Verify multi-GPU systems match correct card by PCI BDF Bug: #380847 Task: #398195 Migrate from develop/eagle PR linuxdeepin#959 to master.
370c5af to
197dd29
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, GongHeng2017, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/merge |
fix: read GPU VRAM via DRM sysfs vram_size fallback
hwinfo, leaving m_GraphicsMemory empty for Turks HD 7600 GPU
/sys/class/drm/card*/device symlinks and read vram_size file,
convert to GB (>=1024 MB) or MB
empty memory, no regression for GPUs already reporting VRAM
Log: GPU VRAM size now displayed for radeon GPUs via DRM sysfs
Influence:
PMS: BUG-380847
Migrate from develop/eagle PR #959 to master.
Summary by Sourcery
Bug Fixes: