[Mesa-dev] [PATCH] radv: Flush before vkCmdWriteTimestamp() if needed

[Samuel Pitoiset]

On 12/5/18 11:15 AM, Alex Smith wrote:
> Thanks. Though this fixes the 100% repro hang, I think your first patch 
> is still needed as well to handle getting 0xffffffff in the low 32 bits.

Yeah, it's still needed. Though I think it should be enough to wait on 
the high 32bits as suggested by Bas.

> 
> On Wed, 5 Dec 2018 at 10:04, Samuel Pitoiset <samuel.pitoiset at gmail.com 
> <mailto:samuel.pitoiset at gmail.com>> wrote:
> 
>     Yes, this is correct, indeed.
> 
>     The issue wasn't present because we used EOP events before removing the
>     availability bit.
> 
>     Btw, just noticed that we should reset pending_reset_query directly in
>     si_emit_cache_flush() to reduce the number of stalls. I will send a
>     patch.
> 
>     Also note that fill CP DMA operations are currently always sync'ed,
>     while CP DMA copies are not. I plan to change this at some point.
> 
>     Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com
>     <mailto:samuel.pitoiset at gmail.com>>
> 
>     On 12/5/18 10:52 AM, Alex Smith wrote:
>      > As done for vkCmdBeginQuery() already. Prevents timestamps from being
>      > overwritten by previous vkCmdResetQueryPool() calls if the shader
>     path
>      > was used to do the reset.
>      >
>      > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108925
>      > Fixes: a41e2e9cf5 ("radv: allow to use a compute shader for
>     resetting the query pool")
>      > Signed-off-by: Alex Smith <asmith at feralinteractive.com
>     <mailto:asmith at feralinteractive.com>>
>      > ---
>      >   src/amd/vulkan/radv_query.c | 30 +++++++++++++++++++-----------
>      >   1 file changed, 19 insertions(+), 11 deletions(-)
>      >
>      > diff --git a/src/amd/vulkan/radv_query.c
>     b/src/amd/vulkan/radv_query.c
>      > index 550abe307a..e226bcef6a 100644
>      > --- a/src/amd/vulkan/radv_query.c
>      > +++ b/src/amd/vulkan/radv_query.c
>      > @@ -1436,6 +1436,22 @@ static unsigned
>     event_type_for_stream(unsigned stream)
>      >       }
>      >   }
>      >
>      > +static void emit_query_flush(struct radv_cmd_buffer *cmd_buffer,
>      > +                          struct radv_query_pool *pool)
>      > +{
>      > +     if (cmd_buffer->pending_reset_query) {
>      > +             if (pool->size >= RADV_BUFFER_OPS_CS_THRESHOLD) {
>      > +                     /* Only need to flush caches if the query
>     pool size is
>      > +                      * large enough to be resetted using the
>     compute shader
>      > +                      * path. Small pools don't need any cache
>     flushes
>      > +                      * because we use a CP dma clear.
>      > +                      */
>      > +                     si_emit_cache_flush(cmd_buffer);
>      > +                     cmd_buffer->pending_reset_query = false;
>      > +             }
>      > +     }
>      > +}
>      > +
>      >   static void emit_begin_query(struct radv_cmd_buffer *cmd_buffer,
>      >                            uint64_t va,
>      >                            VkQueryType query_type,
>      > @@ -1582,17 +1598,7 @@ void radv_CmdBeginQueryIndexedEXT(
>      >
>      >       radv_cs_add_buffer(cmd_buffer->device->ws, cs, pool->bo);
>      >
>      > -     if (cmd_buffer->pending_reset_query) {
>      > -             if (pool->size >= RADV_BUFFER_OPS_CS_THRESHOLD) {
>      > -                     /* Only need to flush caches if the query
>     pool size is
>      > -                      * large enough to be resetted using the
>     compute shader
>      > -                      * path. Small pools don't need any cache
>     flushes
>      > -                      * because we use a CP dma clear.
>      > -                      */
>      > -                     si_emit_cache_flush(cmd_buffer);
>      > -                     cmd_buffer->pending_reset_query = false;
>      > -             }
>      > -     }
>      > +     emit_query_flush(cmd_buffer, pool);
>      >
>      >       va += pool->stride * query;
>      >
>      > @@ -1669,6 +1675,8 @@ void radv_CmdWriteTimestamp(
>      >
>      >       radv_cs_add_buffer(cmd_buffer->device->ws, cs, pool->bo);
>      >
>      > +     emit_query_flush(cmd_buffer, pool);
>      > +
>      >       int num_queries = 1;
>      >       if (cmd_buffer->state.subpass &&
>     cmd_buffer->state.subpass->view_mask)
>      >               num_queries =
>     util_bitcount(cmd_buffer->state.subpass->view_mask);
>      >
>