<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 3, 2018 at 12:03 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">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 or non-zero. 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 non-zero, 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. --- Hey Topi, Just FYI, this patch should fix the MCS bug you reported earlier. src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 33 ++++++++++++++++++--------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 5d3ee569bd8..e70c9ff1ef4 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -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; @@ -1673,9 +1673,9 @@ intel_alloc_aux_buffer(struct brw_context *brw, buf->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. */ @@ -1698,7 +1698,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) { </blockquote><div> </div><div>I don't think the temporary bool is doing you any good. just doing </div><div>if (wants_memset || has_indirect_clear) { </div><div> /* map */ </div><div> if (wants_memset) ... </div><div> if (has_indirect_clear) ... </div><div>} </div><div>is simpler. This needs_memset thing makes it look like we're going "Ha! You have an indirect clear so you're getting a memset even though you didn't ask for one!" </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> assert(!(alloc_flags & BO_ALLOC_BUSY)); void *map = brw_bo_map(brw, buf->bo, MAP_WRITE | MAP_RAW); @@ -1706,11 +1707,21 @@ intel_alloc_aux_buffer(struct brw_context *brw, intel_miptree_aux_buffer_free(buf); return NULL; } - memset(map, memset_value, mt->aux_buf->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); } @@ -1869,7 +1880,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); -- 2.16.2 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>