[6/7] anv: add VK_EXT_display_control to anv driver [v2]
diff mbox

Message ID 20180615025256.10657-7-keithp@keithp.com
State New
Headers show

Commit Message

Keith Packard June 15, 2018, 2:52 a.m. UTC
This extension provides fences and frame count information to direct
display contexts. It uses new kernel ioctls to provide 64-bits of
vblank sequence and nanosecond resolution.

v2:	Adopt Jason Ekstrand's coding conventions

	Declare variables at first use, eliminate extra whitespace between
	types and names. Wrap lines to 80 columns.

	Add extension to list in alphabetical order

	Suggested-by: Jason Ekstrand <jason.ekstrand@intel.com>

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 src/intel/vulkan/anv_extensions.py |  1 +
 src/intel/vulkan/anv_private.h     |  4 ++
 src/intel/vulkan/anv_queue.c       | 22 +++++++
 src/intel/vulkan/anv_wsi_display.c | 97 ++++++++++++++++++++++++++++++
 4 files changed, 124 insertions(+)

Comments

Jason Ekstrand June 20, 2018, 12:33 a.m. UTC | #1
On Thu, Jun 14, 2018 at 7:52 PM, Keith Packard <keithp@keithp.com> wrote:

> This extension provides fences and frame count information to direct
> display contexts. It uses new kernel ioctls to provide 64-bits of
> vblank sequence and nanosecond resolution.
>
> v2:     Adopt Jason Ekstrand's coding conventions
>
>         Declare variables at first use, eliminate extra whitespace between
>         types and names. Wrap lines to 80 columns.
>
>         Add extension to list in alphabetical order
>
>         Suggested-by: Jason Ekstrand <jason.ekstrand@intel.com>
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  src/intel/vulkan/anv_extensions.py |  1 +
>  src/intel/vulkan/anv_private.h     |  4 ++
>  src/intel/vulkan/anv_queue.c       | 22 +++++++
>  src/intel/vulkan/anv_wsi_display.c | 97 ++++++++++++++++++++++++++++++
>  4 files changed, 124 insertions(+)
>
> diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_
> extensions.py
> index 68e545a40f8..8c010e9280b 100644
> --- a/src/intel/vulkan/anv_extensions.py
> +++ b/src/intel/vulkan/anv_extensions.py
> @@ -111,6 +111,7 @@ EXTENSIONS = [
>      Extension('VK_EXT_acquire_xlib_display',              1,
> 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
>      Extension('VK_EXT_debug_report',                      8, True),
>      Extension('VK_EXT_direct_mode_display',               1,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
> +    Extension('VK_EXT_display_control',                   1,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_display_surface_counter',           1,
> 'VK_USE_PLATFORM_DISPLAY_KHR'),
>      Extension('VK_EXT_external_memory_dma_buf',           1, True),
>      Extension('VK_EXT_global_priority',                   1,
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index fb91bc33046..c81885979ad 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -2133,6 +2133,7 @@ enum anv_fence_type {
>     ANV_FENCE_TYPE_NONE = 0,
>     ANV_FENCE_TYPE_BO,
>     ANV_FENCE_TYPE_SYNCOBJ,
> +   ANV_FENCE_TYPE_WSI,
>  };
>
>  enum anv_bo_fence_state {
> @@ -2167,6 +2168,9 @@ struct anv_fence_impl {
>
>        /** DRM syncobj handle for syncobj-based fences */
>        uint32_t syncobj;
> +
> +      /** WSI fence */
> +      struct wsi_fence *fence_wsi;
>     };
>  };
>
> diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
> index 8df99c84549..073e65acf5e 100644
> --- a/src/intel/vulkan/anv_queue.c
> +++ b/src/intel/vulkan/anv_queue.c
> @@ -324,6 +324,10 @@ anv_fence_impl_cleanup(struct anv_device *device,
>        anv_gem_syncobj_destroy(device, impl->syncobj);
>        break;
>
> +   case ANV_FENCE_TYPE_WSI:
> +      impl->fence_wsi->destroy(impl->fence_wsi);
> +      break;
> +
>     default:
>        unreachable("Invalid fence type");
>     }
> @@ -672,6 +676,21 @@ done:
>     return result;
>  }
>
> +static VkResult
> +anv_wait_for_wsi_fence(struct anv_device *device,
> +                       const VkFence _fence,
> +                       uint64_t abs_timeout)
> +{
> +   ANV_FROM_HANDLE(anv_fence, fence, _fence);
> +
> +   struct anv_fence_impl *impl = &fence->permanent;
> +   bool expired = impl->fence_wsi->wait(impl->fence_wsi, true,
> abs_timeout);
> +
> +   if (!expired)
> +      return VK_TIMEOUT;
> +   return VK_SUCCESS;
> +}
> +
>  static VkResult
>  anv_wait_for_fences(struct anv_device *device,
>                      uint32_t fenceCount,
> @@ -694,6 +713,9 @@ anv_wait_for_fences(struct anv_device *device,
>              result = anv_wait_for_syncobj_fences(device, 1, &pFences[i],
>                                                   true, abs_timeout);
>              break;
> +         case ANV_FENCE_TYPE_WSI:
> +            result = anv_wait_for_wsi_fence(device, pFences[i],
> abs_timeout);
> +            break;
>           case ANV_FENCE_TYPE_NONE:
>              result = VK_SUCCESS;
>              break;
> diff --git a/src/intel/vulkan/anv_wsi_display.c
> b/src/intel/vulkan/anv_wsi_display.c
> index f749a8d98f7..cd736bcdd74 100644
> --- a/src/intel/vulkan/anv_wsi_display.c
> +++ b/src/intel/vulkan/anv_wsi_display.c
> @@ -168,3 +168,100 @@ anv_GetRandROutputDisplayEXT(VkPhysicalDevice
> physical_device,
>                                         display);
>  }
>  #endif /* VK_USE_PLATFORM_XLIB_XRANDR_EXT */
> +
> +/* VK_EXT_display_control */
> +
> +VkResult
> +anv_DisplayPowerControlEXT(VkDevice                    _device,
> +                            VkDisplayKHR                display,
> +                            const VkDisplayPowerInfoEXT
> *display_power_info)
> +{
> +   ANV_FROM_HANDLE(anv_device, device, _device);
> +
> +   return wsi_display_power_control(
> +      _device, &device->instance->physicalDevice.wsi_device,
> +      display, display_power_info);
> +}
> +
> +VkResult
> +anv_RegisterDeviceEventEXT(VkDevice _device,
> +                            const VkDeviceEventInfoEXT *device_event_info,
> +                            const VkAllocationCallbacks *allocator,
> +                            VkFence *_fence)
> +{
> +   ANV_FROM_HANDLE(anv_device, device, _device);
> +   const VkAllocationCallbacks *alloc;
> +   struct anv_fence *fence;
> +   VkResult ret;
> +
> +   if (allocator)
> +     alloc = allocator;
> +   else
> +     alloc = &device->instance->alloc;
>

This is what vk_alloc2 is for. :-)


> +
> +   fence = vk_alloc(alloc, sizeof (*fence), 8,
> +                    VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> +   if (!fence)
> +      return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> +
> +   fence->permanent.type = ANV_FENCE_TYPE_WSI;
> +
> +   ret = wsi_register_device_event(_device,
> +                                   &device->instance->
> physicalDevice.wsi_device,
> +                                   device_event_info,
> +                                   alloc,
> +                                   &fence->permanent.fence_wsi);
> +   if (ret == VK_SUCCESS)
> +      *_fence = anv_fence_to_handle(fence);
> +   else
> +      vk_free(alloc, fence);


And vk_free2


> +   return ret;
> +}
> +
> +VkResult
> +anv_RegisterDisplayEventEXT(VkDevice _device,
> +                             VkDisplayKHR display,
> +                             const VkDisplayEventInfoEXT
> *display_event_info,
> +                             const VkAllocationCallbacks *allocator,
> +                             VkFence *_fence)
> +{
> +   ANV_FROM_HANDLE(anv_device, device, _device);
> +   const VkAllocationCallbacks *alloc;
> +   struct anv_fence *fence;
> +   VkResult ret;
> +
> +   if (allocator)
> +     alloc = allocator;
> +   else
> +     alloc = &device->instance->alloc;
>

This isn't needed if you're using vk_alloc2


> +
> +   fence = vk_zalloc2(&device->alloc, allocator, sizeof (*fence), 8,
> +                      VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
>

Above you used vk_alloc and here you're using vk_zalloc.  Mind picking
one?  I don't think zalloc is needed but it doesn't hurt so I don't care
which.


> +   if (!fence)
> +      return VK_ERROR_OUT_OF_HOST_MEMORY;
> +
> +   fence->permanent.type = ANV_FENCE_TYPE_WSI;
> +
> +   ret = wsi_register_display_event(
> +      _device, &device->instance->physicalDevice.wsi_device,
> +      display, display_event_info, alloc, &(fence->permanent.fence_wsi));
>

Indentation could be consistent between the two functions you add.  I don't
care which style.


> +
> +   if (ret == VK_SUCCESS)
> +      *_fence = anv_fence_to_handle(fence);
> +   else
> +      vk_free(alloc, fence);
>

vk_free2?


> +   return ret;
> +}
> +
> +VkResult
> +anv_GetSwapchainCounterEXT(VkDevice _device,
> +                            VkSwapchainKHR swapchain,
> +                            VkSurfaceCounterFlagBitsEXT flag_bits,
> +                            uint64_t *value)
> +{
> +   ANV_FROM_HANDLE(anv_device, device, _device);
> +
> +   return wsi_get_swapchain_counter(
> +      _device, &device->instance->physicalDevice.wsi_device,
> +      swapchain, flag_bits, value);
> +}
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Keith Packard June 20, 2018, 5:31 a.m. UTC | #2
Jason Ekstrand <jason@jlekstrand.net> writes:

>> +   if (allocator)
>> +     alloc = allocator;
>> +   else
>> +     alloc = &device->instance->alloc;
>>
>
> This is what vk_alloc2 is for. :-)
...
> And vk_free2
...
> This isn't needed if you're using vk_alloc2

Yeah, but I need to pass the allocator down to the wsi common code, and
that doesn't have any way to touch the device driver allocator
pointer. I bet I'm just missing something here. Help?

>> +
>> +   fence = vk_zalloc2(&device->alloc, allocator, sizeof (*fence), 8,
>> +                      VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
>>
>
> Above you used vk_alloc and here you're using vk_zalloc.  Mind picking
> one?  I don't think zalloc is needed but it doesn't hurt so I don't care
> which.

Thanks. Existing code is using zalloc for fences; I'll use that everywhere.

> Indentation could be consistent between the two functions you add.  I don't
> care which style.

Sure; these function names are kinda long. I've wrapped the first call
after the (

> vk_free2?

I've had to compute 'alloc' to pass into wsi_common; I figured I might
as well use it.
Jason Ekstrand June 20, 2018, 5:13 p.m. UTC | #3
On Tue, Jun 19, 2018 at 10:31 PM, Keith Packard <keithp@keithp.com> wrote:

> Jason Ekstrand <jason@jlekstrand.net> writes:
>
> >> +   if (allocator)
> >> +     alloc = allocator;
> >> +   else
> >> +     alloc = &device->instance->alloc;
> >>
> >
> > This is what vk_alloc2 is for. :-)
> ...
> > And vk_free2
> ...
> > This isn't needed if you're using vk_alloc2
>
> Yeah, but I need to pass the allocator down to the wsi common code, and
> that doesn't have any way to touch the device driver allocator
> pointer. I bet I'm just missing something here. Help?
>

I believe that the WSI common code should be capable of fishing the
instance allocator out of the wsi_display so we need only pass the
allocator argument unmodified through to the core WSI code.  Make sense?
Yeah, Vulkan allocator fishing is weird.

--Jason
Keith Packard June 20, 2018, 10:45 p.m. UTC | #4
Jason Ekstrand <jason@jlekstrand.net> writes:

> I believe that the WSI common code should be capable of fishing the
> instance allocator out of the wsi_display so we need only pass the
> allocator argument unmodified through to the core WSI code.  Make sense?

Thanks, I think I've sorted it out. I've pushed an updated series with
this change.

> Yeah, Vulkan allocator fishing is weird.

Allowing custom allocators is one of the bad parts of the Vulkan spec;
it will "never" get used, and the chances of it working correctly in any
driver are pretty small. But, we do what we can to implement it.

Patch
diff mbox

diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_extensions.py
index 68e545a40f8..8c010e9280b 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -111,6 +111,7 @@  EXTENSIONS = [
     Extension('VK_EXT_acquire_xlib_display',              1, 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'),
     Extension('VK_EXT_debug_report',                      8, True),
     Extension('VK_EXT_direct_mode_display',               1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
+    Extension('VK_EXT_display_control',                   1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
     Extension('VK_EXT_display_surface_counter',           1, 'VK_USE_PLATFORM_DISPLAY_KHR'),
     Extension('VK_EXT_external_memory_dma_buf',           1, True),
     Extension('VK_EXT_global_priority',                   1,
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index fb91bc33046..c81885979ad 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -2133,6 +2133,7 @@  enum anv_fence_type {
    ANV_FENCE_TYPE_NONE = 0,
    ANV_FENCE_TYPE_BO,
    ANV_FENCE_TYPE_SYNCOBJ,
+   ANV_FENCE_TYPE_WSI,
 };
 
 enum anv_bo_fence_state {
@@ -2167,6 +2168,9 @@  struct anv_fence_impl {
 
       /** DRM syncobj handle for syncobj-based fences */
       uint32_t syncobj;
+
+      /** WSI fence */
+      struct wsi_fence *fence_wsi;
    };
 };
 
diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
index 8df99c84549..073e65acf5e 100644
--- a/src/intel/vulkan/anv_queue.c
+++ b/src/intel/vulkan/anv_queue.c
@@ -324,6 +324,10 @@  anv_fence_impl_cleanup(struct anv_device *device,
       anv_gem_syncobj_destroy(device, impl->syncobj);
       break;
 
+   case ANV_FENCE_TYPE_WSI:
+      impl->fence_wsi->destroy(impl->fence_wsi);
+      break;
+
    default:
       unreachable("Invalid fence type");
    }
@@ -672,6 +676,21 @@  done:
    return result;
 }
 
+static VkResult
+anv_wait_for_wsi_fence(struct anv_device *device,
+                       const VkFence _fence,
+                       uint64_t abs_timeout)
+{
+   ANV_FROM_HANDLE(anv_fence, fence, _fence);
+
+   struct anv_fence_impl *impl = &fence->permanent;
+   bool expired = impl->fence_wsi->wait(impl->fence_wsi, true, abs_timeout);
+
+   if (!expired)
+      return VK_TIMEOUT;
+   return VK_SUCCESS;
+}
+
 static VkResult
 anv_wait_for_fences(struct anv_device *device,
                     uint32_t fenceCount,
@@ -694,6 +713,9 @@  anv_wait_for_fences(struct anv_device *device,
             result = anv_wait_for_syncobj_fences(device, 1, &pFences[i],
                                                  true, abs_timeout);
             break;
+         case ANV_FENCE_TYPE_WSI:
+            result = anv_wait_for_wsi_fence(device, pFences[i], abs_timeout);
+            break;
          case ANV_FENCE_TYPE_NONE:
             result = VK_SUCCESS;
             break;
diff --git a/src/intel/vulkan/anv_wsi_display.c b/src/intel/vulkan/anv_wsi_display.c
index f749a8d98f7..cd736bcdd74 100644
--- a/src/intel/vulkan/anv_wsi_display.c
+++ b/src/intel/vulkan/anv_wsi_display.c
@@ -168,3 +168,100 @@  anv_GetRandROutputDisplayEXT(VkPhysicalDevice  physical_device,
                                        display);
 }
 #endif /* VK_USE_PLATFORM_XLIB_XRANDR_EXT */
+
+/* VK_EXT_display_control */
+
+VkResult
+anv_DisplayPowerControlEXT(VkDevice                    _device,
+                            VkDisplayKHR                display,
+                            const VkDisplayPowerInfoEXT *display_power_info)
+{
+   ANV_FROM_HANDLE(anv_device, device, _device);
+
+   return wsi_display_power_control(
+      _device, &device->instance->physicalDevice.wsi_device,
+      display, display_power_info);
+}
+
+VkResult
+anv_RegisterDeviceEventEXT(VkDevice _device,
+                            const VkDeviceEventInfoEXT *device_event_info,
+                            const VkAllocationCallbacks *allocator,
+                            VkFence *_fence)
+{
+   ANV_FROM_HANDLE(anv_device, device, _device);
+   const VkAllocationCallbacks *alloc;
+   struct anv_fence *fence;
+   VkResult ret;
+
+   if (allocator)
+     alloc = allocator;
+   else
+     alloc = &device->instance->alloc;
+
+   fence = vk_alloc(alloc, sizeof (*fence), 8,
+                    VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+   if (!fence)
+      return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
+
+   fence->permanent.type = ANV_FENCE_TYPE_WSI;
+
+   ret = wsi_register_device_event(_device,
+                                   &device->instance->physicalDevice.wsi_device,
+                                   device_event_info,
+                                   alloc,
+                                   &fence->permanent.fence_wsi);
+   if (ret == VK_SUCCESS)
+      *_fence = anv_fence_to_handle(fence);
+   else
+      vk_free(alloc, fence);
+   return ret;
+}
+
+VkResult
+anv_RegisterDisplayEventEXT(VkDevice _device,
+                             VkDisplayKHR display,
+                             const VkDisplayEventInfoEXT *display_event_info,
+                             const VkAllocationCallbacks *allocator,
+                             VkFence *_fence)
+{
+   ANV_FROM_HANDLE(anv_device, device, _device);
+   const VkAllocationCallbacks *alloc;
+   struct anv_fence *fence;
+   VkResult ret;
+
+   if (allocator)
+     alloc = allocator;
+   else
+     alloc = &device->instance->alloc;
+
+   fence = vk_zalloc2(&device->alloc, allocator, sizeof (*fence), 8,
+                      VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+   if (!fence)
+      return VK_ERROR_OUT_OF_HOST_MEMORY;
+
+   fence->permanent.type = ANV_FENCE_TYPE_WSI;
+
+   ret = wsi_register_display_event(
+      _device, &device->instance->physicalDevice.wsi_device,
+      display, display_event_info, alloc, &(fence->permanent.fence_wsi));
+
+   if (ret == VK_SUCCESS)
+      *_fence = anv_fence_to_handle(fence);
+   else
+      vk_free(alloc, fence);
+   return ret;
+}
+
+VkResult
+anv_GetSwapchainCounterEXT(VkDevice _device,
+                            VkSwapchainKHR swapchain,
+                            VkSurfaceCounterFlagBitsEXT flag_bits,
+                            uint64_t *value)
+{
+   ANV_FROM_HANDLE(anv_device, device, _device);
+
+   return wsi_get_swapchain_counter(
+      _device, &device->instance->physicalDevice.wsi_device,
+      swapchain, flag_bits, value);
+}