<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 03/12/2018 22:08, Jason Ekstrand
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAOFGe94NemDhCa2qGR=Di7-VJWXTctoDsU6CWM_nB4yimGp2Ng@mail.gmail.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<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"
moz-do-not-send="true">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" moz-do-not-send="true">lionel.g.landwerlin@intel.com</a>><br>
Bugzilla: <a
href="https://bugs.freedesktop.org/show_bug.cgi?id=108909"
rel="noreferrer" target="_blank" moz-do-not-send="true">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" moz-do-not-send="true">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>
<p><br>
</p>
<p>I could add another bit like "side buffers writes" which would
call for a pixel scoreboard stall only.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CAOFGe94NemDhCa2qGR=Di7-VJWXTctoDsU6CWM_nB4yimGp2Ng@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<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>
</div>
</blockquote>
<p><br>
</p>
<p>Will do.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CAOFGe94NemDhCa2qGR=Di7-VJWXTctoDsU6CWM_nB4yimGp2Ng@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>Reviewed-by: Jason Ekstrand <<a
href="mailto:jason@jlekstrand.net" moz-do-not-send="true">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>
</blockquote>
<p><br>
</p>
</body>
</html>