<div dir="ltr">Sorry for the multitude of replies. :-( <div class="gmail_extra"> <div class="gmail_quote">On Wed, Mar 7, 2018 at 11:24 PM, Keith Packard <<a href="mailto:keithp@keithp.com" target="_blank">keithp@keithp.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This adds support for the KHR_display extension support to the vulkan WSI layer. Driver support will be added separately. v2: * fix double ;; in wsi_common_display.c * Move mode list from wsi_display to wsi_display_connector * Fix scope for wsi_display_mode andwsi_display_connector allocs * Switch all allocations to vk_zalloc instead of vk_alloc. * Fix DRM failure in wsi_display_get_physical_device_display_properties When DRM fails, or when we don't have a master fd (presumably due to application errors), just return 0 properties from this function, which is at least a valid response. * Use vk_outarray for all property queries This is a bit less error-prone than open-coding the same stuff. * Remove VK_COMPOSITE_ALPHA_INHERIT_BIT_KHR from surface caps Until we have multi-plane support, we shouldn't pretend to have any multi-plane semantics, even if undefined. Suggested-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> * Simplify addition of VK_USE_PLATFORM_DISPLAY_KHR to vulkan_wsi_args Suggested-by: Eric Engestrom <<a href="mailto:eric.engestrom@imgtec.com">eric.engestrom@imgtec.com</a>> v3: Add separate 'display_fd' and 'render_fd' arguments to wsi_device_init API. This allows drivers to use different FDs for the different aspects of the device. Use largest mode as display size when no preferred mode. If the display doesn't provide a preferred mode, we'll assume that the largest supported mode is the "physical size" of the device and report that. v4: Make wsi_image_state enumeration values uppercase. Follow more common mesa conventions. Remove 'render_fd' from wsi_device_init API. The wsi_common_display code doesn't use this fd at all, so stop passing it in. This avoids any potential confusion over which fd to use when creating display-relative object handles. Remove call to wsi_create_prime_image which would never have been reached as the necessary condition (use_prime_blit) is never set. whitespace cleanups in wsi_common_display.c Suggested-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> Add depth/bpp info to available surface formats. Instead of hard-coding depth 24 bpp 32 in the drmModeAddFB call, use the requested format to find suitable values. Destroy kernel buffers and FBs when swapchain is destroyed. We were leaking both of these kernel objects across swapchain destruction. Note that wsi_display_wait_for_event waits for anything to happen. wsi_display_wait_for_event is simply a yield so that the caller can then check to see if the desired state change has occurred. Record swapchain failures in chain for later return. If some asynchronous swapchain activity fails, we need to tell the application eventually. Record the failure in the swapchain and report it at the next acquire_next_image or queue_present call. Fix error returns from wsi_display_setup_connector. If a malloc failed, then the result should be VK_ERROR_OUT_OF_HOST_MEMORY. Otherwise, the associated ioctl failed and we're either VT switched away, or our lease has been revoked, in which case we should return VK_ERROR_OUT_OF_DATE_KHR. Make sure both sides of if/else brace use matches Note that we assume drmModeSetCrtc is synchronous. Add a comment explaining why we can idle any previous displayed image as soon as the mode set returns. Note that EACCES from drmModePageFlip means VT inactive. When vt switched away drmModePageFlip returns EACCES. Poll once a second waiting until we get some other return value back. Clean up after alloc failure in wsi_display_surface_create_swapchain. Destroy any created images, free the swapchain. Remove physical_device from wsi_display_init_wsi. We never need this value, so remove it from the API and from the internal wsi_display structure. Use drmModeAddFB2 in wsi_display_image_init. This takes a drm format instead of depth/bpp, which provides more control over the format of the data. Signed-off-by: Keith Packard <<a href="mailto:keithp@keithp.com">keithp@keithp.com</a>> --- <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> | 1 + meson.build | 4 +- src/amd/vulkan/radv_device.c | 8 + src/amd/vulkan/radv_private.h | 1 + src/amd/vulkan/radv_wsi.c | 3 +- src/intel/vulkan/anv_device.c | 4 + src/intel/vulkan/anv_private.h | 1 + src/intel/vulkan/anv_wsi.c | 3 +- src/vulkan/Makefile.am | 7 + src/vulkan/Makefile.sources | 4 + src/vulkan/wsi/meson.build | 8 + src/vulkan/wsi/wsi_common.c | 19 +- src/vulkan/wsi/wsi_common.h | 5 +- src/vulkan/wsi/wsi_common_display.c | 1401 +++++++++++++++++++++++++++++++++++ src/vulkan/wsi/wsi_common_display.h | 72 ++ src/vulkan/wsi/wsi_common_private.h | 9 + 16 files changed, 1544 insertions(+), 6 deletions(-) create mode 100644 src/vulkan/wsi/wsi_common_display.c create mode 100644 src/vulkan/wsi/wsi_common_display.h diff --git a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> index 172da6b443c..7fcb3220eaa 100644 --- a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> +++ b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> @@ -1856,6 +1856,7 @@ fi AM_CONDITIONAL(HAVE_PLATFORM_X11, echo "$platforms" | grep -q 'x11') AM_CONDITIONAL(HAVE_PLATFORM_WAYLAND, echo "$platforms" | grep -q 'wayland') AM_CONDITIONAL(HAVE_PLATFORM_DRM, echo "$platforms" | grep -q 'drm') +AM_CONDITIONAL(HAVE_PLATFORM_DISPLAY, echo "$platforms" | grep -q 'drm') AM_CONDITIONAL(HAVE_PLATFORM_SURFACELESS, echo "$platforms" | grep -q 'surfaceless') AM_CONDITIONAL(HAVE_PLATFORM_ANDROID, echo "$platforms" | grep -q 'android') diff --git a/meson.build b/meson.build index 8a17d7f240a..788aed6e159 100644 --- a/meson.build +++ b/meson.build @@ -239,11 +239,12 @@ with_platform_wayland = false with_platform_x11 = false with_platform_drm = false with_platform_surfaceless = false +with_platform_display = false egl_native_platform = '' _platforms = get_option('platforms') if _platforms == 'auto' if system_has_kms_drm - _platforms = 'x11,wayland,drm,surfaceless' + _platforms = 'x11,wayland,drm,surfaceless,display' elif ['darwin', 'windows', 'cygwin'].contains(host_machine.system()) _platforms = 'x11,surfaceless' elif ['haiku'].contains(host_machine.system()) @@ -260,6 +261,7 @@ if _platforms != '' with_platform_drm = _split.contains('drm') with_platform_haiku = _split.contains('haiku') with_platform_surfaceless = _split.contains('surfaceless') + with_platform_display = _split.contains('display') egl_native_platform = _split[0] endif diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c index 7a11e08f97c..35fd1ef6b29 100644 --- a/src/amd/vulkan/radv_device.c +++ b/src/amd/vulkan/radv_device.c @@ -223,6 +223,7 @@ radv_physical_device_init(struct radv_physical_device *device, VkResult result; drmVersionPtr version; int fd; + int master_fd = -1; fd = open(path, O_RDWR | O_CLOEXEC); if (fd < 0) @@ -237,6 +238,8 @@ radv_physical_device_init(struct radv_physical_device *device, if (strcmp(version->name, "amdgpu")) { drmFreeVersion(version); + if (master_fd != -1) + close(master_fd); close(fd); return VK_ERROR_INCOMPATIBLE_DRIVER; } @@ -254,6 +257,7 @@ radv_physical_device_init(struct radv_physical_device *device, goto fail; } + device->master_fd = master_fd; device->local_fd = fd; device->ws->query_info(device->ws, &device->rad_info); @@ -317,6 +321,8 @@ radv_physical_device_init(struct radv_physical_device *device, fail: close(fd); + if (master_fd != -1) + close(master_fd); return result; } @@ -327,6 +333,8 @@ radv_physical_device_finish(struct radv_physical_device *device) device->ws->destroy(device->ws); disk_cache_destroy(device->disk_cache); close(device->local_fd); + if (device->master_fd != -1) + close(device->master_fd); } static void * diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h index 0f8ddb2e106..35a161c3671 100644 --- a/src/amd/vulkan/radv_private.h +++ b/src/amd/vulkan/radv_private.h @@ -281,6 +281,7 @@ struct radv_physical_device { uint8_t cache_uuid[VK_UUID_SIZE]; int local_fd; + int master_fd; struct wsi_device wsi_device; bool has_rbplus; /* if RB+ register exist */ diff --git a/src/amd/vulkan/radv_wsi.c b/src/amd/vulkan/radv_wsi.c index 927650480a6..2840b666727 100644 --- a/src/amd/vulkan/radv_wsi.c +++ b/src/amd/vulkan/radv_wsi.c @@ -41,7 +41,8 @@ radv_init_wsi(struct radv_physical_device *physical_device) return wsi_device_init(&physical_device->wsi_device, radv_physical_device_to_handle(physical_device), radv_wsi_proc_addr, - &physical_device->instance->alloc); + &physical_device->instance->alloc, + physical_device->master_fd); </blockquote><div> </div><div>We can just pass -1 in here and move the rest of the anv/radv stuff into patches 1 and 2. That would make 1 and 2 a bit easier to review but it doesn't really matter now that I've found this bit of code. :) </div><div>--Jason </div></div></div></div>