Re: [Mesa-dev] [RFC PATCH 13/13] RFC: anv: Drop unneeded struct anv_image_create_info

2017-03-07 Thread Jason Ekstrand 
Yes please!

On Mon, Mar 6, 2017 at 12:40 PM, Chad Versace 
wrote:

> Now that anv_image_create() is gone, there is no need for struct
> anv_image_create_info. The struct just adds an unused layer of
> indirection.
> ---
>  src/intel/vulkan/anv_image.c   | 38 +++---
> 
>  src/intel/vulkan/anv_private.h |  9 -
>  2 files changed, 11 insertions(+), 36 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index 998b0b43391..e5e93f702f6 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -106,7 +106,7 @@ get_surface(struct anv_image *image,
> VkImageAspectFlags aspect)
>  }
>
>  static isl_tiling_flags_t
> -choose_isl_tiling_flags(const struct anv_image_create_info *anv_info,
> +choose_isl_tiling_flags(const VkImageCreateInfo *base_info,
>  const VkImportImageDmaBufInfoMESAX
> *import_dma_buf_info,
>  const VkExportImageDmaBufInfoMESAX
> *export_dma_buf_info)
>  {
> @@ -122,15 +122,12 @@ choose_isl_tiling_flags(const struct
> anv_image_create_info *anv_info,
>   uint64_t mod = export_dma_buf_info->pDrmFormatModifiers[i];
>   flags |= 1 << isl_tiling_from_drm_format_mod(mod);
>}
> -   } else if (anv_info->vk_info->tiling == VK_IMAGE_TILING_LINEAR) {
> +   } else if (base_info->tiling == VK_IMAGE_TILING_LINEAR) {
>flags = ISL_TILING_LINEAR_BIT;
> } else {
>flags = ISL_TILING_ANY_MASK;
> }
>
> -   if (anv_info->isl_tiling_flags)
> -  flags &= anv_info->isl_tiling_flags;
> -
> assert(flags != 0);
>
> return flags;
> @@ -158,22 +155,6 @@ choose_isl_format(const struct anv_device *dev,
> }
>  }
>
> -static uint32_t
> -choose_row_pitch(const struct anv_image_create_info *anv_info,
> - const VkImportImageDmaBufPlaneInfoMESAX *plane_info)
> -{
> -   if (anv_info->stride != 0)
> -  return anv_info->stride;
> -
> -   if (plane_info) {
> -  assert(plane_info->rowPitch > 0);
> -  return plane_info->rowPitch;
> -   }
> -
> -   /* Let isl choose the pitch. */
> -   return 0;
> -}
> -
>  static void
>  set_min_surface_offset(const struct anv_image *image, struct anv_surface
> *surf)
>  {
> @@ -318,14 +299,13 @@ make_aux_surface_maybe(const struct anv_device *dev,
>   */
>  static void
>  make_main_surface(const struct anv_device *dev,
> -  const struct anv_image_create_info *anv_info,
> +  const VkImageCreateInfo *base_info,
>const VkImportImageDmaBufInfoMESAX *import_dma_buf_info,
>const VkExportImageDmaBufInfoMESAX *export_dma_buf_info,
>VkImageAspectFlags aspect,
>VkExternalMemoryHandleTypeFlagsKHX handle_types,
>struct anv_image *image)
>  {
> -   const VkImageCreateInfo *base_info = anv_info->vk_info;
> bool ok UNUSED;
>
> static const enum isl_surf_dim vk_to_isl_surf_dim[] = {
> @@ -339,8 +319,13 @@ make_main_surface(const struct anv_device *dev,
>plane_info = _dma_buf_info->pPlanes[0];
>
> const isl_tiling_flags_t tiling_flags =
> -  choose_isl_tiling_flags(anv_info, import_dma_buf_info,
> export_dma_buf_info);
> -   const uint32_t row_pitch = choose_row_pitch(anv_info, plane_info);
> +  choose_isl_tiling_flags(base_info, import_dma_buf_info,
> export_dma_buf_info);
> +
> +   uint32_t row_pitch = 0;
> +   if (plane_info) {
> +  assert(plane_info->rowPitch > 0);
> +  row_pitch = plane_info->rowPitch;
> +   }
>
> struct anv_surface *anv_surf = get_surface(image, aspect);
>
> @@ -386,7 +371,6 @@ anv_CreateImage(VkDevice _device,
>  VkImage *pImage)
>  {
> ANV_FROM_HANDLE(anv_device, device, _device);
> -   const struct anv_image_create_info anv_info = { .vk_info = base_info };
> const VkImportImageDmaBufInfoMESAX *import_dma_buf_info = NULL;
> const VkExportImageDmaBufInfoMESAX *export_dma_buf_info = NULL;
> VkExternalMemoryHandleTypeFlagsKHX handle_types = 0;
> @@ -444,7 +428,7 @@ anv_CreateImage(VkDevice _device,
> uint32_t b;
> for_each_bit(b, image->aspects) {
>VkImageAspectFlagBits aspect = 1 << b;
> -  make_main_surface(device, _info, import_dma_buf_info,
> +  make_main_surface(device, base_info, import_dma_buf_info,
>  export_dma_buf_info, aspect, handle_types, image);
>make_aux_surface_maybe(device, base_info, aspect, handle_types,
> image);
> }
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index 2f62ec50c5b..a5dd0f72de1 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1860,15 +1860,6 @@ struct anv_image_view {
> struct brw_image_param storage_image_param;
>  };
>
> -struct anv_image_create_info {
> -   const VkImageCreateInfo *vk_info;
> -
> -   /** An opt-in bitmask which filters an ISL-mapping of

[Mesa-dev] [RFC PATCH 13/13] RFC: anv: Drop unneeded struct anv_image_create_info

2017-03-06 Thread Chad Versace 
Now that anv_image_create() is gone, there is no need for struct
anv_image_create_info. The struct just adds an unused layer of
indirection.
---
 src/intel/vulkan/anv_image.c   | 38 +++---
 src/intel/vulkan/anv_private.h |  9 -
 2 files changed, 11 insertions(+), 36 deletions(-)

diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 998b0b43391..e5e93f702f6 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -106,7 +106,7 @@ get_surface(struct anv_image *image, VkImageAspectFlags 
aspect)
 }
 
 static isl_tiling_flags_t
-choose_isl_tiling_flags(const struct anv_image_create_info *anv_info,
+choose_isl_tiling_flags(const VkImageCreateInfo *base_info,
 const VkImportImageDmaBufInfoMESAX 
*import_dma_buf_info,
 const VkExportImageDmaBufInfoMESAX 
*export_dma_buf_info)
 {
@@ -122,15 +122,12 @@ choose_isl_tiling_flags(const struct 
anv_image_create_info *anv_info,
  uint64_t mod = export_dma_buf_info->pDrmFormatModifiers[i];
  flags |= 1 << isl_tiling_from_drm_format_mod(mod);
   }
-   } else if (anv_info->vk_info->tiling == VK_IMAGE_TILING_LINEAR) {
+   } else if (base_info->tiling == VK_IMAGE_TILING_LINEAR) {
   flags = ISL_TILING_LINEAR_BIT;
} else {
   flags = ISL_TILING_ANY_MASK;
}
 
-   if (anv_info->isl_tiling_flags)
-  flags &= anv_info->isl_tiling_flags;
-
assert(flags != 0);
 
return flags;
@@ -158,22 +155,6 @@ choose_isl_format(const struct anv_device *dev,
}
 }
 
-static uint32_t
-choose_row_pitch(const struct anv_image_create_info *anv_info,
- const VkImportImageDmaBufPlaneInfoMESAX *plane_info)
-{
-   if (anv_info->stride != 0)
-  return anv_info->stride;
-
-   if (plane_info) {
-  assert(plane_info->rowPitch > 0);
-  return plane_info->rowPitch;
-   }
-
-   /* Let isl choose the pitch. */
-   return 0;
-}
-
 static void
 set_min_surface_offset(const struct anv_image *image, struct anv_surface *surf)
 {
@@ -318,14 +299,13 @@ make_aux_surface_maybe(const struct anv_device *dev,
  */
 static void
 make_main_surface(const struct anv_device *dev,
-  const struct anv_image_create_info *anv_info,
+  const VkImageCreateInfo *base_info,
   const VkImportImageDmaBufInfoMESAX *import_dma_buf_info,
   const VkExportImageDmaBufInfoMESAX *export_dma_buf_info,
   VkImageAspectFlags aspect,
   VkExternalMemoryHandleTypeFlagsKHX handle_types,
   struct anv_image *image)
 {
-   const VkImageCreateInfo *base_info = anv_info->vk_info;
bool ok UNUSED;
 
static const enum isl_surf_dim vk_to_isl_surf_dim[] = {
@@ -339,8 +319,13 @@ make_main_surface(const struct anv_device *dev,
   plane_info = _dma_buf_info->pPlanes[0];
 
const isl_tiling_flags_t tiling_flags =
-  choose_isl_tiling_flags(anv_info, import_dma_buf_info, 
export_dma_buf_info);
-   const uint32_t row_pitch = choose_row_pitch(anv_info, plane_info);
+  choose_isl_tiling_flags(base_info, import_dma_buf_info, 
export_dma_buf_info);
+
+   uint32_t row_pitch = 0;
+   if (plane_info) {
+  assert(plane_info->rowPitch > 0);
+  row_pitch = plane_info->rowPitch;
+   }
 
struct anv_surface *anv_surf = get_surface(image, aspect);
 
@@ -386,7 +371,6 @@ anv_CreateImage(VkDevice _device,
 VkImage *pImage)
 {
ANV_FROM_HANDLE(anv_device, device, _device);
-   const struct anv_image_create_info anv_info = { .vk_info = base_info };
const VkImportImageDmaBufInfoMESAX *import_dma_buf_info = NULL;
const VkExportImageDmaBufInfoMESAX *export_dma_buf_info = NULL;
VkExternalMemoryHandleTypeFlagsKHX handle_types = 0;
@@ -444,7 +428,7 @@ anv_CreateImage(VkDevice _device,
uint32_t b;
for_each_bit(b, image->aspects) {
   VkImageAspectFlagBits aspect = 1 << b;
-  make_main_surface(device, _info, import_dma_buf_info,
+  make_main_surface(device, base_info, import_dma_buf_info,
 export_dma_buf_info, aspect, handle_types, image);
   make_aux_surface_maybe(device, base_info, aspect, handle_types, image);
}
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 2f62ec50c5b..a5dd0f72de1 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -1860,15 +1860,6 @@ struct anv_image_view {
struct brw_image_param storage_image_param;
 };
 
-struct anv_image_create_info {
-   const VkImageCreateInfo *vk_info;
-
-   /** An opt-in bitmask which filters an ISL-mapping of the Vulkan tiling. */
-   isl_tiling_flags_t isl_tiling_flags;
-
-   uint32_t stride;
-};
-
 const struct anv_surface *
 anv_image_get_surface_for_aspect_mask(const struct anv_image *image,
   VkImageAspectFlags aspect_mask);
-- 
2.12.0

___
mesa-dev