[Mesa-dev] [PATCH 00/23] anv: Format reworks

[Nanley Chery]

On Mon, May 16, 2016 at 12:08:05PM -0700, Jason Ekstrand wrote:
> This series scratches an itch I've had in the Vulkan driver for some time.
> Early on, we added a table of anv_format structs.  This served a bunch of
> different purposes at the same time: basic format introspection, mapping
> from VkFormats to hardware formats, and we also threw them around as a form
> of format handle (we storred one in anv_image for instance).
> 
> As the driver has grown, this really isn't appropreate anymore.  The bit of
> format introspection we do (has_stencil and has_depth) are more easiliy
> served by a simple switch statement.  We no longer have a one-to-one
> mapping from VkFormats to hardware formats because things like RGB formats
> get a different format depending on whether they're linear or tiled.  It
> also doesn't make sense to use it as a format handle because that leads to
> people doing image->format->isl_format because they're lazy and don't want
> to look up the actual format for the aspect they want which may lead to
> them getting the wrong format.
> 
> This series (which is way more patches than I expected) solves this by
> making the following changes:
> 
>  1) vk_format_info.h now provides very basic VkFormat introspection
>  2) anv_image has an aspects field
>  3) struct anv_format is stripped down to just an isl_format and swizzle
>  4) anv_format_for_vk_format is replaced by anv_get_format which also takes
>     a devinfo, tiling, and aspect and returns the actual format+swizzle
>  5) Cleanups to use the right formats various places
> 
> At the end of the series, I have one more patch that adds better 4444
> support on pre-bdw.  This fixes a pile (~165) Vulkan CTS tests on Haswell
> and brings Broadwell back to supporting all the required Vulkan formats.
> 
> Cc: Chad Versace <chad.versace at intel.com>
> Cc: Nanley Chery <nanley.g.chery at intel.com>
> 

Thanks for reworking the format code.
Patches 3, 5-6, 9-10, 12-13, 16-23 are,
Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>

Please find my comments and remaining reviews in the indivdual replies.

- Nanley

> Jason Ekstrand (23):
>   anv/wsi: Make WSI per-physical-device rather than per-instance
>   anv/wayland: Don't allow non-renderable formats
>   anv: Make format_for_descriptor return an isl_format
>   anv/image: Add an aspects field
>   anv/image: Use get_isl_format when creating buffer views
>   anv: Add helpers to provide simple VkFormat introspection
>   anv: Change render_pass_attachment.format to a VkFormat
>   anv/copy: Use the color_surf from the image to get the block size
>   anv/image: Make heavier use of aspects
>   anv/image: Stop using anv_format for image create validation
>   anv/copy: Use the linear format from the image for the buffer block
>     size
>   anv/wsi: Use vk_format_info for asserts rather than anv_format
>   anv: Remove the anv_format member from anv_image
>   anv/image: Use aspects for computing full usage
>   anv/formats: Delete validate_GetPhysicalDeviceFormatProperties
>   anv/format: Simplify anv_format
>   anv/formats: Add an anv_get_format helper
>   anv/formats: Use the isl_channel_select enum for the swizzle
>   anv: Use 16 bits for the isl_format in anv_format
>   anv/formats: Refactor anv_get_format
>   anv/formats: Set the swizzle to RGB1 when using an RGBA format to fake
>     RGB
>   anv: Add a devinfo argument to the get_format functions
>   anv/formats: Add support for VK_FORMAT_B4G4R4A4_UNORM pre-gen8
> 
>  src/intel/vulkan/Makefile.sources     |   3 +-
>  src/intel/vulkan/anv_cmd_buffer.c     |  23 ++--
>  src/intel/vulkan/anv_descriptor_set.c |   6 +-
>  src/intel/vulkan/anv_device.c         |   9 +-
>  src/intel/vulkan/anv_formats.c        | 209 +++++++++++++++++-----------------
>  src/intel/vulkan/anv_image.c          | 149 ++++++++++--------------
>  src/intel/vulkan/anv_meta_copy.c      |  34 ++++--
>  src/intel/vulkan/anv_meta_resolve.c   |   4 +-
>  src/intel/vulkan/anv_pass.c           |   2 +-
>  src/intel/vulkan/anv_private.h        |  63 ++++------
>  src/intel/vulkan/anv_wsi.c            |  27 +++--
>  src/intel/vulkan/anv_wsi.h            |   8 +-
>  src/intel/vulkan/anv_wsi_wayland.c    |  50 ++++----
>  src/intel/vulkan/anv_wsi_x11.c        |  43 +++----
>  src/intel/vulkan/gen7_cmd_buffer.c    |   5 +-
>  src/intel/vulkan/genX_cmd_buffer.c    |   7 +-
>  src/intel/vulkan/genX_pipeline_util.h |   6 +-
>  src/intel/vulkan/vk_format_info.h     |  65 +++++++++++
>  18 files changed, 384 insertions(+), 329 deletions(-)
>  create mode 100644 src/intel/vulkan/vk_format_info.h
> 
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev