Repository navigation
drm/asahi: g17: add g17_tvb_min_blocks to pre-size the tiler heap - #460
jgoyvaerts wants to merge 1 commit into
Conversation
|
Thanks, this matches what we've seen on the J616s, and the dEQP case plus the CS2 load are good evidence. On the design question: please gate the floor to V25G83, or default to 0 and set 256 only when the 25G83 programs are selected, so other G17 parts keep their current sizing. The Neo (G17P) grows its TVB reliably and passes its full Vulkan and GL lists today, so we'd rather not change its memory behaviour without a reason. Keeping the module parameter as an override is fine. Once it's gated, we'll run it on our J616s (glmark2, vkcube, the memory barrier case) and confirm the Neo is unchanged before merging. |
On 25G83 an ordinary scene that overflows its tiler parameter buffer makes the firmware ask the host to grow it one small run (10 blocks) at a time, each a synchronous halt/request/reply round trip. A heavy scene needs dozens, and the firmware's forward-progress watchdog fails the render (recovery reason 0) before they finish; partial renders, the firmware's other overflow path, do not complete on this firmware either. So on 25G83 a scene that outgrows its initial heap hangs. Floor the initial TVB block count to 256 on 25G83 so ordinary heavy scenes fit without growing. Other G17 parts grow their TVB reliably, so they keep their computed tile-count size. A new module parameter, g17_tvb_min_blocks (default 0, clamped to TVB_MAX_BLOCKS), raises the floor on any G17 part for debugging or tuning. dEQP memory.pipeline_barrier.graphics.all_device.1048576_vertex_buffer_stride_2 passes 3/3 on 25G83 with the 256 floor and recovers 3/3 (reason 0) without it; also unblocks CS2 map loads under FEX/muvm, which hit the same growth hang. Signed-off-by: Jonathan Goyvaerts <jonathan.goyvaerts@gmail.com> Assisted-by: LLM
ddb145b to
1d2067a
Compare
|
Done — gated the 256 floor to V25G83 and defaulted the parameter to 0, so other G17 parts keep their computed tile-count sizing; the parameter still overrides the floor on any part. On the M4 Pro / J616s (25G83) behaviour is unchanged (floored to 256). Force-pushed the amended single commit — ready for your J616s + Neo run whenever. |
|
One more point, from a project-wide decision: we're keeping module parameters to a minimum, so behaviour comes from the device tree and the firmware or compatible match data. Rather than a g17_tvb_min_blocks parameter, could the floor be part of the V25G83 programs' match data (or derived from the target's tile count for that firmware), with no module parameter? Happy to help with that change. |
|
I'm actually not happy with it yet, it's still not the perfect fix. Will keep working on this a bit more. |
|
Superseded by #518: the 25G83 growth hang now has a root-cause fix (fragment_busy = 0 and no fragment barrier, matching macOS), so growth works and the tiler heap no longer needs pre-sizing. Closing this one, thanks for the review. |
On 25G83 (Apple M4 Pro) an ordinary scene that overflows its tiler parameter buffer makes the firmware ask the host to grow it 10 blocks at a time, each a synchronous halt/request/reply round trip. A heavy scene needs dozens, and the firmware's forward-progress watchdog fails the render (recovery reason 0) before they finish. Partial renders — the firmware's other overflow path — do not complete on this firmware either, so a scene that outgrows its initial heap hangs.
This adds a module parameter
g17_tvb_min_blocksthat floors the initial TVB block count of every render target (clamped toTVB_MAX_BLOCKS). Pre-sizing the heap so ordinary heavy scenes fit avoids the fatal mid-render growth.Test:
dEQP-VK.memory.pipeline_barrier.graphics.all_device.1048576_vertex_buffer_stride_2passes 3/3 at 256 blocks on 25G83, and recovers 3/3 (reason 0) at the tile-count default. Also unblocks CS2 map loads under FEX/muvm, which hit the same growth hang.Design note for review: the default is 256, which floors all G17 parts, not only 25G83. A chip whose firmware grows the TVB reliably doesn't need it and pays a little pre-committed memory. Gating it to
V25G83or defaulting to 0 are both reasonable — happy to change to whichever you prefer; the knob form keeps it tunable per deployment.Assisted-by: LLM