<div dir="ltr">You're going to want to re-review this one when I send the v2.  It changed quite a bit.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 13, 2018 at 2:44 PM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Tue, Feb 13, 2018 at 11:02 AM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_6257391089241253652HOEnZb"><div class="m_6257391089241253652h5">On Mon, Feb 05, 2018 at 02:34:56PM -0800, Jason Ekstrand wrote:<br>
> This moves the decision out of begin_subpass and into BeginRenderPass<br>
> like the decision for color clears.  We use a similar name for the<br>
> function for depth/stencil as for color even though no aux usage is<br>
> really getting computed.<br>
> ---<br>
>  src/intel/vulkan/genX_cmd_buff<wbr>er.c | 84 +++++++++++++++++++++++-------<wbr>--------<br>
>  1 file changed, 50 insertions(+), 34 deletions(-)<br>
><br>
> diff --git a/src/intel/vulkan/genX_cmd_bu<wbr>ffer.c b/src/intel/vulkan/genX_cmd_bu<wbr>ffer.c<br>
> index 21fdc6b..ab79fbf 100644<br>
> --- a/src/intel/vulkan/genX_cmd_bu<wbr>ffer.c<br>
> +++ b/src/intel/vulkan/genX_cmd_bu<wbr>ffer.c<br>
> @@ -350,6 +350,52 @@ color_attachment_compute_aux_u<wbr>sage(struct anv_device * device,<br>
>  Â  Â }<br>
>  }<br>
><br>
> +static void<br>
> +depth_stencil_attachment_comp<wbr>ute_aux_usage(struct anv_device *device,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â struct anv_cmd_state *cmd_state,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â uint32_t att, VkRect2D render_area)<br>
> +{<br>
> +  Â struct anv_attachment_state *att_state = &cmd_state->attachments[att];<br>
> +  Â struct anv_image_view *iview = cmd_state->framebuffer->attach<wbr>ments[att];<br>
> +<br>
> +  Â /* These will be initialized after the first subpass transition. */<br>
> +  Â att_state->aux_usage = ISL_AUX_USAGE_NONE;<br>
> +  Â att_state->input_aux_usage = ISL_AUX_USAGE_NONE;<br>
> +<br>
> +  Â if (att_state->aux_usage != ISL_AUX_USAGE_HIZ) {<br>
<br>
</div></div>We set this to NONE 3 lines above. I think you meant to have your<br>
variable be: iview->image->planes[plane].au<wbr>x_usage ?<br></blockquote><div><br></div></div></div><div>Yup.  You're right.  Fixed locally.<br><br>This means I accidentally disabled HiZ clears entirely. :(  This is going to need another jenkins run.<br></div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This is a nice cleanup. With the above fixed, this patch is<br>
Reviewed-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com" target="_blank">nanley.g.chery@intel.com</a>><br>
<div><div class="m_6257391089241253652h5"><br>
<br>
> +  Â  Â  att_state->fast_clear = false;<br>
> +  Â  Â  return;<br>
> +  Â } else if (!(att_state->pending_clear_as<wbr>pects & VK_IMAGE_ASPECT_DEPTH_BIT)) {<br>
> +  Â  Â  /* If we're just clearing stencil, we can always HiZ clear */<br>
> +  Â  Â  att_state->fast_clear = true;<br>
> +  Â  Â  return;<br>
> +  Â }<br>
> +<br>
> +  Â if (!blorp_can_hiz_clear_depth(GE<wbr>N_GEN,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  iview->planes[0].isl.format,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  iview->image->samples,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  render_area.offset.x,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  render_area.offset.y,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  render_area.offset.x +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  render_area.extent.width,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  render_area.offset.y +<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  render_area.extent.height)) {<br>
> +  Â  Â  att_state->fast_clear = false;<br>
> +  Â } else if (att_state->clear_value.depthS<wbr>tencil.depth != ANV_HZ_FC_VAL) {<br>
> +  Â  Â  att_state->fast_clear = false;<br>
> +  Â } else if (GEN_GEN == 8 &&<br>
> +  Â  Â  Â  Â  Â  Â  anv_can_sample_with_hiz(&devic<wbr>e->info, iview->image)) {<br>
> +  Â  Â  /* Only gen9+ supports returning ANV_HZ_FC_VAL when sampling a<br>
> +  Â  Â  Â * fast-cleared portion of a HiZ buffer. Testing has revealed that Gen8<br>
> +  Â  Â  Â * only supports returning 0.0f. Gens prior to gen8 do not support this<br>
> +  Â  Â  Â * feature at all.<br>
> +  Â  Â  Â */<br>
> +  Â  Â  att_state->fast_clear = false;<br>
> +  Â } else {<br>
> +  Â  Â  att_state->fast_clear = true;<br>
> +  Â }<br>
> +}<br>
> +<br>
>  static bool<br>
>  need_input_attachment_state(co<wbr>nst struct anv_render_pass_attachment *att)<br>
>  {<br>
> @@ -1125,12 +1171,9 @@ genX(cmd_buffer_setup_attachme<wbr>nts)(struct anv_cmd_buffer *cmd_buffer,<br>
>  Â  Â  Â  Â  Â  Â  add_image_view_relocs(cmd_buff<wbr>er, iview, 0,<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  state->attachments[i].color);<br>
>  Â  Â  Â  Â  Â } else {<br>
> -  Â  Â  Â  Â  Â  /* This field will be initialized after the first subpass<br>
> -  Â  Â  Â  Â  Â  Â * transition.<br>
> -  Â  Â  Â  Â  Â  Â */<br>
> -  Â  Â  Â  Â  Â  state->attachments[i].aux_usag<wbr>e = ISL_AUX_USAGE_NONE;<br>
> -<br>
> -  Â  Â  Â  Â  Â  state->attachments[i].input_au<wbr>x_usage = ISL_AUX_USAGE_NONE;<br>
> +  Â  Â  Â  Â  Â  depth_stencil_attachment_compu<wbr>te_aux_usage(cmd_buffer->devic<wbr>e,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â state, i,<br>
> +  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â begin->renderArea);<br>
>  Â  Â  Â  Â  Â }<br>
><br>
>  Â  Â  Â  Â  Â if (need_input_attachment_state(&<wbr>pass->attachments[i])) {<br>
> @@ -3541,34 +3584,7 @@ cmd_buffer_begin_subpass(struc<wbr>t anv_cmd_buffer *cmd_buffer,<br>
>  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â VK_IMAGE_ASPECT_STENCIL_BIT))<wbr>;<br>
><br>
>  Â  Â  Â  if (att_state->pending_clear_aspe<wbr>cts) {<br>
> -  Â  Â  Â  Â bool clear_with_hiz = att_state->aux_usage == ISL_AUX_USAGE_HIZ;<br>
> -  Â  Â  Â  Â if (clear_with_hiz &&<br>
> -  Â  Â  Â  Â  Â  Â (att_state->pending_clear_asp<wbr>ects & VK_IMAGE_ASPECT_DEPTH_BIT)) {<br>
> -  Â  Â  Â  Â  Â  if (!blorp_can_hiz_clear_depth(GE<wbr>N_GEN,<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â iview->planes[0].isl.format,<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â iview->image->samples,<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â render_area.offset.x,<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â render_area.offset.y,<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â render_area.offset.x +<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â render_area.extent.width,<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â render_area.offset.y +<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â render_area.extent.height)) {<br>
> -  Â  Â  Â  Â  Â  Â  Â clear_with_hiz = false;<br>
> -  Â  Â  Â  Â  Â  } else if (att_state->clear_value.depthS<wbr>tencil.depth != ANV_HZ_FC_VAL) {<br>
> -  Â  Â  Â  Â  Â  Â  Â clear_with_hiz = false;<br>
> -  Â  Â  Â  Â  Â  } else if (GEN_GEN == 8 &&<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â anv_can_sample_with_hiz(&cmd_<wbr>buffer->device->info,<br>
> -  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â iview->image)) {<br>
> -  Â  Â  Â  Â  Â  Â  Â /* Only gen9+ supports returning ANV_HZ_FC_VAL when sampling a<br>
> -  Â  Â  Â  Â  Â  Â  Â  * fast-cleared portion of a HiZ buffer. Testing has revealed<br>
> -  Â  Â  Â  Â  Â  Â  Â  * that Gen8 only supports returning 0.0f. Gens prior to gen8<br>
> -  Â  Â  Â  Â  Â  Â  Â  * do not support this feature at all.<br>
> -  Â  Â  Â  Â  Â  Â  Â  */<br>
> -  Â  Â  Â  Â  Â  Â  Â clear_with_hiz = false;<br>
> -  Â  Â  Â  Â  Â  }<br>
> -  Â  Â  Â  Â }<br>
> -<br>
> -  Â  Â  Â  Â if (clear_with_hiz) {<br>
> +  Â  Â  Â  Â if (att_state->fast_clear) {<br>
>  Â  Â  Â  Â  Â  Â  /* We currently only support HiZ for single-layer images */<br>
>  Â  Â  Â  Â  Â  Â  assert(iview->planes[0].<a href="http://isl.ba">isl.ba</a><wbr>se_level == 0);<br>
>  Â  Â  Â  Â  Â  Â  assert(iview->planes[0].<a href="http://isl.ba">isl.ba</a><wbr>se_array_layer == 0);<br>
> --<br>
> 2.5.0.400.gff86faf<br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">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>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>