[01/21] vulkan: Add KHR_display extension using DRM
diff mbox

Message ID 20180214003134.1552-2-keithp@keithp.com
State New
Headers show

Commit Message

Keith Packard Feb. 14, 2018, 12:31 a.m. UTC
This adds support for the KHR_display extension support to the vulkan
WSI layer. Driver support will be added separately.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 configure.ac                        |    1 +
 meson.build                         |    4 +-
 src/amd/vulkan/radv_wsi.c           |    3 +-
 src/intel/vulkan/anv_wsi.c          |    3 +-
 src/vulkan/Makefile.am              |    7 +
 src/vulkan/Makefile.sources         |    4 +
 src/vulkan/wsi/meson.build          |   10 +
 src/vulkan/wsi/wsi_common.c         |   19 +-
 src/vulkan/wsi/wsi_common.h         |    5 +-
 src/vulkan/wsi/wsi_common_display.c | 1368 +++++++++++++++++++++++++++++++++++
 src/vulkan/wsi/wsi_common_display.h |   72 ++
 src/vulkan/wsi/wsi_common_private.h |   10 +
 12 files changed, 1500 insertions(+), 6 deletions(-)
 create mode 100644 src/vulkan/wsi/wsi_common_display.c
 create mode 100644 src/vulkan/wsi/wsi_common_display.h

Comments

Jason Ekstrand Feb. 24, 2018, 12:43 a.m. UTC | #1
Continuing on the new version of the patch...

On Tue, Feb 13, 2018 at 4:31 PM, Keith Packard <keithp@keithp.com> wrote:

> +enum wsi_image_state {
> +   wsi_image_idle,
> +   wsi_image_drawing,
> +   wsi_image_queued,
> +   wsi_image_flipping,
> +   wsi_image_displaying
> +};
>

With the exception of NIR (which is a bit odd), we usually use ALL_CAPS for
enum values.


> +
> +static const VkPresentModeKHR available_present_modes[] = {
> +   VK_PRESENT_MODE_FIFO_KHR,
>

Yes, this does seem like the only reasonable mode for now.  I suppose you
could check to see if the platform supports ASYNC_FLIP and advertise
IMMEDIATE in that case.  I know intel doesn't advertise it but I thought
amdgpu does.


> +};
> +
> +static VkResult
> +wsi_display_surface_get_present_modes(VkIcdSurfaceBase  *surface,
> +                                      uint32_t
> *present_mode_count,
> +                                      VkPresentModeKHR  *present_modes)
> +{
> +   if (present_modes == NULL) {
> +      *present_mode_count = ARRAY_SIZE(available_present_modes);
> +      return VK_SUCCESS;
> +   }
> +
> +   *present_mode_count = MIN2(*present_mode_count,
> ARRAY_SIZE(available_present_modes));
> +   typed_memcpy(present_modes, available_present_modes,
> *present_mode_count);
> +
> +   if (*present_mode_count < ARRAY_SIZE(available_present_modes))
> +      return VK_INCOMPLETE;
> +   return VK_SUCCESS;
>

vk_outarray, though I suppose you've probably already made that change.


> +}
> +
> +static VkResult
> +wsi_display_image_init(VkDevice                         device_h,
> +                       struct wsi_swapchain             *drv_chain,
> +                       const VkSwapchainCreateInfoKHR   *create_info,
> +                       const VkAllocationCallbacks      *allocator,
> +                       struct wsi_display_image         *image)
> +{
> +   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *)
> drv_chain;
> +   struct wsi_display           *wsi = chain->wsi;
> +   VkResult                     result;
> +   int                          ret;
> +   uint32_t                     image_handle;
> +
> +   if (chain->base.use_prime_blit)
> +      result = wsi_create_prime_image(&chain->base, create_info,
> &image->base);
>

I don't see use_prime_blit being set anywhere.  Perhaps that comes in a
later patch?  The line is obviously correct, so I'm fine with leaving it.


> +   else
> +      result = wsi_create_native_image(&chain->base, create_info,
> &image->base);
>

This will have to be updated for modifiers.  I'm reasonably happy for it to
be the no-op update for now though KHR_display supporting real modifiers
would be nice. :-)


> +   if (result != VK_SUCCESS)
> +      return result;
> +
> +   ret = drmPrimeFDToHandle(wsi->master_fd, image->base.fd,
> &image_handle);
>

This opens up some interesting questions.  GEM handles are not reference
counted by the kernel.  If the driver that we're running on is using
master_fd, then we shouldn't close our handle because that would close the
handle for the driver as well.  If it's not using master_fd, we should
close the handle to avoid leaking it.  The later is only a worry in the
prime case but given that I saw a line above about use_prime_blit, you have
me scared. :-)

One solution to this connundrum would be to simply never use the master FD
for the Vulkan driver itself and always open a render node.  If handed a
master FD, you could check if it matches your render node and set
use_prime_blit accordingly.  Then the driver and WSI would have separate
handle domains and this problem would be solved.  You could also just open
the master FD twice, once for WSI and once for the driver.


> +
> +   close(image->base.fd);
> +   image->base.fd = -1;
> +
> +   if (ret < 0)
> +      goto fail_handle;
> +
> +   image->chain = chain;
> +   image->state = wsi_image_idle;
> +   image->fb_id = 0;
> +
> +   /* XXX extract depth and bpp from image somehow */
>

You have the format in create_info.  We could add some generic VkFormat
introspection or we can just handle the 2-4 cases we care about directly.


> +   ret = drmModeAddFB(wsi->master_fd, create_info->imageExtent.width,
> create_info->imageExtent.height,
> +                      24, 32, image->base.row_pitch, image_handle,
> &image->fb_id);
>

What happens if we close the handle immediately after calling
drmModeAddFB?  Does the FB become invalid?  If so, then we probably want to
stash the handle so we can clean it up when we destroy the swapchain.
Unless, of course, we're willing to make the assumption (as stated above)
that the driver and WSI are always using the same FD.


> +
> +   if (ret)
> +      goto fail_fb;
> +
> +   return VK_SUCCESS;
> +
> +fail_fb:
> +   /* fall through */
> +
> +fail_handle:
> +   wsi_destroy_image(&chain->base, &image->base);
> +
> +   return VK_ERROR_OUT_OF_HOST_MEMORY;
> +}
> +
> +static void
> +wsi_display_image_finish(struct wsi_swapchain           *drv_chain,
> +                         const VkAllocationCallbacks    *allocator,
> +                         struct wsi_display_image       *image)
> +{
> +   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *)
> drv_chain;
> +
> +   wsi_destroy_image(&chain->base, &image->base);
> +}
> +
> +static VkResult
> +wsi_display_swapchain_destroy(struct wsi_swapchain
> *drv_chain,
> +                              const VkAllocationCallbacks
>  *allocator)
> +{
> +   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *)
> drv_chain;
> +
> +   for (uint32_t i = 0; i < chain->base.image_count; i++)
> +      wsi_display_image_finish(drv_chain, allocator, &chain->images[i]);
> +   vk_free(allocator, chain);
> +   return VK_SUCCESS;
> +}
> +
> +static struct wsi_image *
> +wsi_display_get_wsi_image(struct wsi_swapchain  *drv_chain,
> +                          uint32_t              image_index)
> +{
> +   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *)
> drv_chain;
> +
> +   return &chain->images[image_index].base;
> +}
> +
> +static void
> +wsi_display_idle_old_displaying(struct wsi_display_image *active_image)
> +{
> +   struct wsi_display_swapchain *chain = active_image->chain;
> +
> +   wsi_display_debug("idle everyone but %ld\n", active_image -
> &(chain->images[0]));
> +   for (uint32_t i = 0; i < chain->base.image_count; i++)
> +      if (chain->images[i].state == wsi_image_displaying &&
> &chain->images[i] != active_image) {
> +         wsi_display_debug("idle %d\n", i);
> +         chain->images[i].state = wsi_image_idle;
> +      }
> +}
> +
> +static VkResult
> +_wsi_display_queue_next(struct wsi_swapchain     *drv_chain);
> +
> +static void
> +wsi_display_page_flip_handler2(int              fd,
> +                               unsigned int     frame,
> +                               unsigned int     sec,
> +                               unsigned int     usec,
> +                               uint32_t         crtc_id,
> +                               void             *data)
> +{
> +   struct wsi_display_image     *image = data;
> +
> +   wsi_display_debug("image %ld displayed at %d\n", image -
> &(image->chain->images[0]), frame);
> +   image->state = wsi_image_displaying;
> +   wsi_display_idle_old_displaying(image);
> +   (void) _wsi_display_queue_next(&(image->chain->base));
> +}
> +
> +static void wsi_display_page_flip_handler(int fd, unsigned int frame,
> +                                          unsigned int sec, unsigned int
> usec, void *data)
> +{
> +   wsi_display_page_flip_handler2(fd, frame, sec, usec, 0, data);
> +}
> +
> +static drmEventContext event_context = {
> +   .version = DRM_EVENT_CONTEXT_VERSION,
> +   .page_flip_handler = wsi_display_page_flip_handler,
> +#if DRM_EVENT_CONTEXT_VERSION >= 3
> +   .page_flip_handler2 = wsi_display_page_flip_handler2,
> +#endif
> +};
> +
> +static void *
> +wsi_display_wait_thread(void *data)
> +{
> +   struct wsi_display   *wsi = data;
> +   struct pollfd pollfd = {
> +      .fd = wsi->master_fd,
> +      .events = POLLIN
> +   };
> +   int ret;
> +
> +   pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
> +   for (;;) {
> +      ret = poll(&pollfd, 1, -1);
> +      if (ret > 0) {
> +         pthread_mutex_lock(&wsi->wait_mutex);
> +         (void) drmHandleEvent(wsi->master_fd, &event_context);
> +         pthread_mutex_unlock(&wsi->wait_mutex);
> +         pthread_cond_broadcast(&wsi->wait_cond);
> +      }
> +   }
> +   return NULL;
> +}
> +
> +static int
> +wsi_display_start_wait_thread(struct wsi_display        *wsi)
> +{
> +   if (!wsi->wait_thread) {
> +      int ret = pthread_create(&wsi->wait_thread, NULL,
> wsi_display_wait_thread, wsi);
> +      if (ret)
> +         return ret;
> +   }
> +   return 0;
> +}
> +
> +/* call with wait_mutex held */
>

It might be worth a line in the comment to say that this function does not
guarntee that any particular type of event (or indeed any event at all) has
been processed.  It's just a yield.


