Re: [Mesa-dev] [PATCH 15/29] anv/cmd_buffer: Add begin/end_subpass helpers

2018-01-09 Thread Nanley Chery 
On Mon, Nov 27, 2017 at 07:06:05PM -0800, Jason Ekstrand wrote:
> Having begin/end_subpass is a bit nicer than the begin/next/end hooks
> that Vulkan gives us.
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 55 
> +-
>  1 file changed, 31 insertions(+), 24 deletions(-)
> 

This patch is
Reviewed-by: Nanley Chery 

> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index bbe97f5..6f2fa0a 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3138,10 +3138,11 @@ cmd_buffer_subpass_sync_fast_clear_values(struct 
> anv_cmd_buffer *cmd_buffer)
>  
>  
>  static void
> -genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer *cmd_buffer,
> - struct anv_subpass *subpass)
> +cmd_buffer_begin_subpass(struct anv_cmd_buffer *cmd_buffer,
> + struct anv_subpass *subpass)
>  {
> cmd_buffer->state.subpass = subpass;
> +   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
>  
> cmd_buffer->state.dirty |= ANV_CMD_DIRTY_RENDER_TARGETS;
>  
> @@ -3155,6 +3156,10 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer 
> *cmd_buffer,
> if (GEN_GEN == 7)
>cmd_buffer->state.vb_dirty |= ~0;
>  
> +   /* Accumulate any subpass flushes that need to happen before the subpass 
> */
> +   cmd_buffer->state.pending_pipe_bits |=
> +  cmd_buffer->state.pass->subpass_flushes[subpass_id];
> +
> /* Perform transitions to the subpass layout before any writes have
>  * occurred.
>  */
> @@ -3174,6 +3179,26 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer 
> *cmd_buffer,
> anv_cmd_buffer_clear_subpass(cmd_buffer);
>  }
>  
> +static void
> +cmd_buffer_end_subpass(struct anv_cmd_buffer *cmd_buffer)
> +{
> +   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
> +
> +   anv_cmd_buffer_resolve_subpass(cmd_buffer);
> +
> +   /* Perform transitions to the final layout after all writes have occurred.
> +*/
> +   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
> +
> +   /* Accumulate any subpass flushes that need to happen after the subpass.
> +* Yes, they do get accumulated twice in the NextSubpass case but since
> +* genX_CmdNextSubpass just calls end/begin back-to-back, we just end up
> +* ORing the bits in twice so it's harmless.
> +*/
> +   cmd_buffer->state.pending_pipe_bits |=
> +  cmd_buffer->state.pass->subpass_flushes[subpass_id + 1];
> +}
> +
>  void genX(CmdBeginRenderPass)(
>  VkCommandBuffer commandBuffer,
>  const VkRenderPassBeginInfo*pRenderPassBegin,
> @@ -3197,10 +3222,7 @@ void genX(CmdBeginRenderPass)(
>  
> genX(flush_pipeline_select_3d)(cmd_buffer);
>  
> -   cmd_buffer->state.pending_pipe_bits |=
> -  cmd_buffer->state.pass->subpass_flushes[0];
> -
> -   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
> +   cmd_buffer_begin_subpass(cmd_buffer, pass->subpasses);
>  }
>  
>  void genX(CmdNextSubpass)(
> @@ -3214,17 +3236,9 @@ void genX(CmdNextSubpass)(
>  
> assert(cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY);
>  
> -   anv_cmd_buffer_resolve_subpass(cmd_buffer);
> -
> -   /* Perform transitions to the final layout after all writes have occurred.
> -*/
> -   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
> -
> -   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
> -   cmd_buffer->state.pending_pipe_bits |=
> -  cmd_buffer->state.pass->subpass_flushes[subpass_id];
> +   cmd_buffer_end_subpass(cmd_buffer);
>  
> -   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass + 1);
> +   cmd_buffer_begin_subpass(cmd_buffer, cmd_buffer->state.subpass + 1);
>  }
>  
>  void genX(CmdEndRenderPass)(
> @@ -3235,14 +3249,7 @@ void genX(CmdEndRenderPass)(
> if (anv_batch_has_error(_buffer->batch))
>return;
>  
> -   anv_cmd_buffer_resolve_subpass(cmd_buffer);
> -
> -   /* Perform transitions to the final layout after all writes have occurred.
> -*/
> -   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
> -
> -   cmd_buffer->state.pending_pipe_bits |=
> -  
> cmd_buffer->state.pass->subpass_flushes[cmd_buffer->state.pass->subpass_count];
> +   cmd_buffer_end_subpass(cmd_buffer);
>  
> cmd_buffer->state.hiz_enabled = false;
>  
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> 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 15/29] anv/cmd_buffer: Add begin/end_subpass helpers

2017-12-04 Thread Pohjolainen, Topi 
On Mon, Nov 27, 2017 at 07:06:05PM -0800, Jason Ekstrand wrote:
> Having begin/end_subpass is a bit nicer than the begin/next/end hooks
> that Vulkan gives us.
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 55 
> +-
>  1 file changed, 31 insertions(+), 24 deletions(-)

Like said in the previous patch, unlike there, here things make sense as
pending bits are explicitly handled before and after transitions:

Reviewed-by: Topi Pohjolainen 

> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index bbe97f5..6f2fa0a 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3138,10 +3138,11 @@ cmd_buffer_subpass_sync_fast_clear_values(struct 
> anv_cmd_buffer *cmd_buffer)
>  
>  
>  static void
> -genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer *cmd_buffer,
> - struct anv_subpass *subpass)
> +cmd_buffer_begin_subpass(struct anv_cmd_buffer *cmd_buffer,
> + struct anv_subpass *subpass)
>  {
> cmd_buffer->state.subpass = subpass;
> +   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
>  
> cmd_buffer->state.dirty |= ANV_CMD_DIRTY_RENDER_TARGETS;
>  
> @@ -3155,6 +3156,10 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer 
> *cmd_buffer,
> if (GEN_GEN == 7)
>cmd_buffer->state.vb_dirty |= ~0;
>  
> +   /* Accumulate any subpass flushes that need to happen before the subpass 
> */
> +   cmd_buffer->state.pending_pipe_bits |=
> +  cmd_buffer->state.pass->subpass_flushes[subpass_id];
> +
> /* Perform transitions to the subpass layout before any writes have
>  * occurred.
>  */
> @@ -3174,6 +3179,26 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer 
> *cmd_buffer,
> anv_cmd_buffer_clear_subpass(cmd_buffer);
>  }
>  
> +static void
> +cmd_buffer_end_subpass(struct anv_cmd_buffer *cmd_buffer)
> +{
> +   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
> +
> +   anv_cmd_buffer_resolve_subpass(cmd_buffer);
> +
> +   /* Perform transitions to the final layout after all writes have occurred.
> +*/
> +   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
> +
> +   /* Accumulate any subpass flushes that need to happen after the subpass.
> +* Yes, they do get accumulated twice in the NextSubpass case but since
> +* genX_CmdNextSubpass just calls end/begin back-to-back, we just end up
> +* ORing the bits in twice so it's harmless.
> +*/
> +   cmd_buffer->state.pending_pipe_bits |=
> +  cmd_buffer->state.pass->subpass_flushes[subpass_id + 1];
> +}
> +
>  void genX(CmdBeginRenderPass)(
>  VkCommandBuffer commandBuffer,
>  const VkRenderPassBeginInfo*pRenderPassBegin,
> @@ -3197,10 +3222,7 @@ void genX(CmdBeginRenderPass)(
>  
> genX(flush_pipeline_select_3d)(cmd_buffer);
>  
> -   cmd_buffer->state.pending_pipe_bits |=
> -  cmd_buffer->state.pass->subpass_flushes[0];
> -
> -   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
> +   cmd_buffer_begin_subpass(cmd_buffer, pass->subpasses);
>  }
>  
>  void genX(CmdNextSubpass)(
> @@ -3214,17 +3236,9 @@ void genX(CmdNextSubpass)(
>  
> assert(cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY);
>  
> -   anv_cmd_buffer_resolve_subpass(cmd_buffer);
> -
> -   /* Perform transitions to the final layout after all writes have occurred.
> -*/
> -   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
> -
> -   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
> -   cmd_buffer->state.pending_pipe_bits |=
> -  cmd_buffer->state.pass->subpass_flushes[subpass_id];
> +   cmd_buffer_end_subpass(cmd_buffer);
>  
> -   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass + 1);
> +   cmd_buffer_begin_subpass(cmd_buffer, cmd_buffer->state.subpass + 1);
>  }
>  
>  void genX(CmdEndRenderPass)(
> @@ -3235,14 +3249,7 @@ void genX(CmdEndRenderPass)(
> if (anv_batch_has_error(_buffer->batch))
>return;
>  
> -   anv_cmd_buffer_resolve_subpass(cmd_buffer);
> -
> -   /* Perform transitions to the final layout after all writes have occurred.
> -*/
> -   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
> -
> -   cmd_buffer->state.pending_pipe_bits |=
> -  
> cmd_buffer->state.pass->subpass_flushes[cmd_buffer->state.pass->subpass_count];
> +   cmd_buffer_end_subpass(cmd_buffer);
>  
> cmd_buffer->state.hiz_enabled = false;
>  
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> 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 15/29] anv/cmd_buffer: Add begin/end_subpass helpers

2017-11-27 Thread Jason Ekstrand 
Having begin/end_subpass is a bit nicer than the begin/next/end hooks
that Vulkan gives us.
---
 src/intel/vulkan/genX_cmd_buffer.c | 55 +-
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index bbe97f5..6f2fa0a 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -3138,10 +3138,11 @@ cmd_buffer_subpass_sync_fast_clear_values(struct 
anv_cmd_buffer *cmd_buffer)
 
 
 static void
-genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer *cmd_buffer,
- struct anv_subpass *subpass)
+cmd_buffer_begin_subpass(struct anv_cmd_buffer *cmd_buffer,
+ struct anv_subpass *subpass)
 {
cmd_buffer->state.subpass = subpass;
+   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
 
cmd_buffer->state.dirty |= ANV_CMD_DIRTY_RENDER_TARGETS;
 
@@ -3155,6 +3156,10 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer 
*cmd_buffer,
if (GEN_GEN == 7)
   cmd_buffer->state.vb_dirty |= ~0;
 
+   /* Accumulate any subpass flushes that need to happen before the subpass */
+   cmd_buffer->state.pending_pipe_bits |=
+  cmd_buffer->state.pass->subpass_flushes[subpass_id];
+
/* Perform transitions to the subpass layout before any writes have
 * occurred.
 */
@@ -3174,6 +3179,26 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer 
*cmd_buffer,
anv_cmd_buffer_clear_subpass(cmd_buffer);
 }
 
+static void
+cmd_buffer_end_subpass(struct anv_cmd_buffer *cmd_buffer)
+{
+   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
+
+   anv_cmd_buffer_resolve_subpass(cmd_buffer);
+
+   /* Perform transitions to the final layout after all writes have occurred.
+*/
+   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
+
+   /* Accumulate any subpass flushes that need to happen after the subpass.
+* Yes, they do get accumulated twice in the NextSubpass case but since
+* genX_CmdNextSubpass just calls end/begin back-to-back, we just end up
+* ORing the bits in twice so it's harmless.
+*/
+   cmd_buffer->state.pending_pipe_bits |=
+  cmd_buffer->state.pass->subpass_flushes[subpass_id + 1];
+}
+
 void genX(CmdBeginRenderPass)(
 VkCommandBuffer commandBuffer,
 const VkRenderPassBeginInfo*pRenderPassBegin,
@@ -3197,10 +3222,7 @@ void genX(CmdBeginRenderPass)(
 
genX(flush_pipeline_select_3d)(cmd_buffer);
 
-   cmd_buffer->state.pending_pipe_bits |=
-  cmd_buffer->state.pass->subpass_flushes[0];
-
-   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
+   cmd_buffer_begin_subpass(cmd_buffer, pass->subpasses);
 }
 
 void genX(CmdNextSubpass)(
@@ -3214,17 +3236,9 @@ void genX(CmdNextSubpass)(
 
assert(cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY);
 
-   anv_cmd_buffer_resolve_subpass(cmd_buffer);
-
-   /* Perform transitions to the final layout after all writes have occurred.
-*/
-   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
-
-   uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
-   cmd_buffer->state.pending_pipe_bits |=
-  cmd_buffer->state.pass->subpass_flushes[subpass_id];
+   cmd_buffer_end_subpass(cmd_buffer);
 
-   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass + 1);
+   cmd_buffer_begin_subpass(cmd_buffer, cmd_buffer->state.subpass + 1);
 }
 
 void genX(CmdEndRenderPass)(
@@ -3235,14 +3249,7 @@ void genX(CmdEndRenderPass)(
if (anv_batch_has_error(_buffer->batch))
   return;
 
-   anv_cmd_buffer_resolve_subpass(cmd_buffer);
-
-   /* Perform transitions to the final layout after all writes have occurred.
-*/
-   cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
-
-   cmd_buffer->state.pending_pipe_bits |=
-  
cmd_buffer->state.pass->subpass_flushes[cmd_buffer->state.pass->subpass_count];
+   cmd_buffer_end_subpass(cmd_buffer);
 
cmd_buffer->state.hiz_enabled = false;
 
-- 
2.5.0.400.gff86faf

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