[Mesa-dev] [PATCH 00/28] vulkan/wsi: Rework WSI to look a lot more like a layer

[James Jones]

On 11/16/2017 01:28 PM, Jason Ekstrand wrote:
> This patch series is the combined brain-child of Dave and myself.  The
> objective is to rewrite Vulkan WSI to look as much like a layer as possible
> and to reduce the driver <-> WSI interface.  We try very hard to have as
> many of the WSI details as possible in common code and to use standard
> Vulkan interfaces for everything.  Among other things, this means that
> prime support is now implemented in an entirely driver-agnostic way and the
> driver doesn't even know it's happening.  As a side-effect anv now has
> prime support.
> 
> Eventually, someone could pull what's left out into a proper layer and we
> could drop WSI support from our drivers entirely.  There are a fiew pieces
> of work that would be required to do this:
> 
>   1) Write all of the annoying layer bits.  There are some short-cuts that
>      we can take because we're not in a layer and those will have to go.
> 
>   2) Define a VK_MESA_legacy_swapchain_image extension to replace the hack
>      introduced in patch 8.
> 
>   3) It looks like modifiers support will land before the official Vulkan
>      extensions get finished.  It will have to be ported to the official
>      extensions.
> 
>   4) Figure out what to do about the fence in AcquireNextImage. In a future
>      world of explicit synchronization, we can just import the sync_file
>      from X or the Wayland compositor, but with implicit sync like we have
>      today, it's a bit harder.  Right now, the helper in wsi_common does
>      nothing with it and trusts the caller to handle it.
> 
>      The two drivers handle this differently today.  In anv, we do a dummy
>      QueueSubmit to trigger the fence while radv triggers it a bit more
>      manually.  In both cases, we trigger the fence immediately and trust in
>      the GEM's implicit synchronization to sort things out for us.  We can't
>      use the anv method as directly with radv because no queue is passed in
>      so we don't know what queue to use in the dummy QueueSubmit.  (In ANV,
>      we only have the one queue so that isn't a problem.)

I don't have detailed feedback, but I read through the series and this 
is pretty cool.  Glad things are starting to generalize across the 
driver stacks.  I'm optimistic that things have gotten to the point 
where we'll never have to write a separate Wayland WSI for our driver. 
Is it an accurate observation to say there aren't any Vulkan API bits 
missing (other than stuff in the pipeline like modifiers/dma-buf) to 
allow the full-layer solution?  Hopefully we haven't missed anything in 
the external_* extensions at this point.

Thanks,
-James

> Dave, I tried to pull patches from your series where practical but, because
> we did things in a different order, it frequently wasn't.  If you want to
> claim credit for any of these patches, just say so and I'll --reset-author
> on them.
> 
> The series can be found on freedesktop.org here:
> 
> https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/vulkan-wsi-prime
> 
> 
> Cc: Dave Airlie <airlied at redhat.com>
> Cc: Daniel Stone <daniels at collabora.com>
> Cc: Chad Versace <chadversary at chromium.org>
> Cc: James Jones <jajones at nvidia.com>
> 
> Daniel Stone (1):
>    vulkan/wsi: Add a wsi_image structure
> 
> Dave Airlie (4):
>    vulkan/wsi: use function ptr definitions from the spec.
>    radv/wsi: drop allocate memory special case
>    radv/wsi: Move the guts of QueuePresent to wsi common
>    vulkan/wsi: move swapchain create/destroy to common code
> 
> Jason Ekstrand (23):
>    vulkan/wsi/x11: Handle the geometry check earlier in create_swapchain
>    vulkan/wsi: Add a wsi_device_init function
>    vulkan/wsi: Add wsi_swapchain_init/finish functions
>    vulkan/wsi: Implement prime in a completely generic way
>    anv/image: Add a return value to bind_memory_plane
>    vulkan/wsi: Add a mock image creation extension
>    anv/image: Implement the wsi "extension"
>    radv/image: Implement the wsi "extension"
>    vulkan/wsi: Do image creation in common code
>    vulkan/wsi: Add a WSI_FROM_HANDLE macro
>    vulkan/wsi: Refactor result handling in queue_present
>    vulkan/wsi: Only wait on semaphores on the first swapchain
>    vulkan/wsi: Set a proper pWaitDstStageMask on the dummy submit
>    anv/wsi: Use the common QueuePresent code
>    anv/wsi: Enable prime support
>    vulkan/wsi: Move get_images into common code
>    vulkan/wsi: Move prime blitting into queue_present
>    vulkan/wsi: Add a helper for AcquireNextImage
>    vulkan/wsi: Move wsi_swapchain to wsi_common_private.h
>    vulkan/wsi: Drop the can_handle_different_gpu parameter from
>      get_support
>    vulkan/wsi: Add wrappers for all of the surface queries
>    vulkan/wsi: Drop some unneeded cruft from the API
>    vulkan/wsi: Initialize individual WSI interfaces in wsi_device_init
> 
>   src/amd/vulkan/radv_device.c        |  18 +-
>   src/amd/vulkan/radv_image.c         |  15 +-
>   src/amd/vulkan/radv_private.h       |  10 -
>   src/amd/vulkan/radv_wsi.c           | 472 +++-------------------
>   src/intel/vulkan/anv_image.c        |  71 +++-
>   src/intel/vulkan/anv_private.h      |   2 +
>   src/intel/vulkan/anv_wsi.c          | 347 +++-------------
>   src/vulkan/Makefile.sources         |   2 +
>   src/vulkan/wsi/meson.build          |   2 +
>   src/vulkan/wsi/wsi_common.c         | 763 ++++++++++++++++++++++++++++++++++++
>   src/vulkan/wsi/wsi_common.h         | 223 ++++++-----
>   src/vulkan/wsi/wsi_common_private.h | 157 ++++++++
>   src/vulkan/wsi/wsi_common_wayland.c |  90 ++---
>   src/vulkan/wsi/wsi_common_x11.c     | 150 ++-----
>   14 files changed, 1310 insertions(+), 1012 deletions(-)
>   create mode 100644 src/vulkan/wsi/wsi_common.c
>   create mode 100644 src/vulkan/wsi/wsi_common_private.h
>