[Mesa-dev] [PATCH 2/4] anv/image: Simplify initialization of the isl_tiling

[Chad Versace]

On Mon 27 Jun 2016, Nanley Chery wrote:
> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/intel/vulkan/anv_image.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index 77d9931..b3f5f5c 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -120,10 +120,6 @@ make_surface(const struct anv_device *dev,
>        [VK_IMAGE_TYPE_3D] = ISL_SURF_DIM_3D,
>     };
>  
> -   isl_tiling_flags_t tiling_flags = anv_info->isl_tiling_flags;
> -   if (vk_info->tiling == VK_IMAGE_TILING_LINEAR)
> -      tiling_flags = ISL_TILING_LINEAR_BIT;
> -
>     struct anv_surface *anv_surf = get_surface(image, aspect);
>  
>     image->extent = anv_sanitize_image_extent(vk_info->imageType,
> @@ -142,7 +138,7 @@ make_surface(const struct anv_device *dev,
>        .min_alignment = 0,
>        .min_pitch = anv_info->stride,
>        .usage = choose_isl_surf_usage(image->usage, aspect),
> -      .tiling_flags = tiling_flags);
> +      .tiling_flags = anv_info->isl_tiling_flags);
>  
>     /* isl_surf_init() will fail only if provided invalid input. Invalid input
>      * is illegal in Vulkan.
> @@ -260,7 +256,9 @@ anv_CreateImage(VkDevice device,
>     return anv_image_create(device,
>        &(struct anv_image_create_info) {
>           .vk_info = pCreateInfo,
> -         .isl_tiling_flags = ISL_TILING_ANY_MASK,
> +         .isl_tiling_flags = (pCreateInfo->tiling == VK_IMAGE_TILING_LINEAR) ?
> +                             ISL_TILING_LINEAR_BIT : ISL_TILING_ANY_MASK,
> +
>        },
>        pAllocator,
>        pImage);

I don't agree with this patch.

Locally, the patch look correct. But when you consider that
anv_image_create() is public within the driver, the patch makes the code
fragile. Pre-patch, if the caller of anv_image_create() sets
anv_image_create_info::vk_info::tiling and leaves
anv_image_create_info::isl_tiling_flags unset (which I believe should be
a valid combination), then anv_image_create() correctly converts the
VkImageTilingFlags to isl_tiling_flags. Post-patch, that's no longer the
case; anv_image_create() ignores its VkImageTiling input.