Hi Nanley;

On 05/03/2018 10:03 PM, Nanley Chery wrote:
Before this patch, if we failed to initialize an MCS buffer, we'd
end up in a state in which the miptree thinks it has an MCS buffer,
but doesn't. We also leaked the clear_color_bo if it existed.

With this patch, we now free the miptree aux buffer resources and let
intel_miptree_alloc_mcs() know that the MCS buffer no longer exists.

I triggered this by forcing the failure (by inserting 1 instead unlikely(map ==NULL) to mcs buffer map condition) and following happens currently:

--- 8< ---
Failed to map mcs buffer into GTT
deqp-egl: ../src/intel/blorp/blorp_clear.c:313: blorp_fast_clear: Assertion `start_layer + num_layers <= MAX2(surf->aux_surf->logical_level0_px.depth >> level, surf->aux_surf->logical_level0_px.array_len)' failed.
Aborted (core dumped)
--- 8< ---

However even with this fix it seems we will end up in same or similar case (segfault happening in do_single_blorp_clear):

--- 8< ---
Failed to map mcs buffer into GTT
Segmentation fault (core dumped)
--- 8< ---

Test case used was:
   dEQP-EGL.functional.color_clears.single_context.gles2.rgba8888_window


Are we expected to survive without mcs?


Cc: <mesa-sta...@lists.freedesktop.org>
---
  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index b9a564552df..377efae32c9 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1658,7 +1658,7 @@ intel_miptree_copy_teximage(struct brw_context *brw,
     intel_obj->needs_validate = true;
  }
-static void
+static bool
  intel_miptree_init_mcs(struct brw_context *brw,
                         struct intel_mipmap_tree *mt,
                         int init_value)
@@ -1678,13 +1678,14 @@ intel_miptree_init_mcs(struct brw_context *brw,
     void *map = brw_bo_map(brw, mt->aux_buf->bo, MAP_WRITE | MAP_RAW);
     if (unlikely(map == NULL)) {
        fprintf(stderr, "Failed to map mcs buffer into GTT\n");
-      brw_bo_unreference(mt->aux_buf->bo);
-      free(mt->aux_buf);
-      return;
+      intel_miptree_aux_buffer_free(mt->aux_buf);
+      mt->aux_buf = NULL;
+      return false;
     }
     void *data = map;
     memset(data, init_value, mt->aux_buf->size);
     brw_bo_unmap(mt->aux_buf->bo);
+   return true;
  }
static struct intel_miptree_aux_buffer *
@@ -1764,15 +1765,14 @@ intel_miptree_alloc_mcs(struct brw_context *brw,
     const uint32_t alloc_flags = 0;
     mt->aux_buf = intel_alloc_aux_buffer(brw, "mcs-miptree",
                                          &temp_mcs_surf, alloc_flags, mt);
-   if (!mt->aux_buf) {
+   if (!mt->aux_buf ||
+       !intel_miptree_init_mcs(brw, mt, 0xFF)) {
        free(aux_state);
        return false;
     }
mt->aux_state = aux_state; - intel_miptree_init_mcs(brw, mt, 0xFF);
-
     return true;
  }
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to