<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, Jan 11, 2019 at 2:19 AM Iago Toral <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, 2019-01-07 at 09:39 -0600, Jason Ekstrand wrote: > This function is modeled after the aux_op functions except that it > has a > lot more parameters because it deals with two images as well as > source > and destination regions. > --- > src/intel/vulkan/anv_blorp.c | 225 ++++++++++++++----------------- > -- > src/intel/vulkan/anv_private.h | 14 ++ > 2 files changed, 107 insertions(+), 132 deletions(-) > > diff --git a/src/intel/vulkan/anv_blorp.c > b/src/intel/vulkan/anv_blorp.c > index eee7a8c3b3c..2f8d502e289 100644 > --- a/src/intel/vulkan/anv_blorp.c > +++ b/src/intel/vulkan/anv_blorp.c > @@ -1169,63 +1169,52 @@ enum subpass_stage { > SUBPASS_STAGE_RESOLVE, > }; > > -static void > -resolve_surface(struct blorp_batch *batch, > - struct blorp_surf *src_surf, > - uint32_t src_level, uint32_t src_layer, > - struct blorp_surf *dst_surf, > - uint32_t dst_level, uint32_t dst_layer, > - uint32_t src_x, uint32_t src_y, uint32_t dst_x, > uint32_t dst_y, > - uint32_t width, uint32_t height, > - enum blorp_filter filter) > -{ > - blorp_blit(batch, > - src_surf, src_level, src_layer, > - ISL_FORMAT_UNSUPPORTED, ISL_SWIZZLE_IDENTITY, > - dst_surf, dst_level, dst_layer, > - ISL_FORMAT_UNSUPPORTED, ISL_SWIZZLE_IDENTITY, > - src_x, src_y, src_x + width, src_y + height, > - dst_x, dst_y, dst_x + width, dst_y + height, > - filter, false, false); > -} > - > -static void > -resolve_image(struct anv_device *device, > - struct blorp_batch *batch, > - const struct anv_image *src_image, > - VkImageLayout src_image_layout, > - uint32_t src_level, uint32_t src_layer, > - const struct anv_image *dst_image, > - VkImageLayout dst_image_layout, > - uint32_t dst_level, uint32_t dst_layer, > - VkImageAspectFlags aspect_mask, > - uint32_t src_x, uint32_t src_y, uint32_t dst_x, > uint32_t dst_y, > - uint32_t width, uint32_t height) > +void > +anv_image_msaa_resolve(struct anv_cmd_buffer *cmd_buffer, > + const struct anv_image *src_image, > + enum isl_aux_usage src_aux_usage, > + uint32_t src_level, uint32_t src_base_layer, > + const struct anv_image *dst_image, > + enum isl_aux_usage dst_aux_usage, > + uint32_t dst_level, uint32_t dst_base_layer, > + VkImageAspectFlagBits aspect, > + uint32_t src_x, uint32_t src_y, > + uint32_t dst_x, uint32_t dst_y, > + uint32_t width, uint32_t height, > + uint32_t layer_count, > + enum blorp_filter filter) > { > - struct anv_cmd_buffer *cmd_buffer = batch->driver_batch; > + struct blorp_batch batch; > + blorp_batch_init(&cmd_buffer->device->blorp, &batch, cmd_buffer, > 0); > > assert(src_image->type == VK_IMAGE_TYPE_2D); > assert(src_image->samples > 1); > assert(dst_image->type == VK_IMAGE_TYPE_2D); > assert(dst_image->samples == 1); > assert(src_image->n_planes == dst_image->n_planes); > + assert(!src_image->format->can_ycbcr); > + assert(!dst_image->format->can_ycbcr); > > - uint32_t aspect_bit; > - > - anv_foreach_image_aspect_bit(aspect_bit, src_image, aspect_mask) > { > - struct blorp_surf src_surf, dst_surf; > - get_blorp_surf_for_anv_image(device, src_image, 1UL << > aspect_bit, > - src_image_layout, > ISL_AUX_USAGE_NONE, > - &src_surf); > - get_blorp_surf_for_anv_image(device, dst_image, 1UL << > aspect_bit, > - dst_image_layout, > ISL_AUX_USAGE_NONE, > - &dst_surf); > - anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, > - 1UL << aspect_bit, > - dst_surf.aux_usage, > - dst_level, dst_layer, 1); > - > - enum blorp_filter filter; > + struct blorp_surf src_surf, dst_surf; > + get_blorp_surf_for_anv_image(cmd_buffer->device, src_image, > aspect, > + ANV_IMAGE_LAYOUT_EXPLICIT_AUX, > + src_aux_usage, &src_surf); > + if (src_aux_usage == ISL_AUX_USAGE_MCS) { > + src_surf.clear_color_addr = anv_to_blorp_address( > + anv_image_get_clear_color_addr(cmd_buffer->device, > src_image, > + VK_IMAGE_ASPECT_COLOR_BIT)); > + } > + get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image, > aspect, > + ANV_IMAGE_LAYOUT_EXPLICIT_AUX, > + dst_aux_usage, &dst_surf); > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, > + aspect, dst_aux_usage, > + dst_level, dst_base_layer, > layer_count); > + > + if (filter == BLORP_FILTER_NONE) { > + /* If no explicit filter is provided, then it's implied by the > type of > + * the source image. > + */ > if ((src_surf.surf->usage & ISL_SURF_USAGE_DEPTH_BIT) || > (src_surf.surf->usage & ISL_SURF_USAGE_STENCIL_BIT) || > isl_format_has_int_channel(src_surf.surf->format)) { > @@ -1233,15 +1222,20 @@ resolve_image(struct anv_device *device, > } else { > filter = BLORP_FILTER_AVERAGE; > } > + } > > - assert(!src_image->format->can_ycbcr); > - assert(!dst_image->format->can_ycbcr); > - > - resolve_surface(batch, > - &src_surf, src_level, src_layer, > - &dst_surf, dst_level, dst_layer, > - src_x, src_y, dst_x, dst_y, width, height, > filter); > + for (uint32_t l = 0; l < layer_count; l++) { > + blorp_blit(&batch, > + &src_surf, src_level, src_base_layer + l, > + ISL_FORMAT_UNSUPPORTED, ISL_SWIZZLE_IDENTITY, > + &dst_surf, dst_level, dst_base_layer + l, > + ISL_FORMAT_UNSUPPORTED, ISL_SWIZZLE_IDENTITY, > + src_x, src_y, src_x + width, src_y + height, > + dst_x, dst_y, dst_x + width, dst_y + height, > + filter, false, false); > } > + > + blorp_batch_finish(&batch); > } > > void anv_CmdResolveImage( > @@ -1257,8 +1251,7 @@ void anv_CmdResolveImage( > ANV_FROM_HANDLE(anv_image, src_image, srcImage); > ANV_FROM_HANDLE(anv_image, dst_image, dstImage); > > - struct blorp_batch batch; > - blorp_batch_init(&cmd_buffer->device->blorp, &batch, cmd_buffer, > 0); > + assert(!src_image->format->can_ycbcr); > > for (uint32_t r = 0; r < regionCount; r++) { > assert(pRegions[r].srcSubresource.aspectMask == > @@ -1269,27 +1262,38 @@ void anv_CmdResolveImage( > const uint32_t layer_count = > anv_get_layerCount(dst_image, &pRegions[r].dstSubresource); > > - VkImageAspectFlags src_mask = > pRegions[r].srcSubresource.aspectMask, > - dst_mask = pRegions[r].dstSubresource.aspectMask; > + VkImageAspectFlags src_mask = > pRegions[r].srcSubresource.aspectMask; > + VkImageAspectFlags dst_mask = > pRegions[r].dstSubresource.aspectMask; > > assert(anv_image_aspects_compatible(src_mask, dst_mask)); > > - for (uint32_t layer = 0; layer < layer_count; layer++) { > - resolve_image(cmd_buffer->device, &batch, > - src_image, srcImageLayout, > - pRegions[r].srcSubresource.mipLevel, > - pRegions[r].srcSubresource.baseArrayLayer + > layer, > - dst_image, dstImageLayout, > - pRegions[r].dstSubresource.mipLevel, > - pRegions[r].dstSubresource.baseArrayLayer + > layer, > - pRegions[r].dstSubresource.aspectMask, > - pRegions[r].srcOffset.x, > pRegions[r].srcOffset.y, > - pRegions[r].dstOffset.x, > pRegions[r].dstOffset.y, > - pRegions[r].extent.width, > pRegions[r].extent.height); > + uint32_t aspect_bit; > + anv_foreach_image_aspect_bit(aspect_bit, src_image, > + pRegions[r].srcSubresource.aspect > Mask) { > + enum isl_aux_usage src_aux_usage = > + anv_layout_to_aux_usage(&cmd_buffer->device->info, > src_image, > + (1 << aspect_bit), > srcImageLayout); > + enum isl_aux_usage dst_aux_usage = > + anv_layout_to_aux_usage(&cmd_buffer->device->info, > dst_image, > + (1 << aspect_bit), > dstImageLayout); > + > + anv_image_msaa_resolve(cmd_buffer, > + src_image, src_aux_usage, > + pRegions[r].srcSubresource.mipLevel, > + pRegions[r].srcSubresource.baseArray > Layer, > + dst_image, dst_aux_usage, > + pRegions[r].dstSubresource.mipLevel, > + pRegions[r].dstSubresource.baseArray > Layer, > + (1 << aspect_bit), > + pRegions[r].srcOffset.x, > + pRegions[r].srcOffset.y, > + pRegions[r].dstOffset.x, > + pRegions[r].dstOffset.y, > + pRegions[r].extent.width, > + pRegions[r].extent.height, > + layer_count, BLORP_FILTER_NONE); > } > } > - > - blorp_batch_finish(&batch); > } > > static enum isl_aux_usage > @@ -1310,9 +1314,6 @@ anv_cmd_buffer_resolve_subpass(struct > anv_cmd_buffer *cmd_buffer) > struct anv_subpass *subpass = cmd_buffer->state.subpass; > > if (subpass->has_color_resolve) { > - struct blorp_batch batch; > - blorp_batch_init(&cmd_buffer->device->blorp, &batch, > cmd_buffer, 0); > - > /* We are about to do some MSAA resolves. We need to flush so > that the > * result of writes to the MSAA color attachments show up in > the sampler > * when we blit to the single-sampled resolve target. > @@ -1345,70 +1346,30 @@ anv_cmd_buffer_resolve_subpass(struct > anv_cmd_buffer *cmd_buffer) > struct anv_image_view *src_iview = fb- > >attachments[src_att]; > struct anv_image_view *dst_iview = fb- > >attachments[dst_att]; > > + const VkRect2D render_area = cmd_buffer->state.render_area; > + > enum isl_aux_usage src_aux_usage = > cmd_buffer->state.attachments[src_att].aux_usage; > enum isl_aux_usage dst_aux_usage = > cmd_buffer->state.attachments[dst_att].aux_usage; > > - const VkRect2D render_area = cmd_buffer->state.render_area; > - > assert(src_iview->aspect_mask == VK_IMAGE_ASPECT_COLOR_BIT > && > dst_iview->aspect_mask == > VK_IMAGE_ASPECT_COLOR_BIT); > > - enum blorp_filter filter; > - if (isl_format_has_int_channel(src_iview- > >planes[0].isl.format)) { This is a bit different from what anv_image_msaa_resolve() does. In the new helper function we check if the usage is depth or stencil, but this was checking if the format has any integer channel, which I understand would be true even for color attachments with integer formats, right? Since the average filter seems to require a float type, maybe checking for integer channels instead of usage in anv_image_msaa_resolve() would be more correct? </blockquote><div> </div><div>anv_image_msaa_resolve checks usage *and* isl_format_has_int_channel so it does AVERAGE only for color float targets and SAMPLE_0 for everything else. It's a bit hard to see in the diff but it's there.</div><div> </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > - filter = BLORP_FILTER_SAMPLE_0; > - } else { > - filter = BLORP_FILTER_AVERAGE; > - } > - > - struct blorp_surf src_surf, dst_surf; > - get_blorp_surf_for_anv_image(cmd_buffer->device, src_iview- > >image, > - VK_IMAGE_ASPECT_COLOR_BIT, > - ANV_IMAGE_LAYOUT_EXPLICIT_AUX, > - src_aux_usage, &src_surf); > - if (src_aux_usage == ISL_AUX_USAGE_MCS) { > - src_surf.clear_color_addr = anv_to_blorp_address( > - anv_image_get_clear_color_addr(cmd_buffer->device, > - src_iview->image, > - VK_IMAGE_ASPECT_COLOR_ > BIT)); > - } > - get_blorp_surf_for_anv_image(cmd_buffer->device, dst_iview- > >image, > - VK_IMAGE_ASPECT_COLOR_BIT, > - ANV_IMAGE_LAYOUT_EXPLICIT_AUX, > - dst_aux_usage, &dst_surf); > - > - uint32_t base_src_layer = src_iview- > >planes[0].isl.base_array_layer; > - uint32_t base_dst_layer = dst_iview- > >planes[0].isl.base_array_layer; > - > - assert(src_iview->planes[0].isl.array_len >= fb->layers); > - assert(dst_iview->planes[0].isl.array_len >= fb->layers); > - > - 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, > - base_dst_layer, fb- > >layers); > - > - assert(!src_iview->image->format->can_ycbcr); > - assert(!dst_iview->image->format->can_ycbcr); > - > - for (uint32_t i = 0; i < fb->layers; i++) { > - resolve_surface(&batch, > - &src_surf, > - src_iview->planes[0].isl.base_level, > - base_src_layer + i, > - &dst_surf, > - dst_iview->planes[0].isl.base_level, > - base_dst_layer + i, > - render_area.offset.x, > render_area.offset.y, > - render_area.offset.x, > render_area.offset.y, > - render_area.extent.width, > render_area.extent.height, > - filter); > - } > + anv_image_msaa_resolve(cmd_buffer, > + src_iview->image, src_aux_usage, > + src_iview->planes[0].isl.base_level, > + src_iview- > >planes[0].isl.base_array_layer, > + dst_iview->image, dst_aux_usage, > + dst_iview->planes[0].isl.base_level, > + dst_iview- > >planes[0].isl.base_array_layer, > + VK_IMAGE_ASPECT_COLOR_BIT, > + render_area.offset.x, > render_area.offset.y, > + render_area.offset.x, > render_area.offset.y, > + render_area.extent.width, > + render_area.extent.height, > + fb->layers, BLORP_FILTER_NONE); > } > - > - blorp_batch_finish(&batch); > } > } > > diff --git a/src/intel/vulkan/anv_private.h > b/src/intel/vulkan/anv_private.h > index 0b67e7598b4..a064a058822 100644 > --- a/src/intel/vulkan/anv_private.h > +++ b/src/intel/vulkan/anv_private.h > @@ -2923,6 +2923,20 @@ anv_image_clear_depth_stencil(struct > anv_cmd_buffer *cmd_buffer, > VkRect2D area, > float depth_value, uint8_t > stencil_value); > void > +anv_image_msaa_resolve(struct anv_cmd_buffer *cmd_buffer, > + const struct anv_image *src_image, > + enum isl_aux_usage src_aux_usage, > + uint32_t src_level, uint32_t src_base_layer, > + const struct anv_image *dst_image, > + enum isl_aux_usage dst_aux_usage, > + uint32_t dst_level, uint32_t dst_base_layer, > + VkImageAspectFlagBits aspect, > + uint32_t src_x, uint32_t src_y, > + uint32_t dst_x, uint32_t dst_y, > + uint32_t width, uint32_t height, > + uint32_t layer_count, > + enum blorp_filter filter); > +void > anv_image_hiz_op(struct anv_cmd_buffer *cmd_buffer, > const struct anv_image *image, > VkImageAspectFlagBits aspect, uint32_t level, </blockquote></div></div>