<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, May 19, 2017 at 4:51 PM, Nanley Chery <span dir="ltr"><<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>></span> wrote:<br><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 Thu, May 18, 2017 at 02:00:51PM -0700, Jason Ekstrand wrote:<br>
> ---<br>
>  src/intel/vulkan/anv_image.c       | 12 ++++++++++--<br>
>  src/intel/vulkan/genX_cmd_<wbr>buffer.c |  9 +--------<br>
>  2 files changed, 11 insertions(+), 10 deletions(-)<br>
><br>
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c<br>
> index d21e055..d65ef47 100644<br>
> --- a/src/intel/vulkan/anv_image.c<br>
> +++ b/src/intel/vulkan/anv_image.c<br>
> @@ -488,12 +488,20 @@ anv_layout_to_aux_usage(const struct gen_device_info * const devinfo,<br>
>     /* According to the Vulkan Spec, the following layouts are valid only as<br>
>      * initial layouts in a layout transition and don't support device access.<br>
>      */<br>
> -   case VK_IMAGE_LAYOUT_UNDEFINED:<br>
> -   case VK_IMAGE_LAYOUT_<wbr>PREINITIALIZED:<br>
>     case VK_IMAGE_LAYOUT_RANGE_SIZE:<br>
>     case VK_IMAGE_LAYOUT_MAX_ENUM:<br>
>        unreachable("Invalid image layout for device access.");<br>
><br>
> +   /* Undefined layouts<br>
> +    *<br>
> +    * The pre-initialized layout is equivalent to the undefined layout for<br>
> +    * optimally-tiled images.  We can only do color compression (CCS or HiZ)<br>
> +    * on tiled images.<br>
> +    */<br>
> +   case VK_IMAGE_LAYOUT_UNDEFINED:<br>
> +   case VK_IMAGE_LAYOUT_<wbr>PREINITIALIZED:<br>
> +      return ISL_AUX_USAGE_NONE;<br>
> +<br>
><br>
<br>
</div></div>This function is defined to return the isl_aux_usage for "device access"<br>
as described by the Vulkan spec. The user is not allowed to perform<br>
device accesses in either of those layouts (11.4. Image Layouts). My<br>
suggestion for dealing with this can be found below.<span class=""><br></span></blockquote><div><br></div><div>I guess I don't really see why the "device access" distinction is useful.  Perhaps you could explain?  Just plugging the two other layouts in seems to work fairly well.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>     /* Transfer Layouts<br>
>      *<br>
> diff --git a/src/intel/vulkan/genX_cmd_<wbr>buffer.c b/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
> index 1f30a12..8d5f61b 100644<br>
> --- a/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
> +++ b/src/intel/vulkan/genX_cmd_<wbr>buffer.c<br>
> @@ -355,15 +355,8 @@ transition_depth_buffer(struct anv_cmd_buffer *cmd_buffer,<br>
>      * The undefined layout indicates that the user doesn't care about the data<br>
>      * that's currently in the buffer. Therefore, a data-preserving resolve<br>
>      * operation is not needed.<br>
> -    *<br>
> -    * The pre-initialized layout is equivalent to the undefined layout for<br>
> -    * optimally-tiled images. Anv only exposes support for optimally-tiled<br>
> -    * depth buffers.<br>
>      */<br>
> -   if (image->aux_usage != ISL_AUX_USAGE_HIZ ||<br>
> -       initial_layout == final_layout ||<br>
> -       initial_layout == VK_IMAGE_LAYOUT_UNDEFINED ||<br>
> -       initial_layout == VK_IMAGE_LAYOUT_<wbr>PREINITIALIZED)<br>
> +   if (image->aux_usage != ISL_AUX_USAGE_HIZ || initial_layout == final_layout)<br>
>        return;<br>
><br>
<br>
</span>I think we should keep this new condition and add another one below for<br>
UNDEFINED and PREINITIALIZED in which we do the resolve early and<br>
return.<br>
<br>
I don't mind adding a new condition because the original idea I had of<br>
only comparing hiz_enabled and enable_hiz isn't optimal - for example,<br>
we unnecessarily perform a resolve when going from a read-only<br>
hiz-enabled layout to a hiz-disabled layout. (Thankfully, I haven't yet<br>
found any apps that make such a transition.)<br>
</blockquote></div><br></div><div class="gmail_extra">I don't think that's unnecessary.  If the user goes<br><br>DEPTH_STENCIL_OPTIMAL<br>SHADER_READ_ONLY_OPTIMAL<br>TRANSFER_DST_OPTIMAL<br><br>we need to resolve somewhere in the chain.  The best I think we can hope for is another predicated resolve trick like you did for color buffers.<br><br></div><div class="gmail_extra">--Jason<br></div></div>