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

[Emil Velikov]

Hi Danylo,

On 13 July 2018 at 14:57, Danylo Piliaiev <danylo.piliaiev at gmail.com> wrote:
> 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.
>
> 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)
>
> v3: by Dylan Baker
>     - Remove previously added changes to vk.xml and entrypoints
>        generation because vk.xml is meant to be pulled from the external
>        source.
>
> Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
> ---
>  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_extensions.py     |  2 +-
>  src/intel/vulkan/anv_extensions_gen.py |  7 +++++++
>  src/intel/vulkan/anv_wsi_display.c     |  4 ++--
>  src/vulkan/wsi/wsi_common_display.c    |  8 +++++--
>  src/vulkan/wsi/wsi_common_display.h    |  3 ++-
>  9 files changed, 51 insertions(+), 26 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
> +
There is no need for yet another version - just bump LIBDRM_REQUIRED.

>  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
Thinking out loud: I wonder if we cannot use a single variable for all
the xcb 1.13 bits.

>  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"], [])
> +
Why do we need the new define? From a quick look we should be above to
reuse VK_USE_PLATFORM_DISPLAY_KHR, we simply need the ifdef guards.

The comments are more or less also applicable for meson.

HTH
Emil