<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 28, 2017 at 5:54 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"><div class="HOEnZb"><div class="h5">On Mon, Feb 27, 2017 at 08:41:56PM -0800, Jason Ekstrand wrote: > On Feb 27, 2017 5:21 PM, "Nanley Chery" <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> wrote: > > Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> > --- > src/intel/vulkan/genX_cmd_buffer.c | 57 ++++++++++-------------------- > -------- > 1 file changed, 15 insertions(+), 42 deletions(-) > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c > b/src/intel/vulkan/genX_cmd_buffer.c > index 5171e6f587..60230bf14b 100644 > --- a/src/intel/vulkan/genX_cmd_buffer.c > +++ b/src/intel/vulkan/genX_cmd_buffer.c > @@ -326,27 +326,6 @@ need_input_attachment_state(const struct > anv_render_pass_attachment *att) > return vk_format_is_color(att->format); > } > > -static enum isl_aux_usage > -layout_to_hiz_usage(VkImageLayout layout, uint8_t samples) > -{ > - switch (layout) { > - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: > - return ISL_AUX_USAGE_HIZ; > - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL: > - case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: > - if (anv_can_sample_with_hiz(GEN_GEN, samples)) > - return ISL_AUX_USAGE_HIZ; > - /* Fall-through */ > - case VK_IMAGE_LAYOUT_GENERAL: > - /* This buffer could be used as a source or destination in a transfer > - * operation. Transfer operations current don't perform HiZ-enabled > reads > - * and writes. > - */ > - default: > - return ISL_AUX_USAGE_NONE; > - } > -} > - > /* Transitions a HiZ-enabled depth buffer from one layout to another. > Unless > * the initial layout is undefined, the HiZ buffer and depth buffer will > * represent the same data at the end of this operation. > @@ -362,21 +341,15 @@ transition_depth_buffer(struct anv_cmd_buffer > *cmd_buffer, > if (image->aux_usage != ISL_AUX_USAGE_HIZ || final_layout == > initial_layout) > return; > > - const bool hiz_enabled = layout_to_hiz_usage(initial_layout, > image->samples) == > - ISL_AUX_USAGE_HIZ; > - const bool enable_hiz = layout_to_hiz_usage(final_layout, > image->samples) == > - ISL_AUX_USAGE_HIZ; > + const bool hiz_enabled = ISL_AUX_USAGE_HIZ == > + anv_layout_to_aux_usage(GEN_GEN, image, image->aspects, > + initial_layout, final_layout); > + const bool enable_hiz = ISL_AUX_USAGE_HIZ == > + anv_layout_to_aux_usage(GEN_GEN, image, image->aspects, > + final_layout, final_layout); > > enum blorp_hiz_op hiz_op; > - if (initial_layout == VK_IMAGE_LAYOUT_UNDEFINED) { > > > I'm not sure how I feel about handling this in layout_to_aux_usage. It > seems like a conflation of two things. If this is the only reason for the > future_layout parameter, then it seems like it would be better handled > here. In any case, I'll keep reading > </div></div>Yes, that is the only reason for the future_layout parameter. I can remove it if you'd like. Currently the layout_to_aux_usage function determines the optimal aux_usage for all device accesses: sampling, rendering, transferring, and resolves. Removing the future_layout parameter would remove its responsibility for the last access type. </blockquote><div> </div><div>In my brain, a transition from LAYOUT_UNDEFINED to LAYOUT_* is a transition from AUX_USAGE_NONE to AUX_USAGE_*. However, we know based on some other bits of the driver, that we can skip that particular resolve because the HiZ surface is in a non-hanging state. Also, the second parameter makes the function far less obvious and every other call sight of the function, even though it doesn't care, has to think about that parmeter. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> -Nanley <div class="HOEnZb"><div class="h5"> > - /* We've already initialized the aux HiZ buffer at BindImageMemory > time, > - * so there's no need to perform a HIZ resolve or clear to avoid GPU > hangs. > - * This initial layout indicates that the user doesn't care about > the data > - * that's currently in the buffer, so resolves are not necessary > except > - * for the special case noted below. > - */ > - hiz_op = BLORP_HIZ_OP_NONE; > - } else if (hiz_enabled && !enable_hiz) { > + if (hiz_enabled && !enable_hiz) { > hiz_op = BLORP_HIZ_OP_DEPTH_RESOLVE; > } else if (!hiz_enabled && enable_hiz) { > hiz_op = BLORP_HIZ_OP_HIZ_RESOLVE; > @@ -556,12 +529,11 @@ genX(cmd_buffer_setup_attachments)(struct > anv_cmd_buffer *cmd_buffer, > state->attachments[i].aux_usage, > state->attachments[i].color_rt_state); > } else { > - if (iview->image->aux_usage == ISL_AUX_USAGE_HIZ) { > - state->attachments[i].aux_usage = > - layout_to_hiz_usage(att->initial_layout, > iview->image->samples); > - } else { > - state->attachments[i].aux_usage = ISL_AUX_USAGE_NONE; > - } > + /* This field will be initialized after the first subpass > + * transition. > + */ > + state->attachments[i].aux_usage = ISL_AUX_USAGE_NONE; > + > state->attachments[i].input_aux_usage = ISL_AUX_USAGE_NONE; > } > > @@ -2399,8 +2371,9 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer > *cmd_buffer, > cmd_buffer->state.attachments[ds].current_layout = > cmd_buffer->state.subpass->depth_stencil_layout; > cmd_buffer->state.attachments[ds].aux_usage = > - layout_to_hiz_usage(cmd_buffer->state.subpass->depth_stenci > l_layout, > - iview->image->samples); > + anv_layout_to_aux_usage(GEN_GEN, iview->image, iview->aspect_mask, > + cmd_buffer->state.subpass->depth_stencil_layout, > + cmd_buffer->state.subpass->depth_stencil_layout); > } > > cmd_buffer_emit_depth_stencil(cmd_buffer); > -- > 2.11.1 > > _______________________________________________ > 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>