From 5fdc82d5f11e456066c4d983dfa4ac0e8d74bc2a Mon Sep 17 00:00:00 2001 From: "Eric R. Smith" Date: Fri, 13 Sep 2024 22:01:59 -0300 Subject: [PATCH] panfrost: fix earlyzs settings for alpha_to_coverage When alpha_to_coverage is enabled, the zs_update_operation field must be set to force_late, according to (some of) the documentation. Actually the docs are ambiguous; the main thrust is that late coverage updates are only required when Z or S is written, or when occlusion queries are enabled. But there is a side note in a table that indicates force_late should be used for coverage updates even if Z or S is not written. Logically this shouldn't be necessary and the note is probably just lazily written. But it turns out that we do seem to need the force_late setting on valhall. It's currently unclear whether there's a hardware issue on valhall, or some other issue. Fixes piglit ext_framebuffer_multisample-*alpha-to-coverage* tests on valhall. Reviewed-by: Lars-Ivar Hesselberg Simonsen Acked-by: Boris Brezillon Part-of: --- src/panfrost/lib/pan_earlyzs.c | 9 ++++++++- src/panfrost/lib/tests/test-earlyzs.cpp | 8 ++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/panfrost/lib/pan_earlyzs.c b/src/panfrost/lib/pan_earlyzs.c index d3c82c1dc55..eb941847320 100644 --- a/src/panfrost/lib/pan_earlyzs.c +++ b/src/panfrost/lib/pan_earlyzs.c @@ -51,9 +51,16 @@ analyze(const struct pan_shader_info *s, bool writes_zs_or_oq, * be deferred until the value is known after the ZS_EMIT instruction, * if present. ZS_EMIT must precede ATEST, so the value is known when * ATEST executes, justifying the late test/update. + * Also, if alpha_to_coverage is set that also forces a late update. + * NOTE: it's not at all clear why alpha_to_coverage always requires + * a late update; the late update should only really be required if + * we're writing z or stencil, or testing for occlusion queries. + * The docs are somewhat contradictory on this point. + * But empirically we observe this requirement on Valhall, and doing + * the update later is never wrong (just potentially a bit slower). */ bool shader_writes_zs = (s->fs.writes_depth || s->fs.writes_stencil); - bool late_update = shader_writes_zs; + bool late_update = shader_writes_zs || alpha_to_coverage; bool late_kill = shader_writes_zs; /* Late coverage updates are required if the coverage mask depends on diff --git a/src/panfrost/lib/tests/test-earlyzs.cpp b/src/panfrost/lib/tests/test-earlyzs.cpp index 872487b808c..23efd301a29 100644 --- a/src/panfrost/lib/tests/test-earlyzs.cpp +++ b/src/panfrost/lib/tests/test-earlyzs.cpp @@ -116,7 +116,7 @@ TEST(EarlyZS, SideFXNoShaderZS) CASE(FORCE_EARLY, FORCE_LATE, SIDEFX); CASE(FORCE_EARLY, FORCE_LATE, SIDEFX | DISCARD); CASE(FORCE_EARLY, FORCE_LATE, SIDEFX | WRITES_COV); - CASE(FORCE_EARLY, FORCE_LATE, SIDEFX | ALPHA2COV); + CASE(FORCE_LATE, FORCE_LATE, SIDEFX | ALPHA2COV); } TEST(EarlyZS, SideFXNoShaderZSAlt) @@ -124,20 +124,20 @@ TEST(EarlyZS, SideFXNoShaderZSAlt) CASE(WEAK_EARLY, FORCE_LATE, ZS_ALWAYS_PASSES | SIDEFX); CASE(WEAK_EARLY, FORCE_LATE, ZS_ALWAYS_PASSES | SIDEFX | DISCARD); CASE(WEAK_EARLY, FORCE_LATE, ZS_ALWAYS_PASSES | SIDEFX | WRITES_COV); - CASE(WEAK_EARLY, FORCE_LATE, ZS_ALWAYS_PASSES | SIDEFX | ALPHA2COV); + CASE(FORCE_LATE, FORCE_LATE, ZS_ALWAYS_PASSES | SIDEFX | ALPHA2COV); } TEST(EarlyZS, NoSideFXNoShaderZS) { CASE(FORCE_EARLY, FORCE_EARLY, 0); - CASE(FORCE_EARLY, FORCE_EARLY, ALPHA2COV | DISCARD | WRITES_COV); + CASE(FORCE_LATE, FORCE_EARLY, ALPHA2COV | DISCARD | WRITES_COV); CASE(FORCE_EARLY, FORCE_EARLY, ZS_WRITEMASK); } TEST(EarlyZS, NoSideFXNoShaderZSAlt) { CASE(WEAK_EARLY, WEAK_EARLY, ZS_ALWAYS_PASSES); - CASE(WEAK_EARLY, WEAK_EARLY, + CASE(FORCE_LATE, WEAK_EARLY, ZS_ALWAYS_PASSES | ALPHA2COV | DISCARD | WRITES_COV); CASE(WEAK_EARLY, WEAK_EARLY, ZS_ALWAYS_PASSES | ZS_WRITEMASK); }