Skip to content

More correction of GPU classification as integrated/discrete type for DX12 [UMA flag]#254

Merged
kolkov merged 3 commits into
gogpu:mainfrom
Zeroes1:Correct-GPU-type-in-DX12
Jul 14, 2026
Merged

More correction of GPU classification as integrated/discrete type for DX12 [UMA flag]#254
kolkov merged 3 commits into
gogpu:mainfrom
Zeroes1:Correct-GPU-type-in-DX12

Conversation

@Zeroes1

@Zeroes1 Zeroes1 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Continue #252
sorry i cant renew old PR :)

I tested all OK. my video now correctly show as Integrated. On RDP also detect as right GPU type.

@Zeroes1 Zeroes1 requested a review from kolkov as a code owner July 13, 2026 15:24

@kolkov kolkov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work! This is exactly the UMA approach we suggested in PR #252. The implementation matches Rust wgpu — verified against wgpu-hal/src/dx12/adapter.rs:108-117 (UMA query) and lines 153-161 (device type classification).

Verified

  • D3D12_FEATURE_DATA_ARCHITECTURE struct layout: {NodeIndex uint32, TileBasedRenderer int32, UMA int32, CacheCoherentUMA int32} = 16 bytes, ABI-correct (Win32 BOOL = 4 bytes = int32)
  • CheckFeatureSupport(D3D12_FEATURE_ARCHITECTURE, ...) — correct constant, correct sizeof
  • Error guard: if err == nil && arch.UMA != 0 — on failure, defaults to Discrete (correct fallback)
  • Both Adapter and AdapterLegacy updated — no desync
  • deviceType() logic: SOFTWARE → CPU, UMA → Integrated, else → Discrete — exact Rust wgpu match

Requested changes (two small additions for Rust wgpu parity)

1. Store CacheCoherentUMA (required for full Rust wgpu parity)

The struct already has the field — free information from the same CheckFeatureSupport call. Rust wgpu stores it as MemoryArchitecture::Unified { cache_coherent } and uses it for memory pool selection (device.rs:91-93D3D12_MEMORY_POOL_L0 for UMA vs L1 for non-UMA). Please add:

// In AdapterCapabilities:
IsCacheCoherentUMA bool

// In queryArchitecture (Adapter):
a.capabilities.IsCacheCoherentUMA = arch.CacheCoherentUMA != 0

// Same for AdapterLegacy inline block

2. Add diagnostic logging (required for consistency)

The existing queryD3D12Options logs queried values via hal.Logger().Info(...). Architecture query should follow the same pattern for debuggability:

hal.Logger().Info("dx12: adapter architecture",
    "uma", a.capabilities.IsUMA,
    "cacheCoherentUMA", arch.CacheCoherentUMA != 0,
    "tileBasedRenderer", arch.TileBasedRenderer != 0,
)

Both changes are small — a few lines each. Once added, this is ready to merge. Thank you for implementing the UMA approach from our PR #252 feedback!

Store IsCacheCoherentUMA from D3D12_FEATURE_DATA_ARCHITECTURE for
future memory pool selection (D3D12_MEMORY_POOL_L0 vs L1).
Add hal.Logger().Info() for architecture query — matches existing
queryD3D12Options logging pattern. Both Adapter and AdapterLegacy.
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kolkov kolkov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes implemented: CacheCoherentUMA + diagnostic logging. LGTM.

@kolkov kolkov merged commit 5a061ac into gogpu:main Jul 14, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants