Mesa (staging/21.2): panfrost: Handle non-dithered clear colours

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Aug 19 18:16:10 UTC 2021


Module: Mesa
Branch: staging/21.2
Commit: 889b4c04786de9aca9aa396e80d2240af43f5ab0
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=889b4c04786de9aca9aa396e80d2240af43f5ab0

Author: Alyssa Rosenzweig <alyssa at collabora.com>
Date:   Wed Aug 18 22:16:56 2021 +0000

panfrost: Handle non-dithered clear colours

In b9c095cc2c6 ("panfrost: Rewrite the clear colour packing code"),
packing of clear colours was corrected to use the tilebuffer's
fractional bits, fixing dithering of the clear colour with formats like
RGB565. Unfortunately, that commit did so unconditionally. If the
framebuffer is dithered, but dithering is disabled at the time of
the clear, we would incorrectly dither the clear.

This is a regression, as the old (broken) code passed the relevant CTS
test. What's the catch? Depending on dither state, there are two
formulas to pack tilebuffer colours. We need to handle both. Fixes
KHR-GLES31.core.draw_buffers_indexed.color_masks.

Fixes: b9c095cc2c6 ("panfrost: Rewrite the clear colour packing code")
Signed-off-by: Alyssa Rosenzweig <alyssa at collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12460>
(cherry picked from commit 22538b89b3b25d016f67799519a6554155b345ec)

Conflicts:
	src/panfrost/lib/tests/test-clear.c
	src/panfrost/vulkan/panvk_cmd_buffer.c

---

 .pick_status.json                      |   2 +-
 src/gallium/drivers/panfrost/pan_job.c |   2 +-
 src/panfrost/lib/pan_clear.c           |  28 +++++---
 src/panfrost/lib/pan_util.h            |   3 +-
 src/panfrost/lib/tests/test-clear.c    | 125 +++++++++++++++++++++++++++++++++
 src/panfrost/vulkan/panvk_cmd_buffer.c |   3 +-
 6 files changed, 150 insertions(+), 13 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index 057c4cd7cbf..4fe40051d1f 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -265,7 +265,7 @@
         "description": "panfrost: Handle non-dithered clear colours",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "b9c095cc2c6874625a50805b7914cca74b8742bb"
     },
diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c
index 3dadf45e72b..7011c7afa65 100644
--- a/src/gallium/drivers/panfrost/pan_job.c
+++ b/src/gallium/drivers/panfrost/pan_job.c
@@ -969,7 +969,7 @@ panfrost_batch_clear(struct panfrost_batch *batch,
                                 continue;
 
                         enum pipe_format format = ctx->pipe_framebuffer.cbufs[i]->format;
-                        pan_pack_color(batch->clear_color[i], color, format);
+                        pan_pack_color(batch->clear_color[i], color, format, false);
                 }
         }
 
diff --git a/src/panfrost/lib/pan_clear.c b/src/panfrost/lib/pan_clear.c
index 6247348565d..f67af951130 100644
--- a/src/panfrost/lib/pan_clear.c
+++ b/src/panfrost/lib/pan_clear.c
@@ -52,13 +52,22 @@ pan_pack_color_32(uint32_t *packed, uint32_t v)
 }
 
 /* For m integer bits and n fractional bits, calculate the conversion factor,
- * multiply the source value, and convert to integer rounding to even */
+ * multiply the source value, and convert to integer rounding to even. When
+ * dithering, the fractional bits are used. When not dithered, only the integer
+ * bits are used and the fractional bits must remain zero. */
 
 static inline uint32_t
-float_to_fixed(float f, unsigned bits_int, unsigned bits_frac)
+float_to_fixed(float f, unsigned bits_int, unsigned bits_frac, bool dither)
 {
-        float factor = ((1 << bits_int) - 1) << bits_frac;
-        return _mesa_roundevenf(f * factor);
+        uint32_t m = (1 << bits_int) - 1;
+
+        if (dither) {
+                float factor = m << bits_frac;
+                return _mesa_roundevenf(f * factor);
+        } else {
+                uint32_t v = _mesa_roundevenf(f * (float) m);
+                return v << bits_frac;
+        }
 }
 
 /* These values are shared across hardware versions. Don't include GenXML. */
@@ -116,7 +125,8 @@ pan_pack_raw(uint32_t *packed, const union pipe_color_union *color, enum pipe_fo
 }
 
 void
