<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 10, 2016 at 2:23 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Oct 07, 2016 at 09:41:14PM -0700, Jason Ekstrand wrote:<br>
> If we don't, we can end up with corruption in the portion of the depth<br>
> buffer that lies outside the render area when we do a HiZ resolve at the<br>
> end. The only reason we weren't seeing this before was that all of the<br>
> meta-based clears such as VkCmdClearDepthStencilImage were internally using<br>
> HiZ so the HiZ buffer never truly got out-of-sync. If the CTS ever tested<br>
> a depth upload (which doesn't care about HiZ) and then a partial render we<br>
> would have seen problems. Soon, we will be using blorp to do depth clears<br>
> and it won't bother with HiZ so we would get CTS regressions without this.<br>
><br>
<br>
</span>I understand the problem, but I think this solution unnecessarily<br>
penalizes the user's renderpass.<br>
<br>
Since depth buffer updates via vkCopy*ToImage and<br>
vkCmdClearDepthStencilImage cause the HiZ buffer to become stale,<br>
calling<br>
<br>
genX(cmd_buffer_emit_hz_op)(<wbr>cmd_buffer, BLORP_HIZ_OP_HIZ_RESOLVE);<br>
<br>
at the bottom of those commands should fix the issue without the extra<br>
penalty. I'd imagine that as a prequisite, blorp would have to learn to<br>
emit enough depth stencil state for this command.<br></blockquote><div><br></div><div>I think that's dangerously mixing HiZ data validity models. There are 3 basic aux data validity models that we've thrown around:<br><br></div><div> 1) AUX is always correct.<br></div><div> 2) AUX is correct within a render pass and invalid outside.<br></div><div> 3) Track whether or not AUX is valid and resolve only as needed.<br><br></div><div>So far, our initial enabling strategy has been (2). We can move to (1) or maybe even (3) with layout transitions, but that's not what we've done so far. Your suggestion is to mix in a little of (1) because there is a bug in our implementation of (2).<br><br></div><div>I'm Ok, for gen8+ HiZ, with moving to (2) eventually since gen8 is capable of sampling with HiZ. (Maybe we can even move to (3).) However, that's not where we are right now and I don't really want to start mixing mental models.<br><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-Nanley<br>
<div><div class="h5"><br>
> Signed-off-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
> ---<br>
> src/intel/vulkan/gen8_cmd_<wbr>buffer.c | 12 +++++++++++-<br>
> 1 file changed, 11 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/intel/vulkan/gen8_cmd_<wbr>buffer.c b/src/intel/vulkan/gen8_cmd_<wbr>buffer.c<br>
> index e6a3c3d..44ffcbf 100644<br>
> --- a/src/intel/vulkan/gen8_cmd_<wbr>buffer.c<br>
> +++ b/src/intel/vulkan/gen8_cmd_<wbr>buffer.c<br>
> @@ -513,7 +513,17 @@ genX(cmd_buffer_emit_hz_op)(<wbr>struct anv_cmd_buffer *cmd_buffer,<br>
> return;<br>
> break;<br>
> case BLORP_HIZ_OP_HIZ_RESOLVE:<br>
> - if (cmd_buffer->state.pass-><wbr>attachments[ds].load_op !=<br>
> + /* If the render area covers the entire surface *and* load_op is either<br>
> + * CLEAR or DONT_CARE then the previous contents of the depth buffer<br>
> + * will be entirely discarded. In this case, we can skip the HiZ<br>
> + * resolve.<br>
> + *<br>
> + * If the render area is not the full surface, we need to do<br>
> + * the resolve because otherwise data outside the render area may get<br>
> + * garbled by the resolve at the end of the render pass.<br>
> + */<br>
> + if (full_surface_op &&<br>
> + cmd_buffer->state.pass-><wbr>attachments[ds].load_op !=<br>
> VK_ATTACHMENT_LOAD_OP_LOAD)<br>
> return;<br>
> break;<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">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><br></div></div>