[Mesa-dev] [PATCH] i965: Delete pending CCS and HiZ ops in intel_miptree_make_shareable()

[Ben Widawsky]

On 17-01-04 12:33:56, Chad Versace wrote:
>Fixes crash in piglit
>`egl_khr_gl_renderbuffer_image-clear-shared-image GL_DEPTH_COMPONENT24`
>on Skylake.
>
>The crash happened because blorp attempted to execute a pending hiz
>clear after the hiz buffer was deleted. Deleting the pending hiz ops
>when the hiz buffer gets deleted fixes the crash.
>
>For good measure, this patch also deletes all pending CCS/MCS ops when
>the CCS/MCS buffer gets deleted. I'm now aware of any bugs
>caused by the dangling ops, but deleting them is clearly the right thing
>to do.
>
>Cc: Ben Widawsky <ben at bwidawsk.net>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99265
>---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>index 31fb71e196..f2c94b7a04 100644
>--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>@@ -2337,12 +2337,28 @@ intel_miptree_make_shareable(struct brw_context *brw,
>       drm_intel_bo_unreference(mt->mcs_buf->bo);
>       free(mt->mcs_buf);
>       mt->mcs_buf = NULL;
>+
>+      /* Any pending MCS/CCS operations are no longer needed. Trying to
>+       * execute any will likely crash due to the missing aux buffer. So let's
>+       * delete all pending ops.
>+       */
>+      exec_list_make_empty(&mt->color_resolve_map);
>    }
>
>    if (mt->hiz_buf) {
>       intel_miptree_all_slices_resolve_depth(brw, mt);
>       intel_miptree_hiz_buffer_free(mt->hiz_buf);
>       mt->hiz_buf = NULL;
>+
>+      for (uint32_t l = mt->first_level; l <= mt->last_level; ++l) {
>+         mt->level[l].has_hiz = false;
>+      }
>+
>+      /* Any pending HiZ operations are no longer needed. Trying to execute
>+       * any will likely crash due to the missing aux buffer. So let's delete
>+       * all pending ops.
>+       */
>+      exec_list_make_empty(&mt->hiz_map);
>    }
>
>    mt->aux_disable = INTEL_AUX_DISABLE_ALL;

Both of these are:
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

Could you please address my points in patch 1 to make sure I'm not crazy and
this doesn't actually break me?