Re: [Mesa-dev] [PATCH 3/3] radv: only emit cache flushes when the pool size is large enough

2018-02-28 Thread Bas Nieuwenhuizen 
On Wed, Feb 28, 2018 at 9:41 PM, Samuel Pitoiset
 wrote:
>
>
> On 02/28/2018 09:06 PM, Bas Nieuwenhuizen wrote:
>>
>> On Wed, Feb 28, 2018 at 8:31 PM, Samuel Pitoiset
>>  wrote:
>>>
>>> This is an optimization which reduces the number of flushes for
>>> small pool buffers.
>>>
>>> Signed-off-by: Samuel Pitoiset 
>>> ---
>>>   src/amd/vulkan/radv_meta_buffer.c |  6 --
>>>   src/amd/vulkan/radv_private.h |  6 ++
>>>   src/amd/vulkan/radv_query.c   | 12 
>>>   3 files changed, 14 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/amd/vulkan/radv_meta_buffer.c
>>> b/src/amd/vulkan/radv_meta_buffer.c
>>> index e6ad235e93..2e1ba2c7b2 100644
>>> --- a/src/amd/vulkan/radv_meta_buffer.c
>>> +++ b/src/amd/vulkan/radv_meta_buffer.c
>>> @@ -4,12 +4,6 @@
>>>   #include "sid.h"
>>>   #include "radv_cs.h"
>>>
>>> -/*
>>> - * This is the point we switch from using CP to compute shader
>>> - * for certain buffer operations.
>>> - */
>>> -#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
>>> -
>>>   static nir_shader *
>>>   build_buffer_fill_shader(struct radv_device *dev)
>>>   {
>>> diff --git a/src/amd/vulkan/radv_private.h
>>> b/src/amd/vulkan/radv_private.h
>>> index 752b6a7592..0f8ddb2e10 100644
>>> --- a/src/amd/vulkan/radv_private.h
>>> +++ b/src/amd/vulkan/radv_private.h
>>> @@ -95,6 +95,12 @@ typedef uint32_t xcb_window_t;
>>>
>>>   #define NUM_DEPTH_CLEAR_PIPELINES 3
>>>
>>> +/*
>>> + * This is the point we switch from using CP to compute shader
>>> + * for certain buffer operations.
>>> + */
>>> +#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
>>> +
>>>   enum radv_mem_heap {
>>>  RADV_MEM_HEAP_VRAM,
>>>  RADV_MEM_HEAP_VRAM_CPU_ACCESS,
>>> diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
>>> index 82831961ac..da2bcf5212 100644
>>> --- a/src/amd/vulkan/radv_query.c
>>> +++ b/src/amd/vulkan/radv_query.c
>>> @@ -1088,10 +1088,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);
>>> +   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;
>>
>>
>> I think the pending query reset also need to be inside, as otherwise
>> we possibly forget to flush a larger query behind it?
>>
>> I think you can do a similar opt by only setting the reset bit if the
>> fill_buffer command returns non-zero. That way you don't set it for
>> small pools in the first place (though it does not replace this opt).
>
>
> Yes, good point.
>
> I'm going to send a v3 for the backport and I will update the rest later.
>
> Is the Rb for the whole series?

Yup, rest looked good to me.
>
>
>>
>> Otherwise
>>
>> Reviewed-by: Bas Nieuwenhuizen 
>>>
>>>  }
>>>
>>> --
>>> 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

Re: [Mesa-dev] [PATCH 3/3] radv: only emit cache flushes when the pool size is large enough

2018-02-28 Thread Samuel Pitoiset 



On 02/28/2018 09:06 PM, Bas Nieuwenhuizen wrote:

On Wed, Feb 28, 2018 at 8:31 PM, Samuel Pitoiset
 wrote:

This is an optimization which reduces the number of flushes for
small pool buffers.

Signed-off-by: Samuel Pitoiset 
---
  src/amd/vulkan/radv_meta_buffer.c |  6 --
  src/amd/vulkan/radv_private.h |  6 ++
  src/amd/vulkan/radv_query.c   | 12 
  3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/amd/vulkan/radv_meta_buffer.c 
b/src/amd/vulkan/radv_meta_buffer.c
index e6ad235e93..2e1ba2c7b2 100644
--- a/src/amd/vulkan/radv_meta_buffer.c
+++ b/src/amd/vulkan/radv_meta_buffer.c
@@ -4,12 +4,6 @@
  #include "sid.h"
  #include "radv_cs.h"

