<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">On 12.07.18 21:49, Dylan Baker wrote: </div> <blockquote type="cite" cite="mid:153142137982.3604.15501269663172759019@anlee.localdomain"> <pre wrap="">Quoting Danylo Piliaiev (2018-07-12 06:09:57) </pre> <blockquote type="cite"> <pre wrap="">Make xlease automatically enabled only if xcb-randr >= 1.13, check its version if manually enabled. Enable VK_EXT_display_control only when libdrm >= 2.4.89 Check for DRM_EVENT_CONTEXT_VERSION >= 4 to use sequence_handler. Add support for 'protect' attribute to anv_entrypoints_gen. Bugzilla: <a class="moz-txt-link-freetext" href="https://bugs.freedesktop.org/show_bug.cgi?id=107170">https://bugs.freedesktop.org/show_bug.cgi?id=107170</a> <a class="moz-txt-link-freetext" href="https://bugs.freedesktop.org/show_bug.cgi?id=106972">https://bugs.freedesktop.org/show_bug.cgi?id=106972</a> <a class="moz-txt-link-freetext" href="https://bugs.freedesktop.org/show_bug.cgi?id=107176">https://bugs.freedesktop.org/show_bug.cgi?id=107176</a> v2: - Add 'protect="VK_USE_DISPLAY_CONTROL"' attribute to VK_EXT_display_control in vk.xml - Add support for 'protect' attribute to anv_entrypoints_gen (copied from radv_entrypoints_gen) - Turn #if into #ifdef - Remove unnecessary pkg-config call from meson build (Dylan Baker) Signed-off-by: Danylo Piliaiev <a class="moz-txt-link-rfc2396E" href="mailto:danylo.piliaiev@globallogic.com"><danylo.piliaiev@globallogic.com></a> --- Other vulkan extensions are gated behind platforms: wayland, xlib, ... This one depends on library version and I hope I handled it right, did I took the right approach? Also what if extension requires both platform and libary version? Should *_entrypoints_gen be able to support several defines per extension? configure.ac | 29 ++++++++++++------------- meson.build | 10 ++++++++- src/amd/vulkan/radv_extensions.py | 9 +++++++- src/amd/vulkan/radv_wsi_display.c | 5 ++--- src/intel/vulkan/anv_entrypoints_gen.py | 7 ++++++ src/intel/vulkan/anv_extensions.py | 2 +- src/intel/vulkan/anv_extensions_gen.py | 7 ++++++ src/intel/vulkan/anv_wsi_display.c | 4 ++-- src/vulkan/registry/vk.xml | 2 +- src/vulkan/wsi/wsi_common_display.c | 8 +++++-- src/vulkan/wsi/wsi_common_display.h | 3 ++- 11 files changed, 59 insertions(+), 27 deletions(-) diff --git a/configure.ac b/configure.ac index f135d05736..0b04525014 100644 --- a/configure.ac +++ b/configure.ac @@ -82,6 +82,8 @@ LIBDRM_FREEDRENO_REQUIRED=2.4.92 LIBDRM_ETNAVIV_REQUIRED=2.4.89 LIBDRM_VC4_REQUIRED=2.4.89 +LIBDRM_CRT_SEQUENCE_REQUIRED=2.4.89 + dnl Versions for external dependencies DRI2PROTO_REQUIRED=2.8 GLPROTO_REQUIRED=1.4.14 @@ -97,6 +99,7 @@ XCBDRI2_REQUIRED=1.8 XCBDRI3_MODIFIERS_REQUIRED=1.13 XCBGLX_REQUIRED=1.8.1 XCBPRESENT_MODIFIERS_REQUIRED=1.13 +XCBRANDR_XLEASE_REQUIRED=1.13 XDAMAGE_REQUIRED=1.1 XSHMFENCE_REQUIRED=1.1 XVMC_REQUIRED=1.0.6 @@ -1874,20 +1877,6 @@ if test x"$enable_dri3" = xyes; then fi fi - -if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then - have_xlease=yes -else - have_xlease=no -fi - -if test x"$have_xlease" = xyes; then - randr_modules="x11-xcb xcb-randr" - PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules]) - xlib_randr_modules="xrandr" - PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules]) -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') @@ -1905,14 +1894,24 @@ xno) ;; *) if echo "$platforms" | grep -q 'x11' && echo "$platforms" | grep -q 'drm'; then - enable_xlib_lease=yes + xlease_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED xrandr" + PKG_CHECK_EXISTS([$xlease_modules], [enable_xlib_lease=yes], [enable_xlib_lease=no]) else enable_xlib_lease=no fi esac +if test x"$enable_xlib_lease" = xyes; then + randr_modules="x11-xcb xcb-randr >= $XCBRANDR_XLEASE_REQUIRED" + PKG_CHECK_MODULES([XCB_RANDR], [$randr_modules]) + xlib_randr_modules="xrandr" + PKG_CHECK_MODULES([XLIB_RANDR], [$xlib_randr_modules]) +fi + AM_CONDITIONAL(HAVE_XLIB_LEASE, test "x$enable_xlib_lease" = xyes) +PKG_CHECK_EXISTS([libdrm >= $LIBDRM_CRT_SEQUENCE_REQUIRED], [DEFINES="${DEFINES} -DVK_USE_DISPLAY_CONTROL"], []) + dnl dnl More DRI setup dnl diff --git a/meson.build b/meson.build index 7d12af3d51..902074819c 100644 --- a/meson.build +++ b/meson.build @@ -1088,6 +1088,8 @@ _drm_freedreno_ver = '2.4.92' _drm_intel_ver = '2.4.75' _drm_ver = '2.4.75' +_drm_crt_sequence_ver = '2.4.89' + _libdrm_checks = [ ['intel', with_dri_i915 or with_gallium_i915], ['amdgpu', with_amd_vk or with_gallium_radeonsi], @@ -1361,11 +1363,17 @@ if with_platform_x11 dep_xcb_xfixes = dependency('xcb-xfixes') endif if with_xlib_lease - dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.12') + dep_xcb_xrandr = dependency('xcb-randr', version : '>= 1.13') dep_xlib_xrandr = dependency('xrandr', version : '>= 1.3') endif endif +if with_any_vk + if dep_libdrm.version().version_compare('>= ' + _drm_crt_sequence_ver) + pre_args += '-DVK_USE_DISPLAY_CONTROL' + endif +endif + if get_option('gallium-extra-hud') pre_args += '-DHAVE_GALLIUM_EXTRA_HUD=1' endif diff --git a/src/amd/vulkan/radv_extensions.py b/src/amd/vulkan/radv_extensions.py index 094ed3bce3..35b49243a3 100644 --- a/src/amd/vulkan/radv_extensions.py +++ b/src/amd/vulkan/radv_extensions.py @@ -92,7 +92,7 @@ EXTENSIONS = [ Extension('VK_EXT_direct_mode_display', 1, 'VK_USE_PLATFORM_DISPLAY_KHR'), Extension('VK_EXT_acquire_xlib_display', 1, 'VK_USE_PLATFORM_XLIB_XRANDR_EXT'), Extension('VK_EXT_display_surface_counter', 1, 'VK_USE_PLATFORM_DISPLAY_KHR'), - Extension('VK_EXT_display_control', 1, 'VK_USE_PLATFORM_DISPLAY_KHR'), + Extension('VK_EXT_display_control', 1, 'VK_USE_DISPLAY_CONTROL'), Extension('VK_EXT_debug_report', 9, True), Extension('VK_EXT_depth_range_unrestricted', 1, True), Extension('VK_EXT_descriptor_indexing', 2, True), @@ -239,6 +239,13 @@ _TEMPLATE_C = Template(COPYRIGHT + """ # define ANDROID false #endif +#ifdef VK_USE_DISPLAY_CONTROL +# undef VK_USE_DISPLAY_CONTROL +# define VK_USE_DISPLAY_CONTROL true +#else +# define VK_USE_DISPLAY_CONTROL false +#endif + #define RADV_HAS_SURFACE (VK_USE_PLATFORM_WAYLAND_KHR || \\ VK_USE_PLATFORM_XCB_KHR || \\ VK_USE_PLATFORM_XLIB_KHR || \\ diff --git a/src/amd/vulkan/radv_wsi_display.c b/src/amd/vulkan/radv_wsi_display.c index d8743a06e3..a932e05ce8 100644 --- a/src/amd/vulkan/radv_wsi_display.c +++ b/src/amd/vulkan/radv_wsi_display.c @@ -252,8 +252,7 @@ radv_GetRandROutputDisplayEXT(VkPhysicalDevice physical_device, } #endif /* VK_USE_PLATFORM_XLIB_XRANDR_EXT */ -/* VK_EXT_display_control */ - +#ifdef VK_USE_DISPLAY_CONTROL VkResult radv_DisplayPowerControlEXT(VkDevice _device, VkDisplayKHR display, @@ -351,4 +350,4 @@ radv_GetSwapchainCounterEXT(VkDevice _device, flag_bits, value); } - +#endif /* VK_USE_DISPLAY_CONTROL */ diff --git a/src/intel/vulkan/anv_entrypoints_gen.py b/src/intel/vulkan/anv_entrypoints_gen.py index db35069850..901d7f33ae 100644 --- a/src/intel/vulkan/anv_entrypoints_gen.py +++ b/src/intel/vulkan/anv_entrypoints_gen.py @@ -511,6 +511,13 @@ def get_entrypoints_defines(doc): """Maps entry points to extension defines.""" entrypoints_to_defines = {} + for extension in doc.findall('./extensions/extension[@protect]'): + define = extension.attrib['protect'] + + for entrypoint in extension.findall('./require/command'): + fullname = entrypoint.attrib['name'] + entrypoints_to_defines[fullname] = define + </pre> </blockquote> <pre wrap=""> We used to have this in anv, but Jason dropped it. It's not clear to me from the commit message for the review why. Adding him. </pre> </blockquote> See my comments below. <blockquote type="cite" cite="mid:153142137982.3604.15501269663172759019@anlee.localdomain"> <pre wrap=""> </pre> <blockquote type="cite"> <pre wrap=""> for extension in doc.findall('./extensions/extension[@platform]'): platform = extension.attrib['platform'] ext = '_KHR' diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_extensions.py index adc1d75898..defd153095 100644 --- a/src/intel/vulkan/anv_extensions.py +++ b/src/intel/vulkan/anv_extensions.py @@ -116,7 +116,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_control', 1, 'VK_USE_DISPLAY_CONTROL'), 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_extensions_gen.py b/src/intel/vulkan/anv_extensions_gen.py index a140c26745..914db7bd0e 100644 --- a/src/intel/vulkan/anv_extensions_gen.py +++ b/src/intel/vulkan/anv_extensions_gen.py @@ -120,6 +120,13 @@ _TEMPLATE_C = Template(COPYRIGHT + """ # define ANDROID false #endif +#ifdef VK_USE_DISPLAY_CONTROL +# undef VK_USE_DISPLAY_CONTROL +# define VK_USE_DISPLAY_CONTROL true +#else +# define VK_USE_DISPLAY_CONTROL false +#endif + #define ANV_HAS_SURFACE (VK_USE_PLATFORM_WAYLAND_KHR || \\ VK_USE_PLATFORM_XCB_KHR || \\ VK_USE_PLATFORM_XLIB_KHR || \\ diff --git a/src/intel/vulkan/anv_wsi_display.c b/src/intel/vulkan/anv_wsi_display.c index 3212c235ba..a915e1e9f7 100644 --- a/src/intel/vulkan/anv_wsi_display.c +++ b/src/intel/vulkan/anv_wsi_display.c @@ -231,8 +231,7 @@ anv_GetRandROutputDisplayEXT(VkPhysicalDevice physical_device, } #endif /* VK_USE_PLATFORM_XLIB_XRANDR_EXT */ -/* VK_EXT_display_control */ - +#ifdef VK_USE_DISPLAY_CONTROL VkResult anv_DisplayPowerControlEXT(VkDevice _device, VkDisplayKHR display, @@ -315,3 +314,4 @@ anv_GetSwapchainCounterEXT(VkDevice _device, _device, &device->instance->physicalDevice.wsi_device, swapchain, flag_bits, value); } +#endif /* VK_USE_DISPLAY_CONTROL */ diff --git a/src/vulkan/registry/vk.xml b/src/vulkan/registry/vk.xml index 4419c6fbf9..f8bba4c5e1 100644 --- a/src/vulkan/registry/vk.xml +++ b/src/vulkan/registry/vk.xml @@ -7820,7 +7820,7 @@ server. <command name="vkGetPhysicalDeviceSurfaceCapabilities2EXT"/> </require> </extension> - <extension name="VK_EXT_display_control" number="92" type="device" requires="VK_EXT_display_surface_counter,VK_KHR_swapchain" author="NV" contact="James Jones @cubanismo" supported="vulkan"> + <extension name="VK_EXT_display_control" number="92" type="device" requires="VK_EXT_display_surface_counter,VK_KHR_swapchain" author="NV" contact="James Jones @cubanismo" protect="VK_USE_DISPLAY_CONTROL" supported="vulkan"> </pre> </blockquote> <pre wrap=""> Is this change present in the upstream XML? Generally we don't hand edit the XML from Khronos, unless the change is upstream already or on its way upstream, since the next update to the XML would delete this change. </pre> </blockquote> It is not... If it is always copied from the upstream then making changes in it is not feasible. It seems I took a wrong path here. The question is: Is there a need to disable extension entrypoints if it is not supported? Without changes to vk.xml the entry points will be present but extension will be unsupported. After the second look I think that's ok since some extensions availability depends on runtime state (checks for device->something). <blockquote type="cite" cite="mid:153142137982.3604.15501269663172759019@anlee.localdomain"> <blockquote type="cite"> <pre wrap=""> <require> <enum value="1" name="VK_EXT_DISPLAY_CONTROL_SPEC_VERSION"/> <enum value=""VK_EXT_display_control"" name="VK_EXT_DISPLAY_CONTROL_EXTENSION_NAME"/> diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c index ac932d4368..91ce8706b5 100644 --- a/src/vulkan/wsi/wsi_common_display.c +++ b/src/vulkan/wsi/wsi_common_display.c @@ -1131,7 +1131,9 @@ static drmEventContext event_context = { .page_flip_handler2 = wsi_display_page_flip_handler2, #endif .vblank_handler = wsi_display_vblank_handler, +#if DRM_EVENT_CONTEXT_VERSION >= 4 .sequence_handler = wsi_display_sequence_handler, +#endif }; static void * @@ -1459,6 +1461,7 @@ static void wsi_display_fence_event_handler(struct wsi_display_fence *fence) wsi_display_fence_check_free(fence); } +#ifdef VK_USE_DISPLAY_CONTROL static void wsi_display_fence_destroy(struct wsi_fence *fence_wsi) { @@ -1551,6 +1554,7 @@ wsi_register_vblank_event(struct wsi_display_fence *fence, } } } +#endif /* VK_USE_DISPLAY_CONTROL */ /* * Check to see if the kernel has no flip queued and if there's an image @@ -2348,7 +2352,7 @@ wsi_get_randr_output_display(VkPhysicalDevice physical_device, #endif -/* VK_EXT_display_control */ +#ifdef VK_USE_DISPLAY_CONTROL VkResult wsi_display_power_control(VkDevice device, struct wsi_device *wsi_device, @@ -2459,4 +2463,4 @@ wsi_get_swapchain_counter(VkDevice device, return VK_SUCCESS; } - +#endif /* VK_USE_DISPLAY_CONTROL */ diff --git a/src/vulkan/wsi/wsi_common_display.h b/src/vulkan/wsi/wsi_common_display.h index 50d7f836a7..1b2d6c5d76 100644 --- a/src/vulkan/wsi/wsi_common_display.h +++ b/src/vulkan/wsi/wsi_common_display.h @@ -131,7 +131,7 @@ wsi_get_randr_output_display(VkPhysicalDevice physical_device, #endif /* VK_USE_PLATFORM_XLIB_XRANDR_EXT */ -/* VK_EXT_display_control */ +#ifdef VK_USE_DISPLAY_CONTROL VkResult wsi_display_power_control(VkDevice device, struct wsi_device *wsi_device, @@ -159,5 +159,6 @@ wsi_get_swapchain_counter(VkDevice device, VkSwapchainKHR swapchain, VkSurfaceCounterFlagBitsEXT flag_bits, uint64_t *value); +#endif /* VK_USE_DISPLAY_CONTROL */ #endif -- 2.17.1 </pre> </blockquote> </blockquote> </body> </html>