[Mesa-dev] [PATCH 4/4] anv: Use subpass dependencies for flushes

Jason Ekstrand jason at jlekstrand.net
Thu Apr 6 18:48:04 UTC 2017


On Thu, Mar 30, 2017 at 2:06 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> On Tue, Mar 14, 2017 at 07:55:53AM -0700, Jason Ekstrand wrote:
> > Instead of figuring it all out ourselves, just use the information given
> > to us by the client.
> > ---
> >  src/intel/vulkan/anv_blorp.c       | 88 ++++--------------------------
> --------
> >  src/intel/vulkan/genX_cmd_buffer.c | 10 +++++
> >  2 files changed, 18 insertions(+), 80 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > index 8de339c..41966d6 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -1070,80 +1070,6 @@ enum subpass_stage {
> >  };
> >
> >  static bool
> > -attachment_needs_flush(struct anv_cmd_buffer *cmd_buffer,
> > -                       struct anv_render_pass_attachment *att,
> > -                       enum subpass_stage stage)
> > -{
> > -   struct anv_render_pass *pass = cmd_buffer->state.pass;
> > -   const uint32_t subpass_idx = anv_get_subpass_id(&cmd_buffer->state);
> > -
> > -   /* We handle this subpass specially based on the current stage */
> > -   enum anv_subpass_usage usage = att->subpass_usage[subpass_idx];
> > -   switch (stage) {
> > -   case SUBPASS_STAGE_LOAD:
> > -      if (usage & (ANV_SUBPASS_USAGE_INPUT |
> ANV_SUBPASS_USAGE_RESOLVE_SRC))
> > -         return true;
> > -      break;
> > -
> > -   case SUBPASS_STAGE_DRAW:
> > -      if (usage & ANV_SUBPASS_USAGE_RESOLVE_SRC)
> > -         return true;
> > -      break;
> > -
> > -   default:
> > -      break;
> > -   }
> > -
> > -   for (uint32_t s = subpass_idx + 1; s < pass->subpass_count; s++) {
> > -      usage = att->subpass_usage[s];
> > -
> > -      /* If this attachment is going to be used as an input in this or
> any
> > -       * future subpass, then we need to flush its cache and invalidate
> the
> > -       * texture cache.
> > -       */
> > -      if (att->subpass_usage[s] & ANV_SUBPASS_USAGE_INPUT)
> > -         return true;
> > -
> > -      if (usage & (ANV_SUBPASS_USAGE_DRAW |
> ANV_SUBPASS_USAGE_RESOLVE_DST)) {
> > -         /* We found another subpass that draws to this attachment.
> We'll
> > -          * wait to resolve until then.
> > -          */
> > -         return false;
> > -      }
> > -   }
> > -
> > -   return false;
> > -}
> > -
> > -static void
> > -anv_cmd_buffer_flush_attachments(struct anv_cmd_buffer *cmd_buffer,
> > -                                 enum subpass_stage stage)
> > -{
> > -   struct anv_subpass *subpass = cmd_buffer->state.subpass;
> > -   struct anv_render_pass *pass = cmd_buffer->state.pass;
> > -
> > -   for (uint32_t i = 0; i < subpass->color_count; ++i) {
> > -      uint32_t att = subpass->color_attachments[i].attachment;
> > -      assert(att < pass->attachment_count);
> > -      if (attachment_needs_flush(cmd_buffer, &pass->attachments[att],
> stage)) {
> > -         cmd_buffer->state.pending_pipe_bits |=
> > -            ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT |
> > -            ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
> > -      }
> > -   }
> > -
> > -   if (subpass->depth_stencil_attachment.attachment !=
> VK_ATTACHMENT_UNUSED) {
> > -      uint32_t att = subpass->depth_stencil_attachment.attachment;
> > -      assert(att < pass->attachment_count);
> > -      if (attachment_needs_flush(cmd_buffer, &pass->attachments[att],
> stage)) {
> > -         cmd_buffer->state.pending_pipe_bits |=
> > -            ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT |
> > -            ANV_PIPE_DEPTH_CACHE_FLUSH_BIT;
> > -      }
> > -   }
> > -}
> > -
> > -static bool
> >  subpass_needs_clear(const struct anv_cmd_buffer *cmd_buffer)
> >  {
> >     const struct anv_cmd_state *cmd_state = &cmd_buffer->state;
> > @@ -1327,8 +1253,6 @@ anv_cmd_buffer_clear_subpass(struct
> anv_cmd_buffer *cmd_buffer)
> >     }
> >
> >     blorp_batch_finish(&batch);
> > -
> > -   anv_cmd_buffer_flush_attachments(cmd_buffer, SUBPASS_STAGE_LOAD);
> >  }
> >
> >  static void
> > @@ -1554,9 +1478,15 @@ anv_cmd_buffer_resolve_subpass(struct
> anv_cmd_buffer *cmd_buffer)
> >                               subpass->color_attachments[i].attachment);
> >     }
> >
> > -   anv_cmd_buffer_flush_attachments(cmd_buffer, SUBPASS_STAGE_DRAW);
> > -
> >     if (subpass->has_resolve) {
> > +      /* We are about to do some MSAA resolves.  We need to flush so
> that the
> > +       * result of writes to the MSAA color attachments show up in the
> sampler
> > +       * when we blit to the single-sampled resolve target.
> > +       */
> > +      cmd_buffer->state.pending_pipe_bits |=
> > +         ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT |
> > +         ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
> > +
> >        for (uint32_t i = 0; i < subpass->color_count; ++i) {
> >           uint32_t src_att = subpass->color_attachments[i].attachment;
> >           uint32_t dst_att = subpass->resolve_attachments[i].attachment;
> > @@ -1594,8 +1524,6 @@ anv_cmd_buffer_resolve_subpass(struct
> anv_cmd_buffer *cmd_buffer)
> >
> >           ccs_resolve_attachment(cmd_buffer, &batch, dst_att);
> >        }
> > -
> > -      anv_cmd_buffer_flush_attachments(cmd_buffer,
> SUBPASS_STAGE_RESOLVE);
> >     }
> >
> >     blorp_batch_finish(&batch);
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index acb59d5..02dff44 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -2456,6 +2456,9 @@ 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];
> >  }
> >
> >  void genX(CmdNextSubpass)(
> > @@ -2473,6 +2476,10 @@ 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(&cmd_buffer->state);
> > +   cmd_buffer->state.pending_pipe_bits |=
> > +      cmd_buffer->state.pass->subpass_flushes[subpass_id];
>
> Looks like we could move this hunk into genX(cmd_buffer_set_subpass)().
>

We could, yes, and I thought about doing so.  I elected not to because the
subpass flushes are really something that happens *between* subpasses so
you have to have it Begin, Next, and End.  If we rolled it into
set_subpass, I think it would be more confusing what's going on.


> -Nanley
>
> >  }
> >
> >  void genX(CmdEndRenderPass)(
> > @@ -2486,6 +2493,9 @@ void genX(CmdEndRenderPass)(
> >      */
> >     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->state.hiz_enabled = false;
> >
> >  #ifndef NDEBUG
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170406/ff97fb0c/attachment.html>


More information about the mesa-dev mailing list