<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 29, 2018 at 10:58 AM, Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com" target="_blank">rafael.antognolli@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Gen10, whenever we do a fast clear, blorp will update the clear color state buffer for us, as long as we set the clear color address correctly. However, on a hiz clear, if the surface is already on the fast clear state we skip the actual fast clear operation and, before gen10, only updated the miptree. On gen10+ we need to update the clear value state buffer too, since blorp will not be doing a fast clear and updating it for us. v4: - do not use clear_value_size in the for loop - Get the address of the clear color from the aux buffer or the clear_color_bo, depending on which one is available. - let core blorp update the clear color, but also update it when we skip a fast clear depth. v5: Better subject (Jordan). Signed-off-by: Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> --- src/mesa/drivers/dri/i965/brw_blorp.c | 11 +++++++++++ src/mesa/drivers/dri/i965/brw_clear.c | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c index a0977598309..e2287cbad3b 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp.c +++ b/src/mesa/drivers/dri/i965/brw_blorp.c @@ -185,6 +185,17 @@ blorp_surf_for_miptree(struct brw_context *brw, surf->aux_addr.buffer = aux_buf->bo; surf->aux_addr.offset = aux_buf->offset; + + if (devinfo->gen >= 10) { + /* If we have a CCS surface and clear_color_bo set, use that bo as + * storage for the indirect clear color. Otherwise, use the extra + * space at the end of the aux_buffer. + */ </blockquote><div> </div><div>This comment is out of date now that we have a clear_color_bo/offset in aux_buf. Let's just drop the comment. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + surf->clear_color_addr = (struct blorp_address) { + .buffer = aux_buf->clear_color_bo, + .offset = aux_buf->clear_color_offset, + }; + } } else { surf->aux_addr = (struct blorp_address) { .buffer = NULL, diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c index 8aa83722ee9..63c0b241898 100644 --- a/src/mesa/drivers/dri/i965/brw_clear.c +++ b/src/mesa/drivers/dri/i965/brw_clear.c @@ -108,6 +108,7 @@ brw_fast_clear_depth(struct gl_context *ctx) struct intel_mipmap_tree *mt = depth_irb->mt; struct gl_renderbuffer_attachment *depth_att = &fb->Attachment[BUFFER_DEPTH]; const struct gen_device_info *devinfo = &brw->screen->devinfo; + bool same_clear_value = true; if (devinfo->gen < 6) return false; @@ -213,6 +214,7 @@ brw_fast_clear_depth(struct gl_context *ctx) } intel_miptree_set_depth_clear_value(ctx, mt, clear_value); + same_clear_value = false; } bool need_clear = false; @@ -232,6 +234,26 @@ brw_fast_clear_depth(struct gl_context *ctx) * state then simply updating the miptree fast clear value is sufficient * to change their clear value. */ + if (devinfo->gen >= 10 && !same_clear_value) { + /* Before gen10, it was enough to just update the clear value in the + * miptree. But on gen10+, we let blorp update the clear value state + * buffer when doing a fast clear. Since we are skipping the fast + * clear here, we need to update the clear color ourselves. + */ + uint32_t clear_offset = mt->hiz_buf->clear_color_offset; + union isl_color_value clear_color = { .f32 = { clear_value, } }; + + /* We can't update the clear color while the hardware is still using + * the previous one for a resolve or sampling from it. So make sure + * that there's no pending commands at this point. + */ + brw_emit_pipe_control_flush(brw, PIPE_CONTROL_CS_STALL); + for (int i = 0; i < 4; i++) { + brw_store_data_imm32(brw, mt->hiz_buf->clear_color_bo, + clear_offset + i * 4, clear_color.u32[i]); + } + brw_emit_pipe_control_flush(brw, PIPE_CONTROL_STATE_CACHE_INVALIDATE); </blockquote><div> </div><div>I'm still not convinced by those pipe controls but I'm also not too worried about this edge case. Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + } return true; } -- 2.14.3 _______________________________________________ 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>