[Mesa-dev] [PATCH v3 03/16] anv/cmd_buffer: Initialize the clear values buffer
Nanley Chery
nanleychery at gmail.com
Mon Jul 10 21:37:25 UTC 2017
On Mon, Jul 10, 2017 at 09:18:56AM -0700, Jason Ekstrand wrote:
> On Wed, Jun 28, 2017 at 2:14 PM, Nanley Chery <nanleychery at gmail.com> wrote:
>
> > v2: Rewrite functions.
> >
> > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > ---
> > src/intel/vulkan/genX_cmd_buffer.c | 93 ++++++++++++++++++++++++++++++
> > ++++----
> > 1 file changed, 84 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > index 53c58ca5b3..8601d706d1 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -384,6 +384,70 @@ transition_depth_buffer(struct anv_cmd_buffer
> > *cmd_buffer,
> > anv_gen8_hiz_op_resolve(cmd_buffer, image, hiz_op);
> > }
> >
> > +static inline uint32_t
> > +get_fast_clear_state_entry_offset(const struct anv_device *device,
> > + const struct anv_image *image,
> > + unsigned level)
> > +{
> > + assert(device && image);
> > + assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> > + assert(level < anv_image_aux_levels(image));
> > + const uint32_t offset = image->offset + image->aux_surface.offset +
> > + image->aux_surface.isl.size +
> > + anv_fast_clear_state_entry_size(device) *
> > level;
> > + assert(offset < image->offset + image->size);
> > + return offset;
> > +}
> > +
> > +static void
> > +init_fast_clear_state_entry(struct anv_cmd_buffer *cmd_buffer,
> > + const struct anv_image *image,
> > + unsigned level)
> > +{
> > + assert(cmd_buffer && image);
> > + assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> > + assert(level < anv_image_aux_levels(image));
> > +
> > + /* The fast clear value dword(s) will be copied into a surface state
> > object.
> > + * Ensure that the restrictions of the fields in the dword(s) are
> > followed.
> > + *
> > + * CCS buffers on SKL+ can have any value set for the clear colors.
> > + */
> > + if (image->samples == 1 && GEN_GEN >= 9)
> > + return;
> > +
> > + /* Other combinations of auxiliary buffers and platforms require
> > specific
> > + * values in the clear value dword(s).
> > + */
> > + unsigned i = 0;
> > + for (; i < cmd_buffer->device->isl_dev.ss.clear_value_size; i += 4) {
> > + anv_batch_emit(&cmd_buffer->batch, GENX(MI_STORE_DATA_IMM), sdi) {
> > + const uint32_t entry_offset =
> > + get_fast_clear_state_entry_offset(cmd_buffer->device, image,
> > level);
> > + sdi.Address = (struct anv_address) { image->bo, entry_offset + i
> > };
> > +
> > + if (GEN_GEN >= 9) {
> > + /* MCS buffers on SKL+ can only have 1/0 clear colors. */
> > + assert(image->aux_usage == ISL_AUX_USAGE_MCS);
> > + sdi.ImmediateData = 0;
> > + } else {
> > + /* Pre-SKL, the dword containing the clear values also
> > contains
> > + * other fields, so we need to initialize those fields to
> > match the
> > + * values that would be in a color attachment.
> > + */
> > + assert(i == 0);
> > + sdi.ImmediateData = level << 8;
> >
>
> From the Broadwell PRM, RENDER_SURFACE_STATE::Resource Min LOD:
>
> For Sampling Engine Surfaces:
> This field indicates the most detailed LOD that is present in the resource
> underlying the surface.
> Refer to the "LOD Computation Pseudocode" section for the use of this field.
>
> For Other Surfaces:
> This field is ignored.
>
> I think we can safely leave this field zero since this will only ever be
> ORed into render target surfaces.
I agree that we can leave this as zero, but I should mention that we
don't perform any OR operations with this dword and that this field will
be used with input attachments, which are sampling engine surfaces.
> Grepping through isl_surface_state.c also indicates that we never set
> this field in either GL or Vulkan so it's always zero.
>
> --Jason
>
>
Good find. Additionally, going by the description of the field in the HW
docs, setting it to a non-zero value seems incorrect.
Thanks,
Nanley
> > + if (GEN_VERSIONx10 >= 75) {
> > + sdi.ImmediateData |= ISL_CHANNEL_SELECT_RED << 25 |
> > + ISL_CHANNEL_SELECT_GREEN << 22 |
> > + ISL_CHANNEL_SELECT_BLUE << 19 |
> > + ISL_CHANNEL_SELECT_ALPHA << 16;
> >
>
> These, however, are needed. :-)
>
>
> > + }
> > + }
> > + }
> > + }
> > +}
> > +
> > static void
> > transition_color_buffer(struct anv_cmd_buffer *cmd_buffer,
> > const struct anv_image *image,
> > @@ -392,7 +456,9 @@ transition_color_buffer(struct anv_cmd_buffer
> > *cmd_buffer,
> > VkImageLayout initial_layout,
> > VkImageLayout final_layout)
> > {
> > - if (image->aux_usage != ISL_AUX_USAGE_CCS_E)
> > + assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> > +
> > + if (image->aux_surface.isl.size == 0)
> > return;
> >
> > if (initial_layout != VK_IMAGE_LAYOUT_UNDEFINED &&
> > @@ -405,15 +471,24 @@ transition_color_buffer(struct anv_cmd_buffer
> > *cmd_buffer,
> > layer_count = anv_minify(image->extent.depth, base_level);
> > }
> >
> > -#if GEN_GEN >= 9
> > - /* We're transitioning from an undefined layout so it doesn't really
> > matter
> > - * what data ends up in the color buffer. We do, however, need to
> > ensure
> > - * that the CCS has valid data in it. One easy way to do that is to
> > - * fast-clear the specified range.
> > + /* We're interested in the subresource range subset that has aux data.
> > */
> > + level_count = MIN2(level_count, anv_image_aux_levels(image));
> > +
> > + /* We're transitioning from an undefined layout. We must ensure that
> > the
> > + * clear values buffer is filled with valid data.
> > */
> > - anv_image_ccs_clear(cmd_buffer, image, base_level, level_count,
> > - base_layer, layer_count);
> > -#endif
> > + for (unsigned l = 0; l < level_count; l++)
> > + init_fast_clear_state_entry(cmd_buffer, image, base_level + l);
> > +
> > + if (image->aux_usage == ISL_AUX_USAGE_CCS_E) {
> > + /* We're transitioning from an undefined layout so it doesn't really
> > + * matter what data ends up in the color buffer. We do, however,
> > need to
> > + * ensure that the CCS has valid data in it. One easy way to do
> > that is
> > + * to fast-clear the specified range.
> > + */
> > + anv_image_ccs_clear(cmd_buffer, image, base_level, level_count,
> > + base_layer, layer_count);
> > + }
> > }
> >
> > /**
> > --
> > 2.13.1
> >
> > _______________________________________________
> > 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