> +static int
> +wsi_display_wait_for_event(struct wsi_display           *wsi,
> +                           uint64_t                     timeout_ns)
> +{
> +   int ret;
> +
> +   ret = wsi_display_start_wait_thread(wsi);
> +
> +   if (ret)
> +      return ret;
> +
> +   struct timespec abs_timeout = {
> +      .tv_sec = timeout_ns / ((uint64_t) 1000 * (uint64_t) 1000 *
> (uint64_t) 1000),
> +      .tv_nsec = timeout_ns % ((uint64_t) 1000 * (uint64_t) 1000 *
> (uint64_t) 1000)
> +   };
> +
> +   ret = pthread_cond_timedwait(&wsi->wait_cond, &wsi->wait_mutex,
> &abs_timeout);
>
+
> +   wsi_display_debug("%9ld done waiting for event %d\n", pthread_self(),
> ret);
> +   return ret;
> +}
> +
> +static VkResult
> +wsi_display_acquire_next_image(struct wsi_swapchain     *drv_chain,
> +                               uint64_t                 timeout,
> +                               VkSemaphore              semaphore,
> +                               uint32_t                 *image_index)
> +{
> +   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain
> *)drv_chain;
> +   struct wsi_display           *wsi = chain->wsi;
> +   int                          ret = 0;
> +   VkResult                     result = VK_SUCCESS;
> +
> +   if (timeout != 0 && timeout != UINT64_MAX)
> +      timeout += wsi_get_current_monotonic();
> +
> +   pthread_mutex_lock(&wsi->wait_mutex);
> +   for (;;) {
> +      for (uint32_t i = 0; i < chain->base.image_count; i++) {
> +         if (chain->images[i].state == wsi_image_idle) {
> +            *image_index = i;
> +            wsi_display_debug("image %d available\n", i);
> +            chain->images[i].state = wsi_image_drawing;
> +            result = VK_SUCCESS;
> +            goto done;
> +         }
> +         wsi_display_debug("image %d state %d\n", i,
> chain->images[i].state);
> +      }
> +
> +      if (ret == ETIMEDOUT) {
> +         result = VK_TIMEOUT;
> +         goto done;
> +      }
> +
> +      ret = wsi_display_wait_for_event(wsi, timeout);
> +
> +      if (ret && ret != ETIMEDOUT) {
> +         result = VK_ERROR_OUT_OF_DATE_KHR;
>

I don't see anywhere where you track any sort of per-swapchain error
status.  We need something so that, once you get VK_ERROR_OUT_OF_DATE, most
of the other functions will start bailing immediately with OUT_OF_DATE
instead of assuming the client catches it the first time it's returned.  In
particular, if wsi_display_queue_next fails inside an event handler, we
need to know.  In the X11 WSI patches, we just have a VkResult called
"status" in the swapchain.


> +         goto done;
> +      }
> +   }
> +done:
> +   pthread_mutex_unlock(&wsi->wait_mutex);
> +   return result;
> +}
> +
> +/*
> + * Check whether there are any other connectors driven by this crtc
> + */
> +static bool
> +wsi_display_crtc_solo(struct wsi_display        *wsi,
> +                      drmModeResPtr             mode_res,
> +                      drmModeConnectorPtr       connector,
> +                      uint32_t                  crtc_id)
> +{
> +   int                  c, e;
> +
> +   /* See if any other connectors share the same encoder */
> +   for (c = 0; c < mode_res->count_connectors; c++) {
> +      if (mode_res->connectors[c] == connector->connector_id)
> +         continue;
> +
> +      drmModeConnectorPtr       other_connector =
> drmModeGetConnector(wsi->master_fd, mode_res->connectors[c]);
> +      if (other_connector) {
> +         bool                      match = (other_connector->encoder_id
> == connector->encoder_id);
>

Lots of whitespace in here.  We don't usually try to line stuff up in code
unless there's a good reason.


> +         drmModeFreeConnector(other_connector);
> +         if (match)
> +            return false;
> +      }
> +   }
> +
> +   /* See if any other encoders share the same crtc */
> +   for (e = 0; e < mode_res->count_encoders; e++) {
> +      if (mode_res->encoders[e] == connector->encoder_id)
> +         continue;
> +
> +      drmModeEncoderPtr         other_encoder =
> drmModeGetEncoder(wsi->master_fd, mode_res->encoders[e]);
> +      if (other_encoder) {
> +         bool                      match = (other_encoder->crtc_id ==
> crtc_id);
>

Same here.


> +         drmModeFreeEncoder(other_encoder);
> +         if (match)
> +            return false;
> +      }
> +   }
> +   return true;
> +}
> +
> +/*
> + * Pick a suitable CRTC to drive this connector. Prefer a CRTC which is
> + * currently driving this connector and not any others. Settle for a CRTC
> + * which is currently idle.
> + */
> +static uint32_t
> +wsi_display_select_crtc(struct wsi_display_connector    *connector,
> +                        drmModeResPtr                   mode_res,
> +                        drmModeConnectorPtr             drm_connector)
> +{
> +   struct wsi_display   *wsi = connector->wsi;
> +   int                  c;
> +   uint32_t             crtc_id;
> +
> +   /* See what CRTC is currently driving this connector */
> +   if (drm_connector->encoder_id) {
> +      drmModeEncoderPtr encoder = drmModeGetEncoder(wsi->master_fd,
> drm_connector->encoder_id);
> +      if (encoder) {
> +         crtc_id = encoder->crtc_id;
> +         drmModeFreeEncoder(encoder);
> +         if (crtc_id) {
> +            if (wsi_display_crtc_solo(wsi, mode_res, drm_connector,
> crtc_id))
> +               return crtc_id;
> +         }
> +      }
> +   }
> +   crtc_id = 0;
> +   for (c = 0; crtc_id == 0 && c < mode_res->count_crtcs; c++) {
> +      drmModeCrtcPtr crtc = drmModeGetCrtc(wsi->master_fd,
> mode_res->crtcs[c]);
> +      if (crtc && crtc->buffer_id == 0)
> +         crtc_id = crtc->crtc_id;
> +      drmModeFreeCrtc(crtc);
>

I take it that 0 is not a valid crtc_id?


> +   }
> +   return crtc_id;
> +}
> +
> +static VkResult
> +wsi_display_setup_connector(wsi_display_connector       *connector,
> +                            wsi_display_mode            *display_mode)
> +{
> +   struct wsi_display   *wsi = connector->wsi;
> +   drmModeModeInfoPtr   drm_mode;
> +   drmModeConnectorPtr  drm_connector;
> +   drmModeResPtr        mode_res;
> +   VkResult             result;
> +   int                  m;
> +
> +   if (connector->current_mode == display_mode && connector->crtc_id)
> +      return VK_SUCCESS;
> +
> +   mode_res = drmModeGetResources(wsi->master_fd);
> +   if (!mode_res) {
> +      result = VK_ERROR_INITIALIZATION_FAILED;
>

This looks more like an OUT_OF_DATE to me.  Unless, of course, it's
actually an OUT_OF_HOST_MEMORY.


> +      goto bail;
> +   }
> +
> +   drm_connector = drmModeGetConnectorCurrent(wsi->master_fd,
> connector->id);
> +   if (!drm_connector) {
> +      result = VK_ERROR_INITIALIZATION_FAILED;
>

Same here.


> +      goto bail_mode_res;
> +   }
> +
> +   /* Pick a CRTC if we don't have one */
> +   if (!connector->crtc_id) {
> +      connector->crtc_id = wsi_display_select_crtc(connector, mode_res,
> drm_connector);
> +      if (!connector->crtc_id) {
> +         result = VK_ERROR_OUT_OF_DATE_KHR;
> +         goto bail_connector;
> +      }
> +   }
> +
> +   if (connector->current_mode != display_mode) {
> +
>

Extra new line?


> +      /* Find the drm mode cooresponding to the requested VkDisplayMode */
> +      drm_mode = NULL;
> +      for (m = 0; m < drm_connector->count_modes; m++) {
> +         drm_mode = &drm_connector->modes[m];
> +         if (wsi_display_mode_matches_drm(display_mode, drm_mode))
> +            break;
> +         drm_mode = NULL;
> +      }
> +
> +      if (!drm_mode) {
> +         result = VK_ERROR_OUT_OF_DATE_KHR;
> +         goto bail_connector;
> +      }
> +
> +      connector->current_mode = display_mode;
> +      connector->current_drm_mode = *drm_mode;
> +   }
> +
> +   result = VK_SUCCESS;
> +
> +bail_connector:
> +   drmModeFreeConnector(drm_connector);
> +bail_mode_res:
> +   drmModeFreeResources(mode_res);
> +bail:
> +   return result;
> +
> +}
> +
> +/*
> + * Check to see if the kernel has no flip queued and if there's an image
> + * waiting to be displayed.
> + */
> +static VkResult
> +_wsi_display_queue_next(struct wsi_swapchain     *drv_chain)
> +{
> +   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *)
> drv_chain;
> +   struct wsi_display           *wsi = chain->wsi;
> +   uint32_t                     i;
> +   struct wsi_display_image     *image = NULL;
> +   VkIcdSurfaceDisplay          *surface = chain->surface;
> +   wsi_display_mode             *display_mode =
> wsi_display_mode_from_handle(surface->displayMode);
> +   wsi_display_connector        *connector = display_mode->connector;
> +   int                          ret;
> +   VkResult                     result;
> +
> +   if (wsi->master_fd < 0)
> +      return VK_ERROR_INITIALIZATION_FAILED;
>

Is this possible?  If so, it should be OUT_OF_DATE


> +
> +   if (display_mode != connector->current_mode)
> +      connector->active = false;
> +
> +   for (;;) {
> +      /* Check to see if there is an image to display, or if some image
> is already queued */
> +
> +      for (i = 0; i < chain->base.image_count; i++) {
> +         struct wsi_display_image  *tmp_image = &chain->images[i];
> +
> +         switch (tmp_image->state) {
> +         case wsi_image_flipping:
> +            /* already flipping, don't send another to the kernel yet */
> +            return VK_SUCCESS;
> +         case wsi_image_queued:
> +            /* find the oldest queued */
> +            if (!image || tmp_image->flip_sequence < image->flip_sequence)
> +               image = tmp_image;
> +            break;
> +         default:
> +            break;
> +         }
> +      }
> +
> +      if (!image)
> +         return VK_SUCCESS;
> +
> +      if (connector->active) {
> +         ret = drmModePageFlip(wsi->master_fd, connector->crtc_id,
> image->fb_id,
> +                               DRM_MODE_PAGE_FLIP_EVENT, image);
> +         if (ret == 0) {
> +            image->state = wsi_image_flipping;
> +            return VK_SUCCESS;
> +         }
> +         wsi_display_debug("page flip err %d %s\n", ret, strerror(-ret));
> +      } else
> +         ret = -EINVAL;
>

If one half uses braces, please use braces on the other half.


> +
> +      if (ret) {
> +         switch(-ret) {
> +         case EINVAL:
> +
> +            result = wsi_display_setup_connector(connector,
> display_mode);
> +
> +            if (result != VK_SUCCESS) {
> +               image->state = wsi_image_idle;
> +               return result;
>

As I stated earlier, I think we want some sort of chain->status so that we
can easily know that things have gone out-of-date and we don't end up
retrying just because the client called QueuePresent again.


> +            }
> +
> +            /* XXX allow setting of position */
> +
> +            ret = drmModeSetCrtc(wsi->master_fd, connector->crtc_id,
> image->fb_id, 0, 0,
> +                                 &connector->id, 1,
> &connector->current_drm_mode);
> +
> +            if (ret == 0) {
> +               image->state = wsi_image_displaying;
> +               wsi_display_idle_old_displaying(image);
>

Can we really idle them immediately or do we need to wait until this image
has begun to scan out before returning a new image from AcquireNextImage?
I suppose we do use implicit fencing for all WSI images today so this is
probably ok.  Still, might be worth a TODO comment or something.


> +               connector->active = true;
> +               return VK_SUCCESS;
> +            }
> +            break;
> +         case EACCES:
> +            usleep(1000 * 1000);
>

?


> +            connector->active = false;
> +            break;
> +         default:
> +            connector->active = false;
> +            image->state = wsi_image_idle;
> +            return VK_ERROR_OUT_OF_DATE_KHR;
> +         }
> +      }
> +   }
> +}
> +
> +static VkResult
> +wsi_display_queue_present(struct wsi_swapchain          *drv_chain,
> +                          uint32_t                      image_index,
> +                          const VkPresentRegionKHR      *damage)
> +{
> +   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *)
> drv_chain;
> +   struct wsi_display           *wsi = chain->wsi;
> +   struct wsi_display_image     *image = &chain->images[image_index];
> +   VkResult                     result;
> +
> +   assert(image->state == wsi_image_drawing);
> +   wsi_display_debug("present %d\n", image_index);
> +
> +   pthread_mutex_lock(&wsi->wait_mutex);
> +
>

assert(image->state == wsi_image_idle);


