Re: [Mesa-dev] [PATCH v3] radv: make sure to emit cache flushes before starting a query

2018-03-01 Thread Samuel Pitoiset 



On 03/01/2018 10:21 AM, Alex Smith wrote:

Hi Samuel,

On 28 February 2018 at 20:47, Samuel Pitoiset > wrote:


If the query pool has been previously resetted using the compute
shader path.

v3: set pending_reset_query only for the compute shader path
v2: handle multiple commands buffers with same pool

Fixes: a41e2e9cf5 ("radv: allow to use a compute shader for
resetting the query pool")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105292

Cc: "18.0" mailto:mesa-sta...@lists.freedesktop.org>>
Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>>
---
  src/amd/vulkan/radv_cmd_buffer.c |  7 +++
  src/amd/vulkan/radv_private.h    |  5 +
  src/amd/vulkan/radv_query.c      | 28 +---
  3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c
b/src/amd/vulkan/radv_cmd_buffer.c
index 2b41baea3d..cfdc531acd 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -1930,6 +1930,13 @@ VkResult radv_BeginCommandBuffer(

         cmd_buffer->status = RADV_CMD_BUFFER_STATUS_RECORDING;

+       /* Force cache flushes before starting a new query in case the
+        * corresponding pool has been resetted from a different command
+        * buffer. This is because we have to flush caches between
reset and
+        * begin if the compute shader path has been used.
+        */
+       cmd_buffer->pending_reset_query = true;


Since this just ends up calling si_emit_cache_flush, doesn't flush_bits 
need to be set accordingly for it to actually do anything? If the reset 
is done in a previous command buffer, I think the flush would already 
have been done in EndCommandBuffer on that?


You are right, this is just useless because we always flushes caches in 
EndCommandBuffer(). Thanks for pointing this out. Will remove that hunk.




Thanks,
Alex

+
         return result;
  }

diff --git a/src/amd/vulkan/radv_private.h
b/src/amd/vulkan/radv_private.h
index c72df5a737..b76d2eb5cb 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -1003,6 +1003,11 @@ struct radv_cmd_buffer {
         uint32_t gfx9_fence_offset;
         struct radeon_winsys_bo *gfx9_fence_bo;
         uint32_t gfx9_fence_idx;
+
+       /**
+        * Whether a query pool has been resetted and we have to
flush caches.
+        */
+       bool pending_reset_query;
  };

  struct radv_image;
diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
index ace745e4e6..b1393a2ec7 100644
--- a/src/amd/vulkan/radv_query.c
+++ b/src/amd/vulkan/radv_query.c
@@ -1058,17 +1058,23 @@ void radv_CmdResetQueryPool(
  {
         RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
         RADV_FROM_HANDLE(radv_query_pool, pool, queryPool);
-       struct radv_cmd_state *state = &cmd_buffer->state;
+       uint32_t flush_bits = 0;

-       state->flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
-                                             firstQuery * pool->stride,
-                                             queryCount *
pool->stride, 0);
+       flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
+                                      firstQuery * pool->stride,
+                                      queryCount * pool->stride, 0);

         if (pool->type == VK_QUERY_TYPE_TIMESTAMP ||
             pool->type == VK_QUERY_TYPE_PIPELINE_STATISTICS) {
-               state->flush_bits |= radv_fill_buffer(cmd_buffer,
pool->bo,
-   
  pool->availability_offset + firstQuery * 4,

-                                                     queryCount *
4, 0);
+               flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
+ 
pool->availability_offset + firstQuery * 4,

+                                              queryCount * 4, 0);
+       }
+
+       if (flush_bits) {
+               /* Only need to flush caches for the compute shader
path. */
+               cmd_buffer->pending_reset_query = true;
+               cmd_buffer->state.flush_bits |= flush_bits;
         }
  }

