[Mesa-dev] [PATCH v2] vulkan: Fix compilation on older platforms
Danylo Piliaiev
danylo.piliaiev at gmail.com
Fri Jul 13 07:27:39 UTC 2018
On 12.07.18 21:49, Dylan Baker wrote:
> Quoting Danylo Piliaiev (2018-07-12 06:09:57)
>> 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: https://bugs.freedesktop.org/show_bug.cgi?id=107170
>> https://bugs.freedesktop.org/show_bug.cgi?id=106972
>> https://bugs.freedesktop.org/show_bug.cgi?id=107176
>>
>> 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 <danylo.piliaiev at globallogic.com>
>> ---
>> 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
>> +
> 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.
See my comments below.
>> 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">
> 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.
>
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).
>> <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
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180713/295729a9/attachment-0001.html>
More information about the mesa-dev
mailing list