[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