[Mesa-dev] [PATCH v2 04/11] anv: prepare formats to handle disjoints sets

[Chad Versace]

On Tue 03 Oct 2017, Lionel Landwerlin wrote:
> Newer format enums start at offset 1000000000, making it impossible to
> have them all in one table. This change splits the formats into sets
> that we then access through indirection.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/intel/vulkan/anv_formats.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
> index 049ffe17ac0..71824256b25 100644
> --- a/src/intel/vulkan/anv_formats.c
> +++ b/src/intel/vulkan/anv_formats.c
> @@ -58,7 +58,7 @@
>   * other.  The reason for this is that, for packed formats, the ISL (and
>   * bspec) names are in LSB -> MSB order while VK formats are MSB -> LSB.
>   */
> -static const struct anv_format anv_formats[] = {
> +static const struct anv_format main_formats[] = {
>     fmt(VK_FORMAT_UNDEFINED,               ISL_FORMAT_UNSUPPORTED),
>     fmt(VK_FORMAT_R4G4_UNORM_PACK8,        ISL_FORMAT_UNSUPPORTED),
>     fmt(VK_FORMAT_R4G4B4A4_UNORM_PACK16,   ISL_FORMAT_A4B4G4R4_UNORM),
> @@ -251,13 +251,30 @@ static const struct anv_format anv_formats[] = {
>  
>  #undef fmt
>  
> +static const struct {
> +   const struct anv_format *formats;
> +   uint32_t n_formats;
> +} anv_formats[] = {
> +   [0] = { .formats = main_formats, .n_formats = ARRAY_SIZE(main_formats), },
> +};
> +
> +static struct anv_format
> +format_extract(VkFormat vk_format)
> +{
> +   uint32_t enum_offset = vk_enum_offset(vk_format);
> +   uint32_t ext_number = vk_enum_extension(vk_format);
> +
> +   if (ext_number >= ARRAY_SIZE(anv_formats) ||
> +       enum_offset >= anv_formats[ext_number].n_formats)
> +      return (struct anv_format) { .isl_format = ISL_FORMAT_UNSUPPORTED };
> +
> +   return anv_formats[ext_number].formats[enum_offset];
> +}
> +
>  static bool
>  format_supported(VkFormat vk_format)
>  {
> -   if (vk_format >= ARRAY_SIZE(anv_formats))
> -      return false;
> -
> -   return anv_formats[vk_format].isl_format != ISL_FORMAT_UNSUPPORTED;
> +   return format_extract(vk_format).isl_format != ISL_FORMAT_UNSUPPORTED;
>  }
>  
>  /**
> @@ -267,10 +284,10 @@ struct anv_format
>  anv_get_format(const struct gen_device_info *devinfo, VkFormat vk_format,
>                 VkImageAspectFlags aspect, VkImageTiling tiling)
>  {
> -   if (!format_supported(vk_format))
> -      return anv_formats[VK_FORMAT_UNDEFINED];
> +   struct anv_format format = format_extract(vk_format);
>  
> -   struct anv_format format = anv_formats[vk_format];
> +   if (format.isl_format == ISL_FORMAT_UNSUPPORTED)
> +      return format;
>  
>     if (aspect == VK_IMAGE_ASPECT_STENCIL_BIT) {
>        assert(vk_format_aspects(vk_format) & VK_IMAGE_ASPECT_STENCIL_BIT);
> @@ -553,7 +570,7 @@ anv_get_image_format_properties(
>      *    * This field cannot be ASTC format if the Surface Type is SURFTYPE_1D.
>      */
>     if (info->type == VK_IMAGE_TYPE_1D &&
> -       isl_format_is_compressed(anv_formats[info->format].isl_format)) {
> +       isl_format_is_compressed(format_extract(info->format).isl_format)) {

When I see format_extract(), I have no idea what it does. How about
giving it a name like format_to_anv(), vk_format_to_anv(), or even
something as simple as format_get()? Everyone has a general
understanding of what foobar_get() and foobar_to_stuff() does, but
I don't think foobar_extract() is as well understand.

As precedent, there is only one other function with 'extract' in its
name under src/intel, brw_vec4_surface_builder.cpp:emit_extract(). And,
as expected, emit_extract() is not a getter.