[Mesa-dev] [PATCH 1/2] anv/cmd_buffer: Don't call set_subpass in a secondary
Nanley Chery
nanleychery at gmail.com
Thu Oct 6 20:33:14 UTC 2016
On Thu, Oct 06, 2016 at 12:35:53PM -0700, Jason Ekstrand wrote:
> On Thu, Oct 6, 2016 at 12:30 PM, Nanley Chery <nanleychery at gmail.com> wrote:
>
> > On Wed, Oct 05, 2016 at 05:36:43PM -0700, Jason Ekstrand wrote:
> > > Initially, we had intended set_subpass to be an interesting function that
> > > did whatever (presumably a lot) setup we needed for a subpass. In
> > reality,
> > > it just sets a pointer and a dirty bit and then emits depth and stencil
> > > state. When we call BeginCommandBuffer on a secondary, all of the dirty
> > > bits are already set and there's no point in setting depth and stencil
> > > state since it will already be set by the primary. Instead, the only
> > thing
> > > we need to do at the start of a secondary is set the subpass pointer.
> > >
> > > Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> > > ---
> > > src/intel/vulkan/anv_cmd_buffer.c | 39 +-----------------------------
> > --------
> > > src/intel/vulkan/anv_genX.h | 3 ---
> > > src/intel/vulkan/anv_private.h | 3 ---
> > > src/intel/vulkan/genX_cmd_buffer.c | 5 +----
> > > 4 files changed, 2 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_cmd_buffer.c
> > b/src/intel/vulkan/anv_cmd_buffer.c
> > > index 9dedde8..ef13dfc 100644
> > > --- a/src/intel/vulkan/anv_cmd_buffer.c
> > > +++ b/src/intel/vulkan/anv_cmd_buffer.c
> > > @@ -407,10 +407,8 @@ VkResult anv_BeginCommandBuffer(
> > > cmd_buffer->state.pass =
> > > anv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->
> > renderPass);
> > >
> > > - struct anv_subpass *subpass =
> > > + cmd_buffer->state.subpass =
> > > &cmd_buffer->state.pass->subpasses[pBeginInfo->
> > pInheritanceInfo->subpass];
> > > -
> > > - anv_cmd_buffer_set_subpass(cmd_buffer, subpass);
> >
> > I'm not sure why we always set the fragment descriptor bit in
> > set_subpass, but it seems like we need to do it here as well to keep
> > the logic the same. I don't see where we set the dirty bits on a
> > secondary command buffer at BeginCommandBuffer. Aside from that, this
> > patch looks good.
> >
>
> Initially, I think we did it to ensure that binding tables got re-emitted.
> However, we're now also re-emitting binding tables on pipeline changes and
> you have a pipeline change at the top of every subpass, so it shouldn't be
> needed either place.
>
>
That makes sense. This series is
Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>
> > -Nanley
> >
> > > }
> > >
> > > return VK_SUCCESS;
> > > @@ -1050,41 +1048,6 @@ anv_cmd_buffer_merge_dynamic(struct
> > anv_cmd_buffer *cmd_buffer,
> > > return state;
> > > }
> > >
> > > -/**
> > > - * @brief Setup the command buffer for recording commands inside the
> > given
> > > - * subpass.
> > > - *
> > > - * This does not record all commands needed for starting the subpass.
> > > - * Starting the subpass may require additional commands.
> > > - *
> > > - * Note that vkCmdBeginRenderPass, vkCmdNextSubpass, and
> > vkBeginCommandBuffer
> > > - * with VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT, all setup the
> > > - * command buffer for recording commands for some subpass. But only
> > the first
> > > - * two, vkCmdBeginRenderPass and vkCmdNextSubpass, can start a subpass.
> > > - */
> > > -void
> > > -anv_cmd_buffer_set_subpass(struct anv_cmd_buffer *cmd_buffer,
> > > - struct anv_subpass *subpass)
> > > -{
> > > - switch (cmd_buffer->device->info.gen) {
> > > - case 7:
> > > - if (cmd_buffer->device->info.is_haswell) {
> > > - gen75_cmd_buffer_set_subpass(cmd_buffer, subpass);
> > > - } else {
> > > - gen7_cmd_buffer_set_subpass(cmd_buffer, subpass);
> > > - }
> > > - break;
> > > - case 8:
> > > - gen8_cmd_buffer_set_subpass(cmd_buffer, subpass);
> > > - break;
> > > - case 9:
> > > - gen9_cmd_buffer_set_subpass(cmd_buffer, subpass);
> > > - break;
> > > - default:
> > > - unreachable("unsupported gen\n");
> > > - }
> > > -}
> > > -
> > > struct anv_state
> > > anv_cmd_buffer_push_constants(struct anv_cmd_buffer *cmd_buffer,
> > > gl_shader_stage stage)
> > > diff --git a/src/intel/vulkan/anv_genX.h b/src/intel/vulkan/anv_genX.h
> > > index 02e79c2..dc2dd5d 100644
> > > --- a/src/intel/vulkan/anv_genX.h
> > > +++ b/src/intel/vulkan/anv_genX.h
> > > @@ -36,9 +36,6 @@ struct anv_state
> > > genX(cmd_buffer_alloc_null_surface_state)(struct anv_cmd_buffer
> > *cmd_buffer,
> > > struct anv_framebuffer *fb);
> > >
> > > -void genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer *cmd_buffer,
> > > - struct anv_subpass *subpass);
> > > -
> > > void genX(cmd_buffer_apply_pipe_flushes)(struct anv_cmd_buffer
> > *cmd_buffer);
> > >
> > > void genX(flush_pipeline_select_3d)(struct anv_cmd_buffer *cmd_buffer);
> > > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> > private.h
> > > index 443c31f..4fa403f 100644
> > > --- a/src/intel/vulkan/anv_private.h
> > > +++ b/src/intel/vulkan/anv_private.h
> > > @@ -1394,9 +1394,6 @@ void anv_cmd_buffer_emit_state_base_address(struct
> > anv_cmd_buffer *cmd_buffer);
> > > void anv_cmd_state_setup_attachments(struct anv_cmd_buffer *cmd_buffer,
> > > const VkRenderPassBeginInfo *info);
> > >
> > > -void anv_cmd_buffer_set_subpass(struct anv_cmd_buffer *cmd_buffer,
> > > - struct anv_subpass *subpass);
> > > -
> > > struct anv_state
> > > anv_cmd_buffer_push_constants(struct anv_cmd_buffer *cmd_buffer,
> > > gl_shader_stage stage);
> > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > > index 6a84383..1dff6a1 100644
> > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > @@ -1292,10 +1292,7 @@ cmd_buffer_emit_depth_stencil(struct
> > anv_cmd_buffer *cmd_buffer)
> > > anv_batch_emit(&cmd_buffer->batch, GENX(3DSTATE_CLEAR_PARAMS), cp);
> > > }
> > >
> > > -/**
> > > - * @see anv_cmd_buffer_set_subpass()
> > > - */
> > > -void
> > > +static void
> > > genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer *cmd_buffer,
> > > struct anv_subpass *subpass)
> > > {
> > > --
> > > 2.5.0.400.gff86faf
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
More information about the mesa-dev
mailing list