[Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

Pohjolainen, Topi topi.pohjolainen at gmail.com
Thu Nov 30 16:20:51 UTC 2017


On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> 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       | 36 ++++++++++++++++++++++++++++++++++++
>  src/intel/vulkan/anv_cmd_buffer.c  | 12 ++++++++++++
>  src/intel/vulkan/anv_genX.h        |  6 ++++++
>  src/intel/vulkan/anv_private.h     |  7 +++++++
>  src/intel/vulkan/genX_cmd_buffer.c | 17 +++++++++++++++++
>  5 files changed, 78 insertions(+)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index da273d6..46e2eb0 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
>  
>        assert(anv_image_aspects_compatible(src_mask, dst_mask));
>  
> +      const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
>        if (_mesa_bitcount(src_mask) > 1) {
>           uint32_t aspect_bit;
>           anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask) {
> @@ -281,6 +282,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);
>  
>              for (unsigned i = 0; i < layer_count; i++) {
>                 blorp_copy(&batch, &src_surf, pRegions[r].srcSubresource.mipLevel,
> @@ -298,6 +303,8 @@ 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);
>  
>           for (unsigned i = 0; i < layer_count; i++) {
>              blorp_copy(&batch, &src_surf, pRegions[r].srcSubresource.mipLevel,
> @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer *cmd_buffer,
>                                      extent.width, extent.height,
>                                      buffer_row_pitch, buffer_format,
>                                      &buffer.surf, &buffer_isl_surf);
> +      if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {

In all the other call sites you call anv_cmd_buffer_mark_image_written()
regardless if aux usage is none. Is there something special here?

> +         assert(dst == &image);
> +         anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> +                                           aspect, dst->surf.aux_usage,
> +                                           dst->level);
> +      }
>  
>        for (unsigned z = 0; z < extent.depth; z++) {
>           blorp_copy(&batch, &src->surf, src->level, src->offset.z,
> @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
>        get_blorp_surf_for_anv_image(cmd_buffer->device,
>                                     dst_image, dst_res->aspectMask,
>                                     ANV_AUX_USAGE_DEFAULT, &dst);
> +      anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> +                                        dst_res->aspectMask,
> +                                        dst.aux_usage,
> +                                        dst_res->mipLevel);
>  
>        struct anv_format_plane src_format =
>           anv_get_format_plane(&cmd_buffer->device->info, src_image->vk_format,
> @@ -820,6 +837,10 @@ 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);
> +
>           blorp_clear(&batch, &surf,
>                       src_format.isl_format, src_format.swizzle,
>                       level, base_layer, layer_count,
> @@ -1215,6 +1236,11 @@ 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);
> +
>           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 +1381,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 +1397,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_mask,
> +                                        dst_surf.aux_usage,
> +                                        dst_level);
>  
>        assert(!src_image->format->can_ycbcr);
>        assert(!dst_image->format->can_ycbcr);
> @@ -1498,6 +1530,10 @@ 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);

Indentation of the last three arguments seems to be one step too far. (Even
with that fixed last one seems to run over 80 columns).

>  
>           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..2c9e919 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -353,6 +353,18 @@ 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,
> +                                  unsigned level)
> +{
> +   anv_genX_call(&cmd_buffer->device->info,
> +                 cmd_buffer_mark_image_written,
> +                 cmd_buffer, image, aspect, aux_usage, level);
> +}
> +
>  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..85b893e 100644
> --- a/src/intel/vulkan/anv_genX.h
> +++ b/src/intel/vulkan/anv_genX.h
> @@ -58,6 +58,12 @@ 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,
> +                                         unsigned level);
> +
>  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 461bfed..f805246 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -2530,6 +2530,13 @@ 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,
> +                                  unsigned level);
> +
> +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 65cc85d..7d040bd 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -460,6 +460,15 @@ 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,
> +                                    unsigned level)
> +{
> +}
> +
>  static void
>  init_fast_clear_state_entry(struct anv_cmd_buffer *cmd_buffer,
>                              const struct anv_image *image,
> @@ -3014,6 +3023,14 @@ 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);
>     }
>  }
>  
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> 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