<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Feb 15, 2018 at 7:57 AM, Daniel Stone <span dir="ltr"><<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>><br>
<br>
This involves extending our fake extension a bit to allow for additional<br>
querying and passing of modifier information.  The added bits are<br>
intended to look a lot like the draft of VK_EXT_image_drm_format_modifi<wbr>er.<br>
Once the extension gets finalized, we'll simply transition all of the<br>
structs used in wsi_common to the real extension structs.<br>
<br>
Reviewed-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>><br>
Signed-off-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>><br>
---<br>
 src/vulkan/wsi/wsi_common.c         | 164 ++++++++++++++++++++++++++++++<wbr>++----<br>
 src/vulkan/wsi/wsi_common.h         |  23 +++++<br>
 src/vulkan/wsi/wsi_common_pri<wbr>vate.h |   3 +<br>
 src/vulkan/wsi/wsi_common_way<wbr>land.c |   3 +-<br>
 src/vulkan/wsi/wsi_common_<wbr>x11.c     |   3 +-<br>
 5 files changed, 177 insertions(+), 19 deletions(-)<br>
<br>
diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c<br>
index c235128e562..edba13a13de 100644<br>
--- a/src/vulkan/wsi/wsi_common.c<br>
+++ b/src/vulkan/wsi/wsi_common.c<br>
@@ -26,6 +26,8 @@<br>
 #include "util/macros.h"<br>
 #include "vk_util.h"<br>
