On 14 August 2012 16:58, Chad Versace <<a href="mailto:chad.versace@linux.intel.com" target="_blank">chad.versace@linux.intel.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On creating a hiz miptree, we conservatively marked that each miptree slice needed a hiz resolve. But the resolves are unneeded when creating a non-texture miptree, so this patch removes them. This eliminates one hiz resolve per each creation of a non-texture depth miptree. Hence, this eliminates many resolves when resizing a window. </blockquote><div> So, with this change, are the contents of the HiZ buffer uninitialized for a newly created (or resized) non-texture miptree? The HW docs don't specify the exact format of the HiZ buffer, so it's possible that there may be some possible states of the HiZ buffer that are invalid. If there are, and the hardware doesn't deal with those invalid states well, then we run the risk of sporadic incorrect results (if subsequent rendering fails to bring the HiZ buffer into a valid state) or possibly sporadic GPU hangs (if the hardware is unable to cope with the invalid states). Alternatively, perhaps we could initialize non-texture HiZ buffers to a state that indicates that they need a fast depth clear rather than a HiZ resolve. Clears are likely to be much faster than HiZ resolves, because they don't require reading from the depth buffer. So we would still get most of the benefit of avoiding the HiZ resolve, and no risk of undefined behaviour. In fact, we might conceivably get even more benefit, since after a fast depth clear, the HiZ buffer is in a state where future rendering is very efficient (since the depth buffer doesn't need to be read). </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Signed-off-by: Chad Versace <<a href="mailto:chad.versace@linux.intel.com">chad.versace@linux.intel.com</a>> --- src/mesa/drivers/dri/intel/intel_fbo.c | 4 ++- src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 39 +++++++++++++++++--------- src/mesa/drivers/dri/intel/intel_mipmap_tree.h | 3 +- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c b/src/mesa/drivers/dri/intel/intel_fbo.c index 3a610c2..ada0f69 100644 --- a/src/mesa/drivers/dri/intel/intel_fbo.c +++ b/src/mesa/drivers/dri/intel/intel_fbo.c @@ -538,7 +538,9 @@ intel_renderbuffer_update_wrapper(struct intel_context *intel, if (mt->hiz_mt == NULL && intel->vtbl.is_hiz_depth_format(intel, rb->Format)) { - intel_miptree_alloc_hiz(intel, mt, 0 /* num_samples */); + intel_miptree_alloc_hiz(intel, mt, + 0 /*num_samples*/, + true /*for_texture*/); if (!mt->hiz_mt) return false; } diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c index 24cd9e9..143f2e3 100644 --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c @@ -467,7 +467,8 @@ intel_miptree_create_for_renderbuffer(struct intel_context *intel, goto fail; if (intel->vtbl.is_hiz_depth_format(intel, format)) { - ok = intel_miptree_alloc_hiz(intel, mt, num_samples); + ok = intel_miptree_alloc_hiz(intel, mt, num_samples, + false /*for_texture*/); if (!ok) goto fail; } @@ -825,7 +826,8 @@ intel_miptree_alloc_mcs(struct intel_context *intel, bool intel_miptree_alloc_hiz(struct intel_context *intel, struct intel_mipmap_tree *mt, - GLuint num_samples) + GLuint num_samples, + bool for_texture) { assert(mt->hiz_mt == NULL); /* MSAA HiZ surfaces always use IMS layout. */ @@ -844,18 +846,27 @@ intel_miptree_alloc_hiz(struct intel_context *intel, if (!mt->hiz_mt) return false; - /* Mark that all slices need a HiZ resolve. */ - struct intel_resolve_map *head = &mt->hiz_map; - for (int level = mt->first_level; level <= mt->last_level; ++level) { - for (int layer = 0; layer < mt->level[level].depth; ++layer) { - head->next = malloc(sizeof(*head->next)); - head->next->prev = head; - head->next->next = NULL; - head = head->next; - - head->level = level; - head->layer = layer; - head->need = GEN6_HIZ_OP_HIZ_RESOLVE; + if (for_texture) { + /* Mark that all slices need a HiZ resolve. This is necessary for + * renderbuffers that wrap textures because the user may have previously + * uploaded texture data into the parent depth miptree. + * + * This is skipped for non-texture miptrees. In the non-texture case, + * the depth miptree and the hiz miptree are created together, and hence + * the content of each is undefined here. + */ + struct intel_resolve_map *head = &mt->hiz_map; + for (int level = mt->first_level; level <= mt->last_level; ++level) { + for (int layer = 0; layer < mt->level[level].depth; ++layer) { + head->next = malloc(sizeof(*head->next)); + head->next->prev = head; + head->next->next = NULL; + head = head->next; + + head->level = level; + head->layer = layer; + head->need = GEN6_HIZ_OP_HIZ_RESOLVE; + } } } diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h index 0d0e757..65743fd 100644 --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h @@ -503,7 +503,8 @@ intel_miptree_alloc_mcs(struct intel_context *intel, bool intel_miptree_alloc_hiz(struct intel_context *intel, struct intel_mipmap_tree *mt, - GLuint num_samples); + GLuint num_samples, + bool for_texture); void intel_miptree_slice_set_needs_hiz_resolve(struct intel_mipmap_tree *mt, -- 1.7.11.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>