<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 8, 2016 at 12:21 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Title says: "anv: Add initial for Sky Lake color compression". Did you mean to have something after "initial"? </blockquote><div> </div><div>Yeah, "support" should probably go in there </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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)? </blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Otherwise this patch looks good to me. <div><div class="h5"> > > Signed-off-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > --- > 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]; </div></div>I'm wondering if this holds? assert(!(usage & ANV_SUBPASS_USAGE_INPUT)); <div><div class="h5"> > + > + 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 > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>