<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Apr 27, 2017 at 11:32 AM, 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">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 <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> --- 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; </blockquote><div> </div><div>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. </div><div>Other than that, this patch seems perfectly reasonable. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> /* 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 <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> </blockquote></div> </div></div>