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

[Samuel Pitoiset]

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);
>