[Mesa-dev] [PATCH 16/22] anv/cmd_buffer: Add transition_color_buffer()
Jason Ekstrand
jason at jlekstrand.net
Wed May 10 19:38:50 UTC 2017
On Wed, May 10, 2017 at 11:30 AM, Nanley Chery <nanleychery at gmail.com>
wrote:
> On Wed, May 03, 2017 at 02:24:19PM -0700, Jason Ekstrand wrote:
> > On Thu, Apr 27, 2017 at 11:32 AM, Nanley Chery <nanleychery at gmail.com>
> > wrote:
> >
> > > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > > ---
> > > src/intel/vulkan/genX_cmd_buffer.c | 93
> ++++++++++++++++++++++++++++++
> > > ++++++++
> > > 1 file changed, 93 insertions(+)
> > >
> > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > index d5cc358aec..1ae0c3256e 100644
> > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > @@ -454,6 +454,99 @@ genX(transfer_clear_value)(struct anv_cmd_buffer
> *
> > > const cmd_buffer,
> > > }
> > > }
> > >
> > > +/* Transitions a color buffer from one layout to another. */
> > > +static void
> > > +transition_color_buffer(struct anv_cmd_buffer * const cmd_buffer,
> > > + const struct anv_image * const image,
> > > + const uint32_t base_level, uint32_t
> level_count,
> > > + const uint32_t base_layer, uint32_t
> layer_count,
> > > + const VkImageLayout initial_layout,
> > > + const VkImageLayout final_layout)
> > > +{
> > > + assert(cmd_buffer && image);
> > > +
> > > + /* This must be a color image. */
> > > + assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> > > +
> > > + /* Only color buffers with CCS need resolving. */
> > > + if (image->aux_surface.isl.size == 0 || image->samples > 1)
> > > + return;
> > > +
> > > + /* Don't transition this subresource range if it lacks auxiliary
> data.
> > > */
> > > + if (base_level >= anv_color_aux_levels(image) ||
> > > + base_layer >= anv_color_aux_layers(image, base_level))
> > > + return;
> > > +
> > > + /* The undefined layout indicates that the user doesn't care about
> the
> > > + * data that's currently in the buffer. The pre-initialized layout
> is
> > > + * equivalent to the undefined layout for optimally-tiled images.
> > > + *
> > > + * We can only skip the resolve for CCS_E images in this layout
> > > because it
> > > + * is enabled outside of render passes. This allows previously
> > > fast-cleared
> > > + * and undefined buffers to be defined with transfer operations.
> > > + */
> > > + const bool is_ccs_e = image->aux_usage == ISL_AUX_USAGE_CCS_E;
> > > + const bool undef_layout = initial_layout ==
> VK_IMAGE_LAYOUT_UNDEFINED
> > > ||
> > > + initial_layout == VK_IMAGE_LAYOUT_
> > > PREINITIALIZED;
> > > + if (is_ccs_e && undef_layout)
> > > + return;
> > > +
> > > + /* A resolve isn't necessary when transitioning from a layout that
> > > doesn't
> > > + * have fast-clear data or to a layout that will be aware of the
> > > fast-clear
> > > + * value.
> > > + */
> > > + const bool maybe_fast_cleared = undef_layout || initial_layout ==
> > > + VK_IMAGE_LAYOUT_COLOR_
> > > ATTACHMENT_OPTIMAL;
> > > + if (!maybe_fast_cleared || final_layout ==
> > > + VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL)
> > > + return;
> > > +
> > > + /* Determine the optimal resolve operation. For now we only need to
> > > resolve
> > > + * the clear color.
> > > + */
> > > + const enum blorp_fast_clear_op op = is_ccs_e ?
> > > + BLORP_FAST_CLEAR_OP_RESOLVE_
> PARTIAL
> > > :
> > > + BLORP_FAST_CLEAR_OP_RESOLVE_
> FULL;
> > > +
> > > + /* The actual range that will be transitioned is limited by the
> number
> > > of
> > > + * subresources that have auxiliary data.
> > > + */
> > > + level_count = MIN2(level_count, anv_color_aux_levels(image));
> > > +
> > > + /* A write cache flush with an end-of-pipe sync is required between
> > > + * rendering, clearing, and resolving operations. Perform a flush
> of
> > > the
> > > + * write cache before and after the resolve operation to meet this
> > > + * requirement.
> > > + */
> > >
> >
> > anv_blorp.c has a more detailed comment about this. I think I'd rather
> we
> > copy it here so that it doesn't get lost when we delete
> > ccs_resolve_attachments.
> >
> > Also, I have no idea whether we need to do it per-fast-clear-op or once
> > before we do fast-clear ops and once afterwards. I think this is
> probably
> > fine.
> >
> >
>
> Contrary to the comment in anv_blorp, I found documentation that
> provides us more information as to what's needed. How about I rewrite my
> comment to this instead?
>
> From the Sky Lake PRM Vol. 7, "MCS Buffer for Render Target(s)":
>
> Any transition from any value in {Clear, Render, Resolve} to a
> different value in {Clear, Render, Resolve} requires end of pipe
> synchronization.
>
> Perform a flush of the write cache before and after the resolve
> operation to meet this requirement.
>
That PRM citation is fine and maybe better than the one I used (though they
say about the same thing). The part I was hoping to preserve is
* This comment is a bit cryptic and doesn't really tell you what's going
* or what's really needed. It appears that fast clear ops are not
* properly synchronized with other drawing. We need to use a
PIPE_CONTROL
* to ensure that the contents of the previous draw hit the render target
* before we resolve and then use a second PIPE_CONTROL after the resolve
* to ensure that it is completed before any additional drawing occurs.
Yes, the PRM text tells you that an end of pipe sync is needed, but it
doesn't tell you where. It's empirical evidence from hunting down bugs
that tells us we need it on both sides.
--Jason
> -Nanley
>
> > > + cmd_buffer->state.pending_pipe_bits |=
> > > + ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT | ANV_PIPE_CS_STALL_BIT;
> > > +
> > > + for (uint32_t level = base_level; level < base_level + level_count;
> > > level++) {
> > > +
> > > + layer_count = MIN2(layer_count, anv_color_aux_layers(image,
> level));
> > > + for (uint32_t layer = base_layer; layer < base_layer +
> layer_count;
> > > layer++) {
> > > +
> > > + /* Create a surface state with the right clear color and
> perform
> > > the
> > > + * resolve.
> > > + */
> > > + struct anv_state surface_state =
> > > + anv_cmd_buffer_alloc_surface_state(cmd_buffer);
> > > + anv_fill_ccs_resolve_ss(cmd_buffer->device,
> surface_state.map,
> > > image,
> > > + level, layer);
> > > + add_image_relocs(cmd_buffer, image,
> VK_IMAGE_ASPECT_COLOR_BIT,
> > > + is_ccs_e ? ISL_AUX_USAGE_CCS_E :
> > > ISL_AUX_USAGE_CCS_D,
> > > + surface_state);
> > > + anv_state_flush(cmd_buffer->device, surface_state);
> > > + genX(transfer_clear_value)(cmd_buffer, surface_state, image,
> > > level,
> > > + false);
> > > + anv_ccs_resolve(cmd_buffer, surface_state, image, level,
> layer,
> > > op);
> > > + }
> > > + }
> > > +
> > > + cmd_buffer->state.pending_pipe_bits |=
> > > + ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT | ANV_PIPE_CS_STALL_BIT;
> > > +}
> > > +
> > > /**
> > > * Setup anv_cmd_state::attachments for vkCmdBeginRenderPass.
> > > */
> > > --
> > > 2.12.2
> > >
> > > _______________________________________________
> > > 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/20170510/0b430568/attachment.html>
More information about the mesa-dev
mailing list