> +   image->flip_sequence = ++chain->flip_sequence;
> +   image->state = wsi_image_queued;
> +
> +   result = _wsi_display_queue_next(drv_chain);
> +
> +   pthread_mutex_unlock(&wsi->wait_mutex);
> +
> +   return result;
> +}
> +
> +static VkResult
> +wsi_display_surface_create_swapchain(VkIcdSurfaceBase
>  *icd_surface,
> +                                     VkDevice
>  device,
> +                                     struct wsi_device
> *wsi_device,
> +                                     int
> local_fd,
> +                                     const VkSwapchainCreateInfoKHR
>  *create_info,
> +                                     const VkAllocationCallbacks
> *allocator,
> +                                     struct wsi_swapchain
>  **swapchain_out)
> +{
> +   struct wsi_display *wsi = (struct wsi_display *)
> wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
> +   VkResult result;
> +
> +   assert(create_info->sType == VK_STRUCTURE_TYPE_SWAPCHAIN_
> CREATE_INFO_KHR);
> +
> +   struct wsi_display_swapchain *chain;
> +   const unsigned num_images = create_info->minImageCount;
> +   size_t size = sizeof(*chain) + num_images * sizeof(chain->images[0]);
> +
> +   chain = vk_alloc(allocator, size, 8,
> +                    VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> +
> +   if (chain == NULL)
> +      return VK_ERROR_OUT_OF_HOST_MEMORY;
> +
> +   result = wsi_swapchain_init(wsi_device, &chain->base, device,
> +                               create_info, allocator);
> +
> +   chain->base.destroy = wsi_display_swapchain_destroy;
> +   chain->base.get_wsi_image = wsi_display_get_wsi_image;
> +   chain->base.acquire_next_image = wsi_display_acquire_next_image;
> +   chain->base.queue_present = wsi_display_queue_present;
> +   chain->base.present_mode = create_info->presentMode;
> +   chain->base.image_count = num_images;
> +
> +   chain->wsi = wsi;
> +
> +   chain->surface = (VkIcdSurfaceDisplay *) icd_surface;
> +
> +   for (uint32_t image = 0; image < chain->base.image_count; image++) {
> +      result = wsi_display_image_init(device, &chain->base, create_info,
> allocator,
> +                                      &chain->images[image]);
> +      if (result != VK_SUCCESS)
> +         goto fail_init_images;
> +   }
> +
> +   *swapchain_out = &chain->base;
> +
> +   return VK_SUCCESS;
> +
> +fail_init_images:
>

You need to clean up if you are only able to create some of the images.


> +   return result;
> +}
> +
> +VkResult
> +wsi_display_init_wsi(struct wsi_device *wsi_device,
> +                     const VkAllocationCallbacks *alloc,
> +                     VkPhysicalDevice physical_device,
>

You can get the physical device from the wsi_device.  No need to pass it in
separately.


> +                     int device_fd)
> +{
> +   struct wsi_display *wsi;
> +   VkResult result;
> +
> +   wsi = vk_alloc(alloc, sizeof(*wsi), 8,
> +                   VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
> +   if (!wsi) {
> +      result = VK_ERROR_OUT_OF_HOST_MEMORY;
> +      goto fail;
> +   }
> +   memset(wsi, '\0', sizeof (*wsi));
> +
> +   wsi->master_fd = -1;
> +   if (drmGetNodeTypeFromFd(device_fd) == DRM_NODE_PRIMARY)
> +      wsi->master_fd = device_fd;
> +   wsi->render_fd = device_fd;
> +
> +   pthread_mutex_init(&wsi->wait_mutex, NULL);
> +   wsi->physical_device = physical_device;
>

I don't see this being used anywhere.  Is it needed?


> +   wsi->alloc = alloc;
> +
> +   LIST_INITHEAD(&wsi->display_modes);
> +   LIST_INITHEAD(&wsi->connectors);
> +
> +   pthread_condattr_t condattr;
> +   int ret;
> +
> +   ret = pthread_mutex_init(&wsi->wait_mutex, NULL);
> +   if (ret) {
> +      result = VK_ERROR_OUT_OF_HOST_MEMORY;
> +      goto fail_mutex;
> +   }
> +
> +   ret = pthread_condattr_init(&condattr);
> +   if (ret) {
> +      result = VK_ERROR_OUT_OF_HOST_MEMORY;
> +      goto fail_condattr;
> +   }
> +
> +   ret = pthread_condattr_setclock(&condattr, CLOCK_MONOTONIC);
> +   if (ret) {
> +      result = VK_ERROR_OUT_OF_HOST_MEMORY;
> +      goto fail_setclock;
> +   }
> +
> +   ret = pthread_cond_init(&wsi->wait_cond, &condattr);
> +   if (ret) {
> +      result = VK_ERROR_OUT_OF_HOST_MEMORY;
> +      goto fail_cond;
> +   }
> +
> +   pthread_condattr_destroy(&condattr);
> +
> +   wsi->base.get_support = wsi_display_surface_get_support;
> +   wsi->base.get_capabilities = wsi_display_surface_get_capabilities;
> +   wsi->base.get_capabilities2 = wsi_display_surface_get_capabilities2;
> +   wsi->base.get_formats = wsi_display_surface_get_formats;
> +   wsi->base.get_formats2 = wsi_display_surface_get_formats2;
> +   wsi->base.get_present_modes = wsi_display_surface_get_present_modes;
> +   wsi->base.create_swapchain = wsi_display_surface_create_swapchain;
> +
> +   wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY] = &wsi->base;
> +
> +   return VK_SUCCESS;
> +
> +fail_cond:
> +fail_setclock:
> +   pthread_condattr_destroy(&condattr);
> +fail_condattr:
> +   pthread_mutex_destroy(&wsi->wait_mutex);
> +fail_mutex:
> +   vk_free(alloc, wsi);
> +fail:
> +   return result;
> +}
> +
> +void
> +wsi_display_finish_wsi(struct wsi_device *wsi_device,
> +                       const VkAllocationCallbacks *alloc)
> +{
> +   struct wsi_display *wsi = (struct wsi_display *)
> wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
> +
> +   if (wsi) {
> +
> +      struct wsi_display_connector *connector, *connector_storage;
> +      LIST_FOR_EACH_ENTRY_SAFE(connector, connector_storage,
> &wsi->connectors, list) {
> +         vk_free(wsi->alloc, connector);
> +      }
> +
> +      struct wsi_display_mode *mode, *mode_storage;
> +      LIST_FOR_EACH_ENTRY_SAFE(mode, mode_storage, &wsi->display_modes,
> list) {
> +         vk_free(wsi->alloc, mode);
> +      }
> +
> +      pthread_mutex_lock(&wsi->wait_mutex);
> +      if (wsi->wait_thread) {
> +         pthread_cancel(wsi->wait_thread);
> +         pthread_join(wsi->wait_thread, NULL);
> +      }
> +      pthread_mutex_unlock(&wsi->wait_mutex);
> +      pthread_mutex_destroy(&wsi->wait_mutex);
> +      pthread_cond_destroy(&wsi->wait_cond);
> +
> +      vk_free(alloc, wsi);
> +   }
> +}
> diff --git a/src/vulkan/wsi/wsi_common_display.h
> b/src/vulkan/wsi/wsi_common_display.h
> new file mode 100644
> index 00000000000..b414a226293
> --- /dev/null
> +++ b/src/vulkan/wsi/wsi_common_display.h
> @@ -0,0 +1,72 @@
> +/*
> + * Copyright © 2017 Keith Packard
> + *
> + * Permission to use, copy, modify, distribute, and sell this software
> and its
> + * documentation for any purpose is hereby granted without fee, provided
> that
> + * the above copyright notice appear in all copies and that both that
> copyright
> + * notice and this permission notice appear in supporting documentation,
> and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no
> representations
> + * about the suitability of this software for any purpose.  It is
> provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT
> OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF
> USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#ifndef WSI_COMMON_DISPLAY_H
> +#define WSI_COMMON_DISPLAY_H
> +
> +#include "wsi_common.h"
> +#include <xf86drm.h>
> +#include <xf86drmMode.h>
> +
> +#define typed_memcpy(dest, src, count) ({ \
> +   STATIC_ASSERT(sizeof(*src) == sizeof(*dest)); \
> +   memcpy((dest), (src), (count) * sizeof(*(src))); \
> +})
> +
> +VkResult
> +wsi_display_get_physical_device_display_properties(VkPhysicalDevice
>        physical_device,
> +                                                   struct wsi_device
>       *wsi_device,
> +                                                   uint32_t
>        *property_count,
> +
>  VkDisplayPropertiesKHR       *properties);
> +VkResult
> +wsi_display_get_physical_device_display_plane_properties(VkPhysicalDevice
>              physical_device,
> +                                                         struct
> wsi_device              *wsi_device,
> +                                                         uint32_t
>                *property_count,
> +
>  VkDisplayPlanePropertiesKHR    *properties);
> +
> +VkResult
> +wsi_display_get_display_plane_supported_displays(VkPhysicalDevice
>        physical_device,
> +                                                 struct wsi_device
>       *wsi_device,
> +                                                 uint32_t
>        plane_index,
> +                                                 uint32_t
>        *display_count,
> +                                                 VkDisplayKHR
>        *displays);
> +VkResult
> +wsi_display_get_display_mode_properties(VkPhysicalDevice
>  physical_device,
> +                                        struct wsi_device
> *wsi_device,
> +                                        VkDisplayKHR
>  display,
> +                                        uint32_t
>  *property_count,
> +                                        VkDisplayModePropertiesKHR
>  *properties);
> +
> +VkResult
> +wsi_get_display_plane_capabilities(VkPhysicalDevice
>  physical_device,
> +                                   struct wsi_device
> *wsi_device,
> +                                    VkDisplayModeKHR
> mode_khr,
> +                                    uint32_t
> plane_index,
> +                                    VkDisplayPlaneCapabilitiesKHR
>  *capabilities);
> +
> +VkResult
> +wsi_create_display_surface(VkInstance instance,
> +                           const VkAllocationCallbacks *pAllocator,
> +                           const VkDisplaySurfaceCreateInfoKHR
> *pCreateInfo,
> +                           VkSurfaceKHR *pSurface);
> +
> +#endif
> diff --git a/src/vulkan/wsi/wsi_common_private.h
> b/src/vulkan/wsi/wsi_common_private.h
> index 503b2a015dc..d38d2efa116 100644
> --- a/src/vulkan/wsi/wsi_common_private.h
> +++ b/src/vulkan/wsi/wsi_common_private.h
> @@ -135,6 +135,16 @@ void wsi_wl_finish_wsi(struct wsi_device *wsi_device,
>                         const VkAllocationCallbacks *alloc);
>
>
> +VkResult
> +wsi_display_init_wsi(struct wsi_device *wsi_device,
> +                     const VkAllocationCallbacks *alloc,
> +                     VkPhysicalDevice physical_device,
> +                     int device_fd);
> +
> +void
> +wsi_display_finish_wsi(struct wsi_device *wsi_device,
> +                       const VkAllocationCallbacks *alloc);
> +
>  #define WSI_DEFINE_NONDISP_HANDLE_CASTS(__wsi_type, __VkType)
>   \
>
>   \
>     static inline struct __wsi_type *
>  \
> --
> 2.15.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Daniel Stone Feb. 24, 2018, 9:43 a.m. UTC | #2
Hi,

On 24 February 2018 at 00:43, Jason Ekstrand <jason@jlekstrand.net> wrote:
> On Tue, Feb 13, 2018 at 4:31 PM, Keith Packard <keithp@keithp.com> wrote:
>> +   image->chain = chain;
>> +   image->state = wsi_image_idle;
>> +   image->fb_id = 0;
>> +
>> +   /* XXX extract depth and bpp from image somehow */
>
> You have the format in create_info.  We could add some generic VkFormat
> introspection or we can just handle the 2-4 cases we care about directly.

Or better, just use drmModeAddFB2 and pass the format directly. That
landed back in 3.2 or so, and I don't think there's anyone trying to
use Vulkan on a kernel from 2011.

Cheers,
Daniel
Keith Packard March 7, 2018, 8:15 p.m. UTC | #3
Jason Ekstrand <jason@jlekstrand.net> writes:

Thanks a million for the intense review. I've pushed 15 tiny patches
that address individual questions in this message. If you want to look
at those separately, that would be great. When we're done, I'll merge
them back into the giant patch.

Sorry for the delay in answering; you asked a hard question about GEM
handles and reference counting which I think I've resolved by
eliminating the ability to share the same fd for modesetting and
rendering.

Let me know if I've deleted too much context from your answers.

> With the exception of NIR (which is a bit odd), we usually use ALL_CAPS for
> enum values.

Thanks, fixed.

> Yes, this does seem like the only reasonable mode for now.  I suppose you
> could check to see if the platform supports ASYNC_FLIP and advertise
> IMMEDIATE in that case.  I know intel doesn't advertise it but I thought
> amdgpu does.

Ok, we'll put that on the wish list, along with MAILBOX (which
applications actually want :-)

> vk_outarray, though I suppose you've probably already made that
> change.

Yup, vk_outarray everywhere now.

> I don't see use_prime_blit being set anywhere.  Perhaps that comes in a
> later patch?  The line is obviously correct, so I'm fine with leaving
> it.

You're right -- this was a cult-n-paste error. I don't support prime at
all, so I've removed this bit.

> This will have to be updated for modifiers.  I'm reasonably happy for it to
> be the no-op update for now though KHR_display supporting real modifiers
> would be nice. :-)

Yeah, "patches welcome", of course. I don't have a requirement for them
on Radeon at this point.

>> +   if (result != VK_SUCCESS)
>> +      return result;
>> +
>> +   ret = drmPrimeFDToHandle(wsi->master_fd, image->base.fd,
>> &image_handle);
>>
>
> This opens up some interesting questions.  GEM handles are not reference
> counted by the kernel.  If the driver that we're running on is using
> master_fd, then we shouldn't close our handle because that would close the
> handle for the driver as well.  If it's not using master_fd, we should
> close the handle to avoid leaking it.  The later is only a worry in the
> prime case but given that I saw a line above about use_prime_blit, you have
> me scared. :-)

Ok, as I mentioned in the header of this message, I've eliminated any
code which talks about potentially sharing master_fd and render_fd. I
removed render_fd from the wsi layer entirely as it is no longer used
anywhere.

In the drivers, when you request KHR_display, I attempt to open the
related primary node and if that succeeds, I pass that to the wsi layer
for use there. That fd is otherwise unused by the driver, and in fact
the driver doesn't need to close it as the wsi layer will.

Obviously having two FDs is "wasteful" at some level as we really don't
need that, but that's what the acquire_xlib extension will have to do
anyways.

> One solution to this connundrum would be to simply never use the master FD
> for the Vulkan driver itself and always open a render node.  If handed a
> master FD, you could check if it matches your render node and set
> use_prime_blit accordingly.  Then the driver and WSI would have separate
> handle domains and this problem would be solved.  You could also just open
> the master FD twice, once for WSI and once for the driver.

