[Mesa-dev] [PATCH 13/18] anv: Add initial for Sky Lake color compression

Jason Ekstrand jason at jlekstrand.net
Tue Nov 8 15:50:11 UTC 2016


On Tue, Nov 8, 2016 at 12:21 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

>
> Title says: "anv: Add initial for Sky Lake color compression". Did you mean
> to have something after "initial"?
>

Yeah, "support" should probably go in there


> On Fri, Oct 28, 2016 at 02:17:09AM -0700, Jason Ekstrand wrote:
> > This commit adds basic support for color compression.  For the moment,
> > color compression is only enabled within a render pass and a full resolve
> > is done before the render pass finishes.  All texturing operations still
> > happen with CCS disabled.
>
> I'm not that familiar with all the vulkan concepts so far and need to ask a
> few things. In this patch CCS_E is enabled whenever there is suitable
> render
> target. For surfaces of the type VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT and
> VK_DESCRIPTOR_TYPE_STORAGE_IMAGE aux is explicitly disabled. Does this
> mean
> that it is impossible to have one of these surfaces as render target in
> the same pass (and having compression turned on for writing)?
>

No, not quite.  In this patch, we always do a full resolve at the end of
the render pass.  Since input attachments aren't really supported yet (my
next task), those aren't a problem.  Also, you can't bind one of your
render pass attachments as a storage image or texture so those will never
be used while it's in an unresolved state.


