[Mesa-dev] [PATCH v2] vulkan: Fix compilation on older platforms

Dylan Baker dylan at pnwbakers.com
Thu Jul 12 18:49:39 UTC 2018


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.

>      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.

>              <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 --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180712/0be312db/attachment-0001.sig>


More information about the mesa-dev mailing list