Mesa (master): intel/isl: Set DepthStencilResource based on aux usage

[GitLab Mirror]

Module: Mesa
Branch: master
Commit: 6310c666a4339d8e0460dd2e1daa3fad500ae0ca
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=6310c666a4339d8e0460dd2e1daa3fad500ae0ca

Author: Jason Ekstrand <jason at jlekstrand.net>
Date:   Thu Mar  5 14:50:59 2020 -0600

intel/isl: Set DepthStencilResource based on aux usage

In ISL, usage flags only carry intent and not semantic meaning.  We
don't have a bulletproof way in ISL to specify that an image is of
depth/stencil type.  The usage flags are great but blorp, for instance,
loves to disrespect them.  One proposed solution to this problem is to
add explicit depth/stencil formats which are distinct from the
corresponding color formats.

Fortunately, however, empirical evidence suggests that this bit only
affects the sampler's interpretation of the CCS data.  Therefore, we can
set the bit based off of the aux_usage which is now very specific and
does carry semantic meaning.  In particular, aux_usage now makes a
distinction between color CCS and depth/stencil CCS which appears to be
exactly what the DepthStencilResource bit is for.

Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4056>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4056>

---

 src/intel/isl/isl_surface_state.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/intel/isl/isl_surface_state.c b/src/intel/isl/isl_surface_state.c
index aff8ba5a806..ddc51db8d23 100644
--- a/src/intel/isl/isl_surface_state.c
+++ b/src/intel/isl/isl_surface_state.c
@@ -284,8 +284,36 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev, void *state,
    s.SurfaceFormat = info->view->format;
 
 #if GEN_GEN >= 12
-   s.DepthStencilResource =
-      isl_surf_usage_is_depth_or_stencil(info->surf->usage);
+   /* The BSpec description of this field says:
+    *
+    *    "This bit field, when set, indicates if the resource is created as
+    *    Depth/Stencil resource."
+    *
+    *    "SW must set this bit for any resource that was created with
+    *    Depth/Stencil resource flag. Setting this bit allows HW to properly
+    *    interpret the data-layout for various cases. For any resource that's
+    *    created without Depth/Stencil resource flag, it must be reset."
+    *
+    * Even though the docs for this bit seem to imply that it's required for
+    * anything which might have been used for depth/stencil, empirical
+    * evidence suggests that it only affects CCS compression usage.  There are
+    * a few things which back this up:
+    *
+    *  1. The docs are also pretty clear that this bit was added as part
+    *     of enabling Gen12 depth/stencil lossless compression.
+    *
+    *  2. The only new difference between depth/stencil and color images on
+    *     Gen12 (where the bit was added) is how they treat CCS compression.
+    *     All other differences such as alignment requirements and MSAA layout
+    *     are already covered by other bits.
+    *
+    * Under these assumptions, it makes sense for ISL to model this bit as
+    * being an extension of AuxiliarySurfaceMode where STC_CCS and HIZ_CCS_WT
+    * are indicated by AuxiliarySurfaceMode == CCS_E and DepthStencilResource
+    * == true.
+    */
+   s.DepthStencilResource = info->aux_usage == ISL_AUX_USAGE_HIZ_CCS_WT ||
+                            info->aux_usage == ISL_AUX_USAGE_STC_CCS;
 #endif
 
 #if GEN_GEN <= 5