Re: [Mesa-dev] [PATCH 14/29] anv/cmd_buffer: Apply subpass flushes before set_subpass

[Nanley Chery]

On Mon, Nov 27, 2017 at 07:06:04PM -0800, Jason Ekstrand wrote:
> This seems slightly more correct because it means that the flushes
> happen before any clears or resolves implied by the subpass transition.
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Makes sense. 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 2d47179..bbe97f5 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3197,10 +3197,10 @@ void genX(CmdBeginRenderPass)(
>  
> genX(flush_pipeline_select_3d)(cmd_buffer);
>  
> -   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
> -
> cmd_buffer->state.pending_pipe_bits |=
>cmd_buffer->state.pass->subpass_flushes[0];
> +
> +   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
>  }
>  
>  void genX(CmdNextSubpass)(
> @@ -3220,11 +3220,11 @@ void genX(CmdNextSubpass)(
>  */
> cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
>  
> -   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass + 1);
> -
> uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
> cmd_buffer->state.pending_pipe_bits |=
>cmd_buffer->state.pass->subpass_flushes[subpass_id];
> +
> +   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass + 1);
>  }
>  
>  void genX(CmdEndRenderPass)(
> -- 
> 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 14/29] anv/cmd_buffer: Apply subpass flushes before set_subpass

[Jason Ekstrand]

On Fri, Dec 1, 2017 at 5:47 AM, Pohjolainen, Topi <
topi.pohjolai...@gmail.com> wrote:

> On Mon, Nov 27, 2017 at 07:06:04PM -0800, Jason Ekstrand wrote:
> > This seems slightly more correct because it means that the flushes
> > happen before any clears or resolves implied by the subpass transition.
>
> After reading the next patch this patch seems incomplete both before
> and after. Next patch seems to explicitly consider that flushes are
> needed before and after whereas at this point it would be only
> before (when this patch is applied) or after (without this patch).
>
> I guess something else holds things together, I'm just not seeing
> it?
>

I think so.  In either case, what really matters is that the subpass
flushes happen before the next draw call.  The only change made here is
that before they would get triggered by the next draw call and now they may
get triggered by a clear or resolve that happens as part of set_subpass.


> > ---
> >  src/intel/vulkan/genX_cmd_buffer.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index 2d47179..bbe97f5 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -3197,10 +3197,10 @@ void genX(CmdBeginRenderPass)(
> >
> > genX(flush_pipeline_select_3d)(cmd_buffer);
> >
> > -   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
> > -
> > cmd_buffer->state.pending_pipe_bits |=
> >cmd_buffer->state.pass->subpass_flushes[0];
> > +
> > +   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
> >  }
> >
> >  void genX(CmdNextSubpass)(
> > @@ -3220,11 +3220,11 @@ void genX(CmdNextSubpass)(
> >  */
> > cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
> >
> > -   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass
> + 1);
> > -
> > uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
> > cmd_buffer->state.pending_pipe_bits |=
> >cmd_buffer->state.pass->subpass_flushes[subpass_id];
> > +
> > +   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass
> + 1);
> >  }
> >
> >  void genX(CmdEndRenderPass)(
> > --
> > 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 14/29] anv/cmd_buffer: Apply subpass flushes before set_subpass

[Pohjolainen, Topi]

On Mon, Nov 27, 2017 at 07:06:04PM -0800, Jason Ekstrand wrote:
> This seems slightly more correct because it means that the flushes
> happen before any clears or resolves implied by the subpass transition.

After reading the next patch this patch seems incomplete both before
and after. Next patch seems to explicitly consider that flushes are
needed before and after whereas at this point it would be only
before (when this patch is applied) or after (without this patch).

I guess something else holds things together, I'm just not seeing
it?

> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 2d47179..bbe97f5 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3197,10 +3197,10 @@ void genX(CmdBeginRenderPass)(
>  
> genX(flush_pipeline_select_3d)(cmd_buffer);
>  
> -   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
> -
> cmd_buffer->state.pending_pipe_bits |=
>cmd_buffer->state.pass->subpass_flushes[0];
> +
> +   genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
>  }
>  
>  void genX(CmdNextSubpass)(
> @@ -3220,11 +3220,11 @@ void genX(CmdNextSubpass)(
>  */
> cmd_buffer_subpass_transition_layouts(cmd_buffer, true);
>  
> -   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass + 1);
> -
> uint32_t subpass_id = anv_get_subpass_id(_buffer->state);
> cmd_buffer->state.pending_pipe_bits |=
>cmd_buffer->state.pass->subpass_flushes[subpass_id];
> +
> +   genX(cmd_buffer_set_subpass)(cmd_buffer, cmd_buffer->state.subpass + 1);
>  }
>  
>  void genX(CmdEndRenderPass)(
> -- 
> 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