i965/miptree: Initialize the indirect clear color to zero
authorNanley Chery <nanley.g.chery@intel.com>
Mon, 30 Apr 2018 18:30:32 +0000 (11:30 -0700)
committerNanley Chery <nanley.g.chery@intel.com>
Thu, 17 May 2018 14:06:41 +0000 (07:06 -0700)
The indirect clear color isn't correctly tracked in
intel_miptree::fast_clear_color. The initial value of ::fast_clear_color
is zero, while that of the indirect clear color is undefined.

Topi Pohjolainen discovered this issue with MCS buffers. This issue is
apparent when fast-clearing an MCS buffer for the first time with
glClearColor = {0.0,}. Although the indirect clear color is undefined,
the initial aux state of the MCS is CLEAR and the tracked clear color is
zero, so we avoid updating the indirect clear color with {0.0,}.

Make the indirect clear color match the initial value of
::fast_clear_color.

Note: although we only have to drop HiZ's BO_ALLOC_BUSY flag for gen10+,
we also drop it pre-gen10 to keep things simple. We add this flag back
for pre-gen10 in a later patch.

v2: Add a note about dropping HiZ's BO_ALLOC_BUSY flag (Topi).

Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
src/mesa/drivers/dri/i965/intel_mipmap_tree.c

index d132b20abe83655f60a50bfd4d9fe38b51658f64..bf0731f1f3669c911b0794dce3a007255995acc2 100644 (file)
@@ -978,11 +978,11 @@ create_ccs_buf_for_image(struct brw_context *brw,
     * system with CCS, we don't have the extra space at the end of the aux
     * buffer. So create a new bo here that will store that clear color.
     */
-   const struct gen_device_info *devinfo = &brw->screen->devinfo;
-   if (devinfo->gen >= 10) {
+   if (brw->isl_dev.ss.clear_color_state_size > 0) {
       mt->aux_buf->clear_color_bo =
-         brw_bo_alloc(brw->bufmgr, "clear_color_bo",
-                      brw->isl_dev.ss.clear_color_state_size);
+         brw_bo_alloc_tiled(brw->bufmgr, "clear_color_bo",
+                            brw->isl_dev.ss.clear_color_state_size,
+                            I915_TILING_NONE, 0, BO_ALLOC_ZEROED);
       if (!mt->aux_buf->clear_color_bo) {
          free(mt->aux_buf);
          mt->aux_buf = NULL;
@@ -1670,9 +1670,9 @@ intel_alloc_aux_buffer(struct brw_context *brw,
 
    uint64_t size = aux_surf->size;
 
-   const struct gen_device_info *devinfo = &brw->screen->devinfo;
-   if (devinfo->gen >= 10) {
-      /* On CNL, instead of setting the clear color in the SURFACE_STATE, we
+   const bool has_indirect_clear = brw->isl_dev.ss.clear_color_state_size > 0;
+   if (has_indirect_clear) {
+      /* On CNL+, instead of setting the clear color in the SURFACE_STATE, we
        * will set a pointer to a dword somewhere that contains the color. So,
        * allocate the space for the clear color value here on the aux buffer.
        */
@@ -1693,7 +1693,8 @@ intel_alloc_aux_buffer(struct brw_context *brw,
    }
 
    /* Initialize the bo to the desired value */
-   if (wants_memset) {
+   const bool needs_memset = wants_memset || has_indirect_clear;
+   if (needs_memset) {
       assert(!(alloc_flags & BO_ALLOC_BUSY));
 
       void *map = brw_bo_map(brw, buf->bo, MAP_WRITE | MAP_RAW);
@@ -1701,11 +1702,21 @@ intel_alloc_aux_buffer(struct brw_context *brw,
          intel_miptree_aux_buffer_free(buf);
          return NULL;
       }
-      memset(map, memset_value, aux_surf->size);
+
+      /* Memset the aux_surf portion of the BO. */
+      if (wants_memset)
+         memset(map, memset_value, aux_surf->size);
+
+      /* Zero the indirect clear color to match ::fast_clear_color. */
+      if (has_indirect_clear) {
+         memset((char *)map + buf->clear_color_offset, 0,
+                brw->isl_dev.ss.clear_color_state_size);
+      }
+
       brw_bo_unmap(buf->bo);
    }
 
-   if (devinfo->gen >= 10) {
+   if (has_indirect_clear) {
       buf->clear_color_bo = buf->bo;
       brw_bo_reference(buf->clear_color_bo);
    }
@@ -1864,7 +1875,7 @@ intel_miptree_alloc_hiz(struct brw_context *brw,
       isl_surf_get_hiz_surf(&brw->isl_dev, &mt->surf, &temp_hiz_surf);
    assert(ok);
 
-   const uint32_t alloc_flags = BO_ALLOC_BUSY;
+   const uint32_t alloc_flags = 0;
    mt->aux_buf = intel_alloc_aux_buffer(brw, "hiz-miptree", &temp_hiz_surf,
                                         alloc_flags, false, 0, mt);