<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 2, 2017 at 4:30 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, May 26, 2017 at 04:30:19PM -0700, Jason Ekstrand wrote: <div><div class="h5">> This commit adds a new unified interface for doing resolves. The basic > format is that, prior to any surface access such as texturing or > rendering, you call intel_miptree_prepare_access. If the surface was > written, you call intel_miptree_finish_write. These two functions take > parameters which tell them whether or not auxiliary compression and fast > clears are supported on the surface. Later commits will add wrappers > around these two functions for texturing, rendering, etc. > --- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 156 +++++++++++++++++++++++++- > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 80 +++++++++++++ > 2 files changed, 232 insertions(+), 4 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > index 6cd32ce..0659e75 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > @@ -2028,8 +2028,7 @@ bool > intel_miptree_all_slices_resolve_hiz(struct brw_context *brw, > struct intel_mipmap_tree *mt) > { > - return intel_miptree_depth_hiz_resolve(brw, mt, > - 0, UINT32_MAX, 0, UINT32_MAX, > + return intel_miptree_depth_hiz_resolve(brw, mt, 0, UINT32_MAX, 0, UINT32_MAX, > BLORP_HIZ_OP_HIZ_RESOLVE); > } > > @@ -2037,8 +2036,7 @@ bool > intel_miptree_all_slices_resolve_depth(struct brw_context *brw, > struct intel_mipmap_tree *mt) > { > - return intel_miptree_depth_hiz_resolve(brw, mt, > - 0, UINT32_MAX, 0, UINT32_MAX, > + return intel_miptree_depth_hiz_resolve(brw, mt, 0, UINT32_MAX, 0, UINT32_MAX, > BLORP_HIZ_OP_DEPTH_RESOLVE); > } > > @@ -2221,6 +2219,156 @@ intel_miptree_all_slices_resolve_color(struct brw_context *brw, > intel_miptree_resolve_color(brw, mt, 0, UINT32_MAX, 0, UINT32_MAX, flags); > } > > +void > +intel_miptree_prepare_access(struct brw_context *brw, > + struct intel_mipmap_tree *mt, > + uint32_t start_level, uint32_t num_levels, > + uint32_t start_layer, uint32_t num_layers, > + bool aux_supported, bool fast_clear_supported) > +{ > + if (_mesa_is_format_color_format(mt->format)) { > + if (!mt->mcs_buf) > + return; > + > + if (mt->num_samples > 1) { > + /* Nothing to do for MSAA */ > + } else { > + /* TODO: This is fairly terrible. We can do better. */ > + if (!aux_supported || !fast_clear_supported) { > + intel_miptree_resolve_color(brw, mt, start_level, num_levels, > + start_layer, num_layers, 0); > + } > + } > + } else if (mt->format == MESA_FORMAT_S_UINT8) { > + /* Nothing to do for stencil */ > + } else { > + if (!mt->hiz_buf) > + return; > + > + if (aux_supported) { > + assert(fast_clear_supported); > + intel_miptree_depth_hiz_resolve(brw, mt, start_level, num_levels, > + start_layer, num_layers, > + BLORP_HIZ_OP_HIZ_RESOLVE); > + } else { > + assert(!fast_clear_supported); > + intel_miptree_depth_hiz_resolve(brw, mt, start_level, num_levels, > + start_layer, num_layers, > + BLORP_HIZ_OP_DEPTH_RESOLVE); > + } > + } > +} > + > +void > +intel_miptree_finish_write(struct brw_context *brw, > + struct intel_mipmap_tree *mt, uint32_t level, > + uint32_t start_layer, uint32_t num_layers, > + bool written_with_aux) > +{ > + assert(start_layer < mt->level[level].depth); > + if (num_layers == INTEL_REMAINING_LAYERS) > + num_layers = mt->level[level].depth - start_layer; > + assert(num_layers <= mt->level[level].depth - start_layer); > + > + if (_mesa_is_format_color_format(mt->format)) { > + if (mt->num_samples > 1) { > + /* Nothing to do for MSAA */ > + } else { > + if (written_with_aux) { > + intel_miptree_used_for_rendering(brw, mt, level, > + start_layer, num_layers); > + } > + } > + } else if (mt->format == MESA_FORMAT_S_UINT8) { > + /* Nothing to do for stencil */ > + } else { > + if (written_with_aux) { > + for (unsigned a = 0; a < num_layers; a++) { > + intel_miptree_check_level_layer(mt, level, start_layer); > + intel_miptree_slice_set_needs_depth_resolve(mt, level, > + start_layer + a); > + } > + } else { > + for (unsigned a = 0; a < num_layers; a++) { > + intel_miptree_check_level_layer(mt, level, start_layer); > + intel_miptree_slice_set_needs_hiz_resolve(mt, level, > + start_layer + a); > + } > + } > + } > +} > + > +enum isl_aux_state > +intel_miptree_get_aux_state(const struct intel_mipmap_tree *mt, > + uint32_t level, uint32_t layer) > +{ > + if (_mesa_is_format_color_format(mt->format)) { > + assert(mt->mcs_buf != NULL); > + if (mt->num_samples > 1) { > + return ISL_AUX_STATE_COMPRESSED_CLEAR; > + } else { > + switch (intel_miptree_get_fast_clear_state(mt, level, layer)) { > + case INTEL_FAST_CLEAR_STATE_RESOLVED: > + return ISL_AUX_STATE_RESOLVED; > + case INTEL_FAST_CLEAR_STATE_UNRESOLVED: > + return ISL_AUX_STATE_COMPRESSED_CLEAR; > + case INTEL_FAST_CLEAR_STATE_CLEAR: > + return ISL_AUX_STATE_CLEAR; > + default: > + unreachable("Invalid fast clear state"); > + } > + } > + } else if (mt->format == MESA_FORMAT_S_UINT8) { > + unreachable("Cannot get aux state for stencil"); > + } else { > + assert(mt->hiz_buf != NULL); > + const struct intel_resolve_map *map = > + intel_resolve_map_const_get(&mt->hiz_map, level, layer); > + if (!map) > + return ISL_AUX_STATE_RESOLVED; > + switch (map->need) { > + case BLORP_HIZ_OP_DEPTH_RESOLVE: > + return ISL_AUX_STATE_COMPRESSED_CLEAR; > + case BLORP_HIZ_OP_HIZ_RESOLVE: > + return ISL_AUX_STATE_AUX_INVALID; > + default: > + unreachable("Invalid hiz op"); > + } > + } > +} > + > +void > +intel_miptree_set_aux_state(struct brw_context *brw, > + struct intel_mipmap_tree *mt, uint32_t level, > + uint32_t start_layer, uint32_t num_layers, > + enum isl_aux_state aux_state) > +{ > + assert(start_layer < mt->level[level].depth); > + if (num_layers == INTEL_REMAINING_LAYERS) > + num_layers = mt->level[level].depth - start_layer; > + assert(num_layers <= mt->level[level].depth - start_layer); > + > + /* Right now, this only applies to clears. */ > + assert(aux_state == ISL_AUX_STATE_CLEAR); > + > + if (_mesa_is_format_color_format(mt->format)) { > + if (mt->num_samples > 1) > + assert(mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS); > + > + assert(level == 0 && start_layer == 0 && num_layers == 1); > + intel_miptree_set_fast_clear_state(brw, mt, 0, 0, 1, > + INTEL_FAST_CLEAR_STATE_CLEAR); > + } else if (mt->format == MESA_FORMAT_S_UINT8) { > + assert(!"Cannot set aux state for stencil"); > + } else { > + for (unsigned a = 0; a < num_layers; a++) { > + intel_miptree_check_level_layer(mt, level, start_layer); > + intel_miptree_slice_set_needs_depth_resolve(mt, level, > + start_layer + a); > + } > + } > +} > + > /** > * Make it possible to share the BO backing the given miptree with another > * process or another miptree. > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > index ffcedef..e74fca6 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > @@ -912,6 +912,86 @@ intel_miptree_all_slices_resolve_color(struct brw_context *brw, > struct intel_mipmap_tree *mt, > int flags); > > +#define INTEL_REMAINING_LAYERS UINT32_MAX > +#define INTEL_REMAINING_LEVELS UINT32_MAX > + > +/** Prepare a miptree for access > + * > + * This function should be called prior to any access to miptree in order to > + * perform any needed resolves. > + * > + * \param[in] start_level The first mip level to be accessed > + * > + * \param[in] num_levels The number of miplevels to be accessed or > + * INTEL_REMAINING_LEVELS to indicate every level > + * above start_level will be accessed > + * > + * \param[in] start_layer The first array slice or 3D layer to be accessed > + * > + * \param[in] num_layers The number of array slices or 3D layers be > + * accessed or INTEL_REMAINING_LAYERS to indicate > + * every layer above start_layer will be accessed > + * > + * \param[in] aux_supported Whether or not the access will support the > + * miptree's auxiliary compression format; this > + * must be false for uncompressed miptrees > + * > + * \param[in] fast_clear_supported Whether or not the access will support > + * fast clears in the miptree's auxiliary > + * compression format > + */ > +void > +intel_miptree_prepare_access(struct brw_context *brw, > + struct intel_mipmap_tree *mt, > + uint32_t start_level, uint32_t num_levels, > + uint32_t start_layer, uint32_t num_layers, > + bool aux_supported, bool fast_clear_supported); > + > +/** Complete a write operation > + * > + * This function should be called after any operation writes to a miptree. > + * This will update the miptree's compression state so that future resolves > + * happen correctly. Technically, this function can be called before the > + * write occurs but the caller must ensure that they don't interlace > + * intel_miptree_prepare_access and intel_miptree_finish_write calls to > + * overlapping layer/level ranges. > + * > + * \param[in] level The mip level that was written > + * > + * \param[in] start_layer The first array slice or 3D layer written > + * > + * \param[in] num_layers The number of array slices or 3D layers > + * written or INTEL_REMAINING_LAYERS to indicate > + * every layer above start_layer was written > + * > + * \param[in] written_with_aux Whether or not the write was done with > + * auxiliary compression enabled > + */ > +void > +intel_miptree_finish_write(struct brw_context *brw, > + struct intel_mipmap_tree *mt, uint32_t level, > + uint32_t start_layer, uint32_t num_layers, > + bool written_with_aux); </div></div>I think we discussed this before and I forgot the answer. How does this finish_write function enable future resolves to happen correctly? I'm trying to understand why prepare_access doesn't do this. </blockquote></div> </div><div class="gmail_extra">It avoids an ordering problem. What usually happens is that you do a bunch of prepare_access calls and then draw and then do a bunch of finish_write calls. If you did everything in prepare_access, then you could have "prepare" operations interleaved with "finish" operations and one of the "finish" operations could cause a later "prepare" operation to do a resolve even if it isn't needed. I haven't been able t convince myself that this is actually ever going to be a problem since we do the read-only "prepare" operations first and each of the "finish" operations should happen on unique subresources. That said, I have convinced myself that it's subtle and confusing enough that it's better to just have two functions. </div><div class="gmail_extra">--Jason </div></div>