[Mesa-dev] [PATCH v2 1/4] anv/query: implement multiview interactions

[Iago Toral]

On Tue, 2018-01-16 at 08:07 -0800, Jason Ekstrand wrote:
> On Mon, Jan 8, 2018 at 4:57 AM, Iago Toral Quiroga <itoral at 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
> > 
> > 
> > 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180117/cb8b3c35/attachment-0001.html>