On Thu, Apr 27, 2017 at 11:32 AM, Nanley Chery <nanleych...@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.ch...@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@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