<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, May 8, 2018 at 3:41 PM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Thu, May 3, 2018 at 12:03 PM, Nanley Chery <span dir="ltr"><<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>></span> wrote:<br><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<br>
intel_miptree::fast_clear_colo<wbr>r. The initial value of ::fast_clear_color<br>
is zero, while that of the indirect clear color is undefined or<br>
non-zero.<br>
<br>
Topi Pohjolainen discovered this issue with MCS buffers. This issue is<br>
apparent when fast-clearing an MCS buffer for the first time with<br>
glClearColor = {0.0,}. Although the indirect clear color is non-zero,<br>
the initial aux state of the MCS is CLEAR and the tracked clear color is<br>
zero, so we avoid updating the indirect clear color with {0.0,}.<br>
<br>
Make the indirect clear color match the initial value of<br>
::fast_clear_color.<br>
<br>
---<br>
<br>
Hey Topi,<br>
<br>
Just FYI, this patch should fix the MCS bug you reported earlier.<br>
<br>
src/mesa/drivers/dri/i965/int<wbr>el_mipmap_tree.c | 33 ++++++++++++++++++---------<br>
1 file changed, 22 insertions(+), 11 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/in<wbr>tel_mipmap_tree.c b/src/mesa/drivers/dri/i965/in<wbr>tel_mipmap_tree.c<br>
index 5d3ee569bd8..e70c9ff1ef4 100644<br>
--- a/src/mesa/drivers/dri/i965/in<wbr>tel_mipmap_tree.c<br>
+++ b/src/mesa/drivers/dri/i965/in<wbr>tel_mipmap_tree.c<br>
@@ -978,11 +978,11 @@ create_ccs_buf_for_image(struc<wbr>t brw_context *brw,<br>
* system with CCS, we don't have the extra space at the end of the aux<br>
* buffer. So create a new bo here that will store that clear color.<br>
*/<br>
- const struct gen_device_info *devinfo = &brw->screen->devinfo;<br>
- if (devinfo->gen >= 10) {<br>
+ if (brw->isl_dev.ss.clear_color_s<wbr>tate_size > 0) {<br>
mt->aux_buf->clear_color_bo =<br>
- brw_bo_alloc(brw->bufmgr, "clear_color_bo",<br>
- brw->isl_dev.ss.clear_color_st<wbr>ate_size);<br>
+ brw_bo_alloc_tiled(brw->bufmg<wbr>r, "clear_color_bo",<br>
+ brw->isl_dev.ss.clear_color_st<wbr>ate_size,<br>
+ I915_TILING_NONE, 0, BO_ALLOC_ZEROED);<br>
if (!mt->aux_buf->clear_color_bo) {<br>
free(mt->aux_buf);<br>
mt->aux_buf = NULL;<br>
@@ -1673,9 +1673,9 @@ intel_alloc_aux_buffer(struct brw_context *brw,<br>
<br>
buf->size = aux_surf->size;<br>
<br>
- const struct gen_device_info *devinfo = &brw->screen->devinfo;<br>
- if (devinfo->gen >= 10) {<br>
- /* On CNL, instead of setting the clear color in the SURFACE_STATE, we<br>
+ const bool has_indirect_clear = brw->isl_dev.ss.clear_color_st<wbr>ate_size > 0;<br>
+ if (has_indirect_clear) {<br>
+ /* On CNL+, instead of setting the clear color in the SURFACE_STATE, we<br>
* will set a pointer to a dword somewhere that contains the color. So,<br>
* allocate the space for the clear color value here on the aux buffer.<br>
*/<br>
@@ -1698,7 +1698,8 @@ intel_alloc_aux_buffer(struct brw_context *brw,<br>
}<br>
<br>
/* Initialize the bo to the desired value */<br>
- if (wants_memset) {<br>
+ const bool needs_memset = wants_memset || has_indirect_clear;<br>
+ if (needs_memset) {<br></blockquote><div><br></div></div></div><div>I don't think the temporary bool is doing you any good. just doing<br><br></div><div>if (wants_memset || has_indirect_clear) {<br></div><div> /* map */<br></div><div> if (wants_memset) ...<br></div><div> if (has_indirect_clear) ...<br></div><div>}<br><br></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!"<br></div></div></div></div></blockquote><div><br></div><div>Never mind me. The next patch makes this make sense.<br><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
assert(!(alloc_flags & BO_ALLOC_BUSY));<br>
<br>
void *map = brw_bo_map(brw, buf->bo, MAP_WRITE | MAP_RAW);<br>
@@ -1706,11 +1707,21 @@ intel_alloc_aux_buffer(struct brw_context *brw,<br>
intel_miptree_aux_buffer_free(<wbr>buf);<br>
return NULL;<br>
}<br>
- memset(map, memset_value, mt->aux_buf->size);<br>
+<br>
+ /* Memset the aux_surf portion of the BO. */<br>
+ if (wants_memset)<br>
+ memset(map, memset_value, aux_surf->size);<br>
+<br>
+ /* Zero the indirect clear color to match ::fast_clear_color. */<br>
+ if (has_indirect_clear) {<br>
+ memset((char *)map + buf->clear_color_offset, 0,<br>
+ brw->isl_dev.ss.clear_color_st<wbr>ate_size);<br>
+ }<br>
+<br>
brw_bo_unmap(buf->bo);<br>
}<br>
<br>
- if (devinfo->gen >= 10) {<br>
+ if (has_indirect_clear) {<br>
buf->clear_color_bo = buf->bo;<br>
brw_bo_reference(buf->clear_c<wbr>olor_bo);<br>
}<br>
@@ -1869,7 +1880,7 @@ intel_miptree_alloc_hiz(struct brw_context *brw,<br>
isl_surf_get_hiz_surf(&brw->i<wbr>sl_dev, &mt->surf, &temp_hiz_surf);<br>
assert(ok);<br>
<br>
- const uint32_t alloc_flags = BO_ALLOC_BUSY;<br>
+ const uint32_t alloc_flags = 0;<br>
mt->aux_buf = intel_alloc_aux_buffer(brw, "hiz-miptree", &temp_hiz_surf,<br>
alloc_flags, false, 0, mt);<br>
<span class="m_-7018600372907257637HOEnZb"><font color="#888888"> <br>
-- <br>
2.16.2<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>