<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Feb 15, 2018 at 7:57 AM, Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>> wrote: <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>> This involves extending our fake extension a bit to allow for additional querying and passing of modifier information. The added bits are intended to look a lot like the draft of VK_EXT_image_drm_format_modifier. Once the extension gets finalized, we'll simply transition all of the structs used in wsi_common to the real extension structs. Reviewed-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>> Signed-off-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>> --- src/vulkan/wsi/wsi_common.c | 164 ++++++++++++++++++++++++++++++++---- src/vulkan/wsi/wsi_common.h | 23 +++++ src/vulkan/wsi/wsi_common_private.h | 3 + src/vulkan/wsi/wsi_common_wayland.c | 3 +- src/vulkan/wsi/wsi_common_x11.c | 3 +- 5 files changed, 177 insertions(+), 19 deletions(-) diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c index c235128e562..edba13a13de 100644 --- a/src/vulkan/wsi/wsi_common.c +++ b/src/vulkan/wsi/wsi_common.c @@ -26,6 +26,8 @@ #include "util/macros.h" #include "vk_util.h" +#include <unistd.h> + VkResult wsi_device_init(struct wsi_device *wsi, VkPhysicalDevice pdevice, @@ -36,6 +38,8 @@ wsi_device_init(struct wsi_device *wsi, memset(wsi, 0, sizeof(*wsi)); + wsi->pdevice = pdevice; + #define WSI_GET_CB(func) \ PFN_vk##func func = (PFN_vk##func)proc_addr(pdevice, "vk" #func) WSI_GET_CB(GetPhysicalDeviceMemoryProperties); @@ -69,6 +73,7 @@ wsi_device_init(struct wsi_device *wsi, WSI_GET_CB(GetImageSubresourceLayout); WSI_GET_CB(GetMemoryFdKHR); WSI_GET_CB(GetPhysicalDeviceFormatProperties); + WSI_GET_CB(GetPhysicalDeviceFormatProperties2KHR); WSI_GET_CB(ResetFences); WSI_GET_CB(QueueSubmit); WSI_GET_CB(WaitForFences); @@ -196,6 +201,9 @@ align_u32(uint32_t v, uint32_t a) VkResult wsi_create_native_image(const struct wsi_swapchain *chain, const VkSwapchainCreateInfoKHR *pCreateInfo, + uint32_t num_modifier_lists, + const uint32_t *num_modifiers, + const uint64_t *const *modifiers, struct wsi_image *image) { const struct wsi_device *wsi = chain->wsi; @@ -205,11 +213,91 @@ wsi_create_native_image(const struct wsi_swapchain *chain, for (int i = 0; i < ARRAY_SIZE(image->fds); i++) image->fds[i] = -1; - const struct wsi_image_create_info image_wsi_info = { + struct wsi_image_create_info image_wsi_info = { .sType = VK_STRUCTURE_TYPE_WSI_IMAGE_CREATE_INFO_MESA, .pNext = NULL, - .scanout = true, }; + + uint32_t image_modifier_count = 0, modifier_prop_count = 0; + struct wsi_format_modifier_properties *modifier_props = NULL; + uint64_t *image_modifiers = NULL; + if (num_modifier_lists == 0) { + /* If we don't have modifiers, fall back to the legacy "scanout" flag */ + image_wsi_info.scanout = true; + } else { + /* The winsys can't request modifiers if we don't support them. */ + assert(wsi->supports_modifiers); + struct wsi_format_modifier_properties_list modifier_props_list = { + .sType = VK_STRUCTURE_TYPE_WSI_FORMAT_MODIFIER_PROPERTIES_LIST_MESA, + .pNext = NULL, + }; + VkFormatProperties2KHR format_props = { + .sType = VK_STRUCTURE_TYPE_FORMAT_PROPERTIES_2_KHR, + .pNext = &modifier_props_list, + }; + wsi->GetPhysicalDeviceFormatProperties2KHR(wsi->pdevice, + pCreateInfo->imageFormat, + &format_props); + assert(modifier_props_list.modifier_count > 0); + modifier_props = vk_alloc(&chain->alloc, + sizeof(*modifier_props) * + modifier_props_list.modifier_count, + 8, + VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); + if (!modifier_props) { + result = VK_ERROR_OUT_OF_HOST_MEMORY; + goto fail; + } + + modifier_props_list.modifier_properties = modifier_props; + wsi->GetPhysicalDeviceFormatProperties2KHR(wsi->pdevice, + pCreateInfo->imageFormat, + &format_props); + modifier_prop_count = modifier_props_list.modifier_count; + + uint32_t max_modifier_count = 0; + for (uint32_t l = 0; l < num_modifier_lists; l++) + max_modifier_count = MAX2(max_modifier_count, num_modifiers[l]); + + image_modifiers = vk_alloc(&chain->alloc, + sizeof(*image_modifiers) * + max_modifier_count, + 8, + VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); + if (!image_modifiers) { + result = VK_ERROR_OUT_OF_HOST_MEMORY; + goto fail; + } + + image_modifier_count = 0; + for (uint32_t l = 0; l < num_modifier_lists; l++) { + /* Walk the modifier lists and construct a list of supported + * modifiers. + */ + for (uint32_t i = 0; i < num_modifiers[l]; i++) { + for (uint32_t j = 0; j < modifier_prop_count; j++) { + if (modifier_props[j].modifier == modifiers[l][i]) + image_modifiers[image_modifier_count++] = modifiers[l][i]; + } + } </blockquote><div> </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. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + /* We only want to take the modifiers from the first list */ + if (image_modifier_count > 0) + break; + } + + if (image_modifier_count > 0) { + image_wsi_info.modifier_count = image_modifier_count; + image_wsi_info.modifiers = image_modifiers; + } else { + /* TODO: Add a proper error here */ + assert(!"Failed to find a supported modifier! This should never " + "happen because LINEAR should always be available"); + result = VK_ERROR_OUT_OF_HOST_MEMORY; + goto fail; + } + } + const VkImageCreateInfo image_info = { .sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO, .pNext = &image_wsi_info, @@ -239,15 +327,6 @@ wsi_create_native_image(const struct wsi_swapchain *chain, VkMemoryRequirements reqs; wsi->GetImageMemoryRequirements(chain->device, image->image, &reqs); - VkSubresourceLayout image_layout; - const VkImageSubresource image_subresource = { - .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT, - .mipLevel = 0, - .arrayLayer = 0, - }; - wsi->GetImageSubresourceLayout(chain->device, image->image, - &image_subresource, &image_layout); - const struct wsi_memory_allocate_info memory_wsi_info = { .sType = VK_STRUCTURE_TYPE_WSI_MEMORY_ALLOCATE_INFO_MESA, .pNext = NULL, @@ -292,16 +371,67 @@ wsi_create_native_image(const struct wsi_swapchain *chain, if (result != VK_SUCCESS) goto fail; - image->drm_modifier = DRM_FORMAT_MOD_INVALID; - image->num_planes = 1; - image->sizes[0] = reqs.size; - image->row_pitches[0] = image_layout.rowPitch; - image->offsets[0] = 0; - image->fds[0] = fd; + if (num_modifier_lists > 0) { + image->drm_modifier = wsi->image_get_modifier(image->image); + assert(image->drm_modifier != DRM_FORMAT_MOD_INVALID); + + for (uint32_t j = 0; j < modifier_prop_count; j++) { + if (modifier_props[j].modifier == image->drm_modifier) { + image->num_planes = modifier_props[j].modifier_plane_count; + break; + } + } + + for (uint32_t p = 0; p < image->num_planes; p++) { + const VkImageSubresource image_subresource = { + .aspectMask = VK_IMAGE_ASPECT_PLANE_0_BIT_KHR << p, + .mipLevel = 0, + .arrayLayer = 0, + }; + VkSubresourceLayout image_layout; + wsi->GetImageSubresourceLayout(chain->device, image->image, + &image_subresource, &image_layout); + image->sizes[p] = image_layout.size; + image->row_pitches[p] = image_layout.rowPitch; + image->offsets[p] = image_layout.offset; + if (p == 0) { + image->fds[p] = fd; + } else { + image->fds[p] = dup(fd); + if (image->fds[p] == -1) { + for (uint32_t i = 0; i < p; i++) + close(image->fds[p]); + + goto fail; + } + } + } + } else { + const VkImageSubresource image_subresource = { + .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT, + .mipLevel = 0, + .arrayLayer = 0, + }; + VkSubresourceLayout image_layout; + wsi->GetImageSubresourceLayout(chain->device, image->image, + &image_subresource, &image_layout); + + image->drm_modifier = DRM_FORMAT_MOD_INVALID; + image->num_planes = 1; + image->sizes[0] = reqs.size; + image->row_pitches[0] = image_layout.rowPitch; + image->offsets[0] = 0; + image->fds[0] = fd; + } + + vk_free(&chain->alloc, modifier_props); + vk_free(&chain->alloc, image_modifiers); return VK_SUCCESS; fail: + vk_free(&chain->alloc, modifier_props); + vk_free(&chain->alloc, image_modifiers); wsi_destroy_image(chain, image); return result; diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h index 3e0d3be1c24..6cf729ba025 100644 --- a/src/vulkan/wsi/wsi_common.h +++ b/src/vulkan/wsi/wsi_common.h @@ -35,11 +35,15 @@ */ #define VK_STRUCTURE_TYPE_WSI_IMAGE_CREATE_INFO_MESA (VkStructureType)1000001002 #define VK_STRUCTURE_TYPE_WSI_MEMORY_ALLOCATE_INFO_MESA (VkStructureType)1000001003 +#define VK_STRUCTURE_TYPE_WSI_FORMAT_MODIFIER_PROPERTIES_LIST_MESA (VkStructureType)1000001004 struct wsi_image_create_info { VkStructureType sType; const void *pNext; bool scanout; + + uint32_t modifier_count; + const uint64_t *modifiers; }; struct wsi_memory_allocate_info { @@ -48,14 +52,32 @@ struct wsi_memory_allocate_info { bool implicit_sync; }; +struct wsi_format_modifier_properties { + uint64_t modifier; + uint32_t modifier_plane_count; +}; + +/* Chain in for vkGetPhysicalDeviceFormatProperties2KHR */ +struct wsi_format_modifier_properties_list { + VkStructureType sType; + const void *pNext; + + uint32_t modifier_count; + struct wsi_format_modifier_properties *modifier_properties; +}; + struct wsi_interface; #define VK_ICD_WSI_PLATFORM_MAX 5 struct wsi_device { + VkPhysicalDevice pdevice; VkPhysicalDeviceMemoryProperties memory_props; uint32_t queue_family_count; + bool supports_modifiers; + uint64_t (*image_get_modifier)(VkImage image); + #define WSI_CB(cb) PFN_vk##cb cb WSI_CB(AllocateMemory); WSI_CB(AllocateCommandBuffers); @@ -79,6 +101,7 @@ struct wsi_device { WSI_CB(GetImageSubresourceLayout); WSI_CB(GetMemoryFdKHR); WSI_CB(GetPhysicalDeviceFormatProperties); + WSI_CB(GetPhysicalDeviceFormatProperties2KHR); WSI_CB(ResetFences); WSI_CB(QueueSubmit); WSI_CB(WaitForFences); diff --git a/src/vulkan/wsi/wsi_common_private.h b/src/vulkan/wsi/wsi_common_private.h index 781e84635f9..b608119b969 100644 --- a/src/vulkan/wsi/wsi_common_private.h +++ b/src/vulkan/wsi/wsi_common_private.h @@ -81,6 +81,9 @@ void wsi_swapchain_finish(struct wsi_swapchain *chain); VkResult wsi_create_native_image(const struct wsi_swapchain *chain, const VkSwapchainCreateInfoKHR *pCreateInfo, + uint32_t num_modifier_lists, + const uint32_t *num_modifiers, + const uint64_t *const *modifiers, struct wsi_image *image); VkResult diff --git a/src/vulkan/wsi/wsi_common_wayland.c b/src/vulkan/wsi/wsi_common_wayland.c index 8290170c9ec..1162b92c35f 100644 --- a/src/vulkan/wsi/wsi_common_wayland.c +++ b/src/vulkan/wsi/wsi_common_wayland.c @@ -708,7 +708,8 @@ wsi_wl_image_init(struct wsi_wl_swapchain *chain, { VkResult result; - result = wsi_create_native_image(&chain->base, pCreateInfo, &image->base); + result = wsi_create_native_image(&chain->base, pCreateInfo, + 0, NULL, NULL, &image->base); if (result != VK_SUCCESS) return result; diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c index b42b8234064..2cc7a67c63f 100644 --- a/src/vulkan/wsi/wsi_common_x11.c +++ b/src/vulkan/wsi/wsi_common_x11.c @@ -929,7 +929,8 @@ x11_image_init(VkDevice device_h, struct x11_swapchain *chain, if (chain->base.use_prime_blit) { result = wsi_create_prime_image(&chain->base, pCreateInfo, &image->base); } else { - result = wsi_create_native_image(&chain->base, pCreateInfo, &image->base); + result = wsi_create_native_image(&chain->base, pCreateInfo, + 0, NULL, NULL, &image->base); } if (result != VK_SUCCESS) return result; -- 2.14.3 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>