[Mesa-dev] [PATCH 04/16] anv: Handle transitioning depth from UNDEFINED to other layouts

[Nanley Chery]

On Thu, May 18, 2017 at 02:00:51PM -0700, Jason Ekstrand wrote:
> ---
>  src/intel/vulkan/anv_image.c       | 12 ++++++++++--
>  src/intel/vulkan/genX_cmd_buffer.c |  9 +--------
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index d21e055..d65ef47 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -488,12 +488,20 @@ anv_layout_to_aux_usage(const struct gen_device_info * const devinfo,
>     /* According to the Vulkan Spec, the following layouts are valid only as
>      * initial layouts in a layout transition and don't support device access.
>      */
> -   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 for device access.");
>  
> +   /* Undefined layouts
> +    *
> +    * The pre-initialized layout is equivalent to the undefined layout for
> +    * optimally-tiled images.  We can only do color compression (CCS or HiZ)
> +    * on tiled images.
> +    */
> +   case VK_IMAGE_LAYOUT_UNDEFINED:
> +   case VK_IMAGE_LAYOUT_PREINITIALIZED:
> +      return ISL_AUX_USAGE_NONE;
> +
>  

This function is defined to return the isl_aux_usage for "device access"
as described by the Vulkan spec. The user is not allowed to perform
device accesses in either of those layouts (11.4. Image Layouts). My
suggestion for dealing with this can be found below.

>     /* Transfer Layouts
>      *
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c
> index 1f30a12..8d5f61b 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -355,15 +355,8 @@ transition_depth_buffer(struct anv_cmd_buffer *cmd_buffer,
>      * The undefined layout indicates that the user doesn't care about the data
>      * that's currently in the buffer. Therefore, a data-preserving resolve
>      * operation is not needed.
> -    *
> -    * The pre-initialized layout is equivalent to the undefined layout for
> -    * optimally-tiled images. Anv only exposes support for optimally-tiled
> -    * depth buffers.
>      */
> -   if (image->aux_usage != ISL_AUX_USAGE_HIZ ||
> -       initial_layout == final_layout ||
> -       initial_layout == VK_IMAGE_LAYOUT_UNDEFINED ||
> -       initial_layout == VK_IMAGE_LAYOUT_PREINITIALIZED)
> +   if (image->aux_usage != ISL_AUX_USAGE_HIZ || initial_layout == final_layout)
>        return;
>  

I think we should keep this new condition and add another one below for
UNDEFINED and PREINITIALIZED in which we do the resolve early and
return.

I don't mind adding a new condition because the original idea I had of
only comparing hiz_enabled and enable_hiz isn't optimal - for example,
we unnecessarily perform a resolve when going from a read-only
hiz-enabled layout to a hiz-disabled layout. (Thankfully, I haven't yet
found any apps that make such a transition.)

-Nanley

>     const bool hiz_enabled = ISL_AUX_USAGE_HIZ ==
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev