From 0e1bdd0057a2de6d51cb2f62d80d066b15413714 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Sat, 14 Jan 2023 10:17:53 -0800 Subject: [PATCH] freedreno/a6xx: Invalidate LRZ on blend+depthwrite Writing depth with blend enabled means we need to invalidate LRZ, because the written depth value could mean that a later draw with depth enabled (where we would otherwise write LRZ) could have fragments which don't pass the depth test due to this draw. For example, consider this sequence of draws, with depth mode GREATER: draw A: z=0.1, fragments pass draw B: z=0.4, fragments pass blend enabled (LRZ write disabled) depth write enabled draw C: z=0.2, fragments don't pass blend disabled depth write enabled Normally looking at the state in draw C, we'd assume we could enable LRZ write. But this would cause early-z/lrz to discard fragments from draw A which should be visible due to draw B. Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/8065 Signed-off-by: Rob Clark Part-of: --- src/gallium/drivers/freedreno/a6xx/fd6_emit.c | 45 ++++++++++++++++++- src/gallium/drivers/freedreno/a6xx/fd6_zsa.c | 1 + src/gallium/drivers/freedreno/a6xx/fd6_zsa.h | 2 + 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_emit.c b/src/gallium/drivers/freedreno/a6xx/fd6_emit.c index 0defc197d70..7dc9fcba543 100644 --- a/src/gallium/drivers/freedreno/a6xx/fd6_emit.c +++ b/src/gallium/drivers/freedreno/a6xx/fd6_emit.c @@ -162,11 +162,12 @@ compute_lrz_state(struct fd6_emit *emit) assert_dt struct fd6_blend_stateobj *blend = fd6_blend_stateobj(ctx->blend); struct fd6_zsa_stateobj *zsa = fd6_zsa_stateobj(ctx->zsa); struct fd_resource *rsc = fd_resource(pfb->zsbuf->texture); + bool reads_dest = blend->reads_dest; lrz = zsa->lrz; /* normalize lrz state: */ - if (blend->reads_dest || fs->writes_pos || fs->no_earlyz || fs->has_kill || + if (reads_dest || fs->writes_pos || fs->no_earlyz || fs->has_kill || blend->base.alpha_to_coverage) { lrz.write = false; } @@ -176,8 +177,48 @@ compute_lrz_state(struct fd6_emit *emit) assert_dt * isn't known when blend CSO is constructed so we need to handle * that here. */ - if (ctx->all_mrt_channel_mask & ~blend->all_mrt_write_mask) + if (ctx->all_mrt_channel_mask & ~blend->all_mrt_write_mask) { lrz.write = false; + reads_dest = true; + } + + /* Writing depth with blend enabled means we need to invalidate LRZ, + * because the written depth value could mean that a later draw with + * depth enabled (where we would otherwise write LRZ) could have + * fragments which don't pass the depth test due to this draw. For + * example, consider this sequence of draws, with depth mode GREATER: + * + * draw A: + * z=0.1, fragments pass + * draw B: + * z=0.4, fragments pass + * blend enabled (LRZ write disabled) + * depth write enabled + * draw C: + * z=0.2, fragments don't pass + * blend disabled + * depth write enabled + * + * Normally looking at the state in draw C, we'd assume we could + * enable LRZ write. But this would cause early-z/lrz to discard + * fragments from draw A which should be visible due to draw B. + * + * NOTE: So far invalid rendering due to this case has only been + * found in a single game (The Walking Dead: Season One - + * com.telltalegames.walkingdead100). But other Telltale Games + * use the same Telltale Tool engine, so they might also have the + * same issue. But blend+depthwrite is more common. Hopefully + * it only shows up late in the renderpass where invalidating LRZ + * would not be too costly, but if this is found to cause perf + * regressions, a driconf allowlist/denylist can be added. + */ + if (reads_dest && zsa->writes_z) { + if (!zsa->perf_warn_blend && rsc->lrz_valid) { + perf_debug_ctx(ctx, "Invalidating LRZ due to blend+depthwrite"); + zsa->perf_warn_blend = true; + } + rsc->lrz_valid = false; + } /* if we change depthfunc direction, bail out on using LRZ. The * LRZ buffer encodes a min/max depth value per block, but if diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_zsa.c b/src/gallium/drivers/freedreno/a6xx/fd6_zsa.c index 204bab68ee9..d4dbb3e8b4d 100644 --- a/src/gallium/drivers/freedreno/a6xx/fd6_zsa.c +++ b/src/gallium/drivers/freedreno/a6xx/fd6_zsa.c @@ -103,6 +103,7 @@ fd6_zsa_state_create(struct pipe_context *pctx, so->base = *cso; so->writes_zs = util_writes_depth_stencil(cso); + so->writes_z = util_writes_depth(cso); so->rb_depth_cntl |= A6XX_RB_DEPTH_CNTL_ZFUNC(cso->depth_func); /* maps 1:1 */ diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_zsa.h b/src/gallium/drivers/freedreno/a6xx/fd6_zsa.h index d7b6f61fedf..0de778038e3 100644 --- a/src/gallium/drivers/freedreno/a6xx/fd6_zsa.h +++ b/src/gallium/drivers/freedreno/a6xx/fd6_zsa.h @@ -49,6 +49,7 @@ struct fd6_zsa_stateobj { struct fd6_lrz_state lrz; bool writes_zs : 1; /* writes depth and/or stencil */ + bool writes_z : 1; /* writes depth */ bool invalidate_lrz : 1; bool alpha_test : 1; @@ -56,6 +57,7 @@ struct fd6_zsa_stateobj { * we don't flood the user with LRZ disable warns which can * only be detected at draw time. */ + bool perf_warn_blend : 1; bool perf_warn_zdir : 1; struct fd_ringbuffer *stateobj[4];