On Thu, Feb 15, 2018 at 7:57 AM, Daniel Stone <dani...@collabora.com> wrote:
> From: Jason Ekstrand <ja...@jlekstrand.net>
>
> 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 <dani...@collabora.com>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> ---
> 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_M
> ODIFIER_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];
> + }
> + }
>
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.
> +
> + /* 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
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
