<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Dec 3, 2018 at 11:26 AM Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This change tracks render target writes in the pipeline and applies a<br>
render target flush before copying the query results to make sure the<br>
preceding operations have landed in memory before the command streamer<br>
initiates the copy.<br>
<br>
Signed-off-by: Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com" target="_blank">lionel.g.landwerlin@intel.com</a>><br>
Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=108909" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=108909</a><br>
Fixes: 37f9788e9a8e44 ("anv: flush pipeline before query result copies")<br>
Cc: <a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a><br>
---<br>
src/intel/vulkan/anv_private.h | 7 +++++++<br>
src/intel/vulkan/genX_blorp_exec.c | 1 +<br>
src/intel/vulkan/genX_cmd_buffer.c | 14 ++++++++++++++<br>
src/intel/vulkan/genX_gpu_memcpy.c | 1 +<br>
src/intel/vulkan/genX_query.c | 10 +++++++++-<br>
5 files changed, 32 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h<br>
index 62c563294f5..aff076a55d9 100644<br>
--- a/src/intel/vulkan/anv_private.h<br>
+++ b/src/intel/vulkan/anv_private.h<br>
@@ -1747,6 +1747,13 @@ enum anv_pipe_bits {<br>
* we would have to CS stall on every flush which could be bad.<br>
*/<br>
ANV_PIPE_NEEDS_CS_STALL_BIT = (1 << 21),<br>
+<br>
+ /* This bit does not exist directly in PIPE_CONTROL. It means that render<br>
+ * target operations are ongoing. Some operations like copies on the<br>
+ * command streamer might need to be aware of this to trigger the<br>
+ * appropriate stall before they can proceed with the copy.<br>
+ */<br>
+ ANV_PIPE_RENDER_TARGET_WRITES = (1 << 22),<br>
};<br>
<br>
#define ANV_PIPE_FLUSH_BITS ( \<br>
diff --git a/src/intel/vulkan/genX_blorp_exec.c b/src/intel/vulkan/genX_blorp_exec.c<br>
index 2035017ce0e..c573e890946 100644<br>
--- a/src/intel/vulkan/genX_blorp_exec.c<br>
+++ b/src/intel/vulkan/genX_blorp_exec.c<br>
@@ -263,4 +263,5 @@ genX(blorp_exec)(struct blorp_batch *batch,<br>
cmd_buffer->state.gfx.vb_dirty = ~0;<br>
cmd_buffer->state.gfx.dirty = ~0;<br>
cmd_buffer->state.push_constants_dirty = ~0;<br>
+ cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_RENDER_TARGET_WRITES;<br>
}<br>
diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c<br>
index c7e5ef9596e..fb70cd2e386 100644<br>
--- a/src/intel/vulkan/genX_cmd_buffer.c<br>
+++ b/src/intel/vulkan/genX_cmd_buffer.c<br>
@@ -1766,6 +1766,12 @@ genX(cmd_buffer_apply_pipe_flushes)(struct anv_cmd_buffer *cmd_buffer)<br>
pipe.StallAtPixelScoreboard = true;<br>
}<br>
<br>
+ /* If a render target flush was emitted, then we can toggle off the bit<br>
+ * saying that render target writes are ongoing.<br>
+ */<br>
+ if (bits & ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT)<br>
+ bits &= ~(ANV_PIPE_RENDER_TARGET_WRITES);<br>
+<br>
bits &= ~(ANV_PIPE_FLUSH_BITS | ANV_PIPE_CS_STALL_BIT);<br>
}<br>
<br>
@@ -2777,6 +2783,8 @@ void genX(CmdDraw)(<br>
prim.StartInstanceLocation = firstInstance;<br>
prim.BaseVertexLocation = 0;<br>
}<br>
+<br>
+ cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_RENDER_TARGET_WRITES;<br>
}<br>
<br>
void genX(CmdDrawIndexed)(<br>
@@ -2816,6 +2824,8 @@ void genX(CmdDrawIndexed)(<br>
prim.StartInstanceLocation = firstInstance;<br>
prim.BaseVertexLocation = vertexOffset;<br>
}<br>
+<br>
+ cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_RENDER_TARGET_WRITES;<br>
}<br>
<br>
/* Auto-Draw / Indirect Registers */<br>
@@ -2949,6 +2959,8 @@ void genX(CmdDrawIndirect)(<br>
<br>
offset += stride;<br>
}<br>
+<br>
+ cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_RENDER_TARGET_WRITES;<br>
}<br>
<br>
void genX(CmdDrawIndexedIndirect)(<br>
@@ -2988,6 +3000,8 @@ void genX(CmdDrawIndexedIndirect)(<br>
<br>
offset += stride;<br>
}<br>
+<br>
+ cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_RENDER_TARGET_WRITES;<br>
}<br>
<br>
static VkResult<br>
diff --git a/src/intel/vulkan/genX_gpu_memcpy.c b/src/intel/vulkan/genX_gpu_memcpy.c<br>
index 81522986550..1bee1c6dc17 100644<br>
--- a/src/intel/vulkan/genX_gpu_memcpy.c<br>
+++ b/src/intel/vulkan/genX_gpu_memcpy.c<br>
@@ -302,4 +302,5 @@ genX(cmd_buffer_so_memcpy)(struct anv_cmd_buffer *cmd_buffer,<br>
}<br>
<br>
cmd_buffer->state.gfx.dirty |= ANV_CMD_DIRTY_PIPELINE;<br>
+ cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_RENDER_TARGET_WRITES;<br></blockquote><div><br></div><div>This is a bit odd since there are no RT writes going on.... Still, probably accurate enough so meh.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
}<br>
diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c<br>
index 4831c4ea334..130c9661f15 100644<br>
--- a/src/intel/vulkan/genX_query.c<br>
+++ b/src/intel/vulkan/genX_query.c<br>
@@ -730,7 +730,15 @@ void genX(CmdCopyQueryPoolResults)(<br>
ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);<br>
<br>
if ((flags & VK_QUERY_RESULT_WAIT_BIT) ||<br>
- (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)) {<br>
+ (cmd_buffer->state.pending_pipe_bits &<br>
+ (ANV_PIPE_FLUSH_BITS | ANV_PIPE_RENDER_TARGET_WRITES))) {<br>
+ /* If render target writes are ongoing, request a render target cache<br>
+ * flush.<br>
+ */<br>
+ if (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_RENDER_TARGET_WRITES) {<br>
+ cmd_buffer->state.pending_pipe_bits |=<br>
+ ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;<br>
+ }<br></blockquote><div><br></div><div>You could also just put the above bit above the whole if and you wouldn't need to explicitly check ANV_PIPE_RENDER_TARGET_WRITE there. I'm not sure that's actually better; it just looks like less code. Either way,</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>></div><div><br></div><div>Thanks for sorting this out!</div><div><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">
cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;<br>
genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);<br>
}<br>
-- <br>
2.20.0.rc1<br>
<br>
</blockquote></div></div>