[Mesa-dev] [PATCH 4/6] anv/blorp: Refactor MSAA resolves into an exportable helper function
Iago Toral
itoral at igalia.com
Mon Jan 14 07:07:05 UTC 2019
On Fri, 2019-01-11 at 12:20 -0600, Jason Ekstrand wrote:
> On Fri, Jan 11, 2019 at 2:19 AM Iago Toral <itoral at 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,
> >
> >
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190114/b509e1aa/attachment-0001.html>
More information about the mesa-dev
mailing list