[Mesa-dev] [PATCH v2] i965: prevent using auxilary buffers when an astc5x5 texture is present

Rogovin, Kevin kevin.rogovin at intel.com
Fri Mar 16 10:15:40 UTC 2018


Hi,

 I will just pipe in on the scenario that needs to be checked and ways around the scenario as well. Firstly, What Tapani has below is what I first wrote as well when developing a v4, it is cleaner and from that point of view I prefer it as well. However, the following scenario made me have reservations:

1. The final patch in the series has a simple test if the src in blorp_params HAS an auxiliary buffer, rather than a test if the sampler will read the surface using the auxiliary buffer. Note that on a simple blit from one texture that has an auxiliary (for example lossless color compression) to another surface that blorp needs to use the auxiliary buffer, this case is likely to happen in glBlitFramebuffer).

2. If brw_resolve_framebuffer() triggers a blorp call (for example render to and read from a same texture ala GL_ARB_texture_barrier), then blorp MIGHT set the mode to BRW_ASTC5x5_WA_MODE_HAS_AUX (even though the resolve does not have the sampler read from the surface in the way that helps trigger the hang). However, then if the mode was set in brw_resolve_inputs() to HAS_ASTC5x5, then that information is lost. The state upload for surfaces, because it uses the mode value, will then let auxiliary buffers be used by the sampler.

I do not know the internals of blorp well; if it is the case that the check made in patch 3/3 for having an auxiliary buffer is false in the case of the blorp calls triggered by brw_resolve_framebuffer(), then what Tapani posted is better; if that is not the case, then a nasty, hard to track bug will then lurk.

-Kevin

-----Original Message-----
From: Palli, Tapani 
Sent: Friday, March 16, 2018 11:32 AM
To: mesa-dev at lists.freedesktop.org
Cc: jason at jlekstrand.net; Rogovin, Kevin <kevin.rogovin at intel.com>; Palli, Tapani <tapani.palli at intel.com>
Subject: [PATCH v2] i965: prevent using auxilary buffers when an astc5x5 texture is present

From: Kevin Rogovin <kevin.rogovin at intel.com>

If ASTC5x5 textures are present, resolve all textures that the sampler accesses so that auxilary buffer is unneeded when the astc5x5 workaround is needed and also program the sampler state to not use the auxilary buffer as well.

v2: call gen9_set_astc5x5_wa_mode directly in brw_predraw_resolve_inputs
    (Tapani)

Signed-off-by: Kevin Rogovin <kevin.rogovin at intel.com>
Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
---

I've gone through this patch and wanted to express my thoughts as v2 rather than review. I've also discussed with Kevin offline about a plan to build a proper Piglit test to hit this and attempt to hit blorp as well (currently nothing really hitting that one).

 src/mesa/drivers/dri/i965/brw_draw.c             | 64 +++++++++++++++++++++---
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 ++++-
 2 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
