Re: [Mesa-dev] [PATCH 16/22] anv/cmd_buffer: Add transition_color_buffer()

[Nanley Chery]

On Wed, May 10, 2017 at 02:00:19PM -0700, Jason Ekstrand wrote:
> On Wed, May 10, 2017 at 1:35 PM, Nanley Chery  wrote:
> 
> > On Wed, May 10, 2017 at 12:38:50PM -0700, Jason Ekstrand wrote:
> > > On Wed, May 10, 2017 at 11:30 AM, Nanley Chery 
> > > 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 <
> > nanleych...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Signed-off-by: Nanley Chery 
> > > > > > ---
> > > > > >  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 

Re: [Mesa-dev] [PATCH 16/22] anv/cmd_buffer: Add transition_color_buffer()

[Jason Ekstrand]

On Wed, May 10, 2017 at 1:35 PM, Nanley Chery  wrote:

> On Wed, May 10, 2017 at 12:38:50PM -0700, Jason Ekstrand wrote:
> > On Wed, May 10, 2017 at 11:30 AM, Nanley Chery 
> > 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 <
> nanleych...@gmail.com>
> > > > wrote:
> > > >
> > > > > Signed-off-by: Nanley Chery 
> > > > > ---
> > > > >  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 

Re: [Mesa-dev] [PATCH 16/22] anv/cmd_buffer: Add transition_color_buffer()

[Nanley Chery]

On Wed, May 10, 2017 at 12:38:50PM -0700, Jason Ekstrand wrote:
> On Wed, May 10, 2017 at 11:30 AM, Nanley Chery 
> 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 
> > > wrote:
> > >
> > > > Signed-off-by: Nanley Chery 
> > > > ---
> > > >  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 

Re: [Mesa-dev] [PATCH 16/22] anv/cmd_buffer: Add transition_color_buffer()

[Jason Ekstrand]

On Wed, May 10, 2017 at 11:30 AM, Nanley Chery 
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 
> > wrote:
> >
> > > Signed-off-by: Nanley Chery 
> > > ---
> > >  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 

Re: [Mesa-dev] [PATCH 16/22] anv/cmd_buffer: Add transition_color_buffer()

[Nanley Chery]

On Wed, May 03, 2017 at 02:24:19PM -0700, Jason Ekstrand wrote:
> On Thu, Apr 27, 2017 at 11:32 AM, Nanley Chery 
> wrote:
> 
> > Signed-off-by: Nanley Chery 
> > ---
> >  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.

-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++) {
> > +
> > +  

Re: [Mesa-dev] [PATCH 16/22] anv/cmd_buffer: Add transition_color_buffer()

[Jason Ekstrand]

On Thu, Apr 27, 2017 at 11:32 AM, Nanley Chery 
wrote:

> Signed-off-by: Nanley Chery 
> ---
>  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.


> +   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);
> + 

[Mesa-dev] [PATCH 16/22] anv/cmd_buffer: Add transition_color_buffer()

[Nanley Chery]

Signed-off-by: Nanley Chery 
---
 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.
+*/
+   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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev