Re: [Mesa-dev] [PATCH 1/6] anv/cmd_buffer: Move state base address re-emit into ExecuteCommands

2016-10-17 Thread Anuj Phogat
On Mon, Oct 17, 2016 at 10:35 AM, Jason Ekstrand  wrote:
> This has two primary advantages.  First, it means that the batch_chain code
> knows less about the actual command buffer contents which is good because
> improves separation.  Second, it means that it only gets re-emitted once
> after all of the secondaries instead of once after each secondary which is
> just wasteful.  It also has the advantage of cleaning the code up a bit.
>
> Signed-off-by: Jason Ekstrand 
> ---
>  src/intel/vulkan/anv_batch_chain.c | 6 --
>  src/intel/vulkan/anv_cmd_buffer.c  | 9 +
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_batch_chain.c 
> b/src/intel/vulkan/anv_batch_chain.c
> index a98a0a9..95854f4 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -777,7 +777,6 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer 
> *primary,
> switch (secondary->exec_mode) {
> case ANV_CMD_BUFFER_EXEC_MODE_EMIT:
>anv_batch_emit_batch(>batch, >batch);
> -  anv_cmd_buffer_emit_state_base_address(primary);
>break;
> case ANV_CMD_BUFFER_EXEC_MODE_GROW_AND_EMIT: {
>struct anv_batch_bo *bbo = anv_cmd_buffer_current_batch_bo(primary);
> @@ -785,7 +784,6 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer 
> *primary,
>anv_batch_bo_grow(primary, bbo, >batch, length,
>  GEN8_MI_BATCH_BUFFER_START_length * 4);
>anv_batch_emit_batch(>batch, >batch);
> -  anv_cmd_buffer_emit_state_base_address(primary);
>break;
> }
> case ANV_CMD_BUFFER_EXEC_MODE_CHAIN: {
> @@ -826,8 +824,6 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer 
> *primary,
>  p += CACHELINE_SIZE;
>   }
>}
> -
> -  anv_cmd_buffer_emit_state_base_address(primary);
>break;
> }
> case ANV_CMD_BUFFER_EXEC_MODE_COPY_AND_CHAIN: {
> @@ -851,8 +847,6 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer 
> *primary,
>
>anv_batch_bo_continue(last_bbo, >batch,
>  GEN8_MI_BATCH_BUFFER_START_length * 4);
> -
> -  anv_cmd_buffer_emit_state_base_address(primary);
>break;
> }
> default:
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
> b/src/intel/vulkan/anv_cmd_buffer.c
> index 5bcd5e0..b55a070 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -1180,6 +1180,15 @@ void anv_CmdExecuteCommands(
>
>anv_cmd_buffer_add_secondary(primary, secondary);
> }
> +
> +   /* Each of the secondary command buffers will use its own state base
> +* address.  We need to re-emit state base address for the primary after
> +* all of the secondaries are done.
> +*
> +* TODO: Maybe we want to make this a dirty bit to avoid extra state base
> +* address calls?
> +*/
> +   anv_cmd_buffer_emit_state_base_address(primary);
>  }
>
>  VkResult anv_CreateCommandPool(
> --
> 2.5.0.400.gff86faf
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

For the series:
Reviewed-by: Anuj Phogat 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/6] anv/cmd_buffer: Move state base address re-emit into ExecuteCommands

2016-10-17 Thread Jason Ekstrand
This has two primary advantages.  First, it means that the batch_chain code
knows less about the actual command buffer contents which is good because
improves separation.  Second, it means that it only gets re-emitted once
after all of the secondaries instead of once after each secondary which is
just wasteful.  It also has the advantage of cleaning the code up a bit.

Signed-off-by: Jason Ekstrand 
---
 src/intel/vulkan/anv_batch_chain.c | 6 --
 src/intel/vulkan/anv_cmd_buffer.c  | 9 +
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/intel/vulkan/anv_batch_chain.c 
b/src/intel/vulkan/anv_batch_chain.c
index a98a0a9..95854f4 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -777,7 +777,6 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer *primary,
switch (secondary->exec_mode) {
case ANV_CMD_BUFFER_EXEC_MODE_EMIT:
   anv_batch_emit_batch(>batch, >batch);
-  anv_cmd_buffer_emit_state_base_address(primary);
   break;
case ANV_CMD_BUFFER_EXEC_MODE_GROW_AND_EMIT: {
   struct anv_batch_bo *bbo = anv_cmd_buffer_current_batch_bo(primary);
@@ -785,7 +784,6 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer *primary,
   anv_batch_bo_grow(primary, bbo, >batch, length,
 GEN8_MI_BATCH_BUFFER_START_length * 4);
   anv_batch_emit_batch(>batch, >batch);
-  anv_cmd_buffer_emit_state_base_address(primary);
   break;
}
case ANV_CMD_BUFFER_EXEC_MODE_CHAIN: {
@@ -826,8 +824,6 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer *primary,
 p += CACHELINE_SIZE;
  }
   }
-
-  anv_cmd_buffer_emit_state_base_address(primary);
   break;
}
case ANV_CMD_BUFFER_EXEC_MODE_COPY_AND_CHAIN: {
@@ -851,8 +847,6 @@ anv_cmd_buffer_add_secondary(struct anv_cmd_buffer *primary,
 
   anv_batch_bo_continue(last_bbo, >batch,
 GEN8_MI_BATCH_BUFFER_START_length * 4);
-
-  anv_cmd_buffer_emit_state_base_address(primary);
   break;
}
default:
diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
b/src/intel/vulkan/anv_cmd_buffer.c
index 5bcd5e0..b55a070 100644
--- a/src/intel/vulkan/anv_cmd_buffer.c
+++ b/src/intel/vulkan/anv_cmd_buffer.c
@@ -1180,6 +1180,15 @@ void anv_CmdExecuteCommands(
 
   anv_cmd_buffer_add_secondary(primary, secondary);
}
+
+   /* Each of the secondary command buffers will use its own state base
+* address.  We need to re-emit state base address for the primary after
+* all of the secondaries are done.
+*
+* TODO: Maybe we want to make this a dirty bit to avoid extra state base
+* address calls?
+*/
+   anv_cmd_buffer_emit_state_base_address(primary);
 }
 
 VkResult anv_CreateCommandPool(
-- 
2.5.0.400.gff86faf

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