<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 22, 2018 at 12:07 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"><div class="HOEnZb"><div class="h5">On Fri, Jan 19, 2018 at 03:47:24PM -0800, Jason Ekstrand wrote: > Reviewed-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> > --- > src/intel/vulkan/anv_image.c | 48 ++++++++++++++++++++++++++------------------ > 1 file changed, 29 insertions(+), 19 deletions(-) > > diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c > index 84e4b96..e34ac95 100644 > --- a/src/intel/vulkan/anv_image.c > +++ b/src/intel/vulkan/anv_image.c > @@ -774,12 +774,6 @@ anv_layout_to_aux_usage(const struct gen_device_info * const devinfo, > /* Stencil has no aux */ > assert(aspect != VK_IMAGE_ASPECT_STENCIL_BIT); > > - /* The following switch currently only handles depth stencil aspects. > - * TODO: Handle the color aspect. > - */ > - if (image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) > - return image->planes[plane].aux_usage; > - > switch (layout) { > > /* Invalid Layouts */ > @@ -799,28 +793,38 @@ anv_layout_to_aux_usage(const struct gen_device_info * const devinfo, > > > /* Transfer Layouts > - * > - * This buffer could be a depth buffer used in a transfer operation. BLORP > - * currently doesn't use HiZ for transfer operations so we must use the main > - * buffer for this layout. TODO: Enable HiZ in BLORP. > */ > case VK_IMAGE_LAYOUT_GENERAL: > case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: > case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: > - return ISL_AUX_USAGE_NONE; > + if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT) { > + /* This buffer could be a depth buffer used in a transfer operation. > + * BLORP currently doesn't use HiZ for transfer operations so we must > + * use the main buffer for this layout. TODO: Enable HiZ in BLORP. > + */ > + assert(image->planes[plane].aux_usage == ISL_AUX_USAGE_HIZ); > + return ISL_AUX_USAGE_NONE; > + } else { > + assert(image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV); > + return image->planes[plane].aux_usage; > + } > > > /* Sampling Layouts */ > case VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL: > + case VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL_KHR: > assert((image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) == 0); > /* Fall-through */ > case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: > - case VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL_KHR: > - assert(aspect == VK_IMAGE_ASPECT_DEPTH_BIT); > - if (anv_can_sample_with_hiz(devinfo, image)) > - return ISL_AUX_USAGE_HIZ; > - else > - return ISL_AUX_USAGE_NONE; > + if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT) { > + if (anv_can_sample_with_hiz(devinfo, image)) > + return ISL_AUX_USAGE_HIZ; > + else > + return ISL_AUX_USAGE_NONE; > + } else { > + return image->planes[plane].aux_usage; > + } > + > > case VK_IMAGE_LAYOUT_PRESENT_SRC_KHR: > assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT); > @@ -845,8 +849,14 @@ anv_layout_to_aux_usage(const struct gen_device_info * const devinfo, > > /* Rendering Layouts */ > case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: > - assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT); > - unreachable("Color images are not yet supported."); > + assert(aspect & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV); > + if (image->planes[plane].aux_usage == ISL_AUX_USAGE_NONE) { > + assert(image->samples == 1); > + return ISL_AUX_USAGE_CCS_D; </div></div>Just checking that I understand what is going on. Earlier in this function there is early return for "image->planes[plane].aux_surface.isl.size == 0". That means that we end up here only if there is auxiliary. Moreover if CCS_E should be used then image->planes[plane].aux_usage is set already? </blockquote><div> </div><div>Yes and no. Yes, we only get here if we actually have an aux surface. For color images, aux_usage is the "default" aux_usage. Specifically, it's CCS_E for images where we can enable proper compression and NONE when we cannot. It's a bit of an artifact of history and we should probably change it at some point. For aux_usage == NONE, we can still enable CCS_D for color attachments and we do a full resolve when we transition from COLOR_ATTACHMENT_OPTIMAL to basically any other layout.</div><div> </div><div>We could also enable CCS_E if we wanted in certain cases, but it's not clear that doing so would actually be faster since a full resolve of a CCS_E image can actually be more expensive than a full resolve of CCS_D. (For CCS_D the hardware can just ignore any blocks that aren't clear whereas a full resolve with CCS_E will touch almost every pixel.) </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + } else { > + assert(image->planes[plane].aux_usage != ISL_AUX_USAGE_CCS_D); > + return image->planes[plane].aux_usage; > + } > > case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: > case VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL_KHR: > -- > 2.5.0.400.gff86faf > > _______________________________________________ > 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>