Skip to content
This repository was archived by the owner on Sep 4, 2024. It is now read-only.

Commit 059ae38

Browse files
kaydenl1ace
authored andcommitted
iris: Check prog[] instead of uncompiled[] for BLORP state skipping
Huge thanks to Tapani Pälli for debugging this issue, figuring out what was going wrong, proposing fixes, and walking me through where things were going off the rails. BLORP always disables tessellation and geometry shaders. Our handling tried to look at ice->shaders.uncompiled[] to determine whether the next draw needed those shaders. If not, we can leave BLORP's residual state that disabled those stages in place, and skip looking at it. Unfortunately, predicting the future is a bit fraught, in part due to the uncompiled[] and prog[] arrays being slightly out of sync at times. Consider the following case: 1. Draw with tessellation shaders in place => uncompiled[TES] and prog[TES] will both point at valid shaders. 2. Gallium calls pipe->bind_tes_state(NULL). => This makes uncompiled[TES] point at NULL, and flags IRIS_STAGE_DIRTY_UNCOMPILED_TES. Because iris_update_compiled_shaders() hasn't happened yet, uncompiled[TES] is NULL but prog[TES] has the stale TES from the previous draw still. 3. BLORP operations happen => BLORP sees uncompiled[TES] == NULL and decides that tessellation is off for the upcoming draws. So it skips flagging tess state. 4. Gallium calls pipe->bind_tes_state(shader from step android-rpi#1). => uncompiled[TES] points at the original shader. IRIS_STAGE_DIRTY_UNCOMPILED_TES gets flagged again. 5. Draw again => This calls iris_update_compiled_shaders(), which sees that a TES is bound, and calls iris_update_compiled_tes(). But because the same shader was bound as before, the program it comes up with is identical to the one already bound at ice->shaders.prog[TES]. So, it thinks it doesn't have to flag any tessellation state dirty because it was already set up for the last draw. This random unbind and rebind between draws leads to a situation where, at step android-rpi#3, BLORP thinks it can skip flagging tessellation state (nothing is bound), and at step #5, normal state handling thinks it can skip flagging tessellation state (nothing changed since last time). So nobody does, and things break. This unbind appears to be happening when st_release_variants() decides it wants to free some shaders. Then a rebind happens to put back the actual shader for the draw. So, it's not theoretical. To fix this, we change BLORP to look at ice->shaders.prog[] rather than uncompiled[]. This is equivalent to thinking about the previous draw, rather than the next. If the last draw had tessellation off, then BLORP's disabling was a no-op, and the GPU is still in the same state as the previous draw. This is more reliable than predicting the future. Cc: mesa-stable Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/8308 Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/9678 Reviewed-by: Tapani Pälli <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24880> (cherry picked from commit d693027)
1 parent 117130b commit 059ae38

File tree

2 files changed

+5
-5
lines changed

2 files changed

+5
-5
lines changed

.pick_status.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@
8585
"description": "iris: Check prog[] instead of uncompiled[] for BLORP state skipping",
8686
"nominated": true,
8787
"nomination_type": 0,
88-
"resolution": 0,
88+
"resolution": 1,
8989
"main_sha": null,
9090
"because_sha": null
9191
},

src/gallium/drivers/iris/iris_blorp.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,8 @@ iris_blorp_exec_render(struct blorp_batch *blorp_batch,
364364
IRIS_STAGE_DIRTY_SAMPLER_STATES_TES |
365365
IRIS_STAGE_DIRTY_SAMPLER_STATES_GS);
366366

367-
if (!ice->shaders.uncompiled[MESA_SHADER_TESS_EVAL]) {
368-
/* BLORP disabled tessellation, that's fine for the next draw */
367+
if (!ice->shaders.prog[MESA_SHADER_TESS_EVAL]) {
368+
/* BLORP disabled tessellation, but it was already off anyway */
369369
skip_stage_bits |= IRIS_STAGE_DIRTY_TCS |
370370
IRIS_STAGE_DIRTY_TES |
371371
IRIS_STAGE_DIRTY_CONSTANTS_TCS |
@@ -374,8 +374,8 @@ iris_blorp_exec_render(struct blorp_batch *blorp_batch,
374374
IRIS_STAGE_DIRTY_BINDINGS_TES;
375375
}
376376

377-
if (!ice->shaders.uncompiled[MESA_SHADER_GEOMETRY]) {
378-
/* BLORP disabled geometry shaders, that's fine for the next draw */
377+
if (!ice->shaders.prog[MESA_SHADER_GEOMETRY]) {
378+
/* BLORP disabled geometry shaders, but it was already off anyway */
379379
skip_stage_bits |= IRIS_STAGE_DIRTY_GS |
380380
IRIS_STAGE_DIRTY_CONSTANTS_GS |
381381
IRIS_STAGE_DIRTY_BINDINGS_GS;

0 commit comments

Comments
 (0)