[Mesa-dev] [PATCH] anv: Implement VK_KHR_image_format_list

[Lionel Landwerlin]

On 17/09/17 16:02, Jason Ekstrand wrote:
> On September 17, 2017 3:57:39 AM Lionel Landwerlin 
> <lionel.g.landwerlin at intel.com> wrote:
>
>> On 15/09/17 17:18, Jason Ekstrand wrote:
>>> ---
>>>   src/intel/vulkan/anv_extensions.py |  1 +
>>>   src/intel/vulkan/anv_image.c       | 37 
>>> +++++++++++++++++++++++++++++++++++--
>>>   2 files changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/intel/vulkan/anv_extensions.py 
>>> b/src/intel/vulkan/anv_extensions.py
>>> index acec785..ba7bb56 100644
>>> --- a/src/intel/vulkan/anv_extensions.py
>>> +++ b/src/intel/vulkan/anv_extensions.py
>>> @@ -61,6 +61,7 @@ EXTENSIONS = [
>>>       Extension('VK_KHR_get_memory_requirements2',          1, True),
>>>       Extension('VK_KHR_get_physical_device_properties2',   1, True),
>>>       Extension('VK_KHR_get_surface_capabilities2',         1, True),
>>> +    Extension('VK_KHR_image_format_list',                 1, True),
>>>       Extension('VK_KHR_incremental_present',               1, True),
>>>       Extension('VK_KHR_maintenance1',                      1, True),
>>>       Extension('VK_KHR_push_descriptor',                   1, True),
>>> diff --git a/src/intel/vulkan/anv_image.c 
>>> b/src/intel/vulkan/anv_image.c
>>> index 492b341..eb39a0b 100644
>>> --- a/src/intel/vulkan/anv_image.c
>>> +++ b/src/intel/vulkan/anv_image.c
>>> @@ -30,6 +30,7 @@
>>>
>>>   #include "anv_private.h"
>>>   #include "util/debug.h"
>>> +#include "vk_util.h"
>>>
>>>   #include "vk_format_info.h"
>>>
>>> @@ -116,6 +117,39 @@ add_surface(struct anv_image *image, struct 
>>> anv_surface *surf)
>>>      image->alignment = MAX2(image->alignment, surf->isl.alignment);
>>>   }
>>>
>>> +
>>> +static bool
>>> +all_formats_ccs_e_compatible(const struct gen_device_info *devinfo,
>>> +                             const struct VkImageCreateInfo *vk_info)
>>> +{
>>> +   enum isl_format format =
>>> +      anv_get_isl_format(devinfo, vk_info->format,
>>> +                         VK_IMAGE_ASPECT_COLOR_BIT, vk_info->tiling);
>>> +
>>> +   if (!isl_format_supports_ccs_e(devinfo, format))
>>> +      return false;
>>> +
>>> +   if (!(vk_info->flags & VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT))
>>> +      return true;
>>> +
>>> +   const VkImageFormatListCreateInfoKHR *fmt_list =
>>> +      vk_find_struct_const(vk_info->pNext, 
>>> IMAGE_FORMAT_LIST_CREATE_INFO_KHR);
>>> +
>>> +   if (!fmt_list || fmt_list->viewFormatCount == 0)
>>> +      return false;
>>
>> My reading of the spec is a bit confusing on viewFormatCount == 0...
>> But I would assume that if equal to 0 there is no format to be
>> compatible with, therefore we can enable compression.
>
> No, viewFormatCount == 0 is intended to be identical to not including 
> the struct in the pNext chain at all.

Thanks,

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

>
>>> +
>>> +   for (uint32_t i = 0; i < fmt_list->viewFormatCount; i++) {
>>> +      enum isl_format view_format =
>>> +         anv_get_isl_format(devinfo, fmt_list->pViewFormats[i],
>>> +                            VK_IMAGE_ASPECT_COLOR_BIT, 
>>> vk_info->tiling);
>>> +
>>> +      if (!isl_formats_are_ccs_e_compatible(devinfo, format, 
>>> view_format))
>>> +         return false;
>>> +   }
>>> +
>>> +   return true;
>>> +}
>>> +
>>>   /**
>>>    * For color images that have an auxiliary surface, request 
>>> allocation for an
>>>    * additional buffer that mainly stores fast-clear values. Use of 
>>> this buffer
>>> @@ -319,8 +353,7 @@ make_surface(const struct anv_device *dev,
>>>                * compression on at all times for these formats.
>>>                */
>>>               if (!(vk_info->usage & VK_IMAGE_USAGE_STORAGE_BIT) &&
>>> -                !(vk_info->flags & 
>>> VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT) &&
>>> -                isl_format_supports_ccs_e(&dev->info, format)) {
>>> + all_formats_ccs_e_compatible(&dev->info, vk_info)) {
>>>                  image->aux_usage = ISL_AUX_USAGE_CCS_E;
>>>               }
>>>            }
>>
>>
>
>
>