<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 22, 2018 at 3:22 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Jan 19, 2018 at 03:47:29PM -0800, Jason Ekstrand wrote: <div><div class="h5">> Currently, this helper does nothing but we call it every place where an > image is written through the render pipeline. This will allow us to > properly mark the aux state so that we can handle resolves correctly. > --- > src/intel/vulkan/anv_blorp.c | 44 +++++++++++++++++++++++++++++++++++++- > src/intel/vulkan/anv_cmd_buffer.c | 15 +++++++++++++ > src/intel/vulkan/anv_genX.h | 8 +++++++ > src/intel/vulkan/anv_private.h | 9 ++++++++ > src/intel/vulkan/genX_cmd_buffer.c | 44 ++++++++++++++++++++++++++++++++++++++ > 5 files changed, 119 insertions(+), 1 deletion(-) > > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c > index e4e4135..05efc6d 100644 > --- a/src/intel/vulkan/anv_blorp.c > +++ b/src/intel/vulkan/anv_blorp.c > @@ -283,6 +283,10 @@ void anv_CmdCopyImage( > get_blorp_surf_for_anv_image(cmd_buffer->device, > dst_image, 1UL << aspect_bit, > ANV_AUX_USAGE_DEFAULT, &dst_surf); > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, > + 1UL << aspect_bit, > + dst_surf.aux_usage, dst_level, > + dst_base_layer, layer_count); > > for (unsigned i = 0; i < layer_count; i++) { > blorp_copy(&batch, &src_surf, src_level, src_base_layer + i, > @@ -298,6 +302,9 @@ void anv_CmdCopyImage( > ANV_AUX_USAGE_DEFAULT, &src_surf); > get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image, dst_mask, > ANV_AUX_USAGE_DEFAULT, &dst_surf); > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, dst_mask, > + dst_surf.aux_usage, dst_level, > + dst_base_layer, layer_count); > > for (unsigned i = 0; i < layer_count; i++) { > blorp_copy(&batch, &src_surf, src_level, src_base_layer + i, > @@ -386,6 +393,13 @@ copy_buffer_to_image(struct anv_cmd_buffer *cmd_buffer, > buffer_row_pitch, buffer_format, > &buffer.surf, &buffer_isl_surf); > > + if (&image == dst) { > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image, > + aspect, dst->surf.aux_usage, > + dst->level, > + dst->offset.z, extent.depth); > + } > + > for (unsigned z = 0; z < extent.depth; z++) { > blorp_copy(&batch, &src->surf, src->level, src->offset.z, > &dst->surf, dst->level, dst->offset.z, > @@ -545,6 +559,12 @@ void anv_CmdBlitImage( > bool flip_y = flip_coords(&src_y0, &src_y1, &dst_y0, &dst_y1); > > const unsigned num_layers = dst_end - dst_start; > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, > + dst_res->aspectMask, > + dst.aux_usage, > + dst_res->mipLevel, > + dst_start, num_layers); > + > for (unsigned i = 0; i < num_layers; i++) { > unsigned dst_z = dst_start + i; > unsigned src_z = src_start + i * src_z_step; > @@ -558,7 +578,6 @@ void anv_CmdBlitImage( > dst_x0, dst_y0, dst_x1, dst_y1, > gl_filter, flip_x, flip_y); > } > - </div></div> ^ Random line deletion? Not a big deal. <div><div class="h5"></div></div></blockquote><div> </div><div>Yup. Rebase fail. Dropped. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > } > > blorp_batch_finish(&batch); > @@ -818,6 +837,11 @@ void anv_CmdClearColorImage( > layer_count = anv_minify(image->extent.depth, level); > } > > + anv_cmd_buffer_mark_image_written(cmd_buffer, image, > + pRanges[r].aspectMask, > + surf.aux_usage, level, > + base_layer, layer_count); > + > blorp_clear(&batch, &surf, > src_format.isl_format, src_format.swizzle, > level, base_layer, layer_count, > @@ -1215,6 +1239,13 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer *cmd_buffer) > ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT | ANV_PIPE_CS_STALL_BIT; > } else { > assert(image->n_planes == 1); > + anv_cmd_buffer_mark_image_written(cmd_buffer, image, > + VK_IMAGE_ASPECT_COLOR_BIT, > + att_state->aux_usage, > + iview->planes[0].isl.base_level, > + iview->planes[0].isl.base_array_layer, > + fb->layers); > + > blorp_clear(&batch, &surf, iview->planes[0].isl.format, > anv_swizzle_for_render(iview->planes[0].isl.swizzle), > iview->planes[0].isl.base_level, > @@ -1355,6 +1386,8 @@ resolve_image(struct anv_device *device, > uint32_t src_x, uint32_t src_y, uint32_t dst_x, uint32_t dst_y, > uint32_t width, uint32_t height) > { > + struct anv_cmd_buffer *cmd_buffer = batch->driver_batch; > + > assert(src_image->type == VK_IMAGE_TYPE_2D); > assert(src_image->samples > 1); > assert(dst_image->type == VK_IMAGE_TYPE_2D); > @@ -1369,6 +1402,10 @@ resolve_image(struct anv_device *device, > ANV_AUX_USAGE_DEFAULT, &src_surf); > get_blorp_surf_for_anv_image(device, dst_image, 1UL << aspect_bit, > ANV_AUX_USAGE_DEFAULT, &dst_surf); > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, > + 1UL << aspect_bit, > + dst_surf.aux_usage, > + dst_level, dst_layer, 1); > > assert(!src_image->format->can_ycbcr); > assert(!dst_image->format->can_ycbcr); > @@ -1498,6 +1535,11 @@ anv_cmd_buffer_resolve_subpass(struct anv_cmd_buffer *cmd_buffer) > get_blorp_surf_for_anv_image(cmd_buffer->device, dst_iview->image, > VK_IMAGE_ASPECT_COLOR_BIT, > dst_aux_usage, &dst_surf); > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_iview->image, > + VK_IMAGE_ASPECT_COLOR_BIT, > + dst_surf.aux_usage, > + dst_iview->planes[0].isl.base_level, > + dst_iview->planes[0].isl.base_array_layer, 1); </div></div>As we discussed in the office, this entire function (including this hunk) should be resolving fb->layers. This function was already broken and this new hunk doesn't break it any more, so it's fine to leave it as is. I've filed a bug about the lack of testing for this. <div><div class="h5"></div></div></blockquote><div> </div><div>Sounds good. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > > assert(!src_iview->image->format->can_ycbcr); > assert(!dst_iview->image->format->can_ycbcr); > diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_buffer.c > index 7e7580c..7480ac2 100644 > --- a/src/intel/vulkan/anv_cmd_buffer.c > +++ b/src/intel/vulkan/anv_cmd_buffer.c > @@ -353,6 +353,21 @@ anv_cmd_buffer_emit_state_base_address(struct anv_cmd_buffer *cmd_buffer) > cmd_buffer); > } > > +void > +anv_cmd_buffer_mark_image_written(struct anv_cmd_buffer *cmd_buffer, > + const struct anv_image *image, > + VkImageAspectFlagBits aspect, > + enum isl_aux_usage aux_usage, > + uint32_t level, > + uint32_t base_layer, > + uint32_t layer_count) > +{ > + anv_genX_call(&cmd_buffer->device->info, > + cmd_buffer_mark_image_written, > + cmd_buffer, image, aspect, aux_usage, > + level, base_layer, layer_count); > +} > + > void anv_CmdBindPipeline( > VkCommandBuffer commandBuffer, > VkPipelineBindPoint pipelineBindPoint, > diff --git a/src/intel/vulkan/anv_genX.h b/src/intel/vulkan/anv_genX.h > index 0b7322e..9b56658 100644 > --- a/src/intel/vulkan/anv_genX.h > +++ b/src/intel/vulkan/anv_genX.h > @@ -58,6 +58,14 @@ void genX(cmd_buffer_flush_compute_state)(struct anv_cmd_buffer *cmd_buffer); > void genX(cmd_buffer_enable_pma_fix)(struct anv_cmd_buffer *cmd_buffer, > bool enable); > > +void genX(cmd_buffer_mark_image_written)(struct anv_cmd_buffer *cmd_buffer, > + const struct anv_image *image, > + VkImageAspectFlagBits aspect, > + enum isl_aux_usage aux_usage, > + uint32_t level, > + uint32_t base_layer, > + uint32_t layer_count); > + > void > genX(emit_urb_setup)(struct anv_device *device, struct anv_batch *batch, > const struct gen_l3_config *l3_config, > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h > index f81f8e1..f0251e2 100644 > --- a/src/intel/vulkan/anv_private.h > +++ b/src/intel/vulkan/anv_private.h > @@ -2542,6 +2542,15 @@ anv_can_sample_with_hiz(const struct gen_device_info * const devinfo, > } > > void > +anv_cmd_buffer_mark_image_written(struct anv_cmd_buffer *cmd_buffer, > + const struct anv_image *image, > + VkImageAspectFlagBits aspect, > + enum isl_aux_usage aux_usage, > + uint32_t level, > + uint32_t base_layer, > + uint32_t layer_count); > + > +void > anv_image_hiz_op(struct anv_cmd_buffer *cmd_buffer, > const struct anv_image *image, > VkImageAspectFlagBits aspect, uint32_t level, > diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c > index fd27463..ad7b9fc 100644 > --- a/src/intel/vulkan/genX_cmd_buffer.c > +++ b/src/intel/vulkan/genX_cmd_buffer.c > @@ -460,6 +460,19 @@ genX(load_needs_resolve_predicate)(struct anv_cmd_buffer *cmd_buffer, > } > } > > +void > +genX(cmd_buffer_mark_image_written)(struct anv_cmd_buffer *cmd_buffer, > + const struct anv_image *image, > + VkImageAspectFlagBits aspect, > + enum isl_aux_usage aux_usage, > + uint32_t level, > + uint32_t base_layer, > + uint32_t layer_count) > +{ > + /* The aspect must be exactly one of the image aspects. */ > + assert(_mesa_bitcount(aspect) == 1 && (aspect & image->aspects)); > +} > + > static void > init_fast_clear_state_entry(struct anv_cmd_buffer *cmd_buffer, > const struct anv_image *image, > @@ -2976,6 +2989,27 @@ cmd_buffer_emit_depth_stencil(struct anv_cmd_buffer *cmd_buffer) > isl_emit_depth_stencil_hiz_s(&device->isl_dev, dw, &info); > > cmd_buffer->state.hiz_enabled = info.hiz_usage == ISL_AUX_USAGE_HIZ; > + > + /* We may be writing depth or stencil so we need to mark the surface. > + * Unfortunately, there's no way to know at this point whether the depth or > + * stencil tests used will actually write to the surface. > + */ > + if (image && (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT)) { > + genX(cmd_buffer_mark_image_written)(cmd_buffer, image, > + VK_IMAGE_ASPECT_DEPTH_BIT, > + info.hiz_usage, > + info.view->base_level, > + info.view->base_array_layer, > + info.view->array_len); > + } > + if (image && (image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT)) { > + genX(cmd_buffer_mark_image_written)(cmd_buffer, image, > + VK_IMAGE_ASPECT_STENCIL_BIT, > + ISL_AUX_USAGE_NONE, > + info.view->base_level, > + info.view->base_array_layer, > + info.view->array_len); > + } > } > > > @@ -3174,6 +3208,16 @@ cmd_buffer_subpass_sync_fast_clear_values(struct anv_cmd_buffer *cmd_buffer) > false /* copy to ss */); > } > } > + > + /* We assume that if we're starting a subpass, we're going to do some > + * rendering so we may end up with compressed data. > + */ > + genX(cmd_buffer_mark_image_written)(cmd_buffer, iview->image, > + VK_IMAGE_ASPECT_COLOR_BIT, > + att_state->aux_usage, > + iview->planes[0].isl.base_level, > + iview->planes[0].isl.base_array_layer, > + iview->planes[0].isl.array_len); </div></div>The last argument should be fb->layers right? <div class="HOEnZb"><div class="h5"></div></div></blockquote><div> </div><div>Yup. Thanks for catching that. Fixed locally. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > } > } > > -- > 2.5.0.400.gff86faf > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div></div>