Re: [Mesa-dev] [PATCH 2/2] radv: do not re-emit the index buffer for every draw call

[Samuel Pitoiset]

On 10/18/2017 09:54 PM, Dave Airlie wrote:

On 19 October 2017 at 04:56, Bas Nieuwenhuizen  wrote:

Interesting that we already had RADV_CMD_DIRTY_INDEX_BUFFER. r-b for the series.

On Wed, Oct 18, 2017 at 2:17 PM, Samuel Pitoiset
 wrote:

It can only be changed when CmdBindIndexBuffer() is called
or when a secondary buffer is used. Though not always, but
let's re-emit the packets in this situation for now.


There is some hw issue around resetting the index type that my brain
can't come up with,

/* On CI and later, non-indexed draws overwrite VGT_INDEX_TYPE,
  * so the state must be re-emitted before the next indexed draw.
  */

Just make sure we still do the right thing here before pushing.


Mmmh, I have pushed the patch already, I'm going to have a look.



Dave.


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

Re: [Mesa-dev] [PATCH 2/2] radv: do not re-emit the index buffer for every draw call

[Dave Airlie]

On 19 October 2017 at 04:56, Bas Nieuwenhuizen  wrote:
> Interesting that we already had RADV_CMD_DIRTY_INDEX_BUFFER. r-b for the 
> series.
>
> On Wed, Oct 18, 2017 at 2:17 PM, Samuel Pitoiset
>  wrote:
>> It can only be changed when CmdBindIndexBuffer() is called
>> or when a secondary buffer is used. Though not always, but
>> let's re-emit the packets in this situation for now.

There is some hw issue around resetting the index type that my brain
can't come up with,

/* On CI and later, non-indexed draws overwrite VGT_INDEX_TYPE,
 * so the state must be re-emitted before the next indexed draw.
 */

Just make sure we still do the right thing here before pushing.

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

Re: [Mesa-dev] [PATCH 2/2] radv: do not re-emit the index buffer for every draw call

[Bas Nieuwenhuizen]

Interesting that we already had RADV_CMD_DIRTY_INDEX_BUFFER. r-b for the series.

On Wed, Oct 18, 2017 at 2:17 PM, Samuel Pitoiset
 wrote:
