On Fri, 2019-01-11 at 12:20 -0600, Jason Ekstrand wrote: > On Fri, Jan 11, 2019 at 2:19 AM Iago Toral <ito...@igalia.com> wrote: > > 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? > > 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.
Right, I had missed that, looks good as is then. > --Jason > > > > - 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, > > > > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev