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

[Alex Smith]

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.

On Wed, 5 Dec 2018 at 10:04, Samuel Pitoiset <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>
>
> 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>
> > ---
> >   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);
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181205/84c85fdd/attachment.html>