<br>
+#include <unistd.h><br>
+<br>
 VkResult<br>
 wsi_device_init(struct wsi_device *wsi,<br>
                 VkPhysicalDevice pdevice,<br>
@@ -36,6 +38,8 @@ wsi_device_init(struct wsi_device *wsi,<br>
<br>
    memset(wsi, 0, sizeof(*wsi));<br>
<br>
+   wsi->pdevice = pdevice;<br>
+<br>
 #define WSI_GET_CB(func) \<br>
    PFN_vk##func func = (PFN_vk##func)proc_addr(pdevic<wbr>e, "vk" #func)<br>
    WSI_GET_CB(GetPhysicalDeviceMe<wbr>moryProperties);<br>
@@ -69,6 +73,7 @@ wsi_device_init(struct wsi_device *wsi,<br>
    WSI_GET_CB(GetImageSubresource<wbr>Layout);<br>
    WSI_GET_CB(GetMemoryFdKHR);<br>
    WSI_GET_CB(GetPhysicalDeviceFo<wbr>rmatProperties);<br>
+   WSI_GET_CB(GetPhysicalDeviceF<wbr>ormatProperties2KHR);<br>
    WSI_GET_CB(ResetFences);<br>
    WSI_GET_CB(QueueSubmit);<br>
    WSI_GET_CB(WaitForFences);<br>
@@ -196,6 +201,9 @@ align_u32(uint32_t v, uint32_t a)<br>
 VkResult<br>
 wsi_create_native_image(const struct wsi_swapchain *chain,<br>
                         const VkSwapchainCreateInfoKHR *pCreateInfo,<br>
+                        uint32_t num_modifier_lists,<br>
+                        const uint32_t *num_modifiers,<br>
+                        const uint64_t *const *modifiers,<br>
                         struct wsi_image *image)<br>
 {<br>
    const struct wsi_device *wsi = chain->wsi;<br>
@@ -205,11 +213,91 @@ wsi_create_native_image(const struct wsi_swapchain *chain,<br>
    for (int i = 0; i < ARRAY_SIZE(image->fds); i++)<br>
       image->fds[i] = -1;<br>
<br>
-   const struct wsi_image_create_info image_wsi_info = {<br>
+   struct wsi_image_create_info image_wsi_info = {<br>
       .sType = VK_STRUCTURE_TYPE_WSI_IMAGE_CR<wbr>EATE_INFO_MESA,<br>
       .pNext = NULL,<br>
-      .scanout = true,<br>
    };<br>
+<br>
+   uint32_t image_modifier_count = 0, modifier_prop_count = 0;<br>
+   struct wsi_format_modifier_properties *modifier_props = NULL;<br>
+   uint64_t *image_modifiers = NULL;<br>
+   if (num_modifier_lists == 0) {<br>
+      /* If we don't have modifiers, fall back to the legacy "scanout" flag */<br>
+      image_wsi_info.scanout = true;<br>
+   } else {<br>
+      /* The winsys can't request modifiers if we don't support them. */<br>
+      assert(wsi->supports_modifiers<wbr>);<br>
+      struct wsi_format_modifier_properties<wbr>_list modifier_props_list = {<br>
+         .sType = VK_STRUCTURE_TYPE_WSI_FORMAT_M<wbr>ODIFIER_PROPERTIES_LIST_MESA,<br>
+         .pNext = NULL,<br>
+      };<br>
+      VkFormatProperties2KHR format_props = {<br>
+         .sType = VK_STRUCTURE_TYPE_FORMAT_PROPE<wbr>RTIES_2_KHR,<br>
+         .pNext = &modifier_props_list,<br>
+      };<br>
+      wsi->GetPhysicalDeviceFormatPr<wbr>operties2KHR(wsi->pdevice,<br>
+                                                 pCreateInfo->imageFormat,<br>
+                                                 &format_props);<br>
+      assert(modifier_props_list.mod<wbr>ifier_count > 0);<br>
+      modifier_props = vk_alloc(&chain->alloc,<br>
+                                sizeof(*modifier_props) *<br>
+                                modifier_props_list.modifier_c<wbr>ount,<br>
+                                8,<br>
+                                VK_SYSTEM_ALLOCATION_SCOPE_COM<wbr>MAND);<br>
+      if (!modifier_props) {<br>
+         result = VK_ERROR_OUT_OF_HOST_MEMORY;<br>
+         goto fail;<br>
+      }<br>
+<br>
+      modifier_props_list.modifier_p<wbr>roperties = modifier_props;<br>
+      wsi->GetPhysicalDeviceFormatPr<wbr>operties2KHR(wsi->pdevice,<br>
+                                                 pCreateInfo->imageFormat,<br>
+                                                 &format_props);<br>
+      modifier_prop_count = modifier_props_list.modifier_c<wbr>ount;<br>
+<br>
+      uint32_t max_modifier_count = 0;<br>
+      for (uint32_t l = 0; l < num_modifier_lists; l++)<br>
+         max_modifier_count = MAX2(max_modifier_count, num_modifiers[l]);<br>
+<br>
+      image_modifiers = vk_alloc(&chain->alloc,<br>
+                                 sizeof(*image_modifiers) *<br>
+                                 max_modifier_count,<br>
+                                 8,<br>
+                                 VK_SYSTEM_ALLOCATION_SCOPE_CO<wbr>MMAND);<br>
+      if (!image_modifiers) {<br>
+         result = VK_ERROR_OUT_OF_HOST_MEMORY;<br>
+         goto fail;<br>
+      }<br>
+<br>
+      image_modifier_count = 0;<br>
+      for (uint32_t l = 0; l < num_modifier_lists; l++) {<br>
+         /* Walk the modifier lists and construct a list of supported<br>
+          * modifiers.<br>
+          */<br>
+         for (uint32_t i = 0; i < num_modifiers[l]; i++) {<br>
+            for (uint32_t j = 0; j < modifier_prop_count; j++) {<br>
+               if (modifier_props[j].modifier == modifiers[l][i])<br>
+                  image_modifiers[image_modifier<wbr>_count++] = modifiers[l][i];<br>
+            }<br>
+         }<br></blockquote><div><br></div><div>I know I technically wrote this code, but I'm having a bit of trouble understanding why it's needed.  Are we expecting that the scanout set may not have any supported modifiers in it?  The 3D driver can render to a lot of things so that's very unlikely.  At least on Intel, LINEAR and X will always be there and we can definitely render to those.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+         /* We only want to take the modifiers from the first list */<br>
+         if (image_modifier_count > 0)<br>
+            break;<br>
+      }<br>
+<br>
+      if (image_modifier_count > 0) {<br>
+         image_wsi_info.modifier_count = image_modifier_count;<br>
+         image_wsi_info.modifiers = image_modifiers;<br>
+      } else {<br>
+         /* TODO: Add a proper error here */<br>
+         assert(!"Failed to find a supported modifier!  This should never "<br>
+                 "happen because LINEAR should always be available");<br>
+         result = VK_ERROR_OUT_OF_HOST_MEMORY;<br>
+         goto fail;<br>
+      }<br>
+   }<br>
+<br>
    const VkImageCreateInfo image_info = {<br>
       .sType = VK_STRUCTURE_TYPE_IMAGE_CREATE<wbr>_INFO,<br>
       .pNext = &image_wsi_info,<br>
@@ -239,15 +327,6 @@ wsi_create_native_image(const struct wsi_swapchain *chain,<br>
    VkMemoryRequirements reqs;<br>
    wsi->GetImageMemoryRequirement<wbr>s(chain->device, image->image, &reqs);<br>
<br>
-   VkSubresourceLayout image_layout;<br>
-   const VkImageSubresource image_subresource = {<br>
-      .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT,<br>
-      .mipLevel = 0,<br>
-      .arrayLayer = 0,<br>
-   };<br>
-   wsi->GetImageSubresourceLayou<wbr>t(chain->device, image->image,<br>
-                                  &image_subresource, &image_layout);<br>
-<br>
    const struct wsi_memory_allocate_info memory_wsi_info = {<br>
       .sType = VK_STRUCTURE_TYPE_WSI_MEMORY_A<wbr>LLOCATE_INFO_MESA,<br>
       .pNext = NULL,<br>
@@ -292,16 +371,67 @@ wsi_create_native_image(const struct wsi_swapchain *chain,<br>
    if (result != VK_SUCCESS)<br>
       goto fail;<br>
<br>
-   image->drm_modifier = DRM_FORMAT_MOD_INVALID;<br>
-   image->num_planes = 1;<br>
-   image->sizes[0] = reqs.size;<br>
-   image->row_pitches[0] = image_layout.rowPitch;<br>
-   image->offsets[0] = 0;<br>
-   image->fds[0] = fd;<br>
+   if (num_modifier_lists > 0) {<br>
+      image->drm_modifier = wsi->image_get_modifier(image-<wbr>>image);<br>
+      assert(image->drm_modifier != DRM_FORMAT_MOD_INVALID);<br>
+<br>
+      for (uint32_t j = 0; j < modifier_prop_count; j++) {<br>
+         if (modifier_props[j].modifier == image->drm_modifier) {<br>
+            image->num_planes = modifier_props[j].modifier_pla<wbr>ne_count;<br>
+            break;<br>
+         }<br>
+      }<br>
+<br>
+      for (uint32_t p = 0; p < image->num_planes; p++) {<br>
+         const VkImageSubresource image_subresource = {<br>
+            .aspectMask = VK_IMAGE_ASPECT_PLANE_0_BIT_KH<wbr>R << p,<br>
+            .mipLevel = 0,<br>
+            .arrayLayer = 0,<br>
+         };<br>
+         VkSubresourceLayout image_layout;<br>
+         wsi->GetImageSubresourceLayou<wbr>t(chain->device, image->image,<br>
+                                        &image_subresource, &image_layout);<br>
+         image->sizes[p] = image_layout.size;<br>
+         image->row_pitches[p] = image_layout.rowPitch;<br>
+         image->offsets[p] = image_layout.offset;<br>
+         if (p == 0) {<br>
+            image->fds[p] = fd;<br>
+         } else {<br>
+            image->fds[p] = dup(fd);<br>
+            if (image->fds[p] == -1) {<br>
+               for (uint32_t i = 0; i < p; i++)<br>
+                  close(image->fds[p]);<br>
+<br>
+               goto fail;<br>
+            }<br>
+         }<br>
+      }<br>
+   } else {<br>
+      const VkImageSubresource image_subresource = {<br>
+         .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT,<br>
+         .mipLevel = 0,<br>
+         .arrayLayer = 0,<br>
+      };<br>
+      VkSubresourceLayout image_layout;<br>
+      wsi->GetImageSubresourceLayout<wbr>(chain->device, image->image,<br>
+                                     &image_subresource, &image_layout);<br>
+<br>
+      image->drm_modifier = DRM_FORMAT_MOD_INVALID;<br>
+      image->num_planes = 1;<br>
+      image->sizes[0] = reqs.size;<br>
+      image->row_pitches[0] = image_layout.rowPitch;<br>
+      image->offsets[0] = 0;<br>
+      image->fds[0] = fd;<br>
+   }<br>
+<br>
+   vk_free(&chain->alloc, modifier_props);<br>
+   vk_free(&chain->alloc, image_modifiers);<br>
<br>
    return VK_SUCCESS;<br>
<br>
 fail:<br>
+   vk_free(&chain->alloc, modifier_props);<br>
+   vk_free(&chain->alloc, image_modifiers);<br>
    wsi_destroy_image(chain, image);<br>
<br>
    return result;<br>
diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h<br>
index 3e0d3be1c24..6cf729ba025 100644<br>
--- a/src/vulkan/wsi/wsi_common.h<br>
+++ b/src/vulkan/wsi/wsi_common.h<br>
@@ -35,11 +35,15 @@<br>
  */<br>
 #define VK_STRUCTURE_TYPE_WSI_IMAGE_CR<wbr>EATE_INFO_MESA (VkStructureType)1000001002<br>
 #define VK_STRUCTURE_TYPE_WSI_MEMORY_A<wbr>LLOCATE_INFO_MESA (VkStructureType)1000001003<br>
+#define VK_STRUCTURE_TYPE_WSI_FORMAT_M<wbr>ODIFIER_PROPERTIES_LIST_MESA (VkStructureType)1000001004<br>
<br>
 struct wsi_image_create_info {<br>
     VkStructureType sType;<br>
     const void *pNext;<br>
     bool scanout;<br>
+<br>
+    uint32_t modifier_count;<br>
+    const uint64_t *modifiers;<br>
 };<br>
<br>
 struct wsi_memory_allocate_info {<br>
@@ -48,14 +52,32 @@ struct wsi_memory_allocate_info {<br>
     bool implicit_sync;<br>
 };<br>
<br>
+struct wsi_format_modifier_properties {<br>
+   uint64_t modifier;<br>
+   uint32_t modifier_plane_count;<br>
+};<br>
+<br>
+/* Chain in for vkGetPhysicalDeviceFormatPrope<wbr>rties2KHR */<br>
+struct wsi_format_modifier_properties<wbr>_list {<br>
+   VkStructureType sType;<br>
+   const void *pNext;<br>
+<br>
+   uint32_t modifier_count;<br>
+   struct wsi_format_modifier_properties *modifier_properties;<br>
+};<br>
+<br>
 struct wsi_interface;<br>
<br>
 #define VK_ICD_WSI_PLATFORM_MAX 5<br>
<br>
 struct wsi_device {<br>
+   VkPhysicalDevice pdevice;<br>
    VkPhysicalDeviceMemoryProperti<wbr>es memory_props;<br>
    uint32_t queue_family_count;<br>
<br>
+   bool supports_modifiers;<br>
+   uint64_t (*image_get_modifier)(VkImage image);<br>
+<br>
 #define WSI_CB(cb) PFN_vk##cb cb<br>
    WSI_CB(AllocateMemory);<br>
    WSI_CB(AllocateCommandBuffers)<wbr>;<br>
@@ -79,6 +101,7 @@ struct wsi_device {<br>
    WSI_CB(GetImageSubresourceLayo<wbr>ut);<br>
    WSI_CB(GetMemoryFdKHR);<br>
    WSI_CB(GetPhysicalDeviceFormat<wbr>Properties);<br>
+   WSI_CB(GetPhysicalDeviceForma<wbr>tProperties2KHR);<br>
    WSI_CB(ResetFences);<br>
    WSI_CB(QueueSubmit);<br>
    WSI_CB(WaitForFences);<br>
diff --git a/src/vulkan/wsi/wsi_common_pr<wbr>ivate.h b/src/vulkan/wsi/wsi_common_pr<wbr>ivate.h<br>
index 781e84635f9..b608119b969 100644<br>
--- a/src/vulkan/wsi/wsi_common_pr<wbr>ivate.h<br>
+++ b/src/vulkan/wsi/wsi_common_pr<wbr>ivate.h<br>
@@ -81,6 +81,9 @@ void wsi_swapchain_finish(struct wsi_swapchain *chain);<br>
 VkResult<br>
 wsi_create_native_image(const struct wsi_swapchain *chain,<br>
                         const VkSwapchainCreateInfoKHR *pCreateInfo,<br>
+                        uint32_t num_modifier_lists,<br>
+                        const uint32_t *num_modifiers,<br>
+                        const uint64_t *const *modifiers,<br>
                         struct wsi_image *image);<br>
<br>
 VkResult<br>
diff --git a/src/vulkan/wsi/wsi_common_wa<wbr>yland.c b/src/vulkan/wsi/wsi_common_wa<wbr>yland.c<br>
index 8290170c9ec..1162b92c35f 100644<br>
--- a/src/vulkan/wsi/wsi_common_wa<wbr>yland.c<br>
+++ b/src/vulkan/wsi/wsi_common_wa<wbr>yland.c<br>
@@ -708,7 +708,8 @@ wsi_wl_image_init(struct wsi_wl_swapchain *chain,<br>
 {<br>
    VkResult result;<br>
<br>
-   result = wsi_create_native_image(&chain<wbr>->base, pCreateInfo, &image->base);<br>
+   result = wsi_create_native_image(&chain<wbr>->base, pCreateInfo,<br>
+                                    0, NULL, NULL, &image->base);<br>
    if (result != VK_SUCCESS)<br>
       return result;<br>
<br>
diff --git a/src/vulkan/wsi/wsi_common_x1<wbr>1.c b/src/vulkan/wsi/wsi_common_x1<wbr>1.c<br>
index b42b8234064..2cc7a67c63f 100644<br>
--- a/src/vulkan/wsi/wsi_common_x1<wbr>1.c<br>
+++ b/src/vulkan/wsi/wsi_common_x1<wbr>1.c<br>
@@ -929,7 +929,8 @@ x11_image_init(VkDevice device_h, struct x11_swapchain *chain,<br>
    if (chain->base.use_prime_blit) {<br>
       result = wsi_create_prime_image(&chain-<wbr>>base, pCreateInfo, &image->base);<br>
    } else {<br>
-      result = wsi_create_native_image(&chain<wbr>->base, pCreateInfo, &image->base);<br>
+      result = wsi_create_native_image(&chain<wbr>->base, pCreateInfo,<br>
+                                       0, NULL, NULL, &image->base);<br>
    }<br>
    if (result != VK_SUCCESS)<br>
       return result;<br>
<span class="m_-4827712196147562727HOEnZb"><font color="#888888">--<br>
2.14.3<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div>