<div dir="ltr"><div><div>All of the pre-work patches have been reviewed by myself and Lionel. I've also read through the rest of the series and things look pretty good to me. I did make some scattered comments but they shouldn't be a big deal. </div>My primary concern with the series is the lack of CCS support. Getting that working correctly is clearly the biggest question mark in all of modifiers so I'm hesitant to pass judgment on this as a patch series (I think the spec is ok) with that piece still missing. </div>--Jason </div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Nov 7, 2017 at 6:47 AM, Chad Versace <<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Overview: This series implements 3 extensions: VK_EXT_external_memory_dma_buf VK_EXT_queue_family_foreign VK_EXT_image_drm_format_modifier The patch series lives on my tag 'chadv/review/anv-dma-buf-v01'. <a href="http://git.kiwitree.net/cgit/~chadv/mesa/log/?h=chadv/review/anv-dma-buf-v01" rel="noreferrer" target="_blank">http://git.kiwitree.net/cgit/~chadv/mesa/log/?h=chadv/review/anv-dma-buf-v01</a> The work-in-progress lives on my branch 'wip/anv-dma-buf'. <a href="http://git.kiwitree.net/cgit/~chadv/mesa/log/?h=wip/anv-dma-buf" rel="noreferrer" target="_blank">http://git.kiwitree.net/cgit/~chadv/mesa/log/?h=wip/anv-dma-buf</a> Specifications: For each extension, you can find a git branch of the Vulkan specification, as well as an online build of the spec, at <a href="http://kiwitree.net/~chadv/vulkan/" rel="noreferrer" target="_blank">http://kiwitree.net/~chadv/vulkan/</a>. All 3 extension specifications are still drafts at various stages in the spec lifecycle. - VK_EXT_external_memory_dma_buf I'll ask Khronos tomorrow morning (Wed 8 Nov) to merge this extension. That would schedule it for publication no later Fri 17 Nov, unless something goes wrong. - VK_EXT_queue_family_foreign This extension is a slow, little thorn in my side. But it's needed for spec correctness in the interaction among VK_KHR_external_memory + VK_EXT_external_memory_dma_buf + VK_EXT_image_drm_format_modifier. In anvil today, in this patch series, it's implementation is a no-op. In Khronos, the discussion on this extension is wrapping up. I expect to ask Khronos to merge it no later than Wed 15 Nov, but hopefully sooner. - VK_EXT_image_drm_format_modifier This is the big extension in the series. Its API is complete, in my opinion, and the specification language appears complete to the untrained eye. But there remain a few loose ends in the spec language that I need to finish before submitting it to Khronos. I've documented all the loose ends in the TODO section of the extensions's appendix [1]. [1]: <a href="http://git.kiwitree.net/cgit/~chadv/vulkan-spec/tree/doc/specs/vulkan/appendices/VK_EXT_image_drm_format_modifier.txt?h=1.0-VK_EXT_image_drm_format_modifier" rel="noreferrer" target="_blank">http://git.kiwitree.net/cgit/~chadv/vulkan-spec/tree/doc/specs/vulkan/appendices/VK_EXT_image_drm_format_modifier.txt?h=1.0-VK_EXT_image_drm_format_modifier</a>]. The API is complete, though, modulo review on mesa-dev. So please proceed to review the extension language and the implementation. Testing: - vkcube I've tested portions of VK_EXT_image_drm_format_modifier with a hacked version of krh's vkcube [2]. The following checklist shows what I've tested so far. [x] vkGetPhysicalDeviceFormatProperties2KHR [x] vkGetPhysicalDeviceImageFormatProperties2KHR [x] vkCreateImage [x] VkImageDrmFormatModifierListCreateInfoEXT [ ] VkImageExplicitDrmFormatModifierEXT [ ] Resolves of compression surfaces [x] vkGetImageDrmFormatModifierEXT [x] vkGetImageSubresouceLayout [2]: vkcube: <a href="http://github.com/chadversary/vkcube/commits/wip/vk-drm-format-mods" rel="noreferrer" target="_blank">http://github.com/chadversary/vkcube/commits/wip/vk-drm-format-mods</a> - vk-gl-cts I'm doing a full run of dEQP-VK.*. I'm still waiting. - crucible We really need to write crucible tests to hammer some tricky corner cases. I haven't written them yet. Volunteers? Chad Versace (30): anv: Remove unused variable 'gen' anv: Suffix anv-private 'VK' tokens with 'ANV' anv: Refactor get_buffer_format_properties() anv: Better types for 'aspect' function params anv: Fix get_image_format_properties() - depthstencil (v2) anv: Fix get_image_format_properties() - ASTC anv: Refactor get_image_format_properties() - plane_format anv: Refactor get_image_format_properties() - base_isl_format anv: Refactor get_image_format_properties() - Reduce params anv: Fix get_image_format_properties() - 3-channel formats anv: Fix get_image_format_properties() - YCbCr anv: Rename get_image_format_properties() anv: Simplify anv_get_image_format_properties() anv: Simplify anv_physical_device_get_format_properties() anv: Remove anv_physical_device_get_format_properties() anv: Refactor anv_get_format_plane() - explicit unsupported anv/image: Refactor choice of isl_tiling_flags_t anv: Refactor anv_GetImageSubresourceLayout() HACK: vulkan: Update headers and registry to chadv/1.0-dma-buf@a79a0ab HACK: vulkan: Install Vulkan headers RFC: anv: Implement VK_EXT_external_memory_dma_buf RFC: anv: Implement VK_EXT_queue_family_foreign RFC: anv: Support VkDrmFormatModifierPropertiesListEXT RFC: anv: Support VkPhysicalDeviceImageDrmFormatModifierInfoEXT RFC: anv: Support VkImageDrmFormatModifierListCreateInfoEXT RFC: anv: Support vkGetImageSubresourceLayout with modifiers RFC: anv: Support VkImageExplicitDrmFormatModifierCreateInfoEXT RFC: anv: Enable VK_EXT_image_drm_format_modifier RFC: anv: Kill vkCreateDmaBufImageINTEL() anv/TODO: Updates for VK_EXT_image_drm_format_modifier include/meson.build | 11 + include/vulkan/vulkan.h | 70 ++++- include/vulkan/vulkan_intel.h | 62 ----- src/amd/vulkan/radv_private.h | 1 - src/intel/Makefile.sources | 1 - src/intel/<a href="http://Makefile.vulkan.am" rel="noreferrer" target="_blank">Makefile.vulkan.am</a> | 3 - src/intel/vulkan/TODO | 16 ++ src/intel/vulkan/anv_blorp.c | 6 +- src/intel/vulkan/anv_device.c | 13 +- src/intel/vulkan/anv_entrypoints_gen.py | 10 - src/intel/vulkan/anv_extensions.py | 3 + src/intel/vulkan/anv_formats.c | 475 ++++++++++++++++++++++---------- src/intel/vulkan/anv_image.c | 281 ++++++++++++++----- src/intel/vulkan/anv_intel.c | 117 -------- src/intel/vulkan/anv_private.h | 21 +- src/intel/vulkan/genX_cmd_buffer.c | 24 +- src/intel/vulkan/meson.build | 1 - src/vulkan/registry/vk.xml | 77 +++++- 18 files changed, 738 insertions(+), 454 deletions(-) delete mode 100644 include/vulkan/vulkan_intel.h delete mode 100644 src/intel/vulkan/anv_intel.c -- 2.13.0 </blockquote></div> </div>