[Mesa-dev] [PATCH mesa 2/4] vulkan: add VK_EXT_display_control [v8]
Jason Ekstrand
jason at jlekstrand.net
Thu Jun 21 02:30:28 UTC 2018
On June 20, 2018 15:53:04 Keith Packard <keithp at 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: Remove DRM_CRTC_SEQUENCE_FIRST_PIXEL_OUT flag. This has
> been removed from the proposed kernel API.
>
> Add NULL parameter to drmCrtcQueueSequence ioctl as we
> don't care what sequence the event was actually queued to.
>
> v3: Adapt to pthread clock switch to MONOTONIC
>
> v4: Fix scope for wsi_display_mode andwsi_display_connector allocs
>
> Suggested-by: Jason Ekstrand <jason at jlekstrand.net>
>
> v5: Adopt Jason Ekstrand's coding conventions
>
> Declare variables at first use, eliminate extra whitespace between
> types and names. Wrap lines to 80 columns.
>
> Use wsi_rel_to_abs_time helper function to convert relative
> timeouts to absolute timeouts without causing overflow.
>
> Suggested-by: Jason Ekstrand <jason.ekstrand at intel.com>
>
> v6:
> Change WSI fence wait function to return VkResult instead of
> bool. This makes the meaning of the return value easier to
> understand, and allows for the indication of failure.
>
> Also change the WSI fence wait function to take only absolute
> timeouts and not provide an option for a relative timeout. No
> users wanted relative timeouts, and it's simpler if that option
> isn't available.
>
> Terminate the DPMS property loop once we've found the property.
>
> Assert that the fence hasn't already been destroyed in
> wsi_display_fence_destroy.
>
> Rearrange the event handler function order in the file to place
> routines in an easier to find order.
>
> Suggested-by: Jason Ekstrand <jason.ekstrand at intel.com>
>
> v7:
> Adapt to API changes for surface_get_capabilities
>
> v8:
> Use wsi->alloc in register_display_event so that callers
> don't have to dig out an allocator for us.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
> src/vulkan/wsi/wsi_common.h | 10 +
> src/vulkan/wsi/wsi_common_display.c | 329 +++++++++++++++++++++++++++-
> src/vulkan/wsi/wsi_common_display.h | 29 +++
> 3 files changed, 366 insertions(+), 2 deletions(-)
>
> diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h
> index 07d5e8353b0..33e4f849ac9 100644
> --- a/src/vulkan/wsi/wsi_common.h
> +++ b/src/vulkan/wsi/wsi_common.h
> @@ -73,6 +73,16 @@ struct wsi_surface_supported_counters {
> const void *pNext;
>
> VkSurfaceCounterFlagsEXT supported_surface_counters;
> +
> +};
> +
> +struct wsi_fence {
> + VkDevice device;
> + const struct wsi_device *wsi_device;
> + VkDisplayKHR display;
> + const VkAllocationCallbacks *alloc;
> + VkResult (*wait)(struct wsi_fence *fence, uint64_t
> abs_timeout);
> + void (*destroy)(struct wsi_fence *fence);
> };
>
> struct wsi_interface;
> diff --git a/src/vulkan/wsi/wsi_common_display.c
> b/src/vulkan/wsi/wsi_common_display.c
> index 01150ffbb1b..c786d8befe5 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -79,6 +79,7 @@ typedef struct wsi_display_connector {
> struct list_head display_modes;
> wsi_display_mode *current_mode;
> drmModeModeInfo current_drm_mode;
> + uint32_t dpms_property;
> #ifdef VK_USE_PLATFORM_XLIB_XRANDR_EXT
> xcb_randr_output_t output;
> #endif
> @@ -132,6 +133,15 @@ struct wsi_display_swapchain {
> struct wsi_display_image images[0];
> };
>
> +struct wsi_display_fence {
> + struct wsi_fence base;
> + bool event_received;
> + bool destroyed;
> + uint64_t sequence;
> +};
> +
> +static uint64_t fence_sequence;
> +
> ICD_DEFINE_NONDISP_HANDLE_CASTS(wsi_display_mode, VkDisplayModeKHR)
> ICD_DEFINE_NONDISP_HANDLE_CASTS(wsi_display_connector, VkDisplayKHR)
>
> @@ -307,6 +317,19 @@ wsi_display_get_connector(struct wsi_device *wsi_device,
>
> connector->connected = drm_connector->connection != DRM_MODE_DISCONNECTED;
>
> + /* Look for a DPMS property if we haven't already found one */
> + for (int p = 0; connector->dpms_property == 0 && p <
> drm_connector->count_props; p++) {
I'm guessing this is well over 80 characters
> + drmModePropertyPtr prop = drmModeGetProperty(wsi->fd,
> + drm_connector->props[p]);
> + if (!prop)
> + continue;
> + if (prop->flags & DRM_MODE_PROP_ENUM) {
> + if (!strcmp(prop->name, "DPMS"))
> + connector->dpms_property = drm_connector->props[p];
> + }
> + drmModeFreeProperty(prop);
> + }
> +
> /* Mark all connector modes as invalid */
> wsi_display_invalidate_connector_modes(wsi_device, connector);
>
> @@ -673,15 +696,37 @@ wsi_display_surface_get_capabilities(VkIcdSurfaceBase
> *surface_base,
> return VK_SUCCESS;
> }
>
> +static VkResult
> +wsi_display_surface_get_surface_counters(
> + VkIcdSurfaceBase *surface_base,
> + VkSurfaceCounterFlagsEXT *counters)
> +{
> + *counters = VK_SURFACE_COUNTER_VBLANK_EXT;
> + return VK_SUCCESS;
> +}
> +
> static VkResult
> wsi_display_surface_get_capabilities2(VkIcdSurfaceBase *icd_surface,
> const void *info_next,
> VkSurfaceCapabilities2KHR *caps)
> {
> assert(caps->sType == VK_STRUCTURE_TYPE_SURFACE_CAPABILITIES_2_KHR);
> + VkResult result;
> +
> + result = wsi_display_surface_get_capabilities(icd_surface,
> + &caps->surfaceCapabilities);
> + if (result != VK_SUCCESS)
> + return result;
> +
> + struct wsi_surface_supported_counters *counters =
> + vk_find_struct( caps->pNext, WSI_SURFACE_SUPPORTED_COUNTERS_MESA);
> +
> + if (counters)
> + result = wsi_display_surface_get_surface_counters(
> + icd_surface,
> + &counters->supported_surface_counters);
Please put braces around multi-line if statements.
>
> - return wsi_display_surface_get_capabilities(icd_surface,
> - &caps->surfaceCapabilities);
> + return result;
> }
>
> static const struct {
> @@ -904,6 +949,8 @@ wsi_display_page_flip_handler2(int fd,
> chain->status = result;
> }
>
> +static void wsi_display_fence_event_handler(struct wsi_display_fence *fence);
> +
> static void wsi_display_page_flip_handler(int fd,
> unsigned int frame,
> unsigned int sec,
> @@ -913,12 +960,32 @@ static void wsi_display_page_flip_handler(int fd,
> wsi_display_page_flip_handler2(fd, frame, sec, usec, 0, data);
> }
>
> +static void wsi_display_vblank_handler(int fd, unsigned int frame,
> + unsigned int sec, unsigned int usec,
> + void *data)
> +{
> + struct wsi_display_fence *fence = data;
> +
> + wsi_display_fence_event_handler(fence);
> +}
> +
> +static void wsi_display_sequence_handler(int fd, uint64_t frame,
> + uint64_t nsec, uint64_t user_data)
> +{
> + struct wsi_display_fence *fence =
> + (struct wsi_display_fence *) (uintptr_t) user_data;
> +
> + wsi_display_fence_event_handler(fence);
> +}
> +
> static drmEventContext event_context = {
> .version = DRM_EVENT_CONTEXT_VERSION,
> .page_flip_handler = wsi_display_page_flip_handler,
> #if DRM_EVENT_CONTEXT_VERSION >= 3
> .page_flip_handler2 = wsi_display_page_flip_handler2,
> #endif
> + .vblank_handler = wsi_display_vblank_handler,
> + .sequence_handler = wsi_display_sequence_handler,
> };
>
> static void *
> @@ -1185,6 +1252,151 @@ bail:
>
> }
>
> +static VkResult
> +wsi_display_fence_wait(struct wsi_fence *fence_wsi, uint64_t timeout)
> +{
> + const struct wsi_device *wsi_device = fence_wsi->wsi_device;
> + struct wsi_display *wsi =
> + (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
> + struct wsi_display_fence *fence = (struct wsi_display_fence *) fence_wsi;
> +
> + wsi_display_debug("%9lu wait fence %lu %ld\n",
> + pthread_self(), fence->sequence,
> + (int64_t) (timeout - wsi_get_current_monotonic()));
> + wsi_display_debug_code(uint64_t start_ns = wsi_get_current_monotonic());
> + pthread_mutex_lock(&wsi->wait_mutex);
> +
> + VkResult result;
> + int ret = 0;
> + for (;;) {
> + if (fence->event_received) {
> + wsi_display_debug("%9lu fence %lu passed\n",
> + pthread_self(), fence->sequence);
> + result = VK_SUCCESS;
> + break;
> + }
> +
> + if (ret == ETIMEDOUT) {
> + wsi_display_debug("%9lu fence %lu timeout\n",
> + pthread_self(), fence->sequence);
> + result = VK_TIMEOUT;
> + break;
> + }
> +
> + ret = wsi_display_wait_for_event(wsi, timeout);
> +
> + if (ret && ret != ETIMEDOUT) {
> + wsi_display_debug("%9lu fence %lu error\n",
> + pthread_self(), fence->sequence);
> + result = VK_ERROR_DEVICE_LOST;
> + break;
> + }
> + }
> + pthread_mutex_unlock(&wsi->wait_mutex);
> + wsi_display_debug("%9lu fence wait %f ms\n",
> + pthread_self(),
> + ((int64_t) (wsi_get_current_monotonic() - start_ns)) /
> + 1.0e6);
> + return result;
> +}
> +
> +static void
> +wsi_display_fence_check_free(struct wsi_display_fence *fence)
> +{
> + if (fence->event_received && fence->destroyed)
> + vk_free(fence->base.alloc, fence);
> +}
> +
> +static void wsi_display_fence_event_handler(struct wsi_display_fence *fence)
> +{
> + fence->event_received = true;
> + wsi_display_fence_check_free(fence);
> +}
> +
> +static void
> +wsi_display_fence_destroy(struct wsi_fence *fence_wsi)
> +{
> + struct wsi_display_fence *fence = (struct wsi_display_fence *) fence_wsi;
> +
> + assert(!fence->destroyed);
> + fence->destroyed = true;
> + wsi_display_fence_check_free(fence);
> +}
> +
> +static struct wsi_display_fence *
> +wsi_display_fence_alloc(VkDevice device,
> + const struct wsi_device *wsi_device,
> + VkDisplayKHR display,
> + const VkAllocationCallbacks *allocator)
> +{
> + struct wsi_display *wsi =
> + (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
> + struct wsi_display_fence *fence =
> + vk_zalloc2(wsi->alloc, allocator, sizeof (*fence),
> + 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> +
> + if (!fence)
> + return NULL;
> +
> + fence->base.device = device;
> + fence->base.display = display;
> + fence->base.wsi_device = wsi_device;
> + fence->base.alloc = allocator;
> + fence->base.wait = wsi_display_fence_wait;
> + fence->base.destroy = wsi_display_fence_destroy;
> + fence->event_received = false;
> + fence->destroyed = false;
> + fence->sequence = ++fence_sequence;
> + return fence;
> +}
> +
> +static VkResult
> +wsi_register_vblank_event(struct wsi_display_fence *fence,
> + const struct wsi_device *wsi_device,
> + VkDisplayKHR display,
> + uint32_t flags,
> + uint64_t frame_requested,
> + uint64_t *frame_queued)
> +{
> + struct wsi_display *wsi =
> + (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
> + struct wsi_display_connector *connector =
> + wsi_display_connector_from_handle(display);
> +
> + if (wsi->fd < 0)
> + return VK_ERROR_INITIALIZATION_FAILED;
> +
> + for (;;) {
> + int ret = drmCrtcQueueSequence(wsi->fd, connector->crtc_id,
> + flags,
> + frame_requested,
> + frame_queued,
> + (uint64_t) fence);
> +
> + if (!ret)
> + return VK_SUCCESS;
> +
> + if (errno != ENOMEM) {
>
> This strikes me as a bit odd. What does ENOMEM mean if not "out of memory"?
>
> + wsi_display_debug("queue vblank event %lu failed\n",
> fence->sequence);
> + struct timespec delay = {
> + .tv_sec = 0,
> + .tv_nsec = 100000000ull,
> + };
> + nanosleep(&delay, NULL);
>
> Given your previous explanation, I think this sleep is ok but it really
> needs a comment.
>
> + return VK_ERROR_OUT_OF_HOST_MEMORY;
> + }
> +
> + pthread_mutex_lock(&wsi->wait_mutex);
> + ret = wsi_display_wait_for_event(wsi,
> wsi_rel_to_abs_time(100000000ull));
> + pthread_mutex_unlock(&wsi->wait_mutex);
> +
> + if (ret) {
> + wsi_display_debug("vblank queue full, event wait failed\n");
> + return VK_ERROR_OUT_OF_HOST_MEMORY;
> + }
> + }
> +}
> +
> /*
> * Check to see if the kernel has no flip queued and if there's an image
> * waiting to be displayed.
> @@ -1980,3 +2192,116 @@ wsi_get_randr_output_display(VkPhysicalDevice
> physical_device,
> }
>
> #endif
> +
> +/* VK_EXT_display_control */
> +VkResult
> +wsi_display_power_control(VkDevice device,
> + struct wsi_device *wsi_device,
> + VkDisplayKHR display,
> + const VkDisplayPowerInfoEXT *display_power_info)
> +{
> + struct wsi_display *wsi =
> + (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
> + struct wsi_display_connector *connector =
> + wsi_display_connector_from_handle(display);
> + int mode;
> +
> + if (wsi->fd < 0)
> + return VK_ERROR_INITIALIZATION_FAILED;
> +
> + switch (display_power_info->powerState) {
> + case VK_DISPLAY_POWER_STATE_OFF_EXT:
> + mode = DRM_MODE_DPMS_OFF;
> + break;
> + case VK_DISPLAY_POWER_STATE_SUSPEND_EXT:
> + mode = DRM_MODE_DPMS_SUSPEND;
> + break;
> + default:
> + mode = DRM_MODE_DPMS_ON;
> + break;
> + }
> + drmModeConnectorSetProperty(wsi->fd,
> + connector->id,
> + connector->dpms_property,
> + mode);
> + return VK_SUCCESS;
> +}
> +
> +VkResult
> +wsi_register_device_event(VkDevice device,
> + struct wsi_device *wsi_device,
> + const VkDeviceEventInfoEXT *device_event_info,
> + const VkAllocationCallbacks *allocator,
> + struct wsi_fence **fence_p)
> +{
> + return VK_ERROR_FEATURE_NOT_PRESENT;
> +}
> +
> +VkResult
> +wsi_register_display_event(VkDevice device,
> + struct wsi_device *wsi_device,
> + VkDisplayKHR display,
> + const VkDisplayEventInfoEXT *display_event_info,
> + const VkAllocationCallbacks *allocator,
> + struct wsi_fence **fence_p)
> +{
> + struct wsi_display *wsi =
> + (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
> + struct wsi_display_fence *fence;
> + VkResult ret;
> +
> + switch (display_event_info->displayEvent) {
> + case VK_DISPLAY_EVENT_TYPE_FIRST_PIXEL_OUT_EXT:
> +
> + fence = wsi_display_fence_alloc(device, wsi_device, display, allocator);
> +
> + if (!fence)
> + return VK_ERROR_OUT_OF_HOST_MEMORY;
> +
> + ret = wsi_register_vblank_event(fence, wsi_device, display,
> + DRM_CRTC_SEQUENCE_RELATIVE, 1, NULL);
> +
> + if (ret == VK_SUCCESS)
> + *fence_p = &fence->base;
> + else if (fence != NULL)
> + vk_free2(wsi->alloc, allocator, fence);
> +
> + break;
> + default:
> + ret = VK_ERROR_FEATURE_NOT_PRESENT;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +
> +VkResult
> +wsi_get_swapchain_counter(VkDevice device,
> + struct wsi_device *wsi_device,
> + VkSwapchainKHR _swapchain,
> + VkSurfaceCounterFlagBitsEXT flag_bits,
> + uint64_t *value)
> +{
> + struct wsi_display *wsi =
> + (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
> + struct wsi_display_swapchain *swapchain =
> + (struct wsi_display_swapchain *) wsi_swapchain_from_handle(_swapchain);
> + struct wsi_display_connector *connector =
> +
> wsi_display_mode_from_handle(swapchain->surface->displayMode)->connector;
> +
> + if (wsi->fd < 0)
> + return VK_ERROR_INITIALIZATION_FAILED;
> +
> + if (!connector->active) {
> + *value = 0;
> + return VK_SUCCESS;
> + }
> +
> + int ret = drmCrtcGetSequence(wsi->fd, connector->crtc_id, value, NULL);
> + if (ret)
> + *value = 0;
> +
> + return VK_SUCCESS;
> +}
> +
> diff --git a/src/vulkan/wsi/wsi_common_display.h
> b/src/vulkan/wsi/wsi_common_display.h
> index eaa0ba727b0..941ce8d5d66 100644
> --- a/src/vulkan/wsi/wsi_common_display.h
> +++ b/src/vulkan/wsi/wsi_common_display.h
> @@ -104,4 +104,33 @@ wsi_get_randr_output_display(VkPhysicalDevice
> physical_device,
>
> #endif /* VK_USE_PLATFORM_XLIB_XRANDR_EXT */
>
> +/* VK_EXT_display_control */
> +VkResult
> +wsi_display_power_control(VkDevice device,
> + struct wsi_device *wsi_device,
> + VkDisplayKHR display,
> + const VkDisplayPowerInfoEXT *display_power_info);
> +
> +VkResult
> +wsi_register_device_event(VkDevice device,
> + struct wsi_device *wsi_device,
> + const VkDeviceEventInfoEXT *device_event_info,
> + const VkAllocationCallbacks *allocator,
> + struct wsi_fence **fence);
> +
> +VkResult
> +wsi_register_display_event(VkDevice device,
> + struct wsi_device *wsi_device,
> + VkDisplayKHR display,
> + const VkDisplayEventInfoEXT *display_event_info,
> + const VkAllocationCallbacks *allocator,
> + struct wsi_fence **fence);
> +
> +VkResult
> +wsi_get_swapchain_counter(VkDevice device,
> + struct wsi_device *wsi_device,
> + VkSwapchainKHR swapchain,
> + VkSurfaceCounterFlagBitsEXT flag_bits,
> + uint64_t *value);
> +
> #endif
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list