Yup, two FDs, one master, one render. That way, the kludge
seen in this patch where it opens the master when you request the
KHR_display extension works the same as the acquire_xlib code in the
later patch.

> You have the format in create_info.  We could add some generic VkFormat
> introspection or we can just handle the 2-4 cases we care about
> directly.

Given that this layer provides the list of valid surface formats which
that image could contain, I've added depth/bpp information to that list
and the image_init code walks over the list of possible formats to find
the associated depth/bpp values.

If the application provides an invalid format, I'm returning
VK_ERROR_DEVICE_LOST as that is a valid error return and I couldn't find
anything else that seemed like a better value. If we get here, the
application made a mistake anyways.

> What happens if we close the handle immediately after calling
> drmModeAddFB?  Does the FB become invalid?  If so, then we probably want to
> stash the handle so we can clean it up when we destroy the swapchain.
> Unless, of course, we're willing to make the assumption (as stated above)
> that the driver and WSI are always using the same FD.

It looks like the FB ends up with a reference to the object, so it would
should be safe to just close the handle at this point.  However, to make
it less dependent on the kernel behavior, I've changed the code to store
the buffer_object handle in the image and then added code to free both
the buffer object and the frame buffer in wsi_display_image_finish.

Thanks for finding the leak.

>> +/* call with wait_mutex held */
>>
>
> It might be worth a line in the comment to say that this function does not
> guarntee that any particular type of event (or indeed any event at all) has
> been processed.  It's just a yield.

Yeah, more comments are always good :-)

> I don't see anywhere where you track any sort of per-swapchain error
> status.  We need something so that, once you get VK_ERROR_OUT_OF_DATE, most
> of the other functions will start bailing immediately with OUT_OF_DATE
> instead of assuming the client catches it the first time it's returned.  In
> particular, if wsi_display_queue_next fails inside an event handler, we
> need to know.  In the X11 WSI patches, we just have a VkResult called
> "status" in the swapchain.

Failure is not an option? In reality, failures in the display backend
should only result from an actual unexpected error condition, unlike the
X backend where windows get destroyed or resized without the
applications knowledge.

However, that doesn't mean we shouldn't handle them correctly. I now
record asynchronous failures in a 'chain->status' value and return that
to applications in acquire_next_image and queue_present.

> Lots of whitespace in here.  We don't usually try to line stuff up in code
> unless there's a good reason.

> Same here.

Fixed.

> I take it that 0 is not a valid crtc_id?

Correct.

>> +   mode_res = drmModeGetResources(wsi->master_fd);
>> +   if (!mode_res) {
>> +      result = VK_ERROR_INITIALIZATION_FAILED;
>>
>
> This looks more like an OUT_OF_DATE to me.  Unless, of course, it's
> actually an OUT_OF_HOST_MEMORY.

Yeah, VK_ERROR_OUT_OF_HOST_MEMORY if errno is ENOMEM, otherwise the
ioctl failed and we've lost our ability to do master operations (vt
switched away, or a revoked lease), in which case OUT_OF_DATE_KHR seems appropriate.

>> +   if (!drm_connector) {
>> +      result = VK_ERROR_INITIALIZATION_FAILED;
>>
>
> Same here.

Same fix.

> Extra new line?

There's a comment afterwards, so I like a bit of whitespace to highlight that.

>> +   if (wsi->master_fd < 0)
>> +      return VK_ERROR_INITIALIZATION_FAILED;
>>
>
> Is this possible?  If so, it should be OUT_OF_DATE

It's only possible if you call into this API without having gotten a
master_fd set, or after calling vkReleaseDisplay (part of the
direct_mode_display extension).

I don't know what the right error is in this case as the application did
something wrong or didn't have the right permissions. It "can't happen"
in normal operation, so OUT_OF_DATE seems inappropriate to me.

Suggestions welcome.

> If one half uses braces, please use braces on the other half.

I found another case doing this 

> As I stated earlier, I think we want some sort of chain->status so that we
> can easily know that things have gone out-of-date and we don't end up
> retrying just because the client called QueuePresent again.

All fixed -- queue_next returns are captured in chain->status and
returned at subsequent calls to queue_present or acquire_next_image.


>> +            ret = drmModeSetCrtc(wsi->master_fd, connector->crtc_id,
>> image->fb_id, 0, 0,
>> +                                 &connector->id, 1,
>> &connector->current_drm_mode);
>> +
>> +            if (ret == 0) {
>> +               image->state = wsi_image_displaying;
>> +               wsi_display_idle_old_displaying(image);
>>
>
> Can we really idle them immediately or do we need to wait until this image
> has begun to scan out before returning a new image from
> AcquireNextImage?

After ModeSetCrtc, the old image is no longer being used. There's no
event delivered that could tell us that the scanout started later anyways.

> I suppose we do use implicit fencing for all WSI images today so this is
> probably ok.  Still, might be worth a TODO comment or something.

Given that we're not going to receive an event telling us that the image
is idle, I'm not sure what we could ever do, but a comment is probably wise.

>> +         case EACCES:
>> +            usleep(1000 * 1000);
>>
>
> ?

We get EACCES when VT switched away, so we just want to idle until VT
switched back. After we get back, we'll get EINVAL from the page flip
ioctl, which will cause us to re-set the mode.

If there were an event delivered on VT switch, I could monitor for that
and make the switch back happen faster.

I'll add a comment.

>> +static VkResult
>> +wsi_display_queue_present(struct wsi_swapchain          *drv_chain,
>> +                          uint32_t                      image_index,
>> +                          const VkPresentRegionKHR      *damage)
>> +{
>> +   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *)
>> drv_chain;
>> +   struct wsi_display           *wsi = chain->wsi;
>> +   struct wsi_display_image     *image = &chain->images[image_index];
>> +   VkResult                     result;
>> +
>> +   assert(image->state == wsi_image_drawing);
>> +   wsi_display_debug("present %d\n", image_index);
>> +
>> +   pthread_mutex_lock(&wsi->wait_mutex);
>> +
>>
>
> assert(image->state == wsi_image_idle);

I don't understand. I'm asserting that you can't queue an image that you
haven't gotten from acquire_next_image, which puts the image in
WSI_IMAGE_DRAWING state.

> You need to clean up if you are only able to create some of the
> images.

Oops.

>> +VkResult
>> +wsi_display_init_wsi(struct wsi_device *wsi_device,
>> +                     const VkAllocationCallbacks *alloc,
>> +                     VkPhysicalDevice physical_device,
>>
>
> You can get the physical device from the wsi_device.  No need to pass it in
> separately.

No, you can't (there's very little in wsi_device), but it turns out that
I never needed this value anyways. I've removed it from the API and from
'struct wsi_display'.

> I don't see this being used anywhere.  Is it needed?

Nope. See above.
Keith Packard March 7, 2018, 8:28 p.m. UTC | #4
Daniel Stone <daniel@fooishbar.org> writes:

> Or better, just use drmModeAddFB2 and pass the format directly. That
> landed back in 3.2 or so, and I don't think there's anyone trying to
> use Vulkan on a kernel from 2011.

Yeah, that's probably a better plan. I've pushed a patch that does this
on top of the long list of patches made in response to Jason's email.

Once he's replied to that, I'll go ahead and smash the new patches back
on top of the original series and re-publish that.

Patch
diff mbox

diff --git a/configure.ac b/configure.ac
index 8ed606c7694..46318365603 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1849,6 +1849,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 b39e2f8ab96..aeb7f5e2917 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'
   else
@@ -257,6 +258,7 @@  if _platforms != ''
   with_platform_wayland = _split.contains('wayland')
   with_platform_drm = _split.contains('drm')
   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_wsi.c b/src/amd/vulkan/radv_wsi.c
index e016e837102..9bdd55ef11c 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->local_fd);
 }
 
 void
diff --git a/src/intel/vulkan/anv_wsi.c b/src/intel/vulkan/anv_wsi.c
index 6082c3dd093..f86d83589ea 100644
--- a/src/intel/vulkan/anv_wsi.c
+++ b/src/intel/vulkan/anv_wsi.c
@@ -39,7 +39,8 @@  anv_init_wsi(struct anv_physical_device *physical_device)
    return wsi_device_init(&physical_device->wsi_device,
                           anv_physical_device_to_handle(physical_device),
                           anv_wsi_proc_addr,
-                          &physical_device->instance->alloc);
+                          &physical_device->instance->alloc,
+                          physical_device->local_fd);
 }
 
 void
diff --git a/src/vulkan/Makefile.am b/src/vulkan/Makefile.am
index 037436c1cd7..c33ac5758f7 100644
--- a/src/vulkan/Makefile.am
+++ b/src/vulkan/Makefile.am
@@ -57,6 +57,13 @@  AM_CPPFLAGS += \
 VULKAN_WSI_SOURCES += $(VULKAN_WSI_X11_FILES)
 endif
 
+if HAVE_PLATFORM_DISPLAY
+AM_CPPFLAGS += \
+	-DVK_USE_PLATFORM_DISPLAY_KHR
+
+VULKAN_WSI_SOURCES += $(VULKAN_WSI_DISPLAY_FILES)
+endif
+
 BUILT_SOURCES += $(VULKAN_WSI_WAYLAND_GENERATED_FILES)
 CLEANFILES = $(BUILT_SOURCES)
 
diff --git a/src/vulkan/Makefile.sources b/src/vulkan/Makefile.sources
index a0a24ce7de8..3642c7662c4 100644
--- a/src/vulkan/Makefile.sources
+++ b/src/vulkan/Makefile.sources
@@ -17,6 +17,10 @@  VULKAN_WSI_X11_FILES := \
 	wsi/wsi_common_x11.c \
 	wsi/wsi_common_x11.h
 
+VULKAN_WSI_DISPLAY_FILES := \
+	wsi/wsi_common_display.c \
+	wsi/wsi_common_display.h
+
 VULKAN_UTIL_FILES := \
 	util/vk_alloc.h \
 	util/vk_debug_report.c \
diff --git a/src/vulkan/wsi/meson.build b/src/vulkan/wsi/meson.build
index bd0fd3cc53e..743631a6113 100644
--- a/src/vulkan/wsi/meson.build
+++ b/src/vulkan/wsi/meson.build
@@ -57,6 +57,16 @@  if with_platform_wayland
   ]
 endif
 
+if with_platform_display
+  vulkan_wsi_args += [
+    '-DVK_USE_PLATFORM_DISPLAY_KHR',
+  ]
+  files_vulkan_wsi += files(
+    'wsi_common_display.c',
+    'wsi_common_display.h',
+  )
+endif
+
 libvulkan_wsi = static_library(
   'vulkan_wsi',
   files_vulkan_wsi,
diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
index 90ed07b7857..c0a285e5814 100644
--- a/src/vulkan/wsi/wsi_common.c
+++ b/src/vulkan/wsi/wsi_common.c
@@ -29,7 +29,8 @@  VkResult
 wsi_device_init(struct wsi_device *wsi,
                 VkPhysicalDevice pdevice,
                 WSI_FN_GetPhysicalDeviceProcAddr proc_addr,
-                const VkAllocationCallbacks *alloc)
+                const VkAllocationCallbacks *alloc,
+                int device_fd)
 {
    VkResult result;
 
@@ -89,6 +90,19 @@  wsi_device_init(struct wsi_device *wsi,
    }
 #endif
 
+#ifdef VK_USE_PLATFORM_DISPLAY_KHR
+   result = wsi_display_init_wsi(wsi, alloc, pdevice, device_fd);
+   if (result != VK_SUCCESS) {
+#ifdef VK_USE_PLATFORM_WAYLAND_KHR
+      wsi_wl_finish_wsi(wsi, alloc);
+#endif
+#ifdef VK_USE_PLATFORM_XCB_KHR
+      wsi_x11_finish_wsi(wsi, alloc);
+#endif
+      return result;
+   }
+#endif
+
    return VK_SUCCESS;
 }
 
