<html><head></head><body><div>On Tue, 2018-01-16 at 08:07 -0800, Jason Ekstrand wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 8, 2018 at 4:57 AM, Iago Toral Quiroga <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><span class="">From the Vulkan spec with KHX extensions:<br>
<br>
"If queries are used while executing a render pass instance that has<br>
multiview enabled, the query uses N consecutive query indices<br>
in the query pool (starting at query) where N is the number of bits<br>
set in the view mask in the subpass the query is used in.<br>
<br>
How the numerical results of the query are distributed among the<br>
queries is implementation-dependent. For example, some implementations<br>
may write each view's results to a distinct query, while other<br>
implementations may write the total result to the first query and write<br>
zero to the other queries. However, the sum of the results in all the<br>
queries must accurately reflect the total result of the query summed<br>
over all views. Applications can sum the results from all the queries to<br>
compute the total result."<br>
<br>
In our case we only really emit a single query (in the first query index)<br>
that stores the aggregated result for all views, but we still need to manage<br>
availability for all the other query indices involved, even if we don't<br>
actually use them.<br>
<br>
This is relevant when clients call vkGetQueryPoolResults and pass all N<br>
queries to retrieve the results. In that scenario, without this patch,<br>
we will never see queries other than the first being available since we<br>
never emit them.<br>
<br>
Fixes test failures in some work-in-progress CTS multiview+query tests.<br>
---<br>
</span> src/intel/vulkan/genX_query.c | 36 ++++++++++++++++++++++++++++++++++++<br>
1 file changed, 36 insertions(+)<br>
<br>
diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c<br>
index 7683d0d1e3..231c605b6b 100644<br>
<span class="">--- a/src/intel/vulkan/genX_query.<wbr>c<br>
+++ b/src/intel/vulkan/genX_query.<wbr>c<br>
@@ -462,6 +462,24 @@ void genX(CmdEndQuery)(<br>
default:<br>
unreachable("");<br>
}<br>
+<br>
+ /* When multiview is active the spec requires that N consecutive query<br>
+ * indices are used, where N is the number of active views in the subpass.<br>
+ * The spec allows that we only write the results to one of the queries<br>
+ * but we still need to manage result availability for all the query indices.<br>
+ * Since we only emit a single query for all active views in the<br>
+ * first index, mark the other query indices as being already available<br>
+ * with result 0.<br>
+ */<br>
+ if (!cmd_buffer->state.subpass || !cmd_buffer->state.subpass-><wbr>view_mask)<br>
+ return;<br></span><br></blockquote><div><br></div><div>I think this would be better as just an if instead of an early return.<br></div><div> </div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><span class="">
+<br>
+ uint32_t num_queries = _mesa_bitcount(cmd_buffer-><wbr>state.subpass->view_mask);<br>
+ for (uint32_t i = 1; i < num_queries; i++) {<br>
+ uint64_t *slot = pool->bo.map + (query + i) * pool->stride;<br>
+ slot[0] = 1;<br>
+ memset(&slot[1], 0, sizeof(uint64_t) * pool->stride);<br></span><br></blockquote><div><br></div><div>We can't set this from the CPU, we need to emit an MI_STORE_DATA_IMM and call emit_query_availability.<br></div></div></div></div></blockquote><div><br></div><div>Oh ok, I'll do that then. Thanks for the feedback! Out of curiosity, what is the problem with setting this on the CPU side? I was thinking that since we are not emitting these queries at all the GPU is not going to touch the memory for them anyway...</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><span class="">
+ }<br>
}<br>
<br>
#define TIMESTAMP 0x2358<br>
</span>@@ -504,6 +522,24 @@ void genX(CmdWriteTimestamp)(<br>
}<br>
<br>
emit_query_availability(cmd_buffer, &pool->bo, offset);<br>
<span class="">+<br>
+ /* When multiview is active the spec requires that N consecutive query<br>
+ * indices are used, where N is the number of active views in the subpass.<br>
+ * The spec allows that we only write the results to one of the queries<br>
+ * but we still need to manage result availability for all the query indices.<br>
+ * Since we only emit a single query for all active views in the<br>
+ * first index, mark the other query indices as being already available<br>
+ * with result 0.<br>
+ */<br>
+ if (!cmd_buffer->state.subpass || !cmd_buffer->state.subpass-><wbr>view_mask)<br>
+ return;<br>
+<br>
+ uint32_t num_queries = _mesa_bitcount(cmd_buffer-><wbr>state.subpass->view_mask);<br>
+ for (uint32_t i = 1; i < num_queries; i++) {<br>
+ uint64_t *slot = pool->bo.map + (query + i) * pool->stride;<br>
+ slot[0] = 1;<br>
+ memset(&slot[1], 0, sizeof(uint64_t) * pool->stride);<br></span><br></blockquote><div><br></div><div>Same comments here.<br></div><div> </div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><span class="">
+ }<br>
}<br>
<br>
</span> #if GEN_GEN > 7 || GEN_IS_HASWELL<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.11.0<br>
<br>
</font></span></blockquote></div><br></div></div>
</blockquote></body></html>