On Tue, 2018-01-16 at 08:07 -0800, Jason Ekstrand wrote:
> On Mon, Jan 8, 2018 at 4:57 AM, Iago Toral Quiroga <ito...@igalia.com
> > wrote:
> > From the Vulkan spec with KHX extensions:
> >
> >
> >
> > "If queries are used while executing a render pass instance that
> > has
> >
> > multiview enabled, the query uses N consecutive query indices
> >
> > in the query pool (starting at query) where N is the number of
> > bits
> >
> > set in the view mask in the subpass the query is used in.
> >
> >
> >
> > How the numerical results of the query are distributed among the
> >
> > queries is implementation-dependent. For example, some
> > implementations
> >
> > may write each view's results to a distinct query, while other
> >
> > implementations may write the total result to the first query
> > and write
> >
> > zero to the other queries. However, the sum of the results in
> > all the
> >
> > queries must accurately reflect the total result of the query
> > summed
> >
> > over all views. Applications can sum the results from all the
> > queries to
> >
> > compute the total result."
> >
> >
> >
> > In our case we only really emit a single query (in the first query
> > index)
> >
> > that stores the aggregated result for all views, but we still need
> > to manage
> >
> > availability for all the other query indices involved, even if we
> > don't
> >
> > actually use them.
> >
> >
> >
> > This is relevant when clients call vkGetQueryPoolResults and pass
> > all N
> >
> > queries to retrieve the results. In that scenario, without this
> > patch,
> >
> > we will never see queries other than the first being available
> > since we
> >
> > never emit them.
> >
> >
> >
> > Fixes test failures in some work-in-progress CTS multiview+query
> > tests.
> >
> > ---
> >
> > src/intel/vulkan/genX_query.c | 36
> > ++++++++++++++++++++++++++++++++++++
> >
> > 1 file changed, 36 insertions(+)
> >
> >
> >
> > diff --git a/src/intel/vulkan/genX_query.c
> > b/src/intel/vulkan/genX_query.c
> >
> > index 7683d0d1e3..231c605b6b 100644
> >
> > --- a/src/intel/vulkan/genX_query.c
> >
> > +++ b/src/intel/vulkan/genX_query.c
> >
> > @@ -462,6 +462,24 @@ void genX(CmdEndQuery)(
> >
> > default:
> >
> > unreachable("");
> >
> > }
> >
> > +
> >
> > + /* When multiview is active the spec requires that N
> > consecutive query
> >
> > + * indices are used, where N is the number of active views in
> > the subpass.
> >
> > + * The spec allows that we only write the results to one of the
> > queries
> >
> > + * but we still need to manage result availability for all the
> > query indices.
> >
> > + * Since we only emit a single query for all active views in
> > the
> >
> > + * first index, mark the other query indices as being already
> > available
> >
> > + * with result 0.
> >
> > + */
> >
> > + if (!cmd_buffer->state.subpass || !cmd_buffer->state.subpass-
> > >view_mask)
> >
> > + return;
> >
>
> I think this would be better as just an if instead of an early
> return.
>
> > +
> >
> > + uint32_t num_queries = _mesa_bitcount(cmd_buffer-
> > >state.subpass->view_mask);
> >
> > + for (uint32_t i = 1; i < num_queries; i++) {
> >
> > + uint64_t *slot = pool->bo.map + (query + i) * pool->stride;
> >
> > + slot[0] = 1;
> >
> > + memset(&slot[1], 0, sizeof(uint64_t) * pool->stride);
> >
>
> We can't set this from the CPU, we need to emit an MI_STORE_DATA_IMM
> and call emit_query_availability.
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...
> > + }
> >
> > }
> >
> >
> >
> > #define TIMESTAMP 0x2358
> >
> > @@ -504,6 +522,24 @@ void genX(CmdWriteTimestamp)(
> >
> > }
> >
> >
> >
> > emit_query_availability(cmd_buffer, &pool->bo, offset);
> >
> > +
> >
> > + /* When multiview is active the spec requires that N
> > consecutive query
> >
> > + * indices are used, where N is the number of active views in
> > the subpass.
> >
> > + * The spec allows that we only write the results to one of the
> > queries
> >
> > + * but we still need to manage result availability for all the
> > query indices.
> >
> > + * Since we only emit a single query for all active views in
> > the
> >
> > + * first index, mark the other query indices as being already
> > available
> >
> > + * with result 0.
> >
> > + */
> >
> > + if (!cmd_buffer->state.subpass || !cmd_buffer->state.subpass-
> > >view_mask)
> >
> > + return;
> >
> > +
> >
> > + uint32_t num_queries = _mesa_bitcount(cmd_buffer-
> > >state.subpass->view_mask);
> >
> > + for (uint32_t i = 1; i < num_queries; i++) {
> >
> > + uint64_t *slot = pool->bo.map + (query + i) * pool->stride;
> >
> > + slot[0] = 1;
> >
> > + memset(&slot[1], 0, sizeof(uint64_t) * pool->stride);
> >
>
> Same comments here.
>
> > + }
> >
> > }
> >
> >
> >
> > #if GEN_GEN > 7 || GEN_IS_HASWELL
> >
> > --
> >
> > 2.11.0
> >
> >
> > _______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
