[bimaniamd]

runtime gets cwsr size from KFD, and if KFD does not expose them,
then Runtime tries to do the calculations. Changes are made; so now
we should do the same for DRM.

Motivation

Technical Details

Issue Tracking

Test Plan

Test Result

Submission Checklist

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

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd)

projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp looks correct overall — the fallback logic structure and the call-site integration both look sound.

[Required] PR title format: feat (rocr): → feat(rocr): (no space before the parenthesis). ROCm squash-merges, so the PR title becomes the permanent commit title. The type(component): conventional-commit form requires the component immediately following the type with no space.

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd)

[Question] ComputeCwsrSizeFallback() duplicates the VGPR/SGPR/hwreg/wave-count sizing chain from libhsakmt/src/queues.c:update_ctx_save_restore_size(). When is amdgpu_query_cwsr_info expected to fail in practice — older kernels, DRM-without-KFD, or something else? Is there CI that exercises this path, so a future GPU addition to the thunk table that isn't mirrored here doesn't silently produce an undersized CWSR buffer?

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after author force-push (2026-10-16)

Re-checked each raised issue against the current diff. Note: the original commit 3df77f03 was amended in place (same author-date, commit count unchanged) and the rework added amd_drm_driver.cpp/.h, so the change is easy to miss; all gating threads were marked resolved without reply. Per-issue status:

Process / [Required]:

1. PR title feat (rocr): → feat(rocr): (stray space before the paren) — ⛔ Still open. Title is unchanged, and it becomes the permanent squash-merge commit title.

@dayatsin-amd (gating, CHANGES_REQUESTED):
2. Move the CWSR calc into amd_drm_driver's CreateQueue(...); amd_aql_queue should only hold code common to all driver classes — ⚠️ Partially addressed. ComputeCwsrSizeFallback is now a DrmDriver method and is called from DrmDriver::CreateQueue (good). But amd_aql_queue.cpp still contains the DRM-specific block — static_cast<DrmDriver&>(agent_->driver()).QueryCwsrInfo(...) / .ComputeCwsrSizeFallback(...) plus the cwsr_buf_ sizing — so the fallback is now invoked from two sites and amd_aql_queue is still not driver-common-only as requested. Suggest removing the DRM-specific path from amd_aql_queue.cpp so CreateQueue is the single owner.
3. "Start support with gfx12 and later" — ✅ Addressed. ComputeCwsrSizeFallback returns an error for gfxv < GFX_VERSION_GFX1200, gating the fallback to GFX12+.

@cfreeamd (AI pre-review):
4. The fallback duplicates the thunk's update_ctx_save_restore_size sizing chain; when is amdgpu_query_cwsr_info expected to fail, and is there CI that exercises this path? — ⛔ Still open. No reply and the thread is unresolved; the duplication now lives in amd_drm_driver.cpp::ComputeCwsrSizeFallback, and no CI was added. A future GPU added to the thunk table but not mirrored here would still silently undersize the CWSR buffer.

Net: 1 of 4 fully resolved (gfx12 gating). dayatsin's #2 is only partially met — DRM-specific CWSR code still lives in amd_aql_queue.cpp and the fallback now has two call sites. The [Required] title fix and the duplication/CI question remain open. Heads-up @cfreeamd: your earlier APPROVE (2026-10-15) was against the pre-rework version; @dayatsin-amd's CHANGES_REQUESTED still stands and has not been re-reviewed since the 10-16 force-push.