[Mesa-dev] [PATCH mesa 01/21] vulkan: Add KHR_display extension using DRM [v4]

Jason Ekstrand jason at jlekstrand.net
Mon Apr 9 23:18:14 UTC 2018


Sorry for the multitude of replies. :-(

On Wed, Mar 7, 2018 at 11:24 PM, Keith Packard <keithp at keithp.com> wrote:

> 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 <jason at jlekstrand.net>
>
>         * Simplify addition of VK_USE_PLATFORM_DISPLAY_KHR to
>           vulkan_wsi_args
>
>         Suggested-by: Eric Engestrom <eric.engestrom at imgtec.com>
>
> 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 <jason at jlekstrand.net>
>
>         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 <keithp at keithp.com>
> ---
>  configure.ac                        |    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/configure.ac b/configure.ac
> index 172da6b443c..7fcb3220eaa 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -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);
>

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

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180409/478bfa6d/attachment.html>


More information about the mesa-dev mailing list