index ef7c6a7df9..0b4e993576 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -392,6 +392,8 @@ mark_textures_used_for_txf(BITSET_WORD *used_for_txf,
  *
  * Resolve the depth buffer's HiZ buffer, resolve the depth buffer of each
  * enabled depth texture, and flush the render cache for any dirty textures.
+ * In addition, if the ASTC5x5 workaround is needed and if ASTC5x5 
+ textures
+ * are present, resolve textures so that auxilary buffers are not needed.
  */
 void
 brw_predraw_resolve_inputs(struct brw_context *brw, bool rendering, @@ -412,8 +414,33 @@ brw_predraw_resolve_inputs(struct brw_context *brw, bool rendering,
       mark_textures_used_for_txf(used_for_txf, ctx->ComputeProgram._Current);
    }
 
-   /* Resolve depth buffer and render cache of each enabled texture. */
    int maxEnabledUnit = ctx->Texture._MaxEnabledTexImageUnit;
+   bool disable_aux = false;
+   bool texture_astc5x5_present = false;
+   bool texture_with_auxilary_present = false;
+
+   if (gen9_astc5x5_wa_required(brw)) {
+      /* walk through all the texture units to see if an ASTC5x5 and/or
+       * a texture with an auxilary buffer is to be accessed.
+       */
+      for (int i = 0; i <= maxEnabledUnit; i++) {
+         if (!ctx->Texture.Unit[i]._Current)
+            continue;
+         tex_obj = intel_texture_object(ctx->Texture.Unit[i]._Current);
+         if (!tex_obj)
+            continue;
+         if (tex_obj->mt && tex_obj->mt->aux_usage != ISL_AUX_USAGE_NONE) {
+            texture_with_auxilary_present = true;
+         }
+         if (tex_obj->_Format == MESA_FORMAT_RGBA_ASTC_5x5 ||
+             tex_obj->_Format == MESA_FORMAT_SRGB8_ALPHA8_ASTC_5x5) {
+            texture_astc5x5_present = true;
+         }
+      }
+      disable_aux = texture_astc5x5_present;
+   }
+
+   /* Resolve depth buffer and render cache of each enabled texture. */
    for (int i = 0; i <= maxEnabledUnit; i++) {
       if (!ctx->Texture.Unit[i]._Current)
 	 continue;
@@ -445,9 +472,16 @@ brw_predraw_resolve_inputs(struct brw_context *brw, bool rendering,
                                      "for sampling");
       }
 
-      intel_miptree_prepare_texture(brw, tex_obj->mt, view_format,
-                                    min_level, num_levels,
-                                    min_layer, num_layers);
+      if (!disable_aux) {
+         intel_miptree_prepare_texture(brw, tex_obj->mt, view_format,
+                                       min_level, num_levels,
+                                       min_layer, num_layers);
+      } else {
+         intel_miptree_prepare_access(brw, tex_obj->mt,
+                                      min_level, num_levels,
+                                      min_layer, num_layers,
+                                      ISL_AUX_USAGE_NONE, false);
+      }
 
       /* If any programs are using it with texelFetch, we may need to also do
        * a prepare with an sRGB format to ensure texelFetch works "properly".
@@ -456,9 +490,16 @@ brw_predraw_resolve_inputs(struct brw_context *brw, bool rendering,
          enum isl_format txf_format =
             translate_tex_format(brw, tex_obj->_Format, GL_DECODE_EXT);
          if (txf_format != view_format) {
-            intel_miptree_prepare_texture(brw, tex_obj->mt, txf_format,
-                                          min_level, num_levels,
-                                          min_layer, num_layers);
+            if (!disable_aux) {
+               intel_miptree_prepare_texture(brw, tex_obj->mt, txf_format,
+                                             min_level, num_levels,
+                                             min_layer, num_layers);
+            } else {
+               intel_miptree_prepare_access(brw, tex_obj->mt,
+                                            min_level, num_levels,
+                                            min_layer, num_layers,
+                                            ISL_AUX_USAGE_NONE, false);
+            }
          }
       }
 
@@ -494,6 +535,15 @@ brw_predraw_resolve_inputs(struct brw_context *brw, bool rendering,
          }
       }
    }
+
+   if (gen9_astc5x5_wa_required(brw)) {
+      if (texture_astc5x5_present) {
+         assert(disable_aux);
+         gen9_set_astc5x5_wa_mode(brw, BRW_ASTC5x5_WA_MODE_HAS_ASTC5x5);
+      } else if (texture_with_auxilary_present) {
+         gen9_set_astc5x5_wa_mode(brw, BRW_ASTC5x5_WA_MODE_HAS_AUX);
+      }
+   }
 }
 
 static void
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 23bf5a266c..05205200c6 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -581,9 +581,19 @@ static void brw_update_texture_surface(struct gl_context *ctx,
           obj->Target == GL_TEXTURE_CUBE_MAP_ARRAY)
          view.usage |= ISL_SURF_USAGE_CUBE_BIT;
 
-      enum isl_aux_usage aux_usage =
+      bool disable_aux =
+         (brw->astc5x5_wa_mode == BRW_ASTC5x5_WA_MODE_HAS_ASTC5x5);
+      enum isl_aux_usage aux_usage = (disable_aux) ? ISL_AUX_USAGE_NONE :
          intel_miptree_texture_aux_usage(brw, mt, format);
 
+      assert(!gen9_astc5x5_wa_required(brw) ||
+             brw->astc5x5_wa_mode != BRW_ASTC5x5_WA_MODE_HAS_ASTC5x5 ||
+             aux_usage == ISL_AUX_USAGE_NONE);
+      assert(!gen9_astc5x5_wa_required(brw) ||
+             brw->astc5x5_wa_mode != BRW_ASTC5x5_WA_MODE_HAS_AUX ||
+             !(mesa_fmt == MESA_FORMAT_RGBA_ASTC_5x5 ||
+               mesa_fmt == MESA_FORMAT_SRGB8_ALPHA8_ASTC_5x5));
+
       brw_emit_surface_state(brw, mt, mt->target, view, aux_usage,
                              surf_offset, surf_index,
                              0);
--
2.14.3



More information about the mesa-dev mailing list