On Fri, Nov 4, 2016 at 8:58 AM, Eric Engestrom <eric.engest...@imgtec.com>
wrote:
> On Thursday, 2016-11-03 20:51:59 -0700, Jason Ekstrand wrote:
> > From: Kevin Strasser <kevin.stras...@intel.com>
> >
> > In order to support FIFO mode without blocking the application on calls
> > to vkQueuePresentKHR it is necessary to enqueue the request and defer
> > calling the server until the next vblank period. The xcb present api
> > doesn't offer a way to register a callback, so we will have to spawn a
> > worker thread that will wait for a request to be added to the queue, call
> > to the server, and then make the image available for reuse. This commit
> > introduces the queue data structure needed to implement this.
> >
> > Signed-off-by: Jason Ekstrand <ja...@jlekstrand.net>
> > ---
> > src/vulkan/wsi/Makefile.sources | 3 +-
> > src/vulkan/wsi/wsi_common_queue.h | 156 ++++++++++++++++++++++++++++++
> ++++++++
> > 2 files changed, 158 insertions(+), 1 deletion(-)
> > create mode 100644 src/vulkan/wsi/wsi_common_queue.h
> >
> > diff --git a/src/vulkan/wsi/Makefile.sources b/src/vulkan/wsi/Makefile.
> sources
> > index 3139e6d..50660f9 100644
> > --- a/src/vulkan/wsi/Makefile.sources
> > +++ b/src/vulkan/wsi/Makefile.sources
> > @@ -1,6 +1,7 @@
> >
> > VULKAN_WSI_FILES := \
> > - wsi_common.h
> > + wsi_common.h \
> > + wsi_common_queue.h
> >
> > VULKAN_WSI_WAYLAND_FILES := \
> > wsi_common_wayland.c \
> > diff --git a/src/vulkan/wsi/wsi_common_queue.h
> b/src/vulkan/wsi/wsi_common_queue.h
> > new file mode 100644
> > index 0000000..ec2a925
> > --- /dev/null
> > +++ b/src/vulkan/wsi/wsi_common_queue.h
> > @@ -0,0 +1,156 @@
> > +/*
> > + * Copyright © 2016 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> obtaining a
> > + * copy of this software and associated documentation files (the
> "Software"),
> > + * to deal in the Software without restriction, including without
> limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> next
> > + * paragraph) shall be included in all copies or substantial portions
> of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +#ifndef VULKAN_WSI_COMMON_QUEUE_H
> > +#define VULKAN_WSI_COMMON_QUEUE_H
> > +
> > +#include <time.h>
> > +#include <pthread.h>
> > +#include "util/u_vector.h"
> > +
> > +struct wsi_queue {
> > + struct u_vector vector;
> > + pthread_mutex_t mutex;
> > + pthread_cond_t cond;
> > +};
> > +
> > +static inline int
> > +wsi_queue_init(struct wsi_queue *queue, int length)
> > +{
> > + int ret;
> > +
> > + uint32_t length_pow2 = 4;
> > + while (length_pow2 < length)
> > + length_pow2 *= 2;
> > +
> > + ret = u_vector_init(&queue->vector, sizeof(uint32_t),
> > + sizeof(uint32_t) * length_pow2);
> > + if (!ret)
> > + return -ENOMEM;
>
> I thought pthread_* functions returned positive errors?
> I couldn't find a confirmation of that in the docs (and I was unable to
> actually make a test app that makes one fail), and the actual return
> value is currently unused (even after patch 2) so it doesn't matter yet,
> but it'd be nice to have the same sign-ness so that the value can be
> used at a later point.
> Otherwise, we might as well make this a boolean, I don't think much
> information would be lost really :)
>
Sure. I'll make it positive.
> > +
> > + pthread_condattr_t condattr;
> > + ret = pthread_condattr_init(&condattr);
> > + if (ret)
> > + goto fail_vector;
> > +
> > + ret = pthread_condattr_setclock(&condattr, CLOCK_MONOTONIC);
> > + if (ret)
> > + goto fail_condattr;
> > +
> > + ret = pthread_cond_init(&queue->cond, &condattr);
> > + if (ret)
> > + goto fail_condattr;
> > +
> > + ret = pthread_mutex_init(&queue->mutex, NULL);
> > + if (ret) {
> > + pthread_cond_destroy(&queue->cond);
>
> Double-free.
>
Good catch!
> Other than that, the series is:
> Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>
>
Thanks!
> > + goto fail_cond;
> > + }
> > +
> > + return 0;
> > +
> > +fail_cond:
> > + pthread_cond_destroy(&queue->cond);
> > +fail_condattr:
> > + pthread_condattr_destroy(&condattr);
> > +fail_vector:
> > + u_vector_finish(&queue->vector);
> > +
> > + return ret;
> > +}
> > +
> > +static inline void
> > +wsi_queue_destroy(struct wsi_queue *queue)
> > +{
> > + u_vector_finish(&queue->vector);
> > + pthread_mutex_destroy(&queue->mutex);
> > + pthread_cond_destroy(&queue->cond);
> > +}
> > +
> > +static inline void
> > +wsi_queue_push(struct wsi_queue *queue, uint32_t index)
> > +{
> > + uint32_t *elem;
> > +
> > + pthread_mutex_lock(&queue->mutex);
> > +
> > + if (u_vector_length(&queue->vector) == 0)
> > + pthread_cond_signal(&queue->cond);
> > +
> > + elem = u_vector_add(&queue->vector);
> > + *elem = index;
> > +
> > + pthread_mutex_unlock(&queue->mutex);
> > +}
> > +
> > +#define NSEC_PER_SEC 1000000000
> > +#define INT_TYPE_MAX(type) ((1ull << (sizeof(type) * 8 - 1)) - 1)
> > +
> > +static inline VkResult
> > +wsi_queue_pull(struct wsi_queue *queue, uint32_t *index, uint64_t
> timeout)
> > +{
> > + VkResult result;
> > + int32_t ret;
> > +
> > + pthread_mutex_lock(&queue->mutex);
> > +
> > + struct timespec now;
> > + clock_gettime(CLOCK_MONOTONIC, &now);
> > +
> > + uint32_t abs_nsec = now.tv_nsec + timeout % NSEC_PER_SEC;
> > + uint64_t abs_sec = now.tv_sec + (abs_nsec / NSEC_PER_SEC) +
> > + (timeout / NSEC_PER_SEC);
> > + abs_nsec %= NSEC_PER_SEC;
> > +
> > + /* Avoid roll-over in tv_sec on 32-bit systems if the user provided
> timeout
> > + * is UINT64_MAX
> > + */
> > + struct timespec abstime;
> > + abstime.tv_nsec = abs_nsec;
> > + abstime.tv_sec = MIN2(abs_sec, INT_TYPE_MAX(abstime.tv_sec));
> > +
> > + while (u_vector_length(&queue->vector) == 0) {
> > + ret = pthread_cond_timedwait(&queue->cond, &queue->mutex,
> &abstime);
> > + if (ret == 0) {
> > + continue;
> > + } else if (ret == ETIMEDOUT) {
> > + result = VK_TIMEOUT;
> > + goto end;
> > + } else {
> > + /* Something went badly wrong */
> > + result = VK_ERROR_OUT_OF_DATE_KHR;
> > + goto end;
> > + }
> > + }
> > +
> > + uint32_t *elem = u_vector_remove(&queue->vector);
> > + *index = *elem;
> > + result = VK_SUCCESS;
> > +
> > +end:
> > + pthread_mutex_unlock(&queue->mutex);
> > +
> > + return result;
> > +}
> > +
> > +#endif /* VULKAN_WSI_COMMON_QUEUE_H */
> > --
> > 2.5.0.400.gff86faf
> >
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
