<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Wed, Oct 19, 2016 at 10:47 AM, 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 HZ sequence modifies less state than the blorp path and requires less CPU time to generate the necessary packets. Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> --- v2: Don't combine the depth alignment if statements src/intel/vulkan/gen8_cmd_buffer.c | 46 +++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/src/intel/vulkan/gen8_cmd_buffer.c b/src/intel/vulkan/gen8_cmd_buffer.c index 204542e..d4410d4 100644 --- a/src/intel/vulkan/gen8_cmd_buffer.c +++ b/src/intel/vulkan/gen8_cmd_buffer.c @@ -350,15 +350,19 @@ genX(cmd_buffer_emit_hz_op)(struct anv_cmd_buffer *cmd_buffer, assert(cmd_state->render_area.offset.x == 0 && cmd_state->render_area.offset.y == 0); + bool depth_clear; + bool stc_clear; </blockquote><div> </div><div>Mind calling this stencil_clear instead of the abbreviation. While stc is fairly obvious, it's not an abbreviation we usually use. Yes, it's used in the PRM in the docs for WM_HZ_OP, but this is the first time I'd seen it. With that changed, all three are </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div>Sorry it took so long. :-/ </div><div>Feel free to ignore the comment below for now. I'm mostly just pointing it out. (Sorry if I've pointed it out before.) </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + /* This variable corresponds to the Pixel Dim column in the table below */ struct isl_extent2d px_dim; </blockquote><div> Pedanticism: I'd really rather we call this align_px or something because that's really what it is. Yes, it comes from the size of a HiZ block but the way we use it is as an alignment. We could split the difference and call it block_size_px or something. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> /* Validate that we can perform the HZ operation and that it's necessary. */ switch (op) { case BLORP_HIZ_OP_DEPTH_CLEAR: - if (cmd_buffer->state.pass->attachments[ds].load_op != - VK_ATTACHMENT_LOAD_OP_CLEAR) - return; + stc_clear = VK_IMAGE_ASPECT_STENCIL_BIT & + cmd_state->attachments[ds].pending_clear_aspects; + depth_clear = VK_IMAGE_ASPECT_DEPTH_BIT & + cmd_state->attachments[ds].pending_clear_aspects; /* Apply alignment restrictions. Despite the BDW PRM mentioning this is * only needed for a depth buffer surface type of D16_UNORM, testing @@ -396,7 +400,7 @@ genX(cmd_buffer_emit_hz_op)(struct anv_cmd_buffer *cmd_buffer, px_dim = (struct isl_extent2d) { .w = 8, .h = 4}; #endif - if (!full_surface_op) { + if (depth_clear && !full_surface_op) { /* Fast depth clears clear an entire sample block at a time. As a * result, the rectangle must be aligned to the pixel dimensions of * a sample block for a successful operation. @@ -409,15 +413,25 @@ genX(cmd_buffer_emit_hz_op)(struct anv_cmd_buffer *cmd_buffer, */ if (cmd_state->render_area.offset.x % px_dim.w || cmd_state->render_area.offset.y % px_dim.h) - return; + depth_clear = false; if (cmd_state->render_area.offset.x + cmd_state->render_area.extent.width != iview->extent.width && cmd_state->render_area.extent.width % px_dim.w) - return; + depth_clear = false; if (cmd_state->render_area.offset.y + cmd_state->render_area.extent.height != iview->extent.height && cmd_state->render_area.extent.height % px_dim.h) + depth_clear = false; + } + + if (!depth_clear) { + if (stc_clear) { + /* Stencil has no alignment requirements */ + px_dim = (struct isl_extent2d) { .w = 1, .h = 1}; + } else { + /* Nothing to clear */ return; + } </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> } break; case BLORP_HIZ_OP_DEPTH_RESOLVE: @@ -448,10 +462,8 @@ genX(cmd_buffer_emit_hz_op)(struct anv_cmd_buffer *cmd_buffer, anv_batch_emit(&cmd_buffer->batch, GENX(3DSTATE_WM_HZ_OP), hzp) { switch (op) { case BLORP_HIZ_OP_DEPTH_CLEAR: - hzp.StencilBufferClearEnable = VK_IMAGE_ASPECT_STENCIL_BIT & - cmd_state->attachments[ds].pending_clear_aspects; - hzp.DepthBufferClearEnable = VK_IMAGE_ASPECT_DEPTH_BIT & - cmd_state->attachments[ds].pending_clear_aspects; + hzp.StencilBufferClearEnable = stc_clear; + hzp.DepthBufferClearEnable = depth_clear; hzp.FullSurfaceDepthandStencilClear = full_surface_op; hzp.StencilClearValue = cmd_state->attachments[ds].clear_value.depthStencil.stencil & 0xff; @@ -503,16 +515,24 @@ genX(cmd_buffer_emit_hz_op)(struct anv_cmd_buffer *cmd_buffer, anv_batch_emit(&cmd_buffer->batch, GENX(3DSTATE_WM_HZ_OP), hzp); + /* Perform clear specific flushing and state updates */ if (op == BLORP_HIZ_OP_DEPTH_CLEAR) { - if (!full_surface_op) { + if (depth_clear && !full_surface_op) { <div class="HOEnZb"><div class="h5"> anv_batch_emit(&cmd_buffer->batch, GENX(PIPE_CONTROL), pc) { pc.DepthStallEnable = true; pc.DepthCacheFlushEnable = true; } } - /* Mark aspects as cleared */ - cmd_state->attachments[ds].pending_clear_aspects = 0; + /* Remove cleared aspects from the pending mask */ + if (stc_clear) { + cmd_state->attachments[ds].pending_clear_aspects &= + ~VK_IMAGE_ASPECT_STENCIL_BIT; + } + if (depth_clear) { + cmd_state->attachments[ds].pending_clear_aspects &= + ~VK_IMAGE_ASPECT_DEPTH_BIT; + } } } -- 2.10.0 _______________________________________________ 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> </div></div></blockquote></div> </div></div>