[Mesa-dev] [PATCH 1/2] i965/gen9: Configure rbc buffers as plain for non-rbc tex views

[Topi Pohjolainen]

Fixes rendering in Shadow of Mordor with rbc. Application writes
RGBA_UNORM texture filling it with values the application wants to
later on treat as SRGB_ALPHA.
Intel driver enables lossless compression for the buffer by the time
of writing. However, the driver fails to make sure the buffer can be
sampled as something else later on and unfortunately there is
restriction in the hardware for using lossless compression for srgb
formats which looks to extend itself to the sampling engine also.
Requesting srgb to linear conversion on top of compressed buffer
results the color values to be pretty much garbage.

Fortunately none of tracked benchmarks showed a regression with
this.

Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
CC: Kenneth Graunke <kenneth at whitecape.org
---
 src/mesa/drivers/dri/i965/brw_context.c        | 38 ++++++++++++++++++++++++--
 src/mesa/drivers/dri/i965/gen8_surface_state.c | 13 ++++++++-
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index 2504dce..652acbf 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -70,6 +70,7 @@
 #include "tnl/t_pipeline.h"
 #include "util/ralloc.h"
 #include "util/debug.h"
+#include "isl/isl.h"
 
 /***************************************
  * Mesa's Driver Functions
@@ -166,6 +167,38 @@ intel_update_framebuffer(struct gl_context *ctx,
                                  fb->DefaultGeometry.NumSamples);
 }
 
+/* On Gen9 color buffers may be compressed by the hardware (lossless
+ * compression). There are, however, format restrictions and care needs to be
+ * taken that the sampler engine is capable for re-interpreting a buffer with
+ * format different the buffer was originally written with.
+ *
+ * For example, SRGB formats are not compressible and the sampler engine isn't
+ * capable of treating RGBA_UNORM as SRGB_ALPHA. In such a case the underlying
+ * color buffer needs to be resolved so that the sampling surface can be
+ * sampled as non-compressed (i.e., without the auxiliary MCS buffer being
+ * set).
+ */
+static bool
+intel_texture_view_requires_resolve(struct brw_context *brw,
+                                    struct intel_texture_object *intel_tex)
+{
+   if (brw->gen < 9 ||
+      !intel_miptree_is_lossless_compressed(brw, intel_tex->mt))
+     return false;
+
+   const uint32_t brw_format = brw_format_for_mesa_format(intel_tex->_Format);
+
+   if (isl_format_supports_lossless_compression(brw->intelScreen->devinfo,
+                                                brw_format))
+      return false;
+
+   perf_debug("Incompatible sampling format (%s) for rbc (%s)\n",
+              _mesa_get_format_name(intel_tex->_Format),
+              _mesa_get_format_name(intel_tex->mt->format));
+
+   return true;
+}
+
 static void
 intel_update_state(struct gl_context * ctx, GLuint new_state)
 {
@@ -198,8 +231,9 @@ intel_update_state(struct gl_context * ctx, GLuint new_state)
       /* Sampling engine understands lossless compression and resolving
        * those surfaces should be skipped for performance reasons.
        */
-      intel_miptree_resolve_color(brw, tex_obj->mt,
-                                  INTEL_MIPTREE_IGNORE_CCS_E);
+      const int flags = intel_texture_view_requires_resolve(brw, tex_obj) ?
+                           0 : INTEL_MIPTREE_IGNORE_CCS_E;
+      intel_miptree_resolve_color(brw, tex_obj->mt, flags);
       brw_render_cache_set_check_flush(brw, tex_obj->mt->bo);
    }
 
diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
index a3ad108..e7d9306 100644
--- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
@@ -40,6 +40,7 @@
 #include "brw_state.h"
 #include "brw_defines.h"
 #include "brw_wm.h"
+#include "isl/isl.h"
 
 /**
  * Convert an swizzle enumeration (i.e. SWIZZLE_X) to one of the Gen7.5+
@@ -254,8 +255,18 @@ gen8_emit_texture_surface_state(struct brw_context *brw,
     * the color buffer should always have been resolved before it is used as
     * a texture so there is no need for it. On Gen9 it will be uploaded when
     * the surface is losslessly compressed (CCS_E).
+    * However, sampling engine is not capable of re-interpreting the
+    * underlying color buffer in non-compressible formats when the surface
+    * is configured as compressed. Therefore state upload has made sure the
+    * buffer is in resolved state allowing the surface to be configured as
+    * as non-compressed.
     */
-   if (mt->num_samples <= 1 && aux_mode != GEN9_SURFACE_AUX_MODE_CCS_E) {
+   if (mt->num_samples <= 1 &&
+       (aux_mode != GEN9_SURFACE_AUX_MODE_CCS_E ||
+        !isl_format_supports_lossless_compression(
+            brw->intelScreen->devinfo, format))) {
+      assert(!mt->mcs_mt ||
+             mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED);
       aux_mt = NULL;
       aux_mode = GEN8_SURFACE_AUX_MODE_NONE;
    }
-- 
2.5.5