-/*
- * This is the point we switch from using CP to compute shader
- * for certain buffer operations.
- */
-#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
-
  static nir_shader *
  build_buffer_fill_shader(struct radv_device *dev)
  {
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index 752b6a7592..0f8ddb2e10 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -95,6 +95,12 @@ typedef uint32_t xcb_window_t;

  #define NUM_DEPTH_CLEAR_PIPELINES 3

+/*
+ * This is the point we switch from using CP to compute shader
+ * for certain buffer operations.
+ */
+#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
+
  enum radv_mem_heap {
 RADV_MEM_HEAP_VRAM,
 RADV_MEM_HEAP_VRAM_CPU_ACCESS,
diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
index 82831961ac..da2bcf5212 100644
--- a/src/amd/vulkan/radv_query.c
+++ b/src/amd/vulkan/radv_query.c
@@ -1088,10 +1088,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);
+   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;


I think the pending query reset also need to be inside, as otherwise
we possibly forget to flush a larger query behind it?

I think you can do a similar opt by only setting the reset bit if the
fill_buffer command returns non-zero. That way you don't set it for
small pools in the first place (though it does not replace this opt).


Yes, good point.

I'm going to send a v3 for the backport and I will update the rest later.

Is the Rb for the whole series?



Otherwise

Reviewed-by: Bas Nieuwenhuizen 

 }

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

Re: [Mesa-dev] [PATCH 3/3] radv: only emit cache flushes when the pool size is large enough

2018-02-28 Thread Bas Nieuwenhuizen 
On Wed, Feb 28, 2018 at 8:31 PM, Samuel Pitoiset
 wrote:
> This is an optimization which reduces the number of flushes for
> small pool buffers.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/vulkan/radv_meta_buffer.c |  6 --
>  src/amd/vulkan/radv_private.h |  6 ++
>  src/amd/vulkan/radv_query.c   | 12 
>  3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_meta_buffer.c 
> b/src/amd/vulkan/radv_meta_buffer.c
> index e6ad235e93..2e1ba2c7b2 100644
> --- a/src/amd/vulkan/radv_meta_buffer.c
> +++ b/src/amd/vulkan/radv_meta_buffer.c
> @@ -4,12 +4,6 @@
>  #include "sid.h"
>  #include "radv_cs.h"
>
> -/*
> - * This is the point we switch from using CP to compute shader
> - * for certain buffer operations.
> - */
> -#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
> -
>  static nir_shader *
>  build_buffer_fill_shader(struct radv_device *dev)
>  {
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 752b6a7592..0f8ddb2e10 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -95,6 +95,12 @@ typedef uint32_t xcb_window_t;
>
>  #define NUM_DEPTH_CLEAR_PIPELINES 3
>
> +/*
> + * This is the point we switch from using CP to compute shader
> + * for certain buffer operations.
> + */
> +#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
> +
>  enum radv_mem_heap {
> RADV_MEM_HEAP_VRAM,
> RADV_MEM_HEAP_VRAM_CPU_ACCESS,
> diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
> index 82831961ac..da2bcf5212 100644
> --- a/src/amd/vulkan/radv_query.c
> +++ b/src/amd/vulkan/radv_query.c
> @@ -1088,10 +1088,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);
> +   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;

I think the pending query reset also need to be inside, as otherwise
we possibly forget to flush a larger query behind it?

I think you can do a similar opt by only setting the reset bit if the
fill_buffer command returns non-zero. That way you don't set it for
small pools in the first place (though it does not replace this opt).

Otherwise

Reviewed-by: Bas Nieuwenhuizen 
> }
>
> --
> 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 3/3] radv: only emit cache flushes when the pool size is large enough

2018-02-28 Thread Samuel Pitoiset 
This is an optimization which reduces the number of flushes for
small pool buffers.

Signed-off-by: Samuel Pitoiset 
---
 src/amd/vulkan/radv_meta_buffer.c |  6 --
 src/amd/vulkan/radv_private.h |  6 ++
 src/amd/vulkan/radv_query.c   | 12 
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/amd/vulkan/radv_meta_buffer.c 
b/src/amd/vulkan/radv_meta_buffer.c
index e6ad235e93..2e1ba2c7b2 100644
--- a/src/amd/vulkan/radv_meta_buffer.c
+++ b/src/amd/vulkan/radv_meta_buffer.c
@@ -4,12 +4,6 @@
 #include "sid.h"
 #include "radv_cs.h"
 
-/*
- * This is the point we switch from using CP to compute shader
- * for certain buffer operations.
- */
-#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
-
 static nir_shader *
 build_buffer_fill_shader(struct radv_device *dev)
 {
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index 752b6a7592..0f8ddb2e10 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -95,6 +95,12 @@ typedef uint32_t xcb_window_t;
 
 #define NUM_DEPTH_CLEAR_PIPELINES 3
 
+/*
+ * This is the point we switch from using CP to compute shader
+ * for certain buffer operations.
+ */
+#define RADV_BUFFER_OPS_CS_THRESHOLD 4096
+
 enum radv_mem_heap {
RADV_MEM_HEAP_VRAM,
RADV_MEM_HEAP_VRAM_CPU_ACCESS,
diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
index 82831961ac..da2bcf5212 100644
--- a/src/amd/vulkan/radv_query.c
+++ b/src/amd/vulkan/radv_query.c
@@ -1088,10 +1088,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);
+   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;
}
 
-- 
2.16.2

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