On Mon, Jul 10, 2017 at 09:18:56AM -0700, Jason Ekstrand wrote: > On Wed, Jun 28, 2017 at 2:14 PM, Nanley Chery <nanleych...@gmail.com> wrote: > > > v2: Rewrite functions. > > > > Signed-off-by: Nanley Chery <nanley.g.ch...@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev