From 6310c666a4339d8e0460dd2e1daa3fad500ae0ca Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Thu, 5 Mar 2020 14:50:59 -0600 Subject: [PATCH] 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 Tested-by: Marge Bot Part-of: --- 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 -- 2.30.2