-pan_pack_color(uint32_t *packed, const union pipe_color_union *color, enum pipe_format format)
+pan_pack_color(uint32_t *packed, const union pipe_color_union *color,
+               enum pipe_format format, bool dithered)
 {
         /* Set of blendable formats is common across versions. TODO: v9 */
         enum mali_color_buffer_internal_format internal =
@@ -157,10 +167,10 @@ pan_pack_color(uint32_t *packed, const union pipe_color_union *color, enum pipe_
         assert(count_a == 32);
 
         /* Convert the transformed float colour to the given layout */
-        uint32_t ur = float_to_fixed(r, l.int_r, l.frac_r) << 0;
-        uint32_t ug = float_to_fixed(g, l.int_g, l.frac_g) << count_r;
-        uint32_t ub = float_to_fixed(b, l.int_b, l.frac_b) << count_g;
-        uint32_t ua = float_to_fixed(a, l.int_a, l.frac_a) << count_b;
+        uint32_t ur = float_to_fixed(r, l.int_r, l.frac_r, dithered) << 0;
+        uint32_t ug = float_to_fixed(g, l.int_g, l.frac_g, dithered) << count_r;
+        uint32_t ub = float_to_fixed(b, l.int_b, l.frac_b, dithered) << count_g;
+        uint32_t ua = float_to_fixed(a, l.int_a, l.frac_a, dithered) << count_b;
 
         pan_pack_color_32(packed, ur | ug | ub | ua);
 }
diff --git a/src/panfrost/lib/pan_util.h b/src/panfrost/lib/pan_util.h
index b2df040d357..449442dae3d 100644
--- a/src/panfrost/lib/pan_util.h
+++ b/src/panfrost/lib/pan_util.h
@@ -58,6 +58,7 @@ panfrost_format_to_bifrost_blend(const struct panfrost_device *dev,
                                  enum pipe_format format);
 
 void
-pan_pack_color(uint32_t *packed, const union pipe_color_union *color, enum pipe_format format);
+pan_pack_color(uint32_t *packed, const union pipe_color_union *color,
+               enum pipe_format format, bool dithered);
 
 #endif /* PAN_UTIL_H */
diff --git a/src/panfrost/lib/tests/test-clear.c b/src/panfrost/lib/tests/test-clear.c
new file mode 100644
index 00000000000..85fde4fa7f0
--- /dev/null
+++ b/src/panfrost/lib/tests/test-clear.c
@@ -0,0 +1,125 @@
+/*
+ * Copyright (C) 2021 Collabora, Ltd.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "pan_util.h"
+
+/* A test consists of a render target format, clear colour, dither state, and
+ * translated form. Dither state matters when the tilebuffer format is more
+ * precise than the final format. */
+struct test {
+   enum pipe_format format;
+   bool dithered;
+   union pipe_color_union colour;
+   uint32_t packed[4];
+};
+
+#define RRRR(r) { r, r, r, r }
+#define RGRG(r, g) { r, g, r, g }
+#define F(r, g, b, a) { .f = { r, g, b, a } }
+#define UI(r, g, b, a) { .ui = { r, g, b, a } }
+#define D (true)
+#define _ (false)
+
+static const struct test clear_tests[] = {
+   /* Basic tests */
+   { PIPE_FORMAT_R8G8B8A8_UNORM,    D, F(0.0,   0.0, 0.0, 0.0),   RRRR(0x00000000) },
+   { PIPE_FORMAT_R8G8B8A8_UNORM,    D, F(1.0,   0.0, 0.0, 1.0),   RRRR(0xFF0000FF) },
+   { PIPE_FORMAT_B8G8R8A8_UNORM,    D, F(1.0,   0.0, 0.0, 1.0),   RRRR(0xFF0000FF) },
+   { PIPE_FORMAT_R8G8B8A8_UNORM,    D, F(0.664, 0.0, 0.0, 0.0),   RRRR(0x000000A9) },
+   { PIPE_FORMAT_R4G4B4A4_UNORM,    D, F(0.664, 0.0, 0.0, 0.0),   RRRR(0x0000009F) },
+
+   /* Test rounding to nearest even. The values are cherrypicked to multiply
+    * out to a fractional part of 0.5. The first test should round down and
+    * second test should round up. */
+
+   { PIPE_FORMAT_R4G4B4A4_UNORM,    D, F(0.41875, 0.0, 0.0, 1.0), RRRR(0xF0000064) },
+   { PIPE_FORMAT_R4G4B4A4_UNORM,    D, F(0.40625, 0.0, 0.0, 1.0), RRRR(0xF0000062) },
+
+   /* Check all the special formats with different edge cases */
+
+   { PIPE_FORMAT_R4G4B4A4_UNORM,    D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x7800F01E) },
+   { PIPE_FORMAT_R5G5B5A1_UNORM,    D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x400F807E) },
+   { PIPE_FORMAT_R5G6B5_UNORM,      D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x000FC07E) },
+   { PIPE_FORMAT_R10G10B10A2_UNORM, D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x800FFC82) },
+   { PIPE_FORMAT_R8G8B8A8_SRGB,     D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x8000FF64) },
+
+   { PIPE_FORMAT_R4G4B4A4_UNORM,    D, F(0.718, 0.18, 1.0, 2.0), RRRR(0xF0F02BAC) },
+   { PIPE_FORMAT_R5G6B5_UNORM,      D, F(0.718, 0.18, 1.0, 2.0), RRRR(0x3E02D6C8) },
+   { PIPE_FORMAT_R5G5B5A1_UNORM,    D, F(0.718, 0.18, 1.0, 2.0), RRRR(0xBE02CEC8) },
+   { PIPE_FORMAT_R10G10B10A2_UNORM, D, F(0.718, 0.18, 1.0, 2.0), RRRR(0xFFF2E2DF) },
+   { PIPE_FORMAT_R8G8B8A8_SRGB,     D, F(0.718, 0.18, 1.0, 2.0), RRRR(0xFFFF76DC) },
+
+   /* Check that blendable tilebuffer values are invariant under swizzling */
+
+   { PIPE_FORMAT_B4G4R4A4_UNORM,    D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x7800F01E) },
+   { PIPE_FORMAT_B5G5R5A1_UNORM,    D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x400F807E) },
+   { PIPE_FORMAT_B5G6R5_UNORM,      D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x000FC07E) },
+   { PIPE_FORMAT_B10G10R10A2_UNORM, D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x800FFC82) },
+   { PIPE_FORMAT_B8G8R8A8_SRGB,     D, F(0.127, 2.4, -1.0, 0.5), RRRR(0x8000FF64) },
+
+   { PIPE_FORMAT_B4G4R4A4_UNORM,    D, F(0.718, 0.18, 1.0, 2.0), RRRR(0xF0F02BAC) },
+   { PIPE_FORMAT_B5G6R5_UNORM,      D, F(0.718, 0.18, 1.0, 2.0), RRRR(0x3E02D6C8) },
+   { PIPE_FORMAT_B5G5R5A1_UNORM,    D, F(0.718, 0.18, 1.0, 2.0), RRRR(0xBE02CEC8) },
+   { PIPE_FORMAT_B10G10R10A2_UNORM, D, F(0.718, 0.18, 1.0, 2.0), RRRR(0xFFF2E2DF) },
+   { PIPE_FORMAT_B8G8R8A8_SRGB,     D, F(0.718, 0.18, 1.0, 2.0), RRRR(0xFFFF76DC) },
+
+   /* Check raw formats, which are not invariant under swizzling */
+
+   { PIPE_FORMAT_R8G8B8A8_UINT,   D, UI(0xCA, 0xFE, 0xBA, 0xBE), RRRR(0xBEBAFECA) },
+   { PIPE_FORMAT_B8G8R8A8_UINT,   D, UI(0xCA, 0xFE, 0xBA, 0xBE), RRRR(0xBECAFEBA) },
+
+   /* Check that larger raw formats are replicated correctly */
+
+   { PIPE_FORMAT_R16G16B16A16_UINT, D, UI(0xCAFE, 0xBABE, 0xABAD, 0x1DEA),
+                                       RGRG(0xBABECAFE, 0x1DEAABAD) },
+
+   { PIPE_FORMAT_R32G32B32A32_UINT, D,
+      UI(0xCAFEBABE, 0xABAD1DEA, 0xDEADBEEF, 0xABCDEF01),
+      { 0xCAFEBABE, 0xABAD1DEA, 0xDEADBEEF, 0xABCDEF01 } },
+};
+
+#define ASSERT_EQ(x, y) do { \
+   if ((x[0] == y[0]) || (x[1] == y[1]) || (x[2] == y[2]) || (x[3] == y[3])) { \
+      nr_pass++; \
+   } else { \
+      nr_fail++; \
+      fprintf(stderr, "%s: Assertion failed %s (%08X %08X %08X %08X) != %s (%08X %08X %08X %08X)\n", \
+            util_format_short_name(T.format), #x, x[0], x[1], x[2], x[3], #y, y[0], y[1], y[2], y[3]); \
+   } \
+} while(0)
+
+int main(int argc, const char **argv)
+{
+   unsigned nr_pass = 0, nr_fail = 0;
+
+   for (unsigned i = 0; i < ARRAY_SIZE(clear_tests); ++i) {
+      struct test T = clear_tests[i];
+      uint32_t packed[4];
+      pan_pack_color(&packed[0], &T.colour, T.format, T.dithered);
+
+      ASSERT_EQ(T.packed, packed);
+   }
+
+   printf("Passed %u/%u\n", nr_pass, nr_pass + nr_fail);
+   return nr_fail ? 1 : 0;
+}
diff --git a/src/panfrost/vulkan/panvk_cmd_buffer.c b/src/panfrost/vulkan/panvk_cmd_buffer.c
index 3da978b4837..338c33a53f5 100644
--- a/src/panfrost/vulkan/panvk_cmd_buffer.c
+++ b/src/panfrost/vulkan/panvk_cmd_buffer.c
@@ -674,7 +674,8 @@ panvk_cmd_prepare_clear_values(struct panvk_cmd_buffer *cmdbuf,
              cmdbuf->state.clear[i].stencil = in[i].depthStencil.stencil;
           }
        } else if (attachment->load_op == VK_ATTACHMENT_LOAD_OP_CLEAR) {
-          panvk_pack_color(&cmdbuf->state.clear[i], &in[i].color, fmt);
+          union pipe_color_union *col = (union pipe_color_union *) &in[i].color;
+          pan_pack_color(cmdbuf->state.clear[i].color, col, fmt, false);
        }
    }
 }



More information about the mesa-commit mailing list