@@ -96,6 +110,9 @@  void
 wsi_device_finish(struct wsi_device *wsi,
                   const VkAllocationCallbacks *alloc)
 {
+#ifdef VK_USE_PLATFORM_DISPLAY_KHR
+   wsi_display_finish_wsi(wsi, alloc);
+#endif
 #ifdef VK_USE_PLATFORM_WAYLAND_KHR
    wsi_wl_finish_wsi(wsi, alloc);
 #endif
diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h
index 3e0d3be1c24..1cb6aaebca0 100644
--- a/src/vulkan/wsi/wsi_common.h
+++ b/src/vulkan/wsi/wsi_common.h
@@ -50,7 +50,7 @@  struct wsi_memory_allocate_info {
 
 struct wsi_interface;
 
-#define VK_ICD_WSI_PLATFORM_MAX 5
+#define VK_ICD_WSI_PLATFORM_MAX 6
 
 struct wsi_device {
    VkPhysicalDeviceMemoryProperties memory_props;
@@ -93,7 +93,8 @@  VkResult
 wsi_device_init(struct wsi_device *wsi,
                 VkPhysicalDevice pdevice,
                 WSI_FN_GetPhysicalDeviceProcAddr proc_addr,
-                const VkAllocationCallbacks *alloc);
+                const VkAllocationCallbacks *alloc,
+                int device_fd);
 
 void
 wsi_device_finish(struct wsi_device *wsi,
diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c
new file mode 100644
index 00000000000..2732b1dd721
--- /dev/null
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -0,0 +1,1368 @@ 
+/*
+ * Copyright © 2017 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#include "util/macros.h"
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+#include <string.h>
+#include <fcntl.h>
+#include <poll.h>
+#include <stdbool.h>
+#include <math.h>
+#include <xf86drm.h>
+#include <xf86drmMode.h>
+#include "util/hash_table.h"
+#include "util/list.h"
+
+#include "vk_util.h"
+#include "wsi_common_private.h"
+#include "wsi_common_display.h"
+#include "wsi_common_queue.h"
+
+#if 0
+#define wsi_display_debug(...) fprintf(stderr, __VA_ARGS__)
+#define wsi_display_debug_code(...)     __VA_ARGS__
+#else
+#define wsi_display_debug(...)
+#define wsi_display_debug_code(...)
+#endif
+
+/* These have lifetime equal to the instance, so they effectively
+ * never go away. This means we must keep track of them separately
+ * from all other resources.
+ */
+typedef struct wsi_display_mode {
+   struct list_head             list;
+   struct wsi_display_connector *connector;
+   bool                         valid;          /* was found in most recent poll */
+   bool                         preferred;
+   uint32_t                     clock;          /* in kHz */
+   uint16_t                     hdisplay, hsync_start, hsync_end, htotal, hskew;
+   uint16_t                     vdisplay, vsync_start, vsync_end, vtotal, vscan;
+   uint32_t                     flags;
+} wsi_display_mode;
+
+typedef struct wsi_display_connector {
+   struct list_head             list;
+   struct wsi_display           *wsi;
+   uint32_t                     id;
+   uint32_t                     crtc_id;
+   char                         *name;
+   bool                         connected;
+   bool                         active;
+   wsi_display_mode             *current_mode;
+   drmModeModeInfo              current_drm_mode;
+} wsi_display_connector;
+
+struct wsi_display {
+   struct wsi_interface         base;
+
+   const VkAllocationCallbacks  *alloc;
+   VkPhysicalDevice             physical_device;
+
+   int                          master_fd;
+   int                          render_fd;
+
+   pthread_mutex_t              wait_mutex;
+   pthread_cond_t               wait_cond;
+   pthread_t                    wait_thread;
+
+   struct list_head             connectors;
+
+   struct list_head             display_modes;
+};
+
+enum wsi_image_state {
+   wsi_image_idle,
+   wsi_image_drawing,
+   wsi_image_queued,
+   wsi_image_flipping,
+   wsi_image_displaying
+};
+
+struct wsi_display_image {
+   struct wsi_image             base;
+   struct wsi_display_swapchain *chain;
+   enum wsi_image_state         state;
+   uint32_t                     fb_id;
+   uint64_t                     flip_sequence;
+};
+
+struct wsi_display_swapchain {
+   struct wsi_swapchain         base;
+   struct wsi_display           *wsi;
+   VkIcdSurfaceDisplay          *surface;
+   uint64_t                     flip_sequence;
+   struct wsi_display_image     images[0];
+};
+
+ICD_DEFINE_NONDISP_HANDLE_CASTS(wsi_display_mode, VkDisplayModeKHR)
+ICD_DEFINE_NONDISP_HANDLE_CASTS(wsi_display_connector, VkDisplayKHR)
+
+static bool
+wsi_display_mode_matches_drm(wsi_display_mode   *wsi,
+                             drmModeModeInfoPtr drm)
+{
+   return wsi->clock == drm->clock &&
+      wsi->hdisplay == drm->hdisplay &&
+      wsi->hsync_start == drm->hsync_start &&
+      wsi->hsync_end == drm->hsync_end &&
+      wsi->htotal == drm->htotal &&
+      wsi->hskew == drm->hskew &&
+      wsi->vdisplay == drm->vdisplay &&
+      wsi->vsync_start == drm->vsync_start &&
+      wsi->vsync_end == drm->vsync_end &&
+      wsi->vtotal == drm->vtotal &&
+      wsi->vscan == drm->vscan &&
+      wsi->flags == drm->flags;
+}
+
+static double
+wsi_display_mode_refresh(struct wsi_display_mode        *wsi)
+{
+   return (double) wsi->clock * 1000.0 / ((double) wsi->htotal * (double) wsi->vtotal * (double) (wsi->vscan + 1));
+}
+
+static uint64_t wsi_get_current_monotonic(void)
+{
+   struct timespec tv;
+
+   clock_gettime(CLOCK_MONOTONIC, &tv);
+   return tv.tv_nsec + tv.tv_sec*1000000000ull;
+}
+
+static struct wsi_display_mode *
+wsi_display_find_drm_mode(struct wsi_device                 *wsi_device,
+                          struct wsi_display_connector      *connector,
+                          drmModeModeInfoPtr                mode)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_mode      *display_mode;
+
+   LIST_FOR_EACH_ENTRY(display_mode, &wsi->display_modes, list) {
+      if (display_mode->connector == connector &&
+          wsi_display_mode_matches_drm(display_mode, mode))
+         return display_mode;
+   }
+   return NULL;
+}
+
+static void
+wsi_display_invalidate_connector_modes(struct wsi_device            *wsi_device,
+                                       struct wsi_display_connector *connector)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_mode      *display_mode;
+
+   LIST_FOR_EACH_ENTRY(display_mode, &wsi->display_modes, list)
+      if (display_mode->connector == connector)
+         display_mode->valid = false;
+}
+
+static VkResult
+wsi_display_register_drm_mode(struct wsi_device            *wsi_device,
+                              struct wsi_display_connector *connector,
+                              drmModeModeInfoPtr           drm_mode)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_mode      *display_mode;
+
+   display_mode = wsi_display_find_drm_mode(wsi_device, connector, drm_mode);
+
+   if (display_mode) {
+      display_mode->valid = true;
+      return VK_SUCCESS;
+   }
+
+   display_mode = vk_alloc(wsi->alloc, sizeof (struct wsi_display_mode), 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+   if (!display_mode)
+      return VK_ERROR_OUT_OF_HOST_MEMORY;
+
+   display_mode->connector = connector;
+   display_mode->valid = true;
+   display_mode->preferred = (drm_mode->type & DRM_MODE_TYPE_PREFERRED) != 0;
+   display_mode->clock = drm_mode->clock; /* kHz */
+   display_mode->hdisplay = drm_mode->hdisplay;
+   display_mode->hsync_start = drm_mode->hsync_start;
+   display_mode->hsync_end = drm_mode->hsync_end;
+   display_mode->htotal = drm_mode->htotal;
+   display_mode->hskew = drm_mode->hskew;
+   display_mode->vdisplay = drm_mode->vdisplay;
+   display_mode->vsync_start = drm_mode->vsync_start;
+   display_mode->vsync_end = drm_mode->vsync_end;
+   display_mode->vtotal = drm_mode->vtotal;
+   display_mode->vscan = drm_mode->vscan;
+   display_mode->flags = drm_mode->flags;
+
+   LIST_ADDTAIL(&display_mode->list, &wsi->display_modes);
+   return VK_SUCCESS;
+}
+
+/*
+ * Update our information about a specific connector
+ */
+
+static struct wsi_display_connector *
+wsi_display_find_connector(struct wsi_device    *wsi_device,
+                          uint32_t              connector_id)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_connector *connector;
+
+   connector = NULL;
+   LIST_FOR_EACH_ENTRY(connector, &wsi->connectors, list) {
+      if (connector->id == connector_id)
+         return connector;
+   }
+
+   return NULL;
+}
+
+static struct wsi_display_connector *
+wsi_display_alloc_connector(struct wsi_display  *wsi,
+                            uint32_t            connector_id)
+{
+   struct wsi_display_connector *connector;
+
+   connector = vk_alloc(wsi->alloc, sizeof (struct wsi_display_connector), 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+   memset(connector, '\0', sizeof (*connector));
+   connector->id = connector_id;
+   connector->wsi = wsi;
+   connector->active = false;
+   /* XXX use EDID name */
+   connector->name = "monitor";
+   return connector;
+}
+
+static struct wsi_display_connector *
+wsi_display_get_connector(struct wsi_device             *wsi_device,
+                          uint32_t                      connector_id)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_connector *connector;
+   drmModeConnectorPtr          drm_connector;
+   VkResult                     result;
+   int                          m;
+
+   if (wsi->master_fd < 0)
+      return NULL;
+
+   drm_connector = drmModeGetConnector(wsi->master_fd, connector_id);
+   if (!drm_connector)
+      return NULL;
+
+   connector = wsi_display_find_connector(wsi_device, connector_id);
+
+   if (!connector) {
+      connector = wsi_display_alloc_connector(wsi, connector_id);
+      if (!connector) {
+         drmModeFreeConnector(drm_connector);
+         return NULL;
+      }
+      LIST_ADDTAIL(&connector->list, &wsi->connectors);
+   }
+
+   connector->connected = drm_connector->connection != DRM_MODE_DISCONNECTED;
+
+   /* Mark all connector modes as invalid */
+   wsi_display_invalidate_connector_modes(wsi_device, connector);
+
+   /*
+    * List current modes, adding new ones and marking existing ones as
+    * valid
+    */
+   for (m = 0; m < drm_connector->count_modes; m++) {
+      result = wsi_display_register_drm_mode(wsi_device,
+                                             connector,
+                                             &drm_connector->modes[m]);
+      if (result != VK_SUCCESS) {
+         drmModeFreeConnector(drm_connector);
+         return NULL;
+      }
+   }
+
+   drmModeFreeConnector(drm_connector);
+
+   return connector;
+}
+
+#define MM_PER_PIXEL     (1.0/96.0 * 25.4)
+
+static void
+wsi_display_fill_in_display_properties(struct wsi_device                *wsi_device,
+                                       struct wsi_display_connector     *connector,
+                                       VkDisplayPropertiesKHR           *properties)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_mode      *display_mode, *preferred_mode = NULL;;
+
+   properties->display = wsi_display_connector_to_handle(connector);
+   properties->displayName = connector->name;
+
+   /* Find the preferred mode and assume that's the physical resolution */
+
+   LIST_FOR_EACH_ENTRY(display_mode, &wsi->display_modes, list) {
+      if (display_mode->valid && display_mode->connector == connector && display_mode->preferred) {
+         preferred_mode = display_mode;
+         break;
+      }
+   }
+
+   if (preferred_mode) {
+      properties->physicalResolution.width = preferred_mode->hdisplay;
+      properties->physicalResolution.height = preferred_mode->vdisplay;
+   } else {
+      properties->physicalResolution.width = 1024;
+      properties->physicalResolution.height = 768;
+   }
+
+   /* Make up physical size based on 96dpi */
+   properties->physicalDimensions.width = floor(properties->physicalResolution.width * MM_PER_PIXEL + 0.5);
+   properties->physicalDimensions.height = floor(properties->physicalResolution.height * MM_PER_PIXEL + 0.5);
+
+   properties->supportedTransforms = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR;
+   properties->persistentContent = 0;
+}
+
+/*
+ * Implement vkGetPhysicalDeviceDisplayPropertiesKHR (VK_KHR_display)
+ */
+VkResult
+wsi_display_get_physical_device_display_properties(VkPhysicalDevice             physical_device,
+                                                   struct wsi_device            *wsi_device,
+                                                   uint32_t                     *property_count,
+                                                   VkDisplayPropertiesKHR       *properties)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_connector *connector;
+   int                          c;
+   uint32_t                     connected;
+   uint32_t                     property_count_requested = *property_count;
+   drmModeResPtr                mode_res;
+
+   if (wsi->master_fd < 0)
+      return VK_ERROR_INITIALIZATION_FAILED;
+
+   mode_res = drmModeGetResources(wsi->master_fd);
+
+   if (!mode_res)
+      return VK_ERROR_INITIALIZATION_FAILED;
+
+   connected = 0;
+
+   /* Get current information */
+   for (c = 0; c < mode_res->count_connectors; c++) {
+      connector = wsi_display_get_connector(wsi_device, mode_res->connectors[c]);
+
+      if (!connector) {
+         drmModeFreeResources(mode_res);
+         return VK_ERROR_OUT_OF_HOST_MEMORY;
+      }
+
+      if (connector->connected)
+         connected++;
+   }
+
+   /* Fill in property information if requested */
+   if (properties != NULL) {
+      connected = 0;
+
+      for (c = 0; c < mode_res->count_connectors; c++) {
+         connector  = wsi_display_find_connector(wsi_device, mode_res->connectors[c]);
+
+         if (connector && connector->connected) {
+            if (connected < property_count_requested) {
+               wsi_display_fill_in_display_properties(wsi_device,
+                                                      connector,
+                                                      &properties[connected]);
+            }
+            connected++;
+         }
+      }
+   }
+
+   drmModeFreeResources(mode_res);
+
+   *property_count = connected;
+
+   if (connected > property_count_requested && properties != NULL)
+      return VK_INCOMPLETE;
+
+   return VK_SUCCESS;
+}
+
+/*
+ * Implement vkGetPhysicalDeviceDisplayPlanePropertiesKHR (VK_KHR_display
+ */
+VkResult
+wsi_display_get_physical_device_display_plane_properties(VkPhysicalDevice               physical_device,
+                                                         struct wsi_device              *wsi_device,
+                                                         uint32_t                       *property_count,
+                                                         VkDisplayPlanePropertiesKHR    *properties)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_connector *connector;
+   uint32_t                     property_count_requested = *property_count;
+   int                          c;
+
+   if (!properties)
+      property_count_requested = 0;
+
+   c = 0;
+   LIST_FOR_EACH_ENTRY(connector, &wsi->connectors, list) {
+      if (c < property_count_requested) {
+         if (connector && connector->active) {
+            properties[c].currentDisplay = wsi_display_connector_to_handle(connector);
+            properties[c].currentStackIndex = c;
+         } else {
+            properties[c].currentDisplay = NULL;
+            properties[c].currentStackIndex = 0;
+         }
+      }
+      c++;
+   }
+
+   *property_count = c;
+
+   if (c > property_count_requested && properties != NULL)
+      return VK_INCOMPLETE;
+
+   return VK_SUCCESS;
+}
+
+/*
+ * Implement vkGetDisplayPlaneSupportedDisplaysKHR (VK_KHR_display)
+ */
+
+VkResult
+wsi_display_get_display_plane_supported_displays(VkPhysicalDevice               physical_device,
+                                                 struct wsi_device              *wsi_device,
+                                                 uint32_t                       plane_index,
+                                                 uint32_t                       *display_count,
+                                                 VkDisplayKHR                   *displays)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_connector *connector;
+   int                          c;
+
+
+   if (displays == NULL) {
+      *display_count = 1;
+      return VK_SUCCESS;
+   }
+
+   if (*display_count < 1)
+      return VK_INCOMPLETE;
+
+   c = 0;
+   LIST_FOR_EACH_ENTRY(connector, &wsi->connectors, list) {
+      if (c == plane_index) {
+         *displays = wsi_display_connector_to_handle(connector);
+         *display_count = 1;
+         return VK_SUCCESS;
+      }
+      c++;
+   }
+
+   *displays = 0;
+   *display_count = 0;
+
+   return VK_SUCCESS;
+}
+
+/*
+ * Implement vkGetDisplayModePropertiesKHR (VK_KHR_display)
+ */
+
+VkResult
+wsi_display_get_display_mode_properties(VkPhysicalDevice               physical_device,
+                                        struct wsi_device              *wsi_device,
+                                        VkDisplayKHR                   display,
+                                        uint32_t                       *property_count,
+                                        VkDisplayModePropertiesKHR     *properties)
+{
+   struct wsi_display           *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   struct wsi_display_connector *connector = wsi_display_connector_from_handle(display);
+   int                          i;
+   struct wsi_display_mode      *display_mode;
+   uint32_t                     property_count_requested = *property_count;
+
+   i = 0;
+
+   if (properties == NULL)
+      property_count_requested = 0;
+
+   LIST_FOR_EACH_ENTRY(display_mode, &wsi->display_modes, list) {
+      if (display_mode->valid && display_mode->connector == connector) {
+         if (i < property_count_requested) {
+            properties[i].displayMode = wsi_display_mode_to_handle(display_mode);
+            properties[i].parameters.visibleRegion.width = display_mode->hdisplay;
+            properties[i].parameters.visibleRegion.height = display_mode->vdisplay;
+            properties[i].parameters.refreshRate = (uint32_t) (wsi_display_mode_refresh(display_mode) * 1000 + 0.5);
+         }
+         i++;
+      }
+   }
+
+   *property_count = i;
+
+   if (i > property_count_requested && properties != NULL)
+      return VK_INCOMPLETE;
+
+   return VK_SUCCESS;
+
+}
+
+/*
+ * Implement vkGetDisplayPlaneCapabilities
+ */
+VkResult
+wsi_get_display_plane_capabilities(VkPhysicalDevice                     physical_device,
+                                   struct wsi_device                    *wsi_device,
+                                   VkDisplayModeKHR                     mode_khr,
+                                   uint32_t                             plane_index,
+                                   VkDisplayPlaneCapabilitiesKHR        *capabilities)
+{
+   struct wsi_display_mode      *mode = wsi_display_mode_from_handle(mode_khr);
+
+   /* XXX use actual values */
+   capabilities->supportedAlpha = VK_DISPLAY_PLANE_ALPHA_OPAQUE_BIT_KHR;
+   capabilities->minSrcPosition.x = 0;
+   capabilities->minSrcPosition.y = 0;
+   capabilities->maxSrcPosition.x = 0;
+   capabilities->maxSrcPosition.y = 0;
+   capabilities->minSrcExtent.width = mode->hdisplay;
+   capabilities->minSrcExtent.height = mode->vdisplay;
+   capabilities->maxSrcExtent.width = mode->hdisplay;
+   capabilities->maxSrcExtent.height = mode->vdisplay;
+   capabilities->minDstPosition.x = 0;
+   capabilities->minDstPosition.y = 0;
+   capabilities->maxDstPosition.x = 0;
+   capabilities->maxDstPosition.y = 0;
+   capabilities->minDstExtent.width = mode->hdisplay;
+   capabilities->minDstExtent.height = mode->vdisplay;
+   capabilities->maxDstExtent.width = mode->hdisplay;
+   capabilities->maxDstExtent.height = mode->vdisplay;
+   return VK_SUCCESS;
+}
+
+VkResult
+wsi_create_display_surface(VkInstance instance,
+                           const VkAllocationCallbacks   *allocator,
+                           const VkDisplaySurfaceCreateInfoKHR *create_info,
+                           VkSurfaceKHR *surface_khr)
+{
+   VkIcdSurfaceDisplay *surface;
+
+   surface = vk_alloc(allocator, sizeof *surface, 8,
+                      VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+   if (surface == NULL)
+      return VK_ERROR_OUT_OF_HOST_MEMORY;
+
+   surface->base.platform = VK_ICD_WSI_PLATFORM_DISPLAY;
+
+   surface->displayMode = create_info->displayMode;
+   surface->planeIndex = create_info->planeIndex;
+   surface->planeStackIndex = create_info->planeStackIndex;
+   surface->transform = create_info->transform;
+   surface->globalAlpha = create_info->globalAlpha;
+   surface->alphaMode = create_info->alphaMode;
+   surface->imageExtent = create_info->imageExtent;
+
+   *surface_khr = VkIcdSurfaceBase_to_handle(&surface->base);
+   return VK_SUCCESS;
+}
+
+
+static VkResult
+wsi_display_surface_get_support(VkIcdSurfaceBase *surface,
+                                struct wsi_device *wsi_device,
+                                const VkAllocationCallbacks *allocator,
+                                uint32_t queueFamilyIndex,
+                                int local_fd,
+                                VkBool32* pSupported)
+{
+   *pSupported = VK_TRUE;
+   return VK_SUCCESS;
+}
+
+static VkResult
+wsi_display_surface_get_capabilities(VkIcdSurfaceBase *surface_base,
+                                     VkSurfaceCapabilitiesKHR* caps)
+{
+   VkIcdSurfaceDisplay *surface = (VkIcdSurfaceDisplay *) surface_base;
+   wsi_display_mode *mode = wsi_display_mode_from_handle(surface->displayMode);
+
+   caps->currentExtent.width = mode->hdisplay;
+   caps->currentExtent.height = mode->vdisplay;
+
+   /* XXX Figure out extents based on driver capabilities */
+   caps->maxImageExtent = caps->minImageExtent = caps->currentExtent;
+
+   caps->supportedCompositeAlpha = (VK_COMPOSITE_ALPHA_INHERIT_BIT_KHR |
+                                    VK_COMPOSITE_ALPHA_OPAQUE_BIT_KHR);
+
+   caps->minImageCount = 2;
+   caps->maxImageCount = 0;
+
+   caps->supportedTransforms = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR;
+   caps->currentTransform = VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR;
+   caps->maxImageArrayLayers = 1;
+   caps->supportedUsageFlags =
+      VK_IMAGE_USAGE_TRANSFER_SRC_BIT |
+      VK_IMAGE_USAGE_SAMPLED_BIT |
+      VK_IMAGE_USAGE_TRANSFER_DST_BIT |
+      VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT;
+
+   return VK_SUCCESS;
+}
+
+static VkResult
+wsi_display_surface_get_capabilities2(VkIcdSurfaceBase *icd_surface,
+                                      const void *info_next,
+                                      VkSurfaceCapabilities2KHR *caps)
+{
+   assert(caps->sType == VK_STRUCTURE_TYPE_SURFACE_CAPABILITIES_2_KHR);
+
+   return wsi_display_surface_get_capabilities(icd_surface, &caps->surfaceCapabilities);
+}
+
+static const VkFormat available_surface_formats[] = {
+   VK_FORMAT_B8G8R8A8_SRGB,
+   VK_FORMAT_B8G8R8A8_UNORM,
+};
+
+static VkResult
+wsi_display_surface_get_formats(VkIcdSurfaceBase        *icd_surface,
+                                struct wsi_device       *wsi_device,
+                                uint32_t                *surface_format_count,
+                                VkSurfaceFormatKHR      *surface_formats)
+{
+   VK_OUTARRAY_MAKE(out, surface_formats, surface_format_count);
+
+   for (unsigned i = 0; i < ARRAY_SIZE(available_surface_formats); i++) {
+      vk_outarray_append(&out, f) {
+         f->format = available_surface_formats[i];
+         f->colorSpace = VK_COLORSPACE_SRGB_NONLINEAR_KHR;
+      }
+   }
+
+   return vk_outarray_status(&out);
+}
+
+static VkResult
+wsi_display_surface_get_formats2(VkIcdSurfaceBase *surface,
+                                 struct wsi_device *wsi_device,
+                                 const void *info_next,
+                                 uint32_t *surface_format_count,
+                                 VkSurfaceFormat2KHR *surface_formats)
+{
+   VK_OUTARRAY_MAKE(out, surface_formats, surface_format_count);
+
+   for (unsigned i = 0; i < ARRAY_SIZE(available_surface_formats); i++) {
+      vk_outarray_append(&out, f) {
+         assert(f->sType == VK_STRUCTURE_TYPE_SURFACE_FORMAT_2_KHR);
+         f->surfaceFormat.format = available_surface_formats[i];
+         f->surfaceFormat.colorSpace = VK_COLORSPACE_SRGB_NONLINEAR_KHR;
+      }
+   }
+
+   return vk_outarray_status(&out);
+}
+
+static const VkPresentModeKHR available_present_modes[] = {
+   VK_PRESENT_MODE_FIFO_KHR,
+};
+
+static VkResult
+wsi_display_surface_get_present_modes(VkIcdSurfaceBase  *surface,
+                                      uint32_t          *present_mode_count,
+                                      VkPresentModeKHR  *present_modes)
+{
+   if (present_modes == NULL) {
+      *present_mode_count = ARRAY_SIZE(available_present_modes);
+      return VK_SUCCESS;
+   }
+
+   *present_mode_count = MIN2(*present_mode_count, ARRAY_SIZE(available_present_modes));
+   typed_memcpy(present_modes, available_present_modes, *present_mode_count);
+
+   if (*present_mode_count < ARRAY_SIZE(available_present_modes))
+      return VK_INCOMPLETE;
+   return VK_SUCCESS;
+}
+
+static VkResult
+wsi_display_image_init(VkDevice                         device_h,
+                       struct wsi_swapchain             *drv_chain,
+                       const VkSwapchainCreateInfoKHR   *create_info,
+                       const VkAllocationCallbacks      *allocator,
+                       struct wsi_display_image         *image)
+{
+   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *) drv_chain;
+   struct wsi_display           *wsi = chain->wsi;
+   VkResult                     result;
+   int                          ret;
+   uint32_t                     image_handle;
+
+   if (chain->base.use_prime_blit)
+      result = wsi_create_prime_image(&chain->base, create_info, &image->base);
+   else
+      result = wsi_create_native_image(&chain->base, create_info, &image->base);
+   if (result != VK_SUCCESS)
+      return result;
+
+   ret = drmPrimeFDToHandle(wsi->master_fd, image->base.fd, &image_handle);
+
+   close(image->base.fd);
+   image->base.fd = -1;
+
+   if (ret < 0)
+      goto fail_handle;
+
+   image->chain = chain;
+   image->state = wsi_image_idle;
+   image->fb_id = 0;
+
+   /* XXX extract depth and bpp from image somehow */
+   ret = drmModeAddFB(wsi->master_fd, create_info->imageExtent.width, create_info->imageExtent.height,
+                      24, 32, image->base.row_pitch, image_handle, &image->fb_id);
+
+   if (ret)
+      goto fail_fb;
+
+   return VK_SUCCESS;
+
+fail_fb:
+   /* fall through */
+
+fail_handle:
+   wsi_destroy_image(&chain->base, &image->base);
+
+   return VK_ERROR_OUT_OF_HOST_MEMORY;
+}
+
+static void
+wsi_display_image_finish(struct wsi_swapchain           *drv_chain,
+                         const VkAllocationCallbacks    *allocator,
+                         struct wsi_display_image       *image)
+{
+   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *) drv_chain;
+
+   wsi_destroy_image(&chain->base, &image->base);
+}
+
+static VkResult
+wsi_display_swapchain_destroy(struct wsi_swapchain              *drv_chain,
+                              const VkAllocationCallbacks       *allocator)
+{
+   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *) drv_chain;
+
+   for (uint32_t i = 0; i < chain->base.image_count; i++)
+      wsi_display_image_finish(drv_chain, allocator, &chain->images[i]);
+   vk_free(allocator, chain);
+   return VK_SUCCESS;
+}
+
+static struct wsi_image *
+wsi_display_get_wsi_image(struct wsi_swapchain  *drv_chain,
+                          uint32_t              image_index)
+{
+   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *) drv_chain;
+
+   return &chain->images[image_index].base;
+}
+
+static void
+wsi_display_idle_old_displaying(struct wsi_display_image *active_image)
+{
+   struct wsi_display_swapchain *chain = active_image->chain;
+
+   wsi_display_debug("idle everyone but %ld\n", active_image - &(chain->images[0]));
+   for (uint32_t i = 0; i < chain->base.image_count; i++)
+      if (chain->images[i].state == wsi_image_displaying && &chain->images[i] != active_image) {
+         wsi_display_debug("idle %d\n", i);
+         chain->images[i].state = wsi_image_idle;
+      }
+}
+
+static VkResult
+_wsi_display_queue_next(struct wsi_swapchain     *drv_chain);
+
+static void
+wsi_display_page_flip_handler2(int              fd,
+                               unsigned int     frame,
+                               unsigned int     sec,
+                               unsigned int     usec,
+                               uint32_t         crtc_id,
+                               void             *data)
+{
+   struct wsi_display_image     *image = data;
+
+   wsi_display_debug("image %ld displayed at %d\n", image - &(image->chain->images[0]), frame);
+   image->state = wsi_image_displaying;
+   wsi_display_idle_old_displaying(image);
+   (void) _wsi_display_queue_next(&(image->chain->base));
+}
+
+static void wsi_display_page_flip_handler(int fd, unsigned int frame,
+                                          unsigned int sec, unsigned int usec, void *data)
+{
+   wsi_display_page_flip_handler2(fd, frame, sec, usec, 0, data);
+}
+
+static drmEventContext event_context = {
+   .version = DRM_EVENT_CONTEXT_VERSION,
+   .page_flip_handler = wsi_display_page_flip_handler,
+#if DRM_EVENT_CONTEXT_VERSION >= 3
+   .page_flip_handler2 = wsi_display_page_flip_handler2,
+#endif
+};
+
+static void *
+wsi_display_wait_thread(void *data)
+{
+   struct wsi_display   *wsi = data;
+   struct pollfd pollfd = {
+      .fd = wsi->master_fd,
+      .events = POLLIN
+   };
+   int ret;
+
+   pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+   for (;;) {
+      ret = poll(&pollfd, 1, -1);
+      if (ret > 0) {
+         pthread_mutex_lock(&wsi->wait_mutex);
+         (void) drmHandleEvent(wsi->master_fd, &event_context);
+         pthread_mutex_unlock(&wsi->wait_mutex);
+         pthread_cond_broadcast(&wsi->wait_cond);
+      }
+   }
+   return NULL;
+}
+
+static int
+wsi_display_start_wait_thread(struct wsi_display        *wsi)
+{
+   if (!wsi->wait_thread) {
+      int ret = pthread_create(&wsi->wait_thread, NULL, wsi_display_wait_thread, wsi);
+      if (ret)
+         return ret;
+   }
+   return 0;
+}
+
+/* call with wait_mutex held */
+static int
+wsi_display_wait_for_event(struct wsi_display           *wsi,
+                           uint64_t                     timeout_ns)
+{
+   int ret;
+
+   ret = wsi_display_start_wait_thread(wsi);
+
+   if (ret)
+      return ret;
+
+   struct timespec abs_timeout = {
+      .tv_sec = timeout_ns / ((uint64_t) 1000 * (uint64_t) 1000 * (uint64_t) 1000),
+      .tv_nsec = timeout_ns % ((uint64_t) 1000 * (uint64_t) 1000 * (uint64_t) 1000)
+   };
+
+   ret = pthread_cond_timedwait(&wsi->wait_cond, &wsi->wait_mutex, &abs_timeout);
+
+   wsi_display_debug("%9ld done waiting for event %d\n", pthread_self(), ret);
+   return ret;
+}
+
+static VkResult
+wsi_display_acquire_next_image(struct wsi_swapchain     *drv_chain,
+                               uint64_t                 timeout,
+                               VkSemaphore              semaphore,
+                               uint32_t                 *image_index)
+{
+   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *)drv_chain;
+   struct wsi_display           *wsi = chain->wsi;
+   int                          ret = 0;
+   VkResult                     result = VK_SUCCESS;
+
+   if (timeout != 0 && timeout != UINT64_MAX)
+      timeout += wsi_get_current_monotonic();
+
+   pthread_mutex_lock(&wsi->wait_mutex);
+   for (;;) {
+      for (uint32_t i = 0; i < chain->base.image_count; i++) {
+         if (chain->images[i].state == wsi_image_idle) {
+            *image_index = i;
+            wsi_display_debug("image %d available\n", i);
+            chain->images[i].state = wsi_image_drawing;
+            result = VK_SUCCESS;
+            goto done;
+         }
+         wsi_display_debug("image %d state %d\n", i, chain->images[i].state);
+      }
+
+      if (ret == ETIMEDOUT) {
+         result = VK_TIMEOUT;
+         goto done;
+      }
+
+      ret = wsi_display_wait_for_event(wsi, timeout);
+
+      if (ret && ret != ETIMEDOUT) {
+         result = VK_ERROR_OUT_OF_DATE_KHR;
+         goto done;
+      }
+   }
+done:
+   pthread_mutex_unlock(&wsi->wait_mutex);
+   return result;
+}
+
+/*
+ * Check whether there are any other connectors driven by this crtc
+ */
+static bool
+wsi_display_crtc_solo(struct wsi_display        *wsi,
+                      drmModeResPtr             mode_res,
+                      drmModeConnectorPtr       connector,
+                      uint32_t                  crtc_id)
+{
+   int                  c, e;
+
+   /* See if any other connectors share the same encoder */
+   for (c = 0; c < mode_res->count_connectors; c++) {
+      if (mode_res->connectors[c] == connector->connector_id)
+         continue;
+
+      drmModeConnectorPtr       other_connector = drmModeGetConnector(wsi->master_fd, mode_res->connectors[c]);
+      if (other_connector) {
+         bool                      match = (other_connector->encoder_id == connector->encoder_id);
+         drmModeFreeConnector(other_connector);
+         if (match)
+            return false;
+      }
+   }
+
+   /* See if any other encoders share the same crtc */
+   for (e = 0; e < mode_res->count_encoders; e++) {
+      if (mode_res->encoders[e] == connector->encoder_id)
+         continue;
+
+      drmModeEncoderPtr         other_encoder = drmModeGetEncoder(wsi->master_fd, mode_res->encoders[e]);
+      if (other_encoder) {
+         bool                      match = (other_encoder->crtc_id == crtc_id);
+         drmModeFreeEncoder(other_encoder);
+         if (match)
+            return false;
+      }
+   }
+   return true;
+}
+
+/*
+ * Pick a suitable CRTC to drive this connector. Prefer a CRTC which is
+ * currently driving this connector and not any others. Settle for a CRTC
+ * which is currently idle.
+ */
+static uint32_t
+wsi_display_select_crtc(struct wsi_display_connector    *connector,
+                        drmModeResPtr                   mode_res,
+                        drmModeConnectorPtr             drm_connector)
+{
+   struct wsi_display   *wsi = connector->wsi;
+   int                  c;
+   uint32_t             crtc_id;
+
+   /* See what CRTC is currently driving this connector */
+   if (drm_connector->encoder_id) {
+      drmModeEncoderPtr encoder = drmModeGetEncoder(wsi->master_fd, drm_connector->encoder_id);
+      if (encoder) {
+         crtc_id = encoder->crtc_id;
+         drmModeFreeEncoder(encoder);
+         if (crtc_id) {
+            if (wsi_display_crtc_solo(wsi, mode_res, drm_connector, crtc_id))
+               return crtc_id;
+         }
+      }
+   }
+   crtc_id = 0;
+   for (c = 0; crtc_id == 0 && c < mode_res->count_crtcs; c++) {
+      drmModeCrtcPtr crtc = drmModeGetCrtc(wsi->master_fd, mode_res->crtcs[c]);
+      if (crtc && crtc->buffer_id == 0)
+         crtc_id = crtc->crtc_id;
+      drmModeFreeCrtc(crtc);
+   }
+   return crtc_id;
+}
+
+static VkResult
+wsi_display_setup_connector(wsi_display_connector       *connector,
+                            wsi_display_mode            *display_mode)
+{
+   struct wsi_display   *wsi = connector->wsi;
+   drmModeModeInfoPtr   drm_mode;
+   drmModeConnectorPtr  drm_connector;
+   drmModeResPtr        mode_res;
+   VkResult             result;
+   int                  m;
+
+   if (connector->current_mode == display_mode && connector->crtc_id)
+      return VK_SUCCESS;
+
+   mode_res = drmModeGetResources(wsi->master_fd);
+   if (!mode_res) {
+      result = VK_ERROR_INITIALIZATION_FAILED;
+      goto bail;
+   }
+
+   drm_connector = drmModeGetConnectorCurrent(wsi->master_fd, connector->id);
+   if (!drm_connector) {
+      result = VK_ERROR_INITIALIZATION_FAILED;
+      goto bail_mode_res;
+   }
+
+   /* Pick a CRTC if we don't have one */
+   if (!connector->crtc_id) {
+      connector->crtc_id = wsi_display_select_crtc(connector, mode_res, drm_connector);
+      if (!connector->crtc_id) {
+         result = VK_ERROR_OUT_OF_DATE_KHR;
+         goto bail_connector;
+      }
+   }
+
+   if (connector->current_mode != display_mode) {
+
+      /* Find the drm mode cooresponding to the requested VkDisplayMode */
+      drm_mode = NULL;
+      for (m = 0; m < drm_connector->count_modes; m++) {
+         drm_mode = &drm_connector->modes[m];
+         if (wsi_display_mode_matches_drm(display_mode, drm_mode))
+            break;
+         drm_mode = NULL;
+      }
+
+      if (!drm_mode) {
+         result = VK_ERROR_OUT_OF_DATE_KHR;
+         goto bail_connector;
+      }
+
+      connector->current_mode = display_mode;
+      connector->current_drm_mode = *drm_mode;
+   }
+
+   result = VK_SUCCESS;
+
+bail_connector:
+   drmModeFreeConnector(drm_connector);
+bail_mode_res:
+   drmModeFreeResources(mode_res);
+bail:
+   return result;
+
+}
+
+/*
+ * Check to see if the kernel has no flip queued and if there's an image
+ * waiting to be displayed.
+ */
+static VkResult
+_wsi_display_queue_next(struct wsi_swapchain     *drv_chain)
+{
+   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *) drv_chain;
+   struct wsi_display           *wsi = chain->wsi;
+   uint32_t                     i;
+   struct wsi_display_image     *image = NULL;
+   VkIcdSurfaceDisplay          *surface = chain->surface;
+   wsi_display_mode             *display_mode = wsi_display_mode_from_handle(surface->displayMode);
+   wsi_display_connector        *connector = display_mode->connector;
+   int                          ret;
+   VkResult                     result;
+
+   if (wsi->master_fd < 0)
+      return VK_ERROR_INITIALIZATION_FAILED;
+
+   if (display_mode != connector->current_mode)
+      connector->active = false;
+
+   for (;;) {
+      /* Check to see if there is an image to display, or if some image is already queued */
+
+      for (i = 0; i < chain->base.image_count; i++) {
+         struct wsi_display_image  *tmp_image = &chain->images[i];
+
+         switch (tmp_image->state) {
+         case wsi_image_flipping:
+            /* already flipping, don't send another to the kernel yet */
+            return VK_SUCCESS;
+         case wsi_image_queued:
+            /* find the oldest queued */
+            if (!image || tmp_image->flip_sequence < image->flip_sequence)
+               image = tmp_image;
+            break;
+         default:
+            break;
+         }
+      }
+
+      if (!image)
+         return VK_SUCCESS;
+
+      if (connector->active) {
+         ret = drmModePageFlip(wsi->master_fd, connector->crtc_id, image->fb_id,
+                               DRM_MODE_PAGE_FLIP_EVENT, image);
+         if (ret == 0) {
+            image->state = wsi_image_flipping;
+            return VK_SUCCESS;
+         }
+         wsi_display_debug("page flip err %d %s\n", ret, strerror(-ret));
+      } else
+         ret = -EINVAL;
+
+      if (ret) {
+         switch(-ret) {
+         case EINVAL:
+
+            result = wsi_display_setup_connector(connector, display_mode);
+
+            if (result != VK_SUCCESS) {
+               image->state = wsi_image_idle;
+               return result;
+            }
+
+            /* XXX allow setting of position */
+
+            ret = drmModeSetCrtc(wsi->master_fd, connector->crtc_id, image->fb_id, 0, 0,
+                                 &connector->id, 1, &connector->current_drm_mode);
+
+            if (ret == 0) {
+               image->state = wsi_image_displaying;
+               wsi_display_idle_old_displaying(image);
+               connector->active = true;
+               return VK_SUCCESS;
+            }
+            break;
+         case EACCES:
+            usleep(1000 * 1000);
+            connector->active = false;
+            break;
+         default:
+            connector->active = false;
+            image->state = wsi_image_idle;
+            return VK_ERROR_OUT_OF_DATE_KHR;
+         }
+      }
+   }
+}
+
+static VkResult
+wsi_display_queue_present(struct wsi_swapchain          *drv_chain,
+                          uint32_t                      image_index,
+                          const VkPresentRegionKHR      *damage)
+{
+   struct wsi_display_swapchain *chain = (struct wsi_display_swapchain *) drv_chain;
+   struct wsi_display           *wsi = chain->wsi;
+   struct wsi_display_image     *image = &chain->images[image_index];
+   VkResult                     result;
+
+   assert(image->state == wsi_image_drawing);
+   wsi_display_debug("present %d\n", image_index);
+
+   pthread_mutex_lock(&wsi->wait_mutex);
+
+   image->flip_sequence = ++chain->flip_sequence;
+   image->state = wsi_image_queued;
+
+   result = _wsi_display_queue_next(drv_chain);
+
+   pthread_mutex_unlock(&wsi->wait_mutex);
+
+   return result;
+}
+
+static VkResult
+wsi_display_surface_create_swapchain(VkIcdSurfaceBase                   *icd_surface,
+                                     VkDevice                           device,
+                                     struct wsi_device                  *wsi_device,
+                                     int                                local_fd,
+                                     const VkSwapchainCreateInfoKHR     *create_info,
+                                     const VkAllocationCallbacks        *allocator,
+                                     struct wsi_swapchain               **swapchain_out)
+{
+   struct wsi_display *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+   VkResult result;
+
+   assert(create_info->sType == VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR);
+
+   struct wsi_display_swapchain *chain;
+   const unsigned num_images = create_info->minImageCount;
+   size_t size = sizeof(*chain) + num_images * sizeof(chain->images[0]);
+
+   chain = vk_alloc(allocator, size, 8,
+                    VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+
+   if (chain == NULL)
+      return VK_ERROR_OUT_OF_HOST_MEMORY;
+
+   result = wsi_swapchain_init(wsi_device, &chain->base, device,
+                               create_info, allocator);
+
+   chain->base.destroy = wsi_display_swapchain_destroy;
+   chain->base.get_wsi_image = wsi_display_get_wsi_image;
+   chain->base.acquire_next_image = wsi_display_acquire_next_image;
+   chain->base.queue_present = wsi_display_queue_present;
+   chain->base.present_mode = create_info->presentMode;
+   chain->base.image_count = num_images;
+
+   chain->wsi = wsi;
+
+   chain->surface = (VkIcdSurfaceDisplay *) icd_surface;
+
+   for (uint32_t image = 0; image < chain->base.image_count; image++) {
+      result = wsi_display_image_init(device, &chain->base, create_info, allocator,
+                                      &chain->images[image]);
+      if (result != VK_SUCCESS)
+         goto fail_init_images;
+   }
+
+   *swapchain_out = &chain->base;
+
+   return VK_SUCCESS;
+
+fail_init_images:
+   return result;
+}
+
+VkResult
+wsi_display_init_wsi(struct wsi_device *wsi_device,
+                     const VkAllocationCallbacks *alloc,
+                     VkPhysicalDevice physical_device,
+                     int device_fd)
+{
+   struct wsi_display *wsi;
+   VkResult result;
+
+   wsi = vk_alloc(alloc, sizeof(*wsi), 8,
+                   VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
+   if (!wsi) {
+      result = VK_ERROR_OUT_OF_HOST_MEMORY;
+      goto fail;
+   }
+   memset(wsi, '\0', sizeof (*wsi));
+
+   wsi->master_fd = -1;
+   if (drmGetNodeTypeFromFd(device_fd) == DRM_NODE_PRIMARY)
+      wsi->master_fd = device_fd;
+   wsi->render_fd = device_fd;
+
+   pthread_mutex_init(&wsi->wait_mutex, NULL);
+   wsi->physical_device = physical_device;
+   wsi->alloc = alloc;
+
+   LIST_INITHEAD(&wsi->display_modes);
+   LIST_INITHEAD(&wsi->connectors);
+
+   pthread_condattr_t condattr;
+   int ret;
+
+   ret = pthread_mutex_init(&wsi->wait_mutex, NULL);
+   if (ret) {
+      result = VK_ERROR_OUT_OF_HOST_MEMORY;
+      goto fail_mutex;
+   }
+
+   ret = pthread_condattr_init(&condattr);
+   if (ret) {
+      result = VK_ERROR_OUT_OF_HOST_MEMORY;
+      goto fail_condattr;
+   }
+
+   ret = pthread_condattr_setclock(&condattr, CLOCK_MONOTONIC);
+   if (ret) {
+      result = VK_ERROR_OUT_OF_HOST_MEMORY;
+      goto fail_setclock;
+   }
+
+   ret = pthread_cond_init(&wsi->wait_cond, &condattr);
+   if (ret) {
+      result = VK_ERROR_OUT_OF_HOST_MEMORY;
+      goto fail_cond;
+   }
+
+   pthread_condattr_destroy(&condattr);
+
+   wsi->base.get_support = wsi_display_surface_get_support;
+   wsi->base.get_capabilities = wsi_display_surface_get_capabilities;
+   wsi->base.get_capabilities2 = wsi_display_surface_get_capabilities2;
+   wsi->base.get_formats = wsi_display_surface_get_formats;
+   wsi->base.get_formats2 = wsi_display_surface_get_formats2;
+   wsi->base.get_present_modes = wsi_display_surface_get_present_modes;
+   wsi->base.create_swapchain = wsi_display_surface_create_swapchain;
+
+   wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY] = &wsi->base;
+
+   return VK_SUCCESS;
+
+fail_cond:
+fail_setclock:
+   pthread_condattr_destroy(&condattr);
+fail_condattr:
+   pthread_mutex_destroy(&wsi->wait_mutex);
+fail_mutex:
+   vk_free(alloc, wsi);
+fail:
+   return result;
+}
+
+void
+wsi_display_finish_wsi(struct wsi_device *wsi_device,
+                       const VkAllocationCallbacks *alloc)
+{
+   struct wsi_display *wsi = (struct wsi_display *) wsi_device->wsi[VK_ICD_WSI_PLATFORM_DISPLAY];
+
+   if (wsi) {
+
+      struct wsi_display_connector *connector, *connector_storage;
+      LIST_FOR_EACH_ENTRY_SAFE(connector, connector_storage, &wsi->connectors, list) {
+         vk_free(wsi->alloc, connector);
+      }
+
+      struct wsi_display_mode *mode, *mode_storage;
+      LIST_FOR_EACH_ENTRY_SAFE(mode, mode_storage, &wsi->display_modes, list) {
+         vk_free(wsi->alloc, mode);
+      }
+
+      pthread_mutex_lock(&wsi->wait_mutex);
+      if (wsi->wait_thread) {
+         pthread_cancel(wsi->wait_thread);
+         pthread_join(wsi->wait_thread, NULL);
+      }
+      pthread_mutex_unlock(&wsi->wait_mutex);
+      pthread_mutex_destroy(&wsi->wait_mutex);
+      pthread_cond_destroy(&wsi->wait_cond);
+
+      vk_free(alloc, wsi);
+   }
+}
diff --git a/src/vulkan/wsi/wsi_common_display.h b/src/vulkan/wsi/wsi_common_display.h
new file mode 100644
index 00000000000..b414a226293
--- /dev/null
+++ b/src/vulkan/wsi/wsi_common_display.h
@@ -0,0 +1,72 @@ 
+/*
+ * Copyright © 2017 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifndef WSI_COMMON_DISPLAY_H
+#define WSI_COMMON_DISPLAY_H
+
+#include "wsi_common.h"
+#include <xf86drm.h>
+#include <xf86drmMode.h>
+
+#define typed_memcpy(dest, src, count) ({ \
+   STATIC_ASSERT(sizeof(*src) == sizeof(*dest)); \
+   memcpy((dest), (src), (count) * sizeof(*(src))); \
+})
+
+VkResult
+wsi_display_get_physical_device_display_properties(VkPhysicalDevice             physical_device,
+                                                   struct wsi_device            *wsi_device,
+                                                   uint32_t                     *property_count,
+                                                   VkDisplayPropertiesKHR       *properties);
+VkResult
+wsi_display_get_physical_device_display_plane_properties(VkPhysicalDevice               physical_device,
+                                                         struct wsi_device              *wsi_device,
+                                                         uint32_t                       *property_count,
+                                                         VkDisplayPlanePropertiesKHR    *properties);
+
+VkResult
+wsi_display_get_display_plane_supported_displays(VkPhysicalDevice               physical_device,
+                                                 struct wsi_device              *wsi_device,
+                                                 uint32_t                       plane_index,
+                                                 uint32_t                       *display_count,
+                                                 VkDisplayKHR                   *displays);
+VkResult
+wsi_display_get_display_mode_properties(VkPhysicalDevice               physical_device,
+                                        struct wsi_device              *wsi_device,
+                                        VkDisplayKHR                   display,
+                                        uint32_t                       *property_count,
+                                        VkDisplayModePropertiesKHR     *properties);
+
+VkResult
+wsi_get_display_plane_capabilities(VkPhysicalDevice                     physical_device,
+                                   struct wsi_device                    *wsi_device,
+                                    VkDisplayModeKHR                    mode_khr,
+                                    uint32_t                            plane_index,
+                                    VkDisplayPlaneCapabilitiesKHR       *capabilities);
+
+VkResult
+wsi_create_display_surface(VkInstance instance,
+                           const VkAllocationCallbacks *pAllocator,
+                           const VkDisplaySurfaceCreateInfoKHR *pCreateInfo,
+                           VkSurfaceKHR *pSurface);
+
+#endif
diff --git a/src/vulkan/wsi/wsi_common_private.h b/src/vulkan/wsi/wsi_common_private.h
index 503b2a015dc..d38d2efa116 100644
--- a/src/vulkan/wsi/wsi_common_private.h
+++ b/src/vulkan/wsi/wsi_common_private.h
@@ -135,6 +135,16 @@  void wsi_wl_finish_wsi(struct wsi_device *wsi_device,
                        const VkAllocationCallbacks *alloc);
 
 
+VkResult
+wsi_display_init_wsi(struct wsi_device *wsi_device,
+                     const VkAllocationCallbacks *alloc,
+                     VkPhysicalDevice physical_device,
+                     int device_fd);
+
+void
+wsi_display_finish_wsi(struct wsi_device *wsi_device,
+                       const VkAllocationCallbacks *alloc);
+
 #define WSI_DEFINE_NONDISP_HANDLE_CASTS(__wsi_type, __VkType)              \
                                                                            \
    static inline struct __wsi_type *                                       \