[Mesa-dev] [PATCH 15/22] anv/cmd_buffer: Adjust the image view reloc function

Jason Ekstrand jason at jlekstrand.net
Wed May 3 00:15:46 UTC 2017


On Thu, Apr 27, 2017 at 11:32 AM, Nanley Chery <nanleychery at gmail.com>
wrote:

> Make the function take in an image instead of an image view. This
> enables us to record relocations for surfaces states created outside of
> the anv_CreateImageView path.
>
> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 45 +++++++++++++++++++++---------
> --------
>  1 file changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 4698270abb..d5cc358aec 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -167,17 +167,20 @@ add_surface_state_reloc(struct anv_cmd_buffer
> *cmd_buffer,
>  }
>
>  static void
> -add_image_view_relocs(struct anv_cmd_buffer *cmd_buffer,
> -                      const struct anv_image_view *iview,
> -                      enum isl_aux_usage aux_usage,
> -                      struct anv_state state)
> +add_image_relocs(struct anv_cmd_buffer * const cmd_buffer,
> +                 const struct anv_image * const image,
> +                 const VkImageAspectFlags aspect_mask,
> +                 const enum isl_aux_usage aux_usage,
> +                 const struct anv_state state)
>  {
>     const struct isl_device *isl_dev = &cmd_buffer->device->isl_dev;
> +   const uint32_t surf_offset = image->offset +
> +      anv_image_get_surface_for_aspect_mask(image, aspect_mask)->offset;
>
> -   add_surface_state_reloc(cmd_buffer, state, iview->bo, iview->offset);
> +   add_surface_state_reloc(cmd_buffer, state, image->bo, surf_offset);
>
>     if (aux_usage != ISL_AUX_USAGE_NONE) {
> -      uint32_t aux_offset = iview->offset + iview->image->aux_surface.
> offset;
> +      uint32_t aux_offset = surf_offset + image->aux_surface.offset;
>

Ugh... This is not quite the calculation we want (and neither was the old
one!)  surf_offset is image->offset plus the offset of the main color
surface so adding that to aux_surface.offset isn't right.  I think you just
want image->offset + image->aux_surface.offset. That said, it's always safe
because every surface that has aux (color and depth) has surface->offset ==
0.  We should fix it while we're here anyway.

Other than that, this patch seems perfectly reasonable.


>
>        /* On gen7 and prior, the bottom 12 bits of the MCS base address are
>         * used to store other information.  This should be ok, however,
> because
> @@ -191,7 +194,7 @@ add_image_view_relocs(struct anv_cmd_buffer
> *cmd_buffer,
>           anv_reloc_list_add(&cmd_buffer->surface_relocs,
>                              &cmd_buffer->pool->alloc,
>                              state.offset + isl_dev->ss.aux_addr_offset,
> -                            iview->bo, aux_offset);
> +                            image->bo, aux_offset);
>        if (result != VK_SUCCESS)
>           anv_batch_set_error(&cmd_buffer->batch, result);
>     }
> @@ -586,9 +589,9 @@ genX(cmd_buffer_setup_attachments)(struct
> anv_cmd_buffer *cmd_buffer,
>                                  .clear_color = clear_color,
>                                  .mocs = cmd_buffer->device->default_
> mocs);
>
> -            add_image_view_relocs(cmd_buffer, iview,
> -                                  state->attachments[i].aux_usage,
> -                                  state->attachments[i].color_rt_state);
> +            add_image_relocs(cmd_buffer, iview->image, iview->aspect_mask,
> +                             state->attachments[i].aux_usage,
> +                             state->attachments[i].color_rt_state);
>
>              /* Update the image subresource's fast-clear value as
> necessary. */
>              if (state->attachments[i].fast_clear) {
> @@ -635,9 +638,9 @@ genX(cmd_buffer_setup_attachments)(struct
> anv_cmd_buffer *cmd_buffer,
>                                  .clear_color = clear_color,
>                                  .mocs = cmd_buffer->device->default_
> mocs);
>
> -            add_image_view_relocs(cmd_buffer, iview,
> -                                  state->attachments[i].input_aux_usage,
> -                                  state->attachments[i].input_att_state);
> +            add_image_relocs(cmd_buffer, iview->image, iview->aspect_mask,
> +                             state->attachments[i].input_aux_usage,
> +                             state->attachments[i].input_att_state);
>           }
>        }
>
> @@ -1252,8 +1255,9 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
>              desc->image_view->no_aux_sampler_surface_state :
>              desc->image_view->sampler_surface_state;
>           assert(surface_state.alloc_size);
> -         add_image_view_relocs(cmd_buffer, desc->image_view,
> -                               desc->aux_usage, surface_state);
> +         add_image_relocs(cmd_buffer, desc->image_view->image,
> +                          desc->image_view->aspect_mask,
> +                          desc->aux_usage, surface_state);
>           break;
>        case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
>           assert(stage == MESA_SHADER_FRAGMENT);
> @@ -1265,8 +1269,9 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
>                 desc->image_view->no_aux_sampler_surface_state :
>                 desc->image_view->sampler_surface_state;
>              assert(surface_state.alloc_size);
> -            add_image_view_relocs(cmd_buffer, desc->image_view,
> -                                  desc->aux_usage, surface_state);
> +            add_image_relocs(cmd_buffer, desc->image_view->image,
> +                             desc->image_view->aspect_mask,
> +                             desc->aux_usage, surface_state);
>           } else {
>              /* For color input attachments, we create the surface state at
>               * vkBeginRenderPass time so that we can include aux and clear
> @@ -1284,9 +1289,9 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
>              ? desc->image_view->writeonly_storage_surface_state
>              : desc->image_view->storage_surface_state;
>           assert(surface_state.alloc_size);
> -         add_image_view_relocs(cmd_buffer, desc->image_view,
> -                               desc->image_view->image->aux_usage,
> -                               surface_state);
> +         add_image_relocs(cmd_buffer, desc->image_view->image,
> +                          desc->image_view->aspect_mask,
> +                          desc->image_view->image->aux_usage,
> surface_state);
>
>           struct brw_image_param *image_param =
>              &cmd_buffer->state.push_constants[stage]->images[image++];
> --
> 2.12.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170502/cbc88094/attachment-0001.html>


More information about the mesa-dev mailing list