On Tue, Nov 8, 2016 at 12:21 AM, Pohjolainen, Topi <
topi.pohjolai...@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 <ja...@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@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
