panfrost: Disable CRC by default

Known unsound code.

So far I'm not convinced transaction elimination is doing us much good. Even in
synthetic glmark style benchmarks this seems to be a few % hit at most. Given
that transaction elimination is unsound by design, and that panfrost's
implementation is buggy in several places and getting it right (up to the
unsoundness of the hardware feature itself) would take actual engineering
effort, and the priority is making glamor work... disabling is the obvious
choice here.

For now, we leave the code but gate it behind a env var
flag (PAN_MESA_DEBUG=crc) rather than defaulting to enabled unless
PAN_MESA_DEBUG=nocrc is set. This way, we can still experiment with it if we
need that data ("what performance could we gain if we had this feature,
unsoundness be damned?"). That said, I'm not really ok with having unsoundness
on my devices, y'know? Back of the napkin math suggests that it's not unlikely
that somebody has hit a transaction elimination collision in the wild with the
DDK.

Boils down to values.

Closes: #8113
Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21258>

diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
index 6e87fc9..727a509 100644 (file)
--- a/src/gallium/drivers/panfrost/pan_resource.c
+++ b/src/gallium/drivers/panfrost/pan_resource.c
@@ -437,6 +437,10 @@ static bool
 panfrost_should_checksum(const struct panfrost_device *dev,
                          const struct panfrost_resource *pres)
 {
+   /* Checksumming is disabled by default due to fundamental unsoundness */
+   if (!(dev->debug & PAN_DBG_CRC))
+      return false;
+
    /* When checksumming is enabled, the tile data must fit in the
     * size of the writeback buffer, so don't checksum formats
     * that use too much space. */
@@ -447,8 +451,7 @@ panfrost_should_checksum(const struct panfrost_device *dev,
                               util_format_get_blocksize(pres->base.format);
 
    return pres->base.bind & PIPE_BIND_RENDER_TARGET && panfrost_is_2d(pres) &&
-          bytes_per_pixel <= bytes_per_pixel_max &&
-          pres->base.last_level == 0 && !(dev->debug & PAN_DBG_NO_CRC);
+          bytes_per_pixel <= bytes_per_pixel_max && pres->base.last_level == 0;
 }
 
 static void

diff --git a/src/gallium/drivers/panfrost/pan_screen.c b/src/gallium/drivers/panfrost/pan_screen.c
index 4d9aeeb..9f3a1cb 100644 (file)
--- a/src/gallium/drivers/panfrost/pan_screen.c
+++ b/src/gallium/drivers/panfrost/pan_screen.c
@@ -63,7 +63,7 @@ static const struct debug_named_value panfrost_debug_options[] = {
    {"nofp16",     PAN_DBG_NOFP16,    "Disable 16-bit support"},
    {"gl3",        PAN_DBG_GL3,      "Enable experimental GL 3.x implementation, up to 3.3"},
    {"noafbc",     PAN_DBG_NO_AFBC,  "Disable AFBC support"},
-   {"nocrc",      PAN_DBG_NO_CRC,   "Disable transaction elimination"},
+   {"crc",        PAN_DBG_CRC,      "Enable transaction elimination"},
    {"msaa16",     PAN_DBG_MSAA16,   "Enable MSAA 8x and 16x support"},
    {"linear",     PAN_DBG_LINEAR,   "Force linear textures"},
    {"nocache",    PAN_DBG_NO_CACHE, "Disable BO cache"},

diff --git a/src/panfrost/lib/pan_util.h b/src/panfrost/lib/pan_util.h
index e15266b..04514cf 100644 (file)
--- a/src/panfrost/lib/pan_util.h
+++ b/src/panfrost/lib/pan_util.h
@@ -39,7 +39,7 @@
 #define PAN_DBG_SYNC  0x0010
 /* 0x20 unused */
 #define PAN_DBG_NOFP16   0x0040
-#define PAN_DBG_NO_CRC   0x0080
+#define PAN_DBG_CRC      0x0080
 #define PAN_DBG_GL3      0x0100
 #define PAN_DBG_NO_AFBC  0x0200
 #define PAN_DBG_MSAA16   0x0400