I think I'm going to be griping about code duplication...
Keith Packard <keithp at keithp.com> writes:
> diff --git a/configure.ac b/configure.ac
> index 0a25047..074368c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -38,6 +38,9 @@ LIBDRM_NVVIEUX_REQUIRED=2.4.33
> LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
> LIBDRM_FREEDRENO_REQUIRED=2.4.39
> DRI2PROTO_REQUIRED=2.6
> +DRI3PROTO_REQUIRED=1.0
> +PRESENTPROTO_REQUIRED=1.0
> +LIBUDEV_REQUIRED=151
> GLPROTO_REQUIRED=1.4.14
> LIBDRM_XORG_REQUIRED=2.4.24
> LIBKMS_XORG_REQUIRED=1.0.0
> @@ -820,10 +823,13 @@ xyesno)
> fi
> PKG_CHECK_MODULES([DRI2PROTO], [dri2proto >= $DRI2PROTO_REQUIRED])
> GL_PC_REQ_PRIV="$GL_PC_REQ_PRIV libdrm >= $LIBDRM_REQUIRED"
> + PKG_CHECK_MODULES([DRI3PROTO], [dri3proto >= $DRI3PROTO_REQUIRED])
> + PKG_CHECK_MODULES([PRESENTPROTO], [presentproto >=
> $PRESENTPROTO_REQUIRED])
> + PKG_CHECK_MODULES([LIBUDEV], [libudev >= $LIBUDEV_REQUIRED])
> fi
>
> # find the DRI deps for libGL
> - dri_modules="x11 xext xdamage xfixes x11-xcb xcb-glx >= 1.8.1 xcb-dri2
> >= 1.8"
> + dri_modules="x11 xext xdamage xfixes x11-xcb xcb-glx >= 1.8.1 xcb-dri2
> >= 1.8 xcb-dri3 xcb-present xcb-sync xshmfence"
Patches need to land in XCB and get released before this can land. I
don't even see patches on the xcb list yet.
> diff --git a/src/glx/dri3_common.c b/src/glx/dri3_common.c
> new file mode 100644
> index 0000000..c758f96
> --- /dev/null
> +++ b/src/glx/dri3_common.c
> @@ -0,0 +1,146 @@
> +/*
> + * Copyright ? 2013 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.
> + */
> +
> +/*
> + * This code is derived from src/egl/drivers/dri2/common.c which
> + * carries the following copyright:
> + *
> + * Copyright ? 2011 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.
> + *
> + * Authors:
> + * Kristian H?gsberg <krh at bitplanet.net>
> + * Benjamin Franzke <benjaminfranzke at googlemail.com>
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <GL/gl.h>
> +#include "glapi.h"
> +#include "glxclient.h"
> +#include "xf86dri.h"
> +#include <dlfcn.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/mman.h>
> +#include <sys/time.h>
> +#include "xf86drm.h"
> +#include "dri_common.h"
> +#include "dri3_priv.h"
> +
> +#define DRIVER_MAP_DRI3_ONLY
What does this define do?
> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
> new file mode 100644
> index 0000000..4021baa
> --- /dev/null
> +++ b/src/glx/dri3_glx.c
> @@ -0,0 +1,1722 @@
> +static inline void
> +dri3_fence_reset(xcb_connection_t *c, struct dri3_buffer *buffer) {
> + xshmfence_reset(buffer->shm_fence);
> +}
> +
> +static inline void
> +dri3_fence_set(struct dri3_buffer *buffer) {
> + xshmfence_trigger(buffer->shm_fence);
> +}
> +
> +static inline void
> +dri3_fence_trigger(xcb_connection_t *c, struct dri3_buffer *buffer) {
> + xcb_sync_trigger_fence(c, buffer->sync_fence);
> +}
> +
> +static inline void
> +dri3_fence_await(xcb_connection_t *c, struct dri3_buffer *buffer) {
> + xcb_flush(c);
> + xshmfence_await(buffer->shm_fence);
> +}
> +
> +static inline Bool
> +dri3_fence_triggered(struct dri3_buffer *buffer) {
> + return xshmfence_query(buffer->shm_fence);
> +}
'{' on a separate line, please.
> +static void
> +dri3_destroy_context(struct glx_context *context)
> +{
> + struct dri3_context *pcp = (struct dri3_context *) context;
> + struct dri3_screen *psc = (struct dri3_screen *) context->psc;
> +
> + driReleaseDrawables(&pcp->base);
> +
> + free((char *) context->extensions);
> +
> + (*psc->core->destroyContext) (pcp->driContext);
> +
> + free(pcp);
> +}
> +
> +static Bool
> +dri3_bind_context(struct glx_context *context, struct glx_context *old,
> + GLXDrawable draw, GLXDrawable read)
> +{
> + struct dri3_context *pcp = (struct dri3_context *) context;
> + struct dri3_screen *psc = (struct dri3_screen *) pcp->base.psc;
> + struct dri3_drawable *pdraw, *pread;
> +
> + pdraw = (struct dri3_drawable *) driFetchDrawable(context, draw);
> + pread = (struct dri3_drawable *) driFetchDrawable(context, read);
> +
> + driReleaseDrawables(&pcp->base);
> +
> + if (pdraw == NULL || pread == NULL)
> + return GLXBadDrawable;
> +
> + if (!(*psc->core->bindContext) (pcp->driContext,
> + pdraw->driDrawable, pread->driDrawable))
> + return GLXBadContext;
> +
> + return Success;
> +}
> +
> +static void
> +dri3_unbind_context(struct glx_context *context, struct glx_context *new)
> +{
> + struct dri3_context *pcp = (struct dri3_context *) context;
> + struct dri3_screen *psc = (struct dri3_screen *) pcp->base.psc;
> +
> + (*psc->core->unbindContext) (pcp->driContext);
> +}
> +
> +static struct glx_context *
> +dri3_create_context(struct glx_screen *base,
> + struct glx_config *config_base,
> + struct glx_context *shareList, int renderType)
> +{
> + struct dri3_context *pcp, *pcp_shared;
> + struct dri3_screen *psc = (struct dri3_screen *) base;
> + __GLXDRIconfigPrivate *config = (__GLXDRIconfigPrivate *) config_base;
> + __DRIcontext *shared = NULL;
> +
> + if (shareList) {
> + /* If the shareList context is not a DRI3 context, we cannot possibly
> + * create a DRI3 context that shares it.
> + */
> + if (shareList->vtable->destroy != dri3_destroy_context) {
> + return NULL;
> + }
> +
> + pcp_shared = (struct dri3_context *) shareList;
> + shared = pcp_shared->driContext;
> + }
> +
> + pcp = calloc(1, sizeof *pcp);
> + if (pcp == NULL)
> + return NULL;
> +
> + if (!glx_context_init(&pcp->base, &psc->base, &config->base)) {
> + free(pcp);
> + return NULL;
> + }
> +
> + pcp->driContext =
> + (*psc->image_driver->createNewContext) (psc->driScreen,
> + config->driConfig, shared,
> pcp);
> +
> + if (pcp->driContext == NULL) {
> + free(pcp);
> + return NULL;
> + }
> +
> + pcp->base.vtable = &dri3_context_vtable;
> +
> + return &pcp->base;
> +}
This looks completely like dri2_create_context, except for missing
rendertype validation and a different calloc size.
> +static struct glx_context *
> +dri3_create_context_attribs(struct glx_screen *base,
> + struct glx_config *config_base,
> + struct glx_context *shareList,
> + unsigned num_attribs,
> + const uint32_t *attribs,
> + unsigned *error)
> +{
> + struct dri3_context *pcp = NULL;
> + struct dri3_context *pcp_shared = NULL;
> + struct dri3_screen *psc = (struct dri3_screen *) base;
> + __GLXDRIconfigPrivate *config = (__GLXDRIconfigPrivate *) config_base;
> + __DRIcontext *shared = NULL;
> +
> + uint32_t minor_ver = 1;
> + uint32_t major_ver = 2;
> + uint32_t flags = 0;
> + unsigned api;
> + int reset = __DRI_CTX_RESET_NO_NOTIFICATION;
> + uint32_t ctx_attribs[2 * 5];
> + unsigned num_ctx_attribs = 0;
> + uint32_t render_type;
> +
> + /* Remap the GLX tokens to DRI2 tokens.
> + */
> + if (!dri2_convert_glx_attribs(num_attribs, attribs,
> + &major_ver, &minor_ver,
tabs :(
> + &render_type, &flags, &api,
> + &reset, error))
> + goto error_exit;
> +
> + /* Check the renderType value */
> + if (!validate_renderType_against_config(config_base, render_type))
> + goto error_exit;
> +
> + if (shareList) {
> + pcp_shared = (struct dri3_context *) shareList;
> + shared = pcp_shared->driContext;
> + }
> +
> + pcp = calloc(1, sizeof *pcp);
> + if (pcp == NULL) {
> + *error = __DRI_CTX_ERROR_NO_MEMORY;
> + goto error_exit;
> + }
> +
> + if (!glx_context_init(&pcp->base, &psc->base, &config->base))
> + goto error_exit;
> +
> + ctx_attribs[num_ctx_attribs++] = __DRI_CTX_ATTRIB_MAJOR_VERSION;
> + ctx_attribs[num_ctx_attribs++] = major_ver;
> + ctx_attribs[num_ctx_attribs++] = __DRI_CTX_ATTRIB_MINOR_VERSION;
> + ctx_attribs[num_ctx_attribs++] = minor_ver;
> +
> + /* Only send a value when the non-default value is requested. By doing
> + * this we don't have to check the driver's DRI3 version before sending
> the
> + * default value.
> + */
> + if (reset != __DRI_CTX_RESET_NO_NOTIFICATION) {
> + ctx_attribs[num_ctx_attribs++] = __DRI_CTX_ATTRIB_RESET_STRATEGY;
> + ctx_attribs[num_ctx_attribs++] = reset;
> + }
> +
> + if (flags != 0) {
> + ctx_attribs[num_ctx_attribs++] = __DRI_CTX_ATTRIB_FLAGS;
> +
> + /* The current __DRI_CTX_FLAG_* values are identical to the
> + * GLX_CONTEXT_*_BIT values.
> + */
> + ctx_attribs[num_ctx_attribs++] = flags;
> + }
> +
> + pcp->driContext =
> + (*psc->image_driver->createContextAttribs) (psc->driScreen,
> + api,
> + config->driConfig,
> + shared,
> + num_ctx_attribs / 2,
> + ctx_attribs,
> + error,
> + pcp);
> +
> + if (pcp->driContext == NULL)
> + goto error_exit;
> +
> + pcp->base.vtable = &dri3_context_vtable;
> +
> + return &pcp->base;
> +
> +error_exit:
> + free(pcp);
> +
> + return NULL;
> +}
This looks like an exact copy of dri2_create_context_attribs except for
the vtable, the calloc size being different, the reset initialization,
and the createContextAttribs looking in image_driver instead of dri2.
This sucks.
> +
> +static __GLXDRIdrawable *
> +dri3_create_drawable(struct glx_screen *base, XID xDrawable,
> + GLXDrawable drawable, struct glx_config *config_base)
> +{
> + struct dri3_drawable *pdraw;
> + struct dri3_screen *psc = (struct dri3_screen *) base;
> + __GLXDRIconfigPrivate *config = (__GLXDRIconfigPrivate *) config_base;
> + GLint vblank_mode = DRI_CONF_VBLANK_DEF_INTERVAL_1;
> +
> + pdraw = calloc(1, sizeof(*pdraw));
> + if (!pdraw)
> + return NULL;
> +
> + pdraw->base.destroyDrawable = dri3_destroy_drawable;
> + pdraw->base.xDrawable = xDrawable;
> + pdraw->base.drawable = drawable;
> + pdraw->base.psc = &psc->base;
> +// pdraw->bufferCount = 0;
Leftover debug code?
> + pdraw->swap_interval = 1; /* default may be overridden below */
> + pdraw->have_back = 0;
> + pdraw->have_fake_front = 0;
> +
> + if (psc->config)
> + psc->config->configQueryi(psc->driScreen,
> + "vblank_mode", &vblank_mode);
> +
> + switch (vblank_mode) {
> + case DRI_CONF_VBLANK_NEVER:
> + case DRI_CONF_VBLANK_DEF_INTERVAL_0:
> + pdraw->swap_interval = 0;
> + break;
> + case DRI_CONF_VBLANK_DEF_INTERVAL_1:
> + case DRI_CONF_VBLANK_ALWAYS_SYNC:
> + default:
> + pdraw->swap_interval = 1;
> + break;
> + }
> +
> + (void) __glXInitialize(psc->base.dpy);
> +
> + /* Create a new drawable */
> + pdraw->driDrawable =
> + (*psc->image_driver->createNewDrawable) (psc->driScreen,
> + config->driConfig, pdraw);
> +
> + if (!pdraw->driDrawable) {
> + free(pdraw);
> + return NULL;
> + }
> +
> + /*
> + * Make sure server has the same swap interval we do for the new
> + * drawable.
> + */
> + if (psc->vtable.setSwapInterval)
> + psc->vtable.setSwapInterval(&pdraw->base, pdraw->swap_interval);
> +
> + return &pdraw->base;
> +}
Finally, a function different enough that I think it merits being a new
implementation :)
> +static int
> +dri3_wait_for_msc(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t
> divisor,
> + int64_t remainder, int64_t *ust, int64_t *msc, int64_t
> *sbc)
> +{
> + xcb_connection_t *c = XGetXCBConnection(pdraw->psc->dpy);
> + struct dri3_drawable *priv = (struct dri3_drawable *) pdraw;
> + xcb_generic_event_t *ev;
> + xcb_present_generic_event_t *ge;
> +
> + /* Ask for the an event for the target MSC */
> + ++priv->present_msc_request_serial;
> + xcb_present_notify_msc(c,
> + priv->base.xDrawable,
> + priv->present_msc_request_serial,
> + target_msc,
> + divisor,
> + remainder);
> +
> + xcb_flush(c);
> +
> + /* Wait for the event */
> + if (priv->special_event) {
> + while (priv->present_msc_request_serial !=
> priv->present_msc_event_serial) {
> + ev = xcb_wait_for_special_event(c, priv->special_event);
> + if (!ev)
> + break;
> + ge = (void *) ev;
> + present_handle_special_event(priv, ge);
> + }
> + }
> +
> + *ust = priv->ust;
> + *msc = priv->msc;
> +
funny extra newline.
> + *sbc = priv->sbc;
> +
> + return 1;
> +}
> +static int
> +dri3_wait_for_sbc(__GLXDRIdrawable *pdraw, int64_t target_sbc, int64_t *ust,
> + int64_t *msc, int64_t *sbc)
> +{
> + struct dri3_drawable *priv = (struct dri3_drawable *) pdraw;
> +
> + while (priv->sbc < target_sbc) {
> + sleep(1);
> + }
Some sort of comment about what's going on here? Seems like sleep(1)
would always be a wrong thing to execute.
> + return dri3_wait_for_msc(pdraw, 0, 0, 0, ust, msc, sbc);
> +}
> +/**
> + * dri3Throttle - Request driver throttling
> + *
> + * This function uses the DRI2 throttle extension to give the
> + * driver the opportunity to throttle on flush front, copysubbuffer
> + * and swapbuffers.
> + */
> +static void
> +dri3_throttle(struct dri3_screen *psc,
> + struct dri3_drawable *draw,
> + enum __DRI2throttleReason reason)
> +{
> + if (psc->throttle) {
> + __DRIcontext *ctx = dri3_get_current_context();
> +
> + psc->throttle->throttle(ctx, draw->driDrawable, reason);
> + }
> +}
I think we can drop this entirely thanks to flush_with_flags (see
below). The gallium-only implementation of this driver extension is
just a call to flush_with_flags.
> +
> +/**
> + * Asks the driver to flush any queued work necessary for serializing with
> the
> + * X command stream, and optionally the slightly more strict requirement of
> + * glFlush() equivalence (which would require flushing even if nothing had
> + * been drawn to a window system framebuffer, for example).
> + */
> +static void
> +dri3_flush(struct dri3_screen *psc,
> + __DRIcontext *ctx,
> + struct dri3_drawable *draw,
> + unsigned flags,
> + enum __DRI2throttleReason throttle_reason)
> +{
> + if (ctx && psc->f && psc->f->base.version >= 4) {
> + psc->f->flush_with_flags(ctx, draw->driDrawable, flags,
> throttle_reason);
> + } else {
> + if (flags & __DRI2_FLUSH_CONTEXT)
> + glFlush();
> +
> + if (psc->f)
> + psc->f->flush(draw->driDrawable);
> +
> + dri3_throttle(psc, draw, throttle_reason);
> + }
> +}
I'd rather insist that the driver supports flush_with_flags if you do DRI3.
> +static void
> +dri3_copy_sub_buffer(__GLXDRIdrawable *pdraw, int x, int y,
> + int width, int height, Bool flush)
> +{
> + _dri3_copy_sub_buffer(pdraw, x, y, width, height,
> + __DRI2_THROTTLE_COPYSUBBUFFER, flush);
> +}
This appears to be a pointless wrapper.
> +static void
> +dri3_copy_drawable(struct dri3_drawable *priv, Drawable dest, Drawable src)
> +{
> + struct dri3_screen *psc = (struct dri3_screen *) priv->base.psc;
> + xcb_connection_t *c = XGetXCBConnection(priv->base.psc->dpy);
> +
> + if (psc->f)
> + (*psc->f->flush) (priv->driDrawable);
Use flush_with_flags instead.
> + dri3_copy_area(c,
> + src, dest,
> + dri3_drawable_gc(priv),
> + 0, 0, 0, 0, priv->width, priv->height);
> +}
DRI2CopyRegion round-tripped, while this call doesn't. As a result, I
think dri3_wait_x is broken because it doesn't ensure that the copyarea
actually happens before your driver goes rendering again. dri3_wait_gl
may be similarly wrong in the other way.
We don't have testing for glXWaitGL() or glXWaitX() at all, and that's
bad.
> +static void
> +dri3_wait_x(struct glx_context *gc)
> +{
> + struct dri3_drawable *priv = (struct dri3_drawable *)
> + GetGLXDRIDrawable(gc->currentDpy, gc->currentDrawable);
> +
> + if (priv == NULL || !priv->have_fake_front)
> + return;
> +
> + dri3_copy_drawable(priv, dri3_fake_front_buffer(priv)->pixmap,
> priv->base.xDrawable);
> +}
> +
> +static void
> +dri3_wait_gl(struct glx_context *gc)
> +{
> + struct dri3_drawable *priv = (struct dri3_drawable *)
> + GetGLXDRIDrawable(gc->currentDpy, gc->currentDrawable);
> +
> + if (priv == NULL || !priv->have_fake_front)
> + return;
> +
> + dri3_copy_drawable(priv, priv->base.xDrawable,
> dri3_fake_front_buffer(priv)->pixmap);
> +}
What's going on with fence reset/triggering being present in
copysubbuffer but not these entrypoints?
> +static struct dri3_buffer *
> +dri3_alloc_render_buffer(struct glx_screen *glx_screen, Drawable draw,
> unsigned int format, int width, int height, int depth)
80-column wrap
> +{
> + struct dri3_screen *psc = (struct dri3_screen *) glx_screen;
> + Display *dpy = glx_screen->dpy;
> + struct dri3_buffer *buffer;
> + xcb_connection_t *c = XGetXCBConnection(dpy);
> + xcb_pixmap_t pixmap;
> + xcb_sync_fence_t sync_fence;
> + int32_t *shm_fence;
> + int buffer_fd, fence_fd;
> + int stride;
> +
> + fence_fd = xshmfence_alloc_shm();
> + if (fence_fd < 0)
> + return NULL;
> + shm_fence = xshmfence_map_shm(fence_fd);
> + if (shm_fence == NULL)
> + goto no_shm_fence;
> +
> + buffer = calloc(1, sizeof (struct dri3_buffer));
> + if (!buffer)
> + goto no_buffer;
> +
> + buffer->image = (*psc->image->createImage) (psc->driScreen,
> + width, height,
> + format,
> +
> __DRI_IMAGE_USE_SHARE|__DRI_IMAGE_USE_SCANOUT,
> + buffer);
> +
> +
trailing whitespace
> + if (!buffer->image)
> + goto no_image;
> +
> + if (!(*psc->image->queryImage)(buffer->image, __DRI_IMAGE_ATTRIB_STRIDE,
> &stride))
> + goto no_buffer_attrib;
> +
> + buffer->pitch = stride;
> +
> + if (!(*psc->image->queryImage)(buffer->image, __DRI_IMAGE_ATTRIB_FD,
> &buffer_fd))
> + goto no_buffer_attrib;
> +
> + xcb_dri3_pixmap_from_buffer(c,
> + (pixmap = xcb_generate_id(c)),
> + draw,
> + buffer->size,
> + width, height, buffer->pitch,
> + depth, buffer->cpp * 8,
I don't see buffer->cpp initialized anywhere.
> + /* Mark the buffer as idle */
> + dri3_fence_set(buffer);
> +
> + return buffer;
> +
trailing whitespace
> +static void
> +dri3_free_render_buffer(struct dri3_drawable *pdraw, struct dri3_buffer
> *buffer)
> +{
> + struct dri3_screen *psc = (struct dri3_screen *) pdraw->base.psc;
> + xcb_connection_t *c = XGetXCBConnection(pdraw->base.psc->dpy);
> +
> + xcb_free_pixmap(c, buffer->pixmap);
> + xcb_sync_destroy_fence(c, buffer->sync_fence);
> + xshmfence_unmap_shm(buffer->shm_fence);
> + (*psc->image->destroyImage)(buffer->image);
> + free(buffer);
> +}
> +
> +
> +
> +static void
> +present_flush_events(struct dri3_drawable *priv)
> +{
> + xcb_connection_t *c = XGetXCBConnection(priv->base.psc->dpy);
> +
> + /* Check to see if any configuration changes have occurred
> + * since we were last invoked
> + */
> + if (priv->special_event) {
> + xcb_generic_event_t *ev;
> +
> + while ((ev = xcb_check_for_special_event(c, priv->special_event)) !=
> NULL) {
> + xcb_present_generic_event_t *ge = (void *) ev;
> + present_handle_special_event(priv, ge);
> + }
> + }
> +}
> +
> +static int
> +dri3_update_drawable(__DRIdrawable *driDrawable, void *loaderPrivate)
> +{
> + struct dri3_drawable *priv = loaderPrivate;
> + xcb_connection_t *c = XGetXCBConnection(priv->base.psc->dpy);
> +
> + /* First time through, go get the current drawable geometry
> + */
> + if (priv->width == 0 || priv->height == 0 || priv->depth == 0) {
> + xcb_get_geometry_cookie_t geom_cookie;
> + xcb_get_geometry_reply_t *geom_reply;
> + xcb_void_cookie_t cookie;
> + xcb_generic_error_t *error;
> +
> + cookie = xcb_present_select_input_checked(c,
> + (priv->eid =
> xcb_generate_id(c)),
> + priv->base.xDrawable,
> +
> XCB_PRESENT_EVENT_MASK_CONFIGURE_NOTIFY|
> +
> XCB_PRESENT_EVENT_MASK_COMPLETE_NOTIFY|
> +
> XCB_PRESENT_EVENT_MASK_IDLE_NOTIFY);
> +
> + if (!priv->present_extension) {
> + priv->present_extension = xcb_get_extension_data(c,
> &xcb_present_id);
> + if (!priv->present_extension)
> + return false;
> + }
> +
> + priv->special_event = xcb_register_for_special_event(c,
> +
> priv->present_extension->major_opcode,
> + priv->eid,
> + priv->stamp);
> +
> + geom_cookie = xcb_get_geometry(c, priv->base.xDrawable);
> +
> + geom_reply = xcb_get_geometry_reply(c, geom_cookie, NULL);
> +
> + if (!geom_reply)
> + return false;
> +
> + priv->width = geom_reply->width;
> + priv->height = geom_reply->height;
> + priv->depth = geom_reply->depth;
> + priv->is_pixmap = false;
> +
> + free(geom_reply);
> +
> + error = xcb_request_check(c, cookie);
> +
> + if (error) {
> + if (error->error_code != BadWindow) {
> + free(error);
> + return false;
> + }
> + priv->is_pixmap = true;
> + xcb_unregister_for_special_event(c, priv->special_event);
> + priv->special_event = NULL;
> + }
> + }
You should probably comment what's going on here. Is an error going to
be returned iff it's a pixmap?
> +
trailing whitespace.
> +static int
> +image_format_to_fourcc(int format)
> +{
> +
> + /* Convert from __DRI_IMAGE_FORMAT to __DRI_IMAGE_FOURCC (sigh) */
> + switch (format) {
> + case __DRI_IMAGE_FORMAT_RGB565: return __DRI_IMAGE_FOURCC_RGB565;
> + case __DRI_IMAGE_FORMAT_XRGB8888: return __DRI_IMAGE_FOURCC_XRGB8888;
> + case __DRI_IMAGE_FORMAT_ARGB8888: return __DRI_IMAGE_FOURCC_ARGB8888;
> + case __DRI_IMAGE_FORMAT_ABGR8888: return __DRI_IMAGE_FOURCC_ABGR8888;
> + case __DRI_IMAGE_FORMAT_XBGR8888: return __DRI_IMAGE_FOURCC_XBGR8888;
> +// case __DRI_IMAGE_FORMAT_R8: return __DRI_IMAGE_FOURCC_R8;
> +// case __DRI_IMAGE_FORMAT_GR88: return __DRI_IMAGE_FOURCC_GR88;
> +// case __DRI_IMAGE_FORMAT_NONE: return __DRI_IMAGE_FOURCC_NONE;
> +// case __DRI_IMAGE_FORMAT_XRGB2101010: return
> __DRI_IMAGE_FOURCC_XRGB2101010;
> +// case __DRI_IMAGE_FORMAT_ARGB2101010: return
> __DRI_IMAGE_FOURCC_ARGB2101010;
What's up with commented out formats?
> + }
> + return 0;
> +}
> +
> +static struct dri3_buffer *
> +dri3_get_pixmap_buffer(__DRIdrawable *driDrawable,
> + unsigned int format,
> + enum dri3_buffer_type buffer_type,
> + void *loaderPrivate)
> +{
> + struct dri3_drawable *pdraw = loaderPrivate;
> + int buf_id = buffer_type ==
> dri3_pixmap_buf_id(buffer_type);
> + struct dri3_buffer *buffer = pdraw->buffers[buf_id];
> + Pixmap pixmap;
> + xcb_dri3_buffer_from_pixmap_cookie_t bp_cookie;
> + xcb_dri3_buffer_from_pixmap_reply_t *bp_reply;
> + int *fds;
> + int buffer_fd;
> + Display *dpy;
> + struct dri3_screen *psc;
> + xcb_connection_t *c;
> + xcb_sync_fence_t sync_fence;
> + int32_t *shm_fence;
> + int fence_fd;
> + __DRIimage *image_planar;
> + int stride, offset;
> +
> + if (buffer)
> + return buffer;
> +
> + pixmap = pdraw->base.xDrawable;
> + psc = (struct dri3_screen *) pdraw->base.psc;
> + dpy = psc->base.dpy;
> + c = XGetXCBConnection(dpy);
> +
> + buffer = calloc(1, sizeof (struct dri3_buffer));
> + if (!buffer)
> + goto no_buffer;
> +
> + image_planar = (*psc->image->createImageFromFds) (psc->driScreen,
> + bp_reply->width,
> + bp_reply->height,
> +
> image_format_to_fourcc(format),
> + fds, 1,
> + &stride, &offset,
> buffer);
> + close(buffer_fd);
just drop buffer_fd and reference fds[0] again?
> + if (!image_planar)
> + goto no_image;
> +
> + buffer->image = (*psc->image->fromPlanar)(image_planar, 0, buffer);
> +
> + (*psc->image->destroyImage)(image_planar);
Is the fromPlanar step here actually needed? It looks like since
num_fds == 1 you get a functional image from the first step.
> +static int
> +dri3_get_buffers(__DRIdrawable *driDrawable,
> + int *width, int *height,
> + unsigned int format,
> + uint32_t *stamp,
> + void *loaderPrivate,
> + uint32_t buffer_mask,
> + struct __DRIimageList *buffers)
> +{
> + struct dri3_drawable *priv = loaderPrivate;
> + struct dri3_buffer *front, *back;
> +
> + buffers->front = NULL;
> + buffers->back = NULL;
> +
> + front = NULL;
> + back = NULL;
> +
> + if (!dri3_update_drawable(driDrawable, loaderPrivate))
> + return false;
> +
> + if (priv->is_pixmap)
> + buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
> +
> + if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
> + if (priv->is_pixmap)
> + front = dri3_get_pixmap_buffer(driDrawable,
> + format,
> + dri3_buffer_front,
> + loaderPrivate);
> + else
> + front = dri3_get_buffer(driDrawable,
> + format,
> + dri3_buffer_front,
> + loaderPrivate);
> +
> + if (!front)
> + return false;
> + priv->have_fake_front = !priv->is_pixmap;
> + } else {
> + dri3_free_buffers(driDrawable, dri3_buffer_front, loaderPrivate);
> + priv->have_fake_front = 0;
> + }
> +
> + if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
> + back = dri3_get_buffer(driDrawable,
> + format,
> + dri3_buffer_back,
> + loaderPrivate);
> + if (!back)
> + return false;
I think this early return leaks front. Also have_fake_front's setting
leaked in even though we're not updating priv->front, should it have?
> + priv->have_back = 1;
> + } else {
> + dri3_free_buffers(driDrawable, dri3_buffer_back, loaderPrivate);
> + priv->have_back = 0;
> + }
> +
> + if (front)
> + buffers->front = front->image;
> +
> + if (back)
> + buffers->back = back->image;
> +
> + priv->stamp = stamp;
> +
> + /* Report back current geometry */
> + *width = priv->width;
> + *height = priv->height;
> + return true;
> +}
> +
> +
> +static int
> +dri3_query_version(Display *dpy, int *major, int *minor)
> +{
> + xcb_dri3_query_version_cookie_t cookie;
> + xcb_dri3_query_version_reply_t *reply;
trailing whitespace
> + xcb_connection_t *c = XGetXCBConnection(dpy);
> + xcb_generic_error_t *error;
> +
> + cookie = xcb_dri3_query_version(c,
> + XCB_DRI3_MAJOR_VERSION,
> + XCB_DRI3_MINOR_VERSION);
> + reply = xcb_dri3_query_version_reply(c, cookie, &error);
> + if (!reply) {
> + if (error) {
> + free(error);
> + }
No need for NULL-checking free().
Same 2 comments apply to the next 2 functions.
> + return 0;
> + }
> + *major = reply->major_version;
> + *minor = reply->minor_version;
> + free(reply);
> + return 1;
> +}
> +static const __DRIimageLoaderExtension imageLoaderExtension = {
> + {__DRI_IMAGE_LOADER, __DRI_IMAGE_LOADER_VERSION},
> + .getBuffers = dri3_get_buffers,
> + .flushFrontBuffer = dri3_flush_front_buffer,
> +};
> +
> +static void
> +dri3_bind_tex_image(Display * dpy,
> + GLXDrawable drawable,
> + int buffer, const int *attrib_list)
> +{
> + struct glx_context *gc = __glXGetCurrentContext();
> + struct dri3_context *pcp = (struct dri3_context *) gc;
> + __GLXDRIdrawable *base = GetGLXDRIDrawable(dpy, drawable);
> + struct dri3_drawable *pdraw = (struct dri3_drawable *) base;
> + struct dri3_screen *psc;
> +
> + if (pdraw != NULL) {
> + psc = (struct dri3_screen *) base->psc;
> +
> + if (psc->f &&
> + psc->f->base.version >= 3 && psc->f->invalidate)
> + psc->f->invalidate(pdraw->driDrawable);
> +
> + XSync(dpy, false);
> + if (psc->texBuffer->base.version >= 2 &&
> + psc->texBuffer->setTexBuffer2 != NULL) {
> + (*psc->texBuffer->setTexBuffer2) (pcp->driContext,
> + pdraw->base.textureTarget,
> + pdraw->base.textureFormat,
> + pdraw->driDrawable);
> + }
> + else {
> + (*psc->texBuffer->setTexBuffer) (pcp->driContext,
> + pdraw->base.textureTarget,
> + pdraw->driDrawable);
> + }
> + }
> +}
Tab indentation :(
I'd really like to see less loader code duplication. But if you have
to, at least don't support old setTexBuffer when you know the driver's
new enough that it's got DRI3.
> +static void
> +dri3_release_tex_image(Display * dpy, GLXDrawable drawable, int buffer)
> +{
> +#if __DRI_TEX_BUFFER_VERSION >= 3
> + struct glx_context *gc = __glXGetCurrentContext();
> + struct dri3_context *pcp = (struct dri3_context *) gc;
> + __GLXDRIdrawable *base = GetGLXDRIDrawable(dpy, drawable);
> + struct glx_display *dpyPriv = __glXInitialize(dpy);
> + struct dri3_drawable *pdraw = (struct dri3_drawable *) base;
> + struct dri3_display *pdp =
> + (struct dri3_display *) dpyPriv->dri3Display;
> + struct dri3_screen *psc;
> +
> + if (pdraw != NULL) {
> + psc = (struct dri3_screen *) base->psc;
> +
> + if (psc->texBuffer->base.version >= 3 &&
> + psc->texBuffer->releaseTexBuffer != NULL) {
> + (*psc->texBuffer->releaseTexBuffer) (pcp->driContext,
> + pdraw->base.textureTarget,
> + pdraw->driDrawable);
> + }
> + }
> +#endif
Remove the #ifdef. You're in the tree, you know its value.
> +}
> +
> +static const struct glx_context_vtable dri3_context_vtable = {
> + dri3_destroy_context,
> + dri3_bind_context,
> + dri3_unbind_context,
> + dri3_wait_gl,
> + dri3_wait_x,
> + DRI_glXUseXFont,
> + dri3_bind_tex_image,
> + dri3_release_tex_image,
> + NULL, /* get_proc_address */
> +};
> +
> +static void
> +dri3_bind_extensions(struct dri3_screen *psc, struct glx_display * priv,
> + const char *driverName)
> +{
> +// const struct dri3_display *const pdp = (struct dri3_display *)
> priv->dri3Display;
more commented leftovers.
> + const __DRIextension **extensions;
> + unsigned mask;
> + int i;
> +
> + extensions = psc->core->getExtensions(psc->driScreen);
> +
> + __glXEnableDirectExtension(&psc->base, "GLX_SGI_video_sync");
> + __glXEnableDirectExtension(&psc->base, "GLX_SGI_swap_control");
> + __glXEnableDirectExtension(&psc->base, "GLX_MESA_swap_control");
> + __glXEnableDirectExtension(&psc->base, "GLX_SGI_make_current_read");
> +
> + /*
> + * GLX_INTEL_swap_event is broken on the server side, where it's
> + * currently unconditionally enabled. This completely breaks
> + * systems running on drivers which don't support that extension.
> + * There's no way to test for its presence on this side, so instead
> + * of disabling it unconditionally, just disable it for drivers
> + * which are known to not support it, or for DDX drivers supporting
> + * only an older (pre-ScheduleSwap) version of DRI2.
> + *
> + * This is a hack which is required until:
> + * http://lists.x.org/archives/xorg-devel/2013-February/035449.html
> + * is merged and updated xserver makes it's way into distros:
> + */
> +// if (pdp->swapAvailable && strcmp(driverName, "vmwgfx") != 0) {
> +// __glXEnableDirectExtension(&psc->base, "GLX_INTEL_swap_event");
> +// }
more commented leftovers. Are you dropping swap_event support?
> +
> + mask = psc->image_driver->getAPIMask(psc->driScreen);
> +
> + __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context");
> + __glXEnableDirectExtension(&psc->base, "GLX_ARB_create_context_profile");
> +
> + if ((mask & (1 << __DRI_API_GLES2)) != 0)
> + __glXEnableDirectExtension(&psc->base,
> + "GLX_EXT_create_context_es2_profile");
> +
> + for (i = 0; extensions[i]; i++) {
> + if ((strcmp(extensions[i]->name, __DRI_TEX_BUFFER) == 0)) {
> + psc->texBuffer = (__DRItexBufferExtension *) extensions[i];
> + __glXEnableDirectExtension(&psc->base, "GLX_EXT_texture_from_pixmap");
> + }
> +
> + if ((strcmp(extensions[i]->name, __DRI2_FLUSH) == 0)) {
> + psc->f = (__DRI2flushExtension *) extensions[i];
> + /* internal driver extension, no GL extension exposed */
> + }
> +
> + if ((strcmp(extensions[i]->name, __DRI2_CONFIG_QUERY) == 0))
> + psc->config = (__DRI2configQueryExtension *) extensions[i];
> +
> + if (((strcmp(extensions[i]->name, __DRI2_THROTTLE) == 0)))
> + psc->throttle = (__DRI2throttleExtension *) extensions[i];
> +
> + if (strcmp(extensions[i]->name, __DRI2_ROBUSTNESS) == 0)
> + __glXEnableDirectExtension(&psc->base,
> + "GLX_ARB_create_context_robustness");
> + }
> +}
> +
This is horribly duplicated with dri2, but that's also horribly
duplicated with EGL's dri3. I really think we need to do a
dri_loader_common between all of them with a bunch of this crap.
> + tmp = getenv("LIBGL_SHOW_FPS");
> + psc->show_fps = tmp && strcmp(tmp, "1") == 0;
Dead code.
> +/*
> + * Allocate, initialize and return a __DRIdisplayPrivate object.
> + * This is called from __glXInitialize() when we are given a new
> + * display pointer.
> + */
> +_X_HIDDEN __GLXDRIdisplay *
> +dri3_create_display(Display * dpy)
> +{
> + struct dri3_display *pdp;
> + int i;
> +
> + pdp = malloc(sizeof *pdp);
> + if (pdp == NULL)
> + return NULL;
> +
> + if (!dri3_query_version(dpy, &pdp->dri3Major, &pdp->dri3Minor))
> + goto no_extension;
> +
> + if (!present_query_version(dpy, &pdp->presentMajor, &pdp->presentMinor))
> + goto no_extension;
> +
> + pdp->base.destroyDisplay = dri3_destroy_display;
> + pdp->base.createScreen = dri3_create_screen;
> +
> + i = 0;
> +
> + pdp->loader_extensions[i++] = &imageLoaderExtension.base;
> +
trailing whitespace
> diff --git a/src/glx/dri3_priv.h b/src/glx/dri3_priv.h
> new file mode 100644
> index 0000000..2873919
> --- /dev/null
> +++ b/src/glx/dri3_priv.h
> +
> +struct dri3_buffer {
> + __DRIimage *image;
> + uint32_t pixmap;
> + uint32_t sync_fence;
> + int32_t *shm_fence;
> + GLboolean busy;
Can we get some comments on what these 3 fields do? These
synchronization details were a huge part of the dri3 discussions, and I
know you've gone several ways about things in the process of
development, but I see just a single comment about what fences do:
+ /* Mark the buffer as idle */
That's... not enough.
> +struct dri3_drawable
> +{
> + /* For WaitMSC */
> + uint32_t present_msc_request_serial;
> + uint32_t present_msc_event_serial;
> +
whitespace
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20131105/65066d87/attachment-0001.pgp>
