<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jan 11, 2017 at 5:55 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>><br>
---<br>
 src/intel/vulkan/genX_cmd_<wbr>buffer.c | 54 +++++++++++++++++++++++++++++-<wbr>--------<br>
 1 file changed, 41 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/src/intel/vulkan/genX_cmd_<wbr>buffer.c b/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
index 447baa08b2..11745f8b9e 100644<br>
--- a/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
+++ b/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
@@ -311,11 +311,21 @@ need_input_attachment_state(<wbr>const struct anv_render_pass_attachment *att)<br>
 }<br>
<br>
 static enum isl_aux_usage<br>
-layout_to_hiz_usage(<wbr>VkImageLayout layout)<br>
+layout_to_hiz_usage(<wbr>VkImageLayout layout, uint8_t samples)<br>
 {<br>
    switch (layout) {<br>
    case VK_IMAGE_LAYOUT_DEPTH_STENCIL_<wbr>ATTACHMENT_OPTIMAL:<br>
       return ISL_AUX_USAGE_HIZ;<br>
+   case VK_IMAGE_LAYOUT_DEPTH_STENCIL_<wbr>READ_ONLY_OPTIMAL:<br>
+   case VK_IMAGE_LAYOUT_SHADER_READ_<wbr>ONLY_OPTIMAL:<br>
+      if (anv_can_sample_with_hiz(GEN_<wbr>GEN, samples))<br>
+         return ISL_AUX_USAGE_HIZ;<br>
+      /* Fall-through */<br>
+   case VK_IMAGE_LAYOUT_GENERAL:<br>
+      /* This buffer could be used as a source or destination in a transfer<br>
+       * operation. Transfer operations current don't perform HiZ-enabled reads<br>
+       * and writes.<br>
+       */<br>
    default:<br>
       return ISL_AUX_USAGE_NONE;<br>
    }<br>
@@ -336,26 +346,43 @@ transition_depth_buffer(struct anv_cmd_buffer *cmd_buffer,<br>
    if (image->aux_usage != ISL_AUX_USAGE_HIZ)<br>
       return;<br>
<br>
-   const bool hiz_enabled = layout_to_hiz_usage(initial_<wbr>layout) ==<br>
+   const bool hiz_enabled = layout_to_hiz_usage(initial_<wbr>layout, image->samples) ==<br>
                             ISL_AUX_USAGE_HIZ;<br>
-   const bool enable_hiz = layout_to_hiz_usage(final_<wbr>layout) ==<br>
+   const bool enable_hiz = layout_to_hiz_usage(final_<wbr>layout, image->samples) ==<br>
                            ISL_AUX_USAGE_HIZ;<br>
<br>
+   /* Images that have sampling with HiZ enabled cause all shader sampling to<br>
+    * load data with the HiZ buffer. Therefore, in the case of transitioning to<br>
+    * the general layout - which currently routes all writes to the depth<br>
+    * buffer - we must ensure that the HiZ buffer remains consistent with the<br>
+    * depth buffer by performing a HIZ resolve after performing the resolve<br>
+    * required by this transition (if not already HiZ).<br>
+    */<br>
+   const bool needs_hiz_resolve = final_layout == VK_IMAGE_LAYOUT_GENERAL &&<br>
+      (hiz_enabled || initial_layout == VK_IMAGE_LAYOUT_UNDEFINED) &&<br>
+      anv_can_sample_with_hiz(GEN_<wbr>GEN, image->samples);<br>
+<br>
    /* We've already initialized the aux HiZ buffer at BindImageMemory time,<br>
     * so there's no need to perform a HIZ resolve or clear to avoid GPU hangs.<br>
     * This initial layout indicates that the user doesn't care about the data<br>
-    * that's currently in the buffer, so no resolves are necessary.<br>
+    * that's currently in the buffer, so resolves are not necessary except for<br>
+    * the case mentioned above.<br>
     */<br>
-   if (initial_layout == VK_IMAGE_LAYOUT_UNDEFINED)<br>
+   if (!needs_hiz_resolve && initial_layout == VK_IMAGE_LAYOUT_UNDEFINED)<br>
       return;<br>
<br>
-   if (hiz_enabled == enable_hiz) {<br>
-      /* The same buffer will be used, no resolves are necessary */<br>
-   } else if (hiz_enabled && !enable_hiz) {<br>
-      anv_gen8_hiz_op_resolve(cmd_<wbr>buffer, image, BLORP_HIZ_OP_DEPTH_RESOLVE);<br>
+   if (!hiz_enabled && enable_hiz) {<br>
+         anv_gen8_hiz_op_resolve(cmd_<wbr>buffer, image, BLORP_HIZ_OP_HIZ_RESOLVE);<br>
    } else {<br>
-      assert(!hiz_enabled && enable_hiz);<br>
-      anv_gen8_hiz_op_resolve(cmd_<wbr>buffer, image, BLORP_HIZ_OP_HIZ_RESOLVE);<br>
+      if (hiz_enabled == enable_hiz) {<br>
+         /* If the same buffer will be used, no resolves are necessary except<br>
+          * for the special case noted above.<br>
+          */<br>
+      } else if (hiz_enabled && !enable_hiz) {<br>
+         anv_gen8_hiz_op_resolve(cmd_<wbr>buffer, image, BLORP_HIZ_OP_DEPTH_RESOLVE);<br>
+      }<br>
+      if (needs_hiz_resolve) <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+         anv_gen8_hiz_op_resolve(cmd_<wbr>buffer, image, BLORP_HIZ_OP_HIZ_RESOLVE);<br></blockquote><div><br></div><div>I think this function would be way easier to read if it was structured a bit differently.  How about</div><div><br></div><div>enum blorp_hiz_op hiz_op;<br></div><div>if (initial_layout == UNDEFINED) {<br></div><div>   /* comment */<br></div><div>   hiz_op = BLORP_HIZ_OP_NONE;<br></div><div>} else if (hiz_enabled && !enable_hiz) {<br></div><div>   hiz_op = BLORP_HIZ_OP_DEPTH_RESOLVE;<br></div><div>} else if (!hiz_enabled && enable_hiz) {<br></div><div>   hiz_op = BLORP_HIZ_OP_HIZ_RESOLVE;<br></div><div>} else {<br></div><div>   hiz_op = BLORP_HIZ_OP_NONE;<br>}<br><br></div><div>if (hiz_op != BLORP_HIZ_OP_NONE)<br></div><div>   anv_gen8_hiz_op_resolve(cmd_buffer, image, hiz_op)<br><br></div><div>/* comment */<br></div><div>if (final_layout == GENERAL && can_sample_from_hiz && hiz_op != BLORP_HIZ_OP_HIZ_RESOLVE)<br></div><div>   anv_gen8_hiz_op_resolve(cmd_buffer, image, BLORP_HIZ_OP_HIZ_RESOLVE);<br></div><br></div><div class="gmail_quote">I *think* that accomplishes the same thing and it makes way more sense in my brain than all of the complicated logic.<br></div><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
    }<br>
 }<br>
<br>
@@ -513,7 +540,7 @@ genX(cmd_buffer_setup_<wbr>attachments)(struct anv_cmd_buffer *cmd_buffer,<br>
             if (iview->image->aux_usage == ISL_AUX_USAGE_HIZ &&<br>
                 iview->aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT) {<br>
                state->attachments[i].aux_<wbr>usage =<br>
-                  layout_to_hiz_usage(att-><wbr>initial_layout);<br>
+                  layout_to_hiz_usage(att-><wbr>initial_layout, iview->image->samples);<br>
             } else {<br>
                state->attachments[i].aux_<wbr>usage = ISL_AUX_USAGE_NONE;<br>
             }<br>
@@ -2319,7 +2346,8 @@ genX(cmd_buffer_set_subpass)(<wbr>struct anv_cmd_buffer *cmd_buffer,<br>
       cmd_buffer->state.attachments[<wbr>ds].current_layout =<br>
          cmd_buffer->state.subpass-><wbr>depth_stencil_layout;<br>
       cmd_buffer->state.attachments[<wbr>ds].aux_usage =<br>
-         layout_to_hiz_usage(cmd_<wbr>buffer->state.subpass->depth_<wbr>stencil_layout);<br>
+         layout_to_hiz_usage(cmd_<wbr>buffer->state.subpass->depth_<wbr>stencil_layout,<br>
+                             iview->image->samples);<br>
    }<br>
<br>
    cmd_buffer_emit_depth_stencil(<wbr>cmd_buffer);<br>
<span class="gmail-HOEnZb"><font color="#888888">--<br>
2.11.0<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">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><br></div></div>