From fc30fe5bc5e744dd1cdb7a69cc8a5c382e940fbe Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Fri, 10 Feb 2023 22:19:37 -0500 Subject: [PATCH] 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 Part-of: --- src/gallium/drivers/panfrost/pan_resource.c | 7 +++++-- src/gallium/drivers/panfrost/pan_screen.c | 2 +- src/panfrost/lib/pan_util.h | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index 6e87fc95e48..727a509aa76 100644 --- 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 4d9aeeb5bae..9f3a1cba0ac 100644 --- 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 e15266b3721..04514cf25fd 100644 --- 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