<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Mon, Sep 26, 2016 at 5:10 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">Create a function that performs one of three HiZ operations - depth/stencil clears, HiZ resolve, and depth resolves. Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> --- v2. Add documentation Fix the alignment check Don't minify clear rectangle (Jason) Use blorp enums (Jason) Enable depth stalls and flushes Use full RT rectangle for resolve ops Add stencil clear todo src/intel/vulkan/anv_genX.h | 3 + src/intel/vulkan/gen7_cmd_buffer.c | 6 ++ src/intel/vulkan/gen8_cmd_buffer.c | 167 +++++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+) diff --git a/src/intel/vulkan/anv_genX.h b/src/intel/vulkan/anv_genX.h index 02e79c2..ad3bec9 100644 --- a/src/intel/vulkan/anv_genX.h +++ b/src/intel/vulkan/anv_genX.h @@ -58,6 +58,9 @@ genX(emit_urb_setup)(struct anv_device *device, struct anv_batch *batch, unsigned vs_entry_size, unsigned gs_entry_size, const struct gen_l3_config *l3_config); +void genX(cmd_buffer_do_hz_op)(struct anv_cmd_buffer *cmd_buffer, + enum blorp_hiz_op op); + VkResult genX(graphics_pipeline_create)(VkDevice _device, struct anv_pipeline_cache *cache, diff --git a/src/intel/vulkan/gen7_cmd_buffer.c b/src/intel/vulkan/gen7_cmd_buffer.c index b627ef0..78b5ac7 100644 --- a/src/intel/vulkan/gen7_cmd_buffer.c +++ b/src/intel/vulkan/gen7_cmd_buffer.c @@ -323,6 +323,12 @@ genX(cmd_buffer_flush_dynamic_state)(struct anv_cmd_buffer *cmd_buffer) cmd_buffer->state.dirty = 0; } +void +genX(cmd_buffer_do_hz_op)(struct anv_cmd_buffer *cmd_buffer, + enum blorp_hiz_op op) +{ </blockquote><div> </div><div>This should have an anv_finishme in it. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> +} + void genX(CmdSetEvent)( VkCommandBuffer commandBuffer, VkEvent event, diff --git a/src/intel/vulkan/gen8_cmd_buffer.c b/src/intel/vulkan/gen8_cmd_buffer.c index 7058608..a13413c 100644 --- a/src/intel/vulkan/gen8_cmd_buffer.c +++ b/src/intel/vulkan/gen8_cmd_buffer.c @@ -399,6 +399,173 @@ genX(cmd_buffer_flush_compute_state)(struct anv_cmd_buffer *cmd_buffer) genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer); } + +/** + * Emit the HZ_OP packet in the sequence specified by the BDW PRM section + * entitled: "Optimized Depth Buffer Clear and/or Stencil Buffer Clear." + * + * \todo Enable Stencil Buffer-only clears + */ +void +genX(cmd_buffer_do_hz_op)(struct anv_cmd_buffer *cmd_buffer, + enum blorp_hiz_op op) +{ + struct anv_cmd_state *cmd_state = &cmd_buffer->state; + const struct anv_image_view *iview = + anv_cmd_buffer_get_depth_stencil_view(cmd_buffer); + + if (iview == NULL || !anv_image_has_hiz(iview->image)) + return; + + const uint32_t ds = cmd_state->subpass->depth_stencil_attachment; + const bool full_surface_op = + cmd_state->render_area.extent.width == iview->extent.width && + cmd_state->render_area.extent.height == iview->extent.height; </blockquote><div> </div><div>It's probably a bit redundant, but we might as well check render_area.offset == 0. I realize that, from API requirements, if the extents match then offset must be 0, but it's not incredibly obvious and the check won't hurt that much. </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; + + /* Apply alignment restrictions. Despite the BDW PRM mentioning this is + * only needed for a depth buffer surface type of D16_UNORM, testing + * showed it to be necessary for other depth formats as well + * (e.g., D32_FLOAT). + */ + if (!full_surface_op) { + + struct isl_extent2d px_dim; </blockquote><div> </div><div>Would it be better to call this hiz_block_size_px? That follows the ISL naming convention a bit better. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> +#if GEN_GEN == 8 </blockquote><div> </div><div>Mind making this <= 8? I know it's in a gen8+ file, but <= 8 makes it clear that the else case is >= 9 and not != 8. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + /* Pre-SKL, HiZ has an 8x4 sample block. As the number of samples + * increases, the number of pixels representable by this block + * decreases by a factor of the sample dimensions. Sample dimensions + * scale following the MSAA interleaved pattern. + * + * Sample|Sample|Pixel + * Count |Dim |Dim + * =================== + * 1 | 1x1 | 8x4 + * 2 | 2x1 | 4x4 + * 4 | 2x2 | 4x2 + * 8 | 4x2 | 2x2 + * 16 | 4x4 | 2x1 + * + * Table: Pixel Dimensions in a HiZ Sample Block Pre-SKL + */ + const struct isl_extent2d sa_dim = </blockquote><div> </div><div>Maybe call this px_size_sa? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + isl_get_interleaved_msaa_px_size_sa(iview->image->samples); + px_dim.w = 8 / sa_dim.w; + px_dim.h = 4 / sa_dim.h; +#else + /* SKL+, the sample block becomes a "pixel block" so the expected + * pixel dimension is a constant 8x4 px for all sample counts. + */ + px_dim = (struct isl_extent2d) { .w = 8, .h = 4}; +#endif + + /* 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. + */ + if (cmd_state->render_area.offset.x % px_dim.w || + cmd_state->render_area.offset.y % px_dim.h || + cmd_state->render_area.extent.width % px_dim.w || + cmd_state->render_area.extent.height % px_dim.h) + return; </blockquote><div> </div><div>What about the case where the offset is aligned and we go to the far right or bottom of the image but the image size is not aligned? I think this case should be possible too. </div><div> </div><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: + if (cmd_buffer->state.pass->attachments[ds].store_op != + VK_ATTACHMENT_STORE_OP_STORE) + return; + break; + case BLORP_HIZ_OP_HIZ_RESOLVE: + if (cmd_buffer->state.pass->attachments[ds].load_op != + VK_ATTACHMENT_LOAD_OP_LOAD) + return; + break; + case BLORP_HIZ_OP_NONE: + unreachable("Invalid HiZ OP"); + break; + } + + 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.FullSurfaceDepthandStencilClear = full_surface_op; + hzp.StencilClearValue = 0xff & + cmd_state->attachments[ds].clear_value.depthStencil.stencil; </blockquote><div> </div><div>nit: I would find having "clear_value & 0xff" and all on the second line more readable. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + /* Mark aspects as cleared */ + cmd_state->attachments[ds].pending_clear_aspects = 0; + break; + case BLORP_HIZ_OP_DEPTH_RESOLVE: + hzp.DepthBufferResolveEnable = true; + break; + case BLORP_HIZ_OP_HIZ_RESOLVE: + hzp.HierarchicalDepthBufferResolveEnable = true; + break; + case BLORP_HIZ_OP_NONE: + unreachable("Invalid HiZ OP"); + break; + } + + if (op != BLORP_HIZ_OP_DEPTH_CLEAR) { + /* The Optimized HiZ resolve rectangle must be the size of the full RT + * and aligned to 8x4. The non-optimized Depth resolve rectangle must + * be the size of the full RT. The same alignment is assumed to be + * required. + * + * TODO: + * Consider changing halign of non-D16 depth formats to 8 as mip 2 may + * get clobbered. </blockquote><div> </div><div>This shouldn't be needed on BDW+ </div><div>Most of my comments were just nits. </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + */ + hzp.ClearRectangleXMax = align_u32(iview->extent.width, 8); + hzp.ClearRectangleYMax = align_u32(iview->extent.height, 4); </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + } else { + /* This clear rectangle is aligned */ + hzp.ClearRectangleXMin = cmd_state->render_area.offset.x; + hzp.ClearRectangleYMin = cmd_state->render_area.offset.y; + hzp.ClearRectangleXMax = cmd_state->render_area.offset.x + + cmd_state->render_area.extent.width; + hzp.ClearRectangleYMax = cmd_state->render_area.offset.y + + cmd_state->render_area.extent.height; + } + + + /* Due to a hardware issue, this bit MBZ */ + hzp.ScissorRectangleEnable = false; + hzp.NumberofMultisamples = ffs(iview->image->samples) - 1; + hzp.SampleMask = 0xFFFF; + } + + anv_batch_emit(&cmd_buffer->batch, GENX(PIPE_CONTROL), pc) { + pc.PostSyncOperation = WriteImmediateData; + pc.Address = + (struct anv_address){ &cmd_buffer->device->workaround_bo, 0 }; + } + + anv_batch_emit(&cmd_buffer->batch, GENX(3DSTATE_WM_HZ_OP), hzp); + + /* TODO: + * Determine if a DepthCacheFlushEnable and DepthStallEnable is really + * necessary for non-full_surface_op clears. Performing a HZ op without + * this pipecontrol showed no impact on rendering results. + */ + if (!full_surface_op && op == BLORP_HIZ_OP_DEPTH_CLEAR) { + anv_batch_emit(&cmd_buffer->batch, GENX(PIPE_CONTROL), pc) { + pc.DepthStallEnable = true; + pc.DepthCacheFlushEnable = true; + } + } +} + void genX(CmdSetEvent)( VkCommandBuffer commandBuffer, VkEvent _event, -- 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> </blockquote></div> </div></div>