@@ -1086,6 +1092,14 @@ void radv_CmdBeginQuery(

         radv_cs_add_buffer(cmd_buffer->device->ws, cs, pool->bo, 8);

+       if (cmd_buffer->pending_reset_query) {
+               /* Make sure to flush caches if the query pool has been
+                * previously resetted using the compute shader path.
+                */
+               si_emit_cache_flush(cmd_buffer);
+   

Re: [Mesa-dev] [PATCH v3] radv: make sure to emit cache flushes before starting a query

2018-03-01 Thread Alex Smith 
Hi Samuel,

On 28 February 2018 at 20:47, Samuel Pitoiset 
wrote:

> If the query pool has been previously resetted using the compute
> shader path.
>
> v3: set pending_reset_query only for the compute shader path
> v2: handle multiple commands buffers with same pool
>
> Fixes: a41e2e9cf5 ("radv: allow to use a compute shader for resetting the
> query pool")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105292
> Cc: "18.0" 
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/vulkan/radv_cmd_buffer.c |  7 +++
>  src/amd/vulkan/radv_private.h|  5 +
>  src/amd/vulkan/radv_query.c  | 28 +---
>  3 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_
> buffer.c
> index 2b41baea3d..cfdc531acd 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -1930,6 +1930,13 @@ VkResult radv_BeginCommandBuffer(
>
> cmd_buffer->status = RADV_CMD_BUFFER_STATUS_RECORDING;
>
> +   /* Force cache flushes before starting a new query in case the
> +* corresponding pool has been resetted from a different command
> +* buffer. This is because we have to flush caches between reset
> and
> +* begin if the compute shader path has been used.
> +*/
> +   cmd_buffer->pending_reset_query = true;
>

Since this just ends up calling si_emit_cache_flush, doesn't flush_bits
need to be set accordingly for it to actually do anything? If the reset is
done in a previous command buffer, I think the flush would already have
been done in EndCommandBuffer on that?

Thanks,
Alex

+
> return result;
>  }
>
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index c72df5a737..b76d2eb5cb 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -1003,6 +1003,11 @@ struct radv_cmd_buffer {
> uint32_t gfx9_fence_offset;
> struct radeon_winsys_bo *gfx9_fence_bo;
> uint32_t gfx9_fence_idx;
> +
> +   /**
> +* Whether a query pool has been resetted and we have to flush
> caches.
> +*/
> +   bool pending_reset_query;
>  };
>
>  struct radv_image;
> diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
> index ace745e4e6..b1393a2ec7 100644
> --- a/src/amd/vulkan/radv_query.c
> +++ b/src/amd/vulkan/radv_query.c
> @@ -1058,17 +1058,23 @@ void radv_CmdResetQueryPool(
>  {
> RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> RADV_FROM_HANDLE(radv_query_pool, pool, queryPool);
> -   struct radv_cmd_state *state = &cmd_buffer->state;
> +   uint32_t flush_bits = 0;
>
> -   state->flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
> - firstQuery * pool->stride,
> - queryCount * pool->stride,
> 0);
> +   flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
> +  firstQuery * pool->stride,
> +  queryCount * pool->stride, 0);
>
> if (pool->type == VK_QUERY_TYPE_TIMESTAMP ||
> pool->type == VK_QUERY_TYPE_PIPELINE_STATISTICS) {
> -   state->flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
> -
>  pool->availability_offset + firstQuery * 4,
> - queryCount * 4, 0);
> +   flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
> +  pool->availability_offset +
> firstQuery * 4,
> +  queryCount * 4, 0);
> +   }
> +
> +   if (flush_bits) {
> +   /* Only need to flush caches for the compute shader path.
> */
> +   cmd_buffer->pending_reset_query = true;
> +   cmd_buffer->state.flush_bits |= flush_bits;
> }
>  }
>
> @@ -1086,6 +1092,14 @@ void radv_CmdBeginQuery(
>
> radv_cs_add_buffer(cmd_buffer->device->ws, cs, pool->bo, 8);
>
> +   if (cmd_buffer->pending_reset_query) {
> +   /* Make sure to flush caches if the query pool has been
> +* previously resetted using the compute shader path.
> +*/
> +   si_emit_cache_flush(cmd_buffer);
> +   cmd_buffer->pending_reset_query = false;
> +   }
> +
> switch (pool->type) {
> case VK_QUERY_TYPE_OCCLUSION:
> radeon_check_space(cmd_buffer->device->ws, cs, 7);
> --
> 2.16.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3] radv: make sure to emit cache flushes before starting a query

2018-02-28 Thread Samuel Pitoiset 
If the query pool has been previously resetted using the compute
shader path.

v3: set pending_reset_query only for the compute shader path
v2: handle multiple commands buffers with same pool

Fixes: a41e2e9cf5 ("radv: allow to use a compute shader for resetting the query 
pool")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105292
Cc: "18.0" 
Signed-off-by: Samuel Pitoiset 
---
 src/amd/vulkan/radv_cmd_buffer.c |  7 +++
 src/amd/vulkan/radv_private.h|  5 +
 src/amd/vulkan/radv_query.c  | 28 +---
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 2b41baea3d..cfdc531acd 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -1930,6 +1930,13 @@ VkResult radv_BeginCommandBuffer(
 
cmd_buffer->status = RADV_CMD_BUFFER_STATUS_RECORDING;
 
+   /* Force cache flushes before starting a new query in case the
+* corresponding pool has been resetted from a different command
+* buffer. This is because we have to flush caches between reset and
+* begin if the compute shader path has been used.
+*/
+   cmd_buffer->pending_reset_query = true;
+
return result;
 }
 
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index c72df5a737..b76d2eb5cb 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -1003,6 +1003,11 @@ struct radv_cmd_buffer {
uint32_t gfx9_fence_offset;
struct radeon_winsys_bo *gfx9_fence_bo;
uint32_t gfx9_fence_idx;
+
+   /**
+* Whether a query pool has been resetted and we have to flush caches.
+*/
+   bool pending_reset_query;
 };
 
 struct radv_image;
diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
index ace745e4e6..b1393a2ec7 100644
--- a/src/amd/vulkan/radv_query.c
+++ b/src/amd/vulkan/radv_query.c
@@ -1058,17 +1058,23 @@ void radv_CmdResetQueryPool(
 {
RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
RADV_FROM_HANDLE(radv_query_pool, pool, queryPool);
-   struct radv_cmd_state *state = &cmd_buffer->state;
+   uint32_t flush_bits = 0;
 
-   state->flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
- firstQuery * pool->stride,
- queryCount * pool->stride, 0);
+   flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
+  firstQuery * pool->stride,
+  queryCount * pool->stride, 0);
 
if (pool->type == VK_QUERY_TYPE_TIMESTAMP ||
pool->type == VK_QUERY_TYPE_PIPELINE_STATISTICS) {
-   state->flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
- pool->availability_offset 
+ firstQuery * 4,
- queryCount * 4, 0);
+   flush_bits |= radv_fill_buffer(cmd_buffer, pool->bo,
+  pool->availability_offset + 
firstQuery * 4,
+  queryCount * 4, 0);
+   }
+
+   if (flush_bits) {
+   /* Only need to flush caches for the compute shader path. */
+   cmd_buffer->pending_reset_query = true;
+   cmd_buffer->state.flush_bits |= flush_bits;
}
 }
 
@@ -1086,6 +1092,14 @@ void radv_CmdBeginQuery(
 
radv_cs_add_buffer(cmd_buffer->device->ws, cs, pool->bo, 8);
 
+   if (cmd_buffer->pending_reset_query) {
+   /* Make sure to flush caches if the query pool has been
+* previously resetted using the compute shader path.
+*/
+   si_emit_cache_flush(cmd_buffer);
+   cmd_buffer->pending_reset_query = false;
+   }
+
switch (pool->type) {
case VK_QUERY_TYPE_OCCLUSION:
radeon_check_space(cmd_buffer->device->ws, cs, 7);
-- 
2.16.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev