Re: [Mesa-dev] [PATCH v2] wsi: allow to override the present mode with MESA_VK_WSI_PRESENT_MODE

2019-04-09 Thread Samuel Pitoiset 


On 4/9/19 3:52 PM, Lionel Landwerlin wrote:

On 09/04/2019 14:31, Samuel Pitoiset wrote:

This is common to all Vulkan drivers and all WSI.

v2: - store the override in wsi_device_init()
 - do not abort when an invalid value is detected
 - check supported present modes

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107391
Signed-off-by: Samuel Pitoiset 
---
  src/vulkan/wsi/wsi_common.c | 67 +
  src/vulkan/wsi/wsi_common.h |  1 +
  src/vulkan/wsi/wsi_common_display.c |  2 +-
  src/vulkan/wsi/wsi_common_private.h |  4 ++
  src/vulkan/wsi/wsi_common_wayland.c |  2 +-
  src/vulkan/wsi/wsi_common_x11.c |  2 +-
  6 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
index 3cba0a4b06e..1d70b04e18b 100644
--- a/src/vulkan/wsi/wsi_common.c
+++ b/src/vulkan/wsi/wsi_common.c
@@ -29,6 +29,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 
    VkResult
  wsi_device_init(struct wsi_device *wsi,
@@ -112,6 +114,8 @@ wsi_device_init(struct wsi_device *wsi,
    goto fail;
  #endif
  +   wsi->override_present_mode = getenv("MESA_VK_WSI_PRESENT_MODE");
+
 return VK_SUCCESS;
    fail:
@@ -202,6 +206,69 @@ fail:
 return result;
  }
  +static bool
+wsi_swapchain_is_present_mode_supported(struct wsi_device *wsi,
+    const 
VkSwapchainCreateInfoKHR *pCreateInfo,

+    VkPresentModeKHR mode)
+{
+  ICD_FROM_HANDLE(VkIcdSurfaceBase, surface, pCreateInfo->surface);
+  struct wsi_interface *iface = wsi->wsi[surface->platform];
+  VkPresentModeKHR *present_modes;
+  uint32_t present_mode_count;
+  bool supported = false;
+  VkResult result;
+
+  result = iface->get_present_modes(surface, 
_mode_count, NULL);

+  if (result != VK_SUCCESS)
+ return supported;
+
+  present_modes = malloc(present_mode_count * 
sizeof(*present_modes));

+  if (!present_modes)
+ return supported;
+
+  result = iface->get_present_modes(surface, _mode_count,
+    present_modes);
+  if (result != VK_SUCCESS)
+ goto fail;
+
+  for (uint32_t i = 0; i < present_mode_count; i++) {
+ if (present_modes[i] == mode)
+    supported = true;
+  }
+
+fail:
+  free(present_modes);
+  return supported;
+}
+
+enum VkPresentModeKHR
+wsi_swapchain_get_present_mode(struct wsi_device *wsi,
+   const VkSwapchainCreateInfoKHR 
*pCreateInfo)

+{
+   VkPresentModeKHR mode;
+
+   if (!wsi->override_present_mode)
+  return pCreateInfo->presentMode;
+
+   if (!strcmp(wsi->override_present_mode, "fifo")) {
+  mode = VK_PRESENT_MODE_FIFO_KHR;
+   } else if (!strcmp(wsi->override_present_mode, "mailbox")) {
+  mode = VK_PRESENT_MODE_MAILBOX_KHR;
+   } else if (!strcmp(wsi->override_present_mode, "immediate")) {
+  mode = VK_PRESENT_MODE_IMMEDIATE_KHR;
+   } else {
+  fprintf(stderr, "Invalid MESA_VK_WSI_PRESENT_MODE value!\n");
+  return pCreateInfo->presentMode;
+   }



I would put the conversion of string -> VkPresentModeKHR in the init 
function.


You can use VK_PRESENT_MODE_MAX_ENUM_KHR as an unset value.


I was going to do that, and I changed my mind. :)

I will send v3 soon.




Otherwise looks good, thanks for the update :)


