[Mesa-dev] [PATCH 02/18] anv/image: Add anv_layout_to_aux_usage()
Jason Ekstrand
jason at jlekstrand.net
Tue Feb 28 23:20:33 UTC 2017
On Tue, Feb 28, 2017 at 3:01 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> On Tue, Feb 28, 2017 at 11:00:14AM -0800, Jason Ekstrand wrote:
> > On Tue, Feb 28, 2017 at 10:53 AM, Nanley Chery <nanleychery at gmail.com>
> > 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 <
> nanleychery at gmail.com>
> > > > 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 <
> nanleychery at gmail.com
> > > >
> > > > > wrote:
> > > > > >
> > > > > > > This function supersedes layout_to_hiz_usage().
> > > > > > >
> > > > > > > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > > > > > > ---
> > > > > > > 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.
> >
>
> Do you want me to use the device_info struct in the v2?
>
I'd prefer it but I'm not going to be too upset if it's not there.
> -Nanley
>
> >
> > > -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
> > > > > > > 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/20170228/759ce40b/attachment-0001.html>
More information about the mesa-dev
mailing list