> Otherwise this patch looks good to me.
>
> >
> > Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> > ---
> >  src/intel/vulkan/anv_blorp.c       | 139 +++++++++++++++++++++++++++++-
> -------
> >  src/intel/vulkan/anv_image.c       |  17 +++--
> >  src/intel/vulkan/anv_private.h     |   1 +
> >  src/intel/vulkan/genX_cmd_buffer.c |  50 ++++++++++++-
> >  4 files changed, 171 insertions(+), 36 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > index 0e70e9b..bf317c7 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -1179,52 +1179,131 @@ void anv_CmdResolveImage(
> >     blorp_batch_finish(&batch);
> >  }
> >
> > +static void
> > +ccs_resolve_attachment(struct anv_cmd_buffer *cmd_buffer,
> > +                       struct blorp_batch *batch,
> > +                       uint32_t att)
> > +{
> > +   struct anv_framebuffer *fb = cmd_buffer->state.framebuffer;
> > +   struct anv_attachment_state *att_state =
> > +      &cmd_buffer->state.attachments[att];
> > +
> > +   assert(att_state->aux_usage != ISL_AUX_USAGE_CCS_D);
> > +   if (att_state->aux_usage != ISL_AUX_USAGE_CCS_E)
> > +      return; /* Nothing to resolve */
> > +
> > +   struct anv_render_pass *pass = cmd_buffer->state.pass;
> > +   struct anv_subpass *subpass = cmd_buffer->state.subpass;
> > +   unsigned subpass_idx = subpass - pass->subpasses;
> > +   assert(subpass_idx < pass->subpass_count);
> > +
> > +   /* Scan forward to see what all ways this attachment will be used.
> > +    * Ideally, we would like to resolve in the same subpass as the last
> write
> > +    * of a particular attachment.  That way we only resolve once but
> it's
> > +    * still hot in the cache.
> > +    */
> > +   for (uint32_t s = subpass_idx + 1; s < pass->subpass_count; s++) {
> > +      enum anv_subpass_usage usage = pass->attachments[att].
> subpass_usage[s];
>
> I'm wondering if this holds?
>
>          assert(!(usage & ANV_SUBPASS_USAGE_INPUT));
>
> > +
> > +      if (usage & (ANV_SUBPASS_USAGE_DRAW |
> ANV_SUBPASS_USAGE_RESOLVE_DST)) {
> > +         /* We found another subpass that draws to this attachment.
> We'll
> > +          * wait to resolve until then.
> > +          */
> > +         return;
> > +      }
> > +   }
> > +
> > +   struct anv_image_view *iview = fb->attachments[att];
> > +   const struct anv_image *image = iview->image;
> > +   assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> > +
> > +   struct blorp_surf surf;
> > +   get_blorp_surf_for_anv_image(image, VK_IMAGE_ASPECT_COLOR_BIT,
> &surf);
> > +   surf.aux_surf = &image->aux_surface.isl;
> > +   surf.aux_addr = (struct blorp_address) {
> > +      .buffer = image->bo,
> > +      .offset = image->offset + image->aux_surface.offset,
> > +   };
> > +   surf.aux_usage = att_state->aux_usage;
> > +
> > +   for (uint32_t layer = 0; layer < fb->layers; layer++) {
> > +      blorp_ccs_resolve(batch, &surf,
> > +                        iview->isl.base_level,
> > +                        iview->isl.base_array_layer + layer,
> > +                        iview->isl.format,
> > +                        BLORP_FAST_CLEAR_OP_RESOLVE_FULL);
> > +   }
> > +}
> > +
> >  void
> >  anv_cmd_buffer_resolve_subpass(struct anv_cmd_buffer *cmd_buffer)
> >  {
> >     struct anv_framebuffer *fb = cmd_buffer->state.framebuffer;
> >     struct anv_subpass *subpass = cmd_buffer->state.subpass;
> >
> > -   if (!subpass->has_resolve)
> > -      return;
> >
> >     struct blorp_batch batch;
> >     blorp_batch_init(&cmd_buffer->device->blorp, &batch, cmd_buffer, 0);
> >
> > +   /* From the Sky Lake PRM Vol. 7, "Render Target Resolve":
> > +    *
> > +    *    "When performing a render target resolve, PIPE_CONTROL with
> end of
> > +    *    pipe sync must be delivered."
> > +    */
> > +   cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
> > +
> >     for (uint32_t i = 0; i < subpass->color_count; ++i) {
> > -      uint32_t src_att = subpass->color_attachments[i];
> > -      uint32_t dst_att = subpass->resolve_attachments[i];
> > +      ccs_resolve_attachment(cmd_buffer, &batch,
> > +                             subpass->color_attachments[i]);
> > +   }
> >
> > -      if (dst_att == VK_ATTACHMENT_UNUSED)
> > -         continue;
> > +   if (subpass->has_resolve) {
> > +      for (uint32_t i = 0; i < subpass->color_count; ++i) {
> > +         uint32_t src_att = subpass->color_attachments[i];
> > +         uint32_t dst_att = subpass->resolve_attachments[i];
> > +
> > +         if (dst_att == VK_ATTACHMENT_UNUSED)
> > +            continue;
> > +
> > +         if (cmd_buffer->state.attachments[dst_att].pending_clear_aspects)
> {
> > +            /* FINISHME(perf): Skip clears for resolve attachments.
> > +             *
> > +             * From the Vulkan 1.0 spec:
> > +             *
> > +             *    If the first use of an attachment in a render pass is
> as a
> > +             *    resolve attachment, then the loadOp is effectively
> ignored
> > +             *    as the resolve is guaranteed to overwrite all pixels
> in the
> > +             *    render area.
> > +             */
> > +            cmd_buffer->state.attachments[dst_att].pending_clear_aspects
> = 0;
> > +         }
> >
> > -      if (cmd_buffer->state.attachments[dst_att].pending_clear_aspects)
> {
> > -         /* FINISHME(perf): Skip clears for resolve attachments.
> > -          *
> > -          * From the Vulkan 1.0 spec:
> > -          *
> > -          *    If the first use of an attachment in a render pass is as
> a
> > -          *    resolve attachment, then the loadOp is effectively
> ignored
> > -          *    as the resolve is guaranteed to overwrite all pixels in
> the
> > -          *    render area.
> > -          */
> > -         cmd_buffer->state.attachments[dst_att].pending_clear_aspects
> = 0;
> > -      }
> > +         struct anv_image_view *src_iview = fb->attachments[src_att];
> > +         struct anv_image_view *dst_iview = fb->attachments[dst_att];
> >
> > -      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;
> >
> > -      const VkRect2D render_area = cmd_buffer->state.render_area;
> > +         assert(src_iview->aspect_mask == dst_iview->aspect_mask);
> > +         resolve_image(&batch, src_iview->image,
> > +                       src_iview->isl.base_level,
> > +                       src_iview->isl.base_array_layer,
> > +                       dst_iview->image,
> > +                       dst_iview->isl.base_level,
> > +                       dst_iview->isl.base_array_layer,
> > +                       src_iview->aspect_mask,
> > +                       render_area.offset.x, render_area.offset.y,
> > +                       render_area.offset.x, render_area.offset.y,
> > +                       render_area.extent.width,
> render_area.extent.height);
> >
> > -      assert(src_iview->aspect_mask == dst_iview->aspect_mask);
> > -      resolve_image(&batch, src_iview->image,
> > -                    src_iview->isl.base_level,
> src_iview->isl.base_array_layer,
> > -                    dst_iview->image,
> > -                    dst_iview->isl.base_level,
> dst_iview->isl.base_array_layer,
> > -                    src_iview->aspect_mask,
> > -                    render_area.offset.x, render_area.offset.y,
> > -                    render_area.offset.x, render_area.offset.y,
> > -                    render_area.extent.width,
> render_area.extent.height);
> > +         /* From the Sky Lake PRM Vol. 7, "Render Target Resolve":
> > +          *
> > +          *    "When performing a render target resolve, PIPE_CONTROL
> with end
> > +          *    of pipe sync must be delivered."
> > +          */
> > +         cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
> > +
> > +         ccs_resolve_attachment(cmd_buffer, &batch, dst_att);
> > +      }
> >     }
> >
> >     blorp_batch_finish(&batch);
> > diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> > index bdf8bca..4bef5ce 100644
> > --- a/src/intel/vulkan/anv_image.c
> > +++ b/src/intel/vulkan/anv_image.c
> > @@ -176,23 +176,32 @@ make_surface(const struct anv_device *dev,
> >
> >     /* Add a HiZ surface to a depth buffer that will be used for
> rendering.
> >      */
> > -   if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT &&
> > -       (image->usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT)) {
> > -
> > +   if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT) {
> >        /* Allow the user to control HiZ enabling. Disable by default on
> gen7
> >         * because resolves are not currently implemented pre-BDW.
> >         */
> > -      if (!env_var_as_boolean("INTEL_VK_HIZ", dev->info.gen >= 8)) {
> > +      if (!(image->usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT))
> {
> > +         /* It will never be used as an attachment, HiZ is pointless. */
> > +      } else if (!env_var_as_boolean("INTEL_VK_HIZ", dev->info.gen >=
> 8)) {
> >           anv_finishme("Implement gen7 HiZ");
> >        } else if (vk_info->mipLevels > 1) {
> >           anv_finishme("Test multi-LOD HiZ");
> >        } else if (dev->info.gen == 8 && vk_info->samples > 1) {
> >           anv_finishme("Test gen8 multisampled HiZ");
> >        } else {
> > +         assert(image->aux_surface.isl.size == 0);
> >           isl_surf_get_hiz_surf(&dev->isl_dev,
> &image->depth_surface.isl,
> >                                 &image->aux_surface.isl);
> >           add_surface(image, &image->aux_surface);
> >        }
> > +   } else if (aspect == VK_IMAGE_ASPECT_COLOR_BIT) {
> > +      if (dev->info.gen >= 9 && vk_info->samples == 1) {
> > +         assert(image->aux_surface.isl.size == 0);
> > +         ok = isl_surf_get_ccs_surf(&dev->isl_dev, &anv_surf->isl,
> > +                                    &image->aux_surface.isl);
> > +         if (ok)
> > +            add_surface(image, &image->aux_surface);
> > +      }
> >     }
> >
> >     return VK_SUCCESS;
> > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> > index 531f2d1..6e8f5e0 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -1059,6 +1059,7 @@ void anv_dynamic_state_copy(struct
> anv_dynamic_state *dest,
> >   * The clear value is valid only if there exists a pending clear.
> >   */
> >  struct anv_attachment_state {
> > +   enum isl_aux_usage                           aux_usage;
> >     struct anv_state                             color_rt_state;
> >
> >     VkImageAspectFlags                           pending_clear_aspects;
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index a87e5a3..3b9f946 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -165,6 +165,7 @@ 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)
> >  {
> >     const struct isl_device *isl_dev = &cmd_buffer->device->isl_dev;
> > @@ -172,6 +173,41 @@ 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.addr_offset,
> >                        iview->bo, iview->offset);
> > +
> > +   if (aux_usage != ISL_AUX_USAGE_NONE) {
> > +      uint32_t aux_offset = iview->offset + iview->image->aux_surface.
> offset;
> > +
> > +      /* 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
> > +       * surface buffer addresses are always 4K page alinged.
> > +       */
> > +      assert((aux_offset & 0xfff) == 0);
> > +      uint32_t *aux_addr_dw = state.map + isl_dev->ss.aux_addr_offset;
> > +      aux_offset += *aux_addr_dw & 0xfff;
> > +
> > +      anv_reloc_list_add(&cmd_buffer->surface_relocs,
> &cmd_buffer->pool->alloc,
> > +                         state.offset + isl_dev->ss.aux_addr_offset,
> > +                         iview->bo, aux_offset);
> > +   }
> > +}
> > +
> > +static enum isl_aux_usage
> > +fb_attachment_get_aux_usage(struct anv_device *device,
> > +                            struct anv_framebuffer *fb,
> > +                            uint32_t attachment)
> > +{
> > +   struct anv_image_view *iview = fb->attachments[attachment];
> > +
> > +   if (iview->image->aux_surface.isl.size == 0)
> > +      return ISL_AUX_USAGE_NONE; /* No aux surface */
> > +
> > +   assert(iview->image->aux_surface.isl.usage &
> ISL_SURF_USAGE_CCS_BIT);
> > +
> > +   if (isl_format_supports_lossless_compression(&device->info,
> > +                                                iview->isl.format))
> > +      return ISL_AUX_USAGE_CCS_E;
> > +
> > +   return ISL_AUX_USAGE_NONE;
> >  }
> >
> >  /**
> > @@ -293,16 +329,24 @@ genX(cmd_buffer_setup_attachments)(struct
> anv_cmd_buffer *cmd_buffer,
> >           assert(iview->image->vk_format == att->format);
> >
> >           if (att_aspects == VK_IMAGE_ASPECT_COLOR_BIT) {
> > +            state->attachments[i].aux_usage =
> > +               fb_attachment_get_aux_usage(cmd_buffer->device,
> framebuffer, i);
> > +
> >              struct isl_view view = iview->isl;
> >              view.usage |= ISL_SURF_USAGE_RENDER_TARGET_BIT;
> >              isl_surf_fill_state(isl_dev,
> >                                  state->attachments[i].color_
> rt_state.map,
> >                                  .surf = &iview->image->color_surface.
> isl,
> >                                  .view = &view,
> > +                                .aux_surf = &iview->image->aux_surface.
> isl,
> > +                                .aux_usage = state->attachments[i].aux_
> usage,
> >                                  .mocs = cmd_buffer->device->default_
> mocs);
> >
> >              add_image_view_relocs(cmd_buffer, iview,
> > +                                  state->attachments[i].aux_usage,
> >                                    state->attachments[i].color_
> rt_state);
> > +         } else {
> > +            state->attachments[i].aux_usage = ISL_AUX_USAGE_NONE;
> >           }
> >        }
> >
> > @@ -912,13 +956,15 @@ emit_binding_table(struct anv_cmd_buffer
> *cmd_buffer,
> >        case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
> >           surface_state = desc->image_view->sampler_surface_state;
> >           assert(surface_state.alloc_size);
> > -         add_image_view_relocs(cmd_buffer, desc->image_view,
> surface_state);
> > +         add_image_view_relocs(cmd_buffer, desc->image_view,
> > +                               ISL_AUX_USAGE_NONE, surface_state);
> >           break;
> >
> >        case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: {
> >           surface_state = desc->image_view->storage_surface_state;
> >           assert(surface_state.alloc_size);
> > -         add_image_view_relocs(cmd_buffer, desc->image_view,
> surface_state);
> > +         add_image_view_relocs(cmd_buffer, desc->image_view,
> > +                               ISL_AUX_USAGE_NONE, surface_state);
> >
> >           struct brw_image_param *image_param =
> >              &cmd_buffer->state.push_constants[stage]->images[image++];
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161108/e61a61ff/attachment-0001.html>


More information about the mesa-dev mailing list