<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 28, 2017 at 3:01 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@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 Tue, Feb 28, 2017 at 11:00:14AM -0800, Jason Ekstrand wrote: > On Tue, Feb 28, 2017 at 10:53 AM, Nanley Chery <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> > wrote: > > > On Tue, Feb 28, 2017 at 10:38:12AM -0800, Jason Ekstrand wrote: > > > On Tue, Feb 28, 2017 at 10:32 AM, Nanley Chery <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> > > > wrote: > > > > > > > On Tue, Feb 28, 2017 at 08:26:56AM -0800, Jason Ekstrand wrote: > > > > > On Mon, Feb 27, 2017 at 5:20 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a> > > > > > > > wrote: > > > > > > > > > > > This function supersedes layout_to_hiz_usage(). > > > > > > > > > > > > Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> > > > > > > --- > > > > > > src/intel/vulkan/anv_image.c | 149 > > ++++++++++++++++++++++++++++++ > > > > > > +++++++++++ > > > > > > src/intel/vulkan/anv_private.h | 4 ++ > > > > > > 2 files changed, 153 insertions(+) > > > > > > > > > > > > diff --git a/src/intel/vulkan/anv_image.c > > > > b/src/intel/vulkan/anv_image.c > > > > > > index cd142938e7..716cdf3a38 100644 > > > > > > --- a/src/intel/vulkan/anv_image.c > > > > > > +++ b/src/intel/vulkan/anv_image.c > > > > > > @@ -432,6 +432,155 @@ void anv_GetImageSubresourceLayout( > > > > > > } > > > > > > } > > > > > > > > > > > > +/** > > > > > > + * @brief This function determines the optimal buffer to use for > > > > device > > > > > > + * accesses given a VkImageLayout and other pieces of information > > > > needed > > > > > > to > > > > > > + * make that determination. Device accesses may include normal > > > > sampling > > > > > > and > > > > > > + * rendering operations or resolves. > > > > > > + * > > > > > > + * @param gen The generation of the Intel GPU. > > > > > > + * @param image The image that may contain a collection of > > buffers. > > > > > > + * @param aspects The aspect(s) of the image to be accessed. > > > > > > + * @param layout The current layout of the image aspect(s). > > > > > > + * @param future_layout The next layout of the image aspect(s), if > > > > known. > > > > > > + * Otherwise this should be equal to the > > current > > > > > > layout. > > > > > > + * > > > > > > + * @return The primary buffer that should be used for the given > > > > layout. > > > > > > + */ > > > > > > +enum isl_aux_usage > > > > > > +anv_layout_to_aux_usage(const uint8_t gen, const struct anv_image > > * > > > > const > > > > > > image, > > > > > > > > > > > > > > > > Might be better to take a gen_device_info rather than just the > > integer. > > > > > Since we're doing run-time checks anyway, the cost should be small. > > > > > > > > > > > > > What does this buy us? > > > > > > > > > > Nothing at the moment. However, it is the more standard thing to pass in > > > and, if we ever need more information in the future, device_info is what > > > we'll want. Or you could just pass in the anv_device to guarantee that > > we > > > have everything we would ever need. > > > > > > > I'm in favor of adding it when we need it, but I can make it take the > > device_info if you insist. My rationale is that if the function takes a > > device_info or anv_device, the function prototype would be unnecessarily > > ambiguous with respect to what device-related information is necessary > > to perform the mapping. > > > > Certainly anv_device is probably a bit much since it doesn't (yet) depend > on any run-time information. I could see that changing in the future > (environment variables, per-app parameters, etc.) but we can cross that > bridge when we come to it. I don't really see an "information" difference > between an integer "gen" and a device info. The one completely describes > the hardware and the other sort-of describes the hardware. Also, I > wouldn't be terribly surprised if, in the future, whether or not we can > sample from HiZ starts to depend on half-gen or whether or not it's an atom > part. > </div></div>Do you want me to use the device_info struct in the v2? </blockquote><div> </div><div>I'd prefer it but I'm not going to be too upset if it's not there. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> -Nanley <div class="HOEnZb"><div class="h5"> > > > -Nanley > > > > > > > > > -Nanley > > > > > > > > > > > > > > > + const VkImageAspectFlags aspects, > > > > VkImageLayout > > > > > > layout, > > > > > > + const VkImageLayout future_layout) > > > > > > +{ > > > > > > + /* Validate the inputs. */ > > > > > > + > > > > > > + /* Intel GPUs prior to Gen7 are not supported in anv. */ > > > > > > + assert(gen >= 7); > > > > > > + > > > > > > + /* The layout of a NULL image is not properly defined. */ > > > > > > + assert(image != NULL); > > > > > > + > > > > > > + /* The aspects must be a subset of the image aspects. */ > > > > > > + assert(aspects & image->aspects && aspects <= image->aspects); > > > > > > + > > > > > > + /* According to the Vulkan Spec, the following layouts are > > valid > > > > only > > > > > > as > > > > > > + * initial layouts in a layout transition and don't support > > device > > > > > > access. > > > > > > + * Therefore, the caller should not be setting the future > > layout to > > > > > > either. > > > > > > + */ > > > > > > + assert(future_layout != VK_IMAGE_LAYOUT_UNDEFINED && > > > > > > + future_layout != VK_IMAGE_LAYOUT_PREINITIALIZED); > > > > > > + > > > > > > + /* Determine the optimal buffer. */ > > > > > > + > > > > > > + /* If there is no auxiliary surface allocated, we must use the > > one > > > > and > > > > > > only > > > > > > + * main buffer. > > > > > > + */ > > > > > > + if (image->aux_surface.isl.size == 0) > > > > > > + return ISL_AUX_USAGE_NONE; > > > > > > + > > > > > > + /* All images that use an auxiliary surface are required to be > > > > tiled. > > > > > > */ > > > > > > + assert(image->tiling == VK_IMAGE_TILING_OPTIMAL); > > > > > > + > > > > > > + /* On BDW+, when clearing the stencil aspect of a depth stencil > > > > image, > > > > > > + * the HiZ buffer allows us to record the clear with a > > relatively > > > > small > > > > > > + * number of packets. Prior to BDW, the HiZ buffer provides no > > > > known > > > > > > benefit > > > > > > + * to the stencil aspect. > > > > > > + */ > > > > > > + if (gen < 8 && aspects == VK_IMAGE_ASPECT_STENCIL_BIT) > > > > > > + return ISL_AUX_USAGE_NONE; > > > > > > + > > > > > > + /* The undefined layout indicates that the user doesn't care > > about > > > > the > > > > > > data > > > > > > + * that's currently in the buffer. Therefore, the optimal > > buffer to > > > > > > use is > > > > > > + * the same buffer that would be used in the next layout. This > > > > avoids > > > > > > the > > > > > > + * possibility of having to resolve in order to maintain > > coherency. > > > > > > + * > > > > > > + * The pre-initialized layout is undefined for optimally-tiled > > > > images. > > > > > > As > > > > > > + * guaranteed by the assertion above, all images that have > > reached > > > > this > > > > > > + * point are tiled. > > > > > > + */ > > > > > > + if (layout == VK_IMAGE_LAYOUT_UNDEFINED || > > > > > > + layout == VK_IMAGE_LAYOUT_PREINITIALIZED) > > > > > > + layout = future_layout; > > > > > > + > > > > > > + const bool has_depth = aspects & VK_IMAGE_ASPECT_DEPTH_BIT; > > > > > > + const bool color_aspect = aspects == VK_IMAGE_ASPECT_COLOR_BIT; > > > > > > + > > > > > > + /* The following switch currently only handles depth stencil > > > > aspects. > > > > > > + * TODO: Handle the color aspect. > > > > > > + */ > > > > > > + if (color_aspect) > > > > > > + return image->aux_usage; > > > > > > + > > > > > > + switch (layout) { > > > > > > + > > > > > > + /* Invalid Layouts */ > > > > > > + case VK_IMAGE_LAYOUT_UNDEFINED: > > > > > > + case VK_IMAGE_LAYOUT_PREINITIALIZED: > > > > > > + case VK_IMAGE_LAYOUT_RANGE_SIZE: > > > > > > + case VK_IMAGE_LAYOUT_MAX_ENUM: > > > > > > + unreachable("Invalid image layout."); > > > > > > + > > > > > > + > > > > > > + /* 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; > > > > > > + > > > > > > + > > > > > > + /* Sampling Layouts */ > > > > > > + case VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL: > > > > > > + assert(!color_aspect); > > > > > > + /* Fall-through */ > > > > > > + case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: > > > > > > + if (has_depth && anv_can_sample_with_hiz(gen, > > image->samples)) > > > > > > + return ISL_AUX_USAGE_HIZ; > > > > > > + else > > > > > > + return ISL_AUX_USAGE_NONE; > > > > > > + > > > > > > + case VK_IMAGE_LAYOUT_PRESENT_SRC_KHR: > > > > > > + assert(color_aspect); > > > > > > + > > > > > > + /* On SKL+, the render buffer can be decompressed by the > > > > > > presentation > > > > > > + * engine. Support for this feature has not yet landed in > > the > > > > wider > > > > > > + * ecosystem. TODO: Update this code when support lands. > > > > > > + * > > > > > > + * From the BDW PRM, Vol 7, Render Target Resolve: > > > > > > + * > > > > > > + * If the MCS is enabled on a non-multisampled render > > > > target, the > > > > > > + * render target must be resolved before being used for > > other > > > > > > + * purposes (display, texture, CPU lock) The clear value > > from > > > > > > + * SURFACE_STATE is written into pixels in the render > > target > > > > > > + * indicated as clear in the MCS. > > > > > > + * > > > > > > + * Pre-SKL, the render buffer must be resolved before being > > > > used for > > > > > > + * presentation. We can infer that the auxiliary buffer is > > not > > > > used. > > > > > > + */ > > > > > > + return ISL_AUX_USAGE_NONE; > > > > > > + > > > > > > + > > > > > > + /* Rendering Layouts */ > > > > > > + case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: > > > > > > + assert(color_aspect); > > > > > > + unreachable("Color images are not yet supported."); > > > > > > + > > > > > > + case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: > > > > > > + assert(!color_aspect); > > > > > > + return ISL_AUX_USAGE_HIZ; > > > > > > + } > > > > > > + > > > > > > + /* If the layout isn't recognized in the exhaustive switch > > above, > > > > the > > > > > > + * VkImageLayout value is not defined in vulkan.h. > > > > > > + */ > > > > > > + unreachable("layout is not a VkImageLayout enumeration > > member."); > > > > > > +} > > > > > > + > > > > > > + > > > > > > static struct anv_state > > > > > > alloc_surface_state(struct anv_device *device) > > > > > > { > > > > > > diff --git a/src/intel/vulkan/anv_private.h > > b/src/intel/vulkan/anv_ > > > > > > private.h > > > > > > index 2527c2cc5a..5ce9027f88 100644 > > > > > > --- a/src/intel/vulkan/anv_private.h > > > > > > +++ b/src/intel/vulkan/anv_private.h > > > > > > @@ -1671,6 +1671,10 @@ anv_gen8_hiz_op_resolve(struct > > anv_cmd_buffer > > > > > > *cmd_buffer, > > > > > > const struct anv_image *image, > > > > > > enum blorp_hiz_op op); > > > > > > > > > > > > +enum isl_aux_usage > > > > > > +anv_layout_to_aux_usage(const uint8_t gen, const struct anv_image > > > > *image, > > > > > > + const VkImageAspectFlags aspects, > > > > VkImageLayout > > > > > > layout, > > > > > > + const VkImageLayout future_layout); > > > > > > static inline uint32_t > > > > > > anv_get_layerCount(const struct anv_image *image, > > > > > > const VkImageSubresourceRange *range) > > > > > > -- > > > > > > 2.11.1 > > > > > > > > > > > > _______________________________________________ > > > > > > 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> > > > > > > > > > > > > </div></div></blockquote></div> </div></div>