-Lionel



+
+   if (!wsi_swapchain_is_present_mode_supported(wsi, pCreateInfo, 
mode)) {

+  fprintf(stderr, "Unsupported MESA_VK_WSI_PRESENT_MODE value!\n");
+  return pCreateInfo->presentMode;
+   }
+
+   return mode;
+}
+
  void
  wsi_swapchain_finish(struct wsi_swapchain *chain)
  {
diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h
index e693e2be425..5ca376b4c49 100644
--- a/src/vulkan/wsi/wsi_common.h
+++ b/src/vulkan/wsi/wsi_common.h
@@ -101,6 +101,7 @@ struct wsi_device {
   bool supports_modifiers;
 uint32_t maxImageDimension2D;
+   const char *override_present_mode;
   uint64_t (*image_get_modifier)(VkImage image);
  diff --git a/src/vulkan/wsi/wsi_common_display.c 
b/src/vulkan/wsi/wsi_common_display.c

index 09c18315623..74ed36ed646 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -1757,7 +1757,7 @@ wsi_display_surface_create_swapchain(
 chain->base.get_wsi_image = wsi_display_get_wsi_image;
 chain->base.acquire_next_image = wsi_display_acquire_next_image;
 chain->base.queue_present = wsi_display_queue_present;
-   chain->base.present_mode = create_info->presentMode;
+   chain->base.present_mode = 
wsi_swapchain_get_present_mode(wsi_device, create_info);

 chain->base.image_count = num_images;
   chain->wsi = wsi;
diff --git a/src/vulkan/wsi/wsi_common_private.h 
b/src/vulkan/wsi/wsi_common_private.h

index a6f49fc3124..6d8f4b7a0e4 100644
--- a/src/vulkan/wsi/wsi_common_private.h
+++ b/src/vulkan/wsi/wsi_common_private.h
@@

Re: [Mesa-dev] [PATCH v2] wsi: allow to override the present mode with MESA_VK_WSI_PRESENT_MODE

2019-04-09 Thread Lionel Landwerlin 

On 09/04/2019 14:31, Samuel Pitoiset wrote:

This is common to all Vulkan drivers and all WSI.

v2: - store the override in wsi_device_init()
 - do not abort when an invalid value is detected
 - check supported present modes

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107391
Signed-off-by: Samuel Pitoiset 
---
  src/vulkan/wsi/wsi_common.c | 67 +
  src/vulkan/wsi/wsi_common.h |  1 +
  src/vulkan/wsi/wsi_common_display.c |  2 +-
  src/vulkan/wsi/wsi_common_private.h |  4 ++
  src/vulkan/wsi/wsi_common_wayland.c |  2 +-
  src/vulkan/wsi/wsi_common_x11.c |  2 +-
  6 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
index 3cba0a4b06e..1d70b04e18b 100644
--- a/src/vulkan/wsi/wsi_common.c
+++ b/src/vulkan/wsi/wsi_common.c
@@ -29,6 +29,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 
  
  VkResult

  wsi_device_init(struct wsi_device *wsi,
@@ -112,6 +114,8 @@ wsi_device_init(struct wsi_device *wsi,
goto fail;
  #endif
  
+   wsi->override_present_mode = getenv("MESA_VK_WSI_PRESENT_MODE");

+
 return VK_SUCCESS;
  
  fail:

@@ -202,6 +206,69 @@ fail:
 return result;
  }
  
+static bool

+wsi_swapchain_is_present_mode_supported(struct wsi_device *wsi,
+const VkSwapchainCreateInfoKHR 
*pCreateInfo,
+VkPresentModeKHR mode)
+{
+  ICD_FROM_HANDLE(VkIcdSurfaceBase, surface, pCreateInfo->surface);
+  struct wsi_interface *iface = wsi->wsi[surface->platform];
+  VkPresentModeKHR *present_modes;
+  uint32_t present_mode_count;
+  bool supported = false;
+  VkResult result;
+
+  result = iface->get_present_modes(surface, _mode_count, NULL);
+  if (result != VK_SUCCESS)
+ return supported;
+
+  present_modes = malloc(present_mode_count * sizeof(*present_modes));
+  if (!present_modes)
+ return supported;
+
+  result = iface->get_present_modes(surface, _mode_count,
+present_modes);
+  if (result != VK_SUCCESS)
+ goto fail;
+
+  for (uint32_t i = 0; i < present_mode_count; i++) {
+ if (present_modes[i] == mode)
+supported = true;
+  }
+
+fail:
+  free(present_modes);
+  return supported;
+}
+
+enum VkPresentModeKHR
+wsi_swapchain_get_present_mode(struct wsi_device *wsi,
+   const VkSwapchainCreateInfoKHR *pCreateInfo)
+{
+   VkPresentModeKHR mode;
+
+   if (!wsi->override_present_mode)
+  return pCreateInfo->presentMode;
+
+   if (!strcmp(wsi->override_present_mode, "fifo")) {
+  mode = VK_PRESENT_MODE_FIFO_KHR;
+   } else if (!strcmp(wsi->override_present_mode, "mailbox")) {
+  mode = VK_PRESENT_MODE_MAILBOX_KHR;
+   } else if (!strcmp(wsi->override_present_mode, "immediate")) {
+  mode = VK_PRESENT_MODE_IMMEDIATE_KHR;
+   } else {
+  fprintf(stderr, "Invalid MESA_VK_WSI_PRESENT_MODE value!\n");
+  return pCreateInfo->presentMode;
+   }



I would put the conversion of string -> VkPresentModeKHR in the init 
function.


You can use VK_PRESENT_MODE_MAX_ENUM_KHR as an unset value.


Otherwise looks good, thanks for the update :)


-Lionel



+
+   if (!wsi_swapchain_is_present_mode_supported(wsi, pCreateInfo, mode)) {
+  fprintf(stderr, "Unsupported MESA_VK_WSI_PRESENT_MODE value!\n");
+  return pCreateInfo->presentMode;
+   }
+
+   return mode;
+}
+
  void
  wsi_swapchain_finish(struct wsi_swapchain *chain)
  {
diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h
index e693e2be425..5ca376b4c49 100644
--- a/src/vulkan/wsi/wsi_common.h
+++ b/src/vulkan/wsi/wsi_common.h
@@ -101,6 +101,7 @@ struct wsi_device {
  
 bool supports_modifiers;

 uint32_t maxImageDimension2D;
+   const char *override_present_mode;
  
 uint64_t (*image_get_modifier)(VkImage image);
  
diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c

index 09c18315623..74ed36ed646 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -1757,7 +1757,7 @@ wsi_display_surface_create_swapchain(
 chain->base.get_wsi_image = wsi_display_get_wsi_image;
 chain->base.acquire_next_image = wsi_display_acquire_next_image;
 chain->base.queue_present = wsi_display_queue_present;
-   chain->base.present_mode = create_info->presentMode;
+   chain->base.present_mode = wsi_swapchain_get_present_mode(wsi_device, 
create_info);
 chain->base.image_count = num_images;
  
 chain->wsi = wsi;

diff --git a/src/vulkan/wsi/wsi_common_private.h 
b/src/vulkan/wsi/wsi_common_private.h
index a6f49fc3124..6d8f4b7a0e4 100644
--- a/src/vulkan/wsi/wsi_common_private.h
+++ b/src/vulkan/wsi/wsi_common_private.h
@@ -79,6 +79,10 @@ wsi_swapchain_init(const struct wsi_device *wsi,
 const VkSwapchainCreateInfoKHR