> It can only be changed when CmdBindIndexBuffer() is called
> or when a secondary buffer is used. Though not always, but
> let's re-emit the packets in this situation for now.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/vulkan/radv_cmd_buffer.c | 57 
> 
>  1 file changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> b/src/amd/vulkan/radv_cmd_buffer.c
> index c6dd620eea..f2e8c0ca5a 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -1400,6 +1400,27 @@ radv_emit_framebuffer_state(struct radv_cmd_buffer 
> *cmd_buffer)
> }
>  }
>
> +static void
> +radv_emit_index_buffer(struct radv_cmd_buffer *cmd_buffer)
> +{
> +   struct radeon_winsys_cs *cs = cmd_buffer->cs;
> +
> +   if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) 
> {
> +   radeon_set_uconfig_reg_idx(cs, R_03090C_VGT_INDEX_TYPE,
> +  2, cmd_buffer->state.index_type);
> +   } else {
> +   radeon_emit(cs, PKT3(PKT3_INDEX_TYPE, 0, 0));
> +   radeon_emit(cs, cmd_buffer->state.index_type);
> +   }
> +
> +   radeon_emit(cs, PKT3(PKT3_INDEX_BASE, 1, 0));
> +   radeon_emit(cs, cmd_buffer->state.index_va);
> +   radeon_emit(cs, cmd_buffer->state.index_va >> 32);
> +
> +   radeon_emit(cs, PKT3(PKT3_INDEX_BUFFER_SIZE, 0, 0));
> +   radeon_emit(cs, cmd_buffer->state.max_index_count);
> +}
> +
>  void radv_set_db_count_control(struct radv_cmd_buffer *cmd_buffer)
>  {
> uint32_t db_count_control;
> @@ -1743,6 +1764,9 @@ radv_cmd_buffer_flush_state(struct radv_cmd_buffer 
> *cmd_buffer,
> if (cmd_buffer->state.dirty & RADV_CMD_DIRTY_RENDER_TARGETS)
> radv_emit_framebuffer_state(cmd_buffer);
>
> +   if (cmd_buffer->state.dirty & RADV_CMD_DIRTY_INDEX_BUFFER)
> +   radv_emit_index_buffer(cmd_buffer);
> +
> ia_multi_vgt_param = si_get_ia_multi_vgt_param(cmd_buffer, 
> instanced_draw, indirect_draw, draw_vertex_count);
> if (cmd_buffer->state.last_ia_multi_vgt_param != ia_multi_vgt_param) {
> if (cmd_buffer->device->physical_device->rad_info.chip_class 
> >= GFX9)
> @@ -2689,8 +2713,9 @@ void radv_CmdExecuteCommands(
> /* After executing commands from secondary buffers we have to dirty
>  * some states.
>  */
> -   primary->state.dirty |= RADV_CMD_DIRTY_PIPELINE;
> -   primary->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_ALL;
> +   primary->state.dirty |= RADV_CMD_DIRTY_PIPELINE |
> +   RADV_CMD_DIRTY_INDEX_BUFFER |
> +   RADV_CMD_DIRTY_DYNAMIC_ALL;
> radv_mark_descriptor_sets_dirty(primary);
>  }
>
> @@ -2926,14 +2951,6 @@ void radv_CmdDrawIndexed(
>
> MAYBE_UNUSED unsigned cdw_max = 
> radeon_check_space(cmd_buffer->device->ws, cmd_buffer->cs, 26 * MAX_VIEWS);
>
> -   if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) 
> {
> -   radeon_set_uconfig_reg_idx(cmd_buffer->cs, 
> R_03090C_VGT_INDEX_TYPE,
> -  2, cmd_buffer->state.index_type);
> -   } else {
> -   radeon_emit(cmd_buffer->cs, PKT3(PKT3_INDEX_TYPE, 0, 0));
> -   radeon_emit(cmd_buffer->cs, cmd_buffer->state.index_type);
> -   }
> -
> assert(cmd_buffer->state.pipeline->graphics.vtx_base_sgpr);
> radeon_set_sh_reg_seq(cmd_buffer->cs, 
> cmd_buffer->state.pipeline->graphics.vtx_base_sgpr,
>   
> cmd_buffer->state.pipeline->graphics.vtx_emit_num);
> @@ -3081,29 +3098,11 @@ radv_cmd_draw_indexed_indirect_count(
> uint32_tstride)
>  {
> RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> -   uint64_t index_va;
> -   radv_cmd_buffer_flush_state(cmd_buffer, true, false, true, 0);
>
> -   index_va = cmd_buffer->state.index_va;
> +   radv_cmd_buffer_flush_state(cmd_buffer, true, false, true, 0);
>
> MAYBE_UNUSED unsigned cdw_max = 
> radeon_check_space(cmd_buffer->device->ws, cmd_buffer->cs, 31 * MAX_VIEWS);
>
> -   if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) 
> {
> -   radeon_set_uconfig_reg_idx(cmd_buffer->cs,
> -  R_03090C_VGT_INDEX_TYPE,
> -  2, cmd_buffer->state.index_type);
> -   } else {
> -   radeon_emit(cmd_buffer->cs, PKT3(PKT3_INDEX_TYPE, 0, 0));
> -   radeon_emit(cmd_buffer->cs, cmd_buffer->state.index_type);
> -   }
> -
> -   radeon_emit(cmd_buffer->cs, PKT3(PKT3_INDEX_BASE, 1, 

[Mesa-dev] [PATCH 2/2] radv: do not re-emit the index buffer for every draw call

[Samuel Pitoiset]

It can only be changed when CmdBindIndexBuffer() is called
or when a secondary buffer is used. Though not always, but
let's re-emit the packets in this situation for now.

Signed-off-by: Samuel Pitoiset 
---
 src/amd/vulkan/radv_cmd_buffer.c | 57 
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index c6dd620eea..f2e8c0ca5a 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -1400,6 +1400,27 @@ radv_emit_framebuffer_state(struct radv_cmd_buffer 
*cmd_buffer)
}
 }
 
+static void
+radv_emit_index_buffer(struct radv_cmd_buffer *cmd_buffer)
+{
+   struct radeon_winsys_cs *cs = cmd_buffer->cs;
+
+   if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) {
+   radeon_set_uconfig_reg_idx(cs, R_03090C_VGT_INDEX_TYPE,
+  2, cmd_buffer->state.index_type);
+   } else {
+   radeon_emit(cs, PKT3(PKT3_INDEX_TYPE, 0, 0));
+   radeon_emit(cs, cmd_buffer->state.index_type);
+   }
+
+   radeon_emit(cs, PKT3(PKT3_INDEX_BASE, 1, 0));
+   radeon_emit(cs, cmd_buffer->state.index_va);
+   radeon_emit(cs, cmd_buffer->state.index_va >> 32);
+
+   radeon_emit(cs, PKT3(PKT3_INDEX_BUFFER_SIZE, 0, 0));
+   radeon_emit(cs, cmd_buffer->state.max_index_count);
+}
+
 void radv_set_db_count_control(struct radv_cmd_buffer *cmd_buffer)
 {
uint32_t db_count_control;
@@ -1743,6 +1764,9 @@ radv_cmd_buffer_flush_state(struct radv_cmd_buffer 
*cmd_buffer,
if (cmd_buffer->state.dirty & RADV_CMD_DIRTY_RENDER_TARGETS)
radv_emit_framebuffer_state(cmd_buffer);
 
+   if (cmd_buffer->state.dirty & RADV_CMD_DIRTY_INDEX_BUFFER)
+   radv_emit_index_buffer(cmd_buffer);
+
ia_multi_vgt_param = si_get_ia_multi_vgt_param(cmd_buffer, 
instanced_draw, indirect_draw, draw_vertex_count);
if (cmd_buffer->state.last_ia_multi_vgt_param != ia_multi_vgt_param) {
if (cmd_buffer->device->physical_device->rad_info.chip_class >= 
GFX9)
@@ -2689,8 +2713,9 @@ void radv_CmdExecuteCommands(
/* After executing commands from secondary buffers we have to dirty
 * some states.
 */
-   primary->state.dirty |= RADV_CMD_DIRTY_PIPELINE;
-   primary->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_ALL;
+   primary->state.dirty |= RADV_CMD_DIRTY_PIPELINE |
+   RADV_CMD_DIRTY_INDEX_BUFFER |
+   RADV_CMD_DIRTY_DYNAMIC_ALL;
radv_mark_descriptor_sets_dirty(primary);
 }
 
@@ -2926,14 +2951,6 @@ void radv_CmdDrawIndexed(
 
MAYBE_UNUSED unsigned cdw_max = 
radeon_check_space(cmd_buffer->device->ws, cmd_buffer->cs, 26 * MAX_VIEWS);
 
-   if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) {
-   radeon_set_uconfig_reg_idx(cmd_buffer->cs, 
R_03090C_VGT_INDEX_TYPE,
-  2, cmd_buffer->state.index_type);
-   } else {
-   radeon_emit(cmd_buffer->cs, PKT3(PKT3_INDEX_TYPE, 0, 0));
-   radeon_emit(cmd_buffer->cs, cmd_buffer->state.index_type);
-   }
-
assert(cmd_buffer->state.pipeline->graphics.vtx_base_sgpr);
radeon_set_sh_reg_seq(cmd_buffer->cs, 
cmd_buffer->state.pipeline->graphics.vtx_base_sgpr,
  
cmd_buffer->state.pipeline->graphics.vtx_emit_num);
@@ -3081,29 +3098,11 @@ radv_cmd_draw_indexed_indirect_count(
uint32_tstride)
 {
RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
-   uint64_t index_va;
-   radv_cmd_buffer_flush_state(cmd_buffer, true, false, true, 0);
 
-   index_va = cmd_buffer->state.index_va;
+   radv_cmd_buffer_flush_state(cmd_buffer, true, false, true, 0);
 
MAYBE_UNUSED unsigned cdw_max = 
radeon_check_space(cmd_buffer->device->ws, cmd_buffer->cs, 31 * MAX_VIEWS);
 
-   if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) {
-   radeon_set_uconfig_reg_idx(cmd_buffer->cs,
-  R_03090C_VGT_INDEX_TYPE,
-  2, cmd_buffer->state.index_type);
-   } else {
-   radeon_emit(cmd_buffer->cs, PKT3(PKT3_INDEX_TYPE, 0, 0));
-   radeon_emit(cmd_buffer->cs, cmd_buffer->state.index_type);
-   }
-
-   radeon_emit(cmd_buffer->cs, PKT3(PKT3_INDEX_BASE, 1, 0));
-   radeon_emit(cmd_buffer->cs, index_va);
-   radeon_emit(cmd_buffer->cs, index_va >> 32);
-
-   radeon_emit(cmd_buffer->cs, PKT3(PKT3_INDEX_BUFFER_SIZE, 0, 0));
-   radeon_emit(cmd_buffer->cs, cmd_buffer->state.max_index_count);
-
radv_emit_indirect_draw(cmd_buffer, buffer, offset,
countBuffer, countBufferOffset,