On 01/19/2015 03:00 AM, Chris Wilson wrote:
> Within the DRI2GetBuffers return packet there is a 4-byte field that is
> currently unused by any driver, i.e. flags. With the co-operation of a
> suitably modified X server, we can pass the last SBC on which the buffer
> was defined (i.e. the last SwapBuffers for which it was used) and 0 if
> it is fresh (with a slight loss of precision). We can then compare the
> flags field of the DRIBuffer against the current swap buffers count and
> so compute the age of the back buffer (thus satisfying
> GLX_EXT_buffer_age).
>
> As we reuse a driver specific field within the DRI2GetBuffers packet, we
> first query whether the X/DDX are ready to supply the new value using a
> DRI2GetParam request.
>
> Another caveat is that we need to complete the SwapBuffers/GetBuffers
> roundtrip before reporting the back buffer age so that it tallies
> against the buffer used for rendering. As with all things X, there is a
> race between the query and the rendering where the buffer may be
> invalidated by the server. However, for the primary usecase (that of a
> compositing manager), the DRI2Drawable is only accessible to a single
> client mitigating the impact of the issue.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> configure.ac | 2 +-
> src/glx/dri2_glx.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 870435c..ca1da86 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -65,7 +65,7 @@ LIBDRM_INTEL_REQUIRED=2.4.52
> LIBDRM_NVVIEUX_REQUIRED=2.4.33
> LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
> LIBDRM_FREEDRENO_REQUIRED=2.4.57
> -DRI2PROTO_REQUIRED=2.6
> +DRI2PROTO_REQUIRED=2.9
> DRI3PROTO_REQUIRED=1.0
> PRESENTPROTO_REQUIRED=1.0
> LIBUDEV_REQUIRED=151
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index 0577804..b43f115 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -917,6 +917,67 @@ dri2GetBuffersWithFormat(__DRIdrawable * driDrawable,
> }
>
> static int
> +dri2HasBufferAge(int screen, struct glx_display * priv)
> +{
> + const struct dri2_display *const pdp =
> + (struct dri2_display *)priv->dri2Display;
> + CARD64 value;
> +
> + if (pdp->driMajor <= 1 && pdp->driMinor < 4)
> + return 0;
> +
> + value = 0;
> + if (!DRI2GetParam(priv->dpy, RootWindow(priv->dpy, screen),
> + DRI2ParamXHasBufferAge, &value))
> + return 0;
> +
> + return value;
> +}
> +
> +static int
> +dri2GetBufferAge(__GLXDRIdrawable *pdraw)
> +{
> + struct dri2_drawable *priv = (struct dri2_drawable *) pdraw;
> + int i, age = 0;
> +
> + if (priv->swap_pending) {
> + unsigned int attachments[5];
I see other callers that have attachments of at least 8 (although it
appears that intel_query_dri2_buffers only needs 2). Could we at least
get an assertion or something that priv->bufferCount <=
ARRAY_SIZE(attachments)? A (hypothetical) driver doing stereo rendering
with separate, DDX managed, depth and stencil buffers would need 6. A
(again, hypothetical) driver with AUX buffers could need... more.
> + DRI2Buffer *buffers;
> +
> + for (i = 0; i < priv->bufferCount; i++)
> + attachments[i] = priv->buffers[i].attachment;
> +
> + buffers = DRI2GetBuffers(priv->base.psc->dpy, priv->base.xDrawable,
> + &priv->width, &priv->height,
> + attachments, i, &i);
Most drivers prefer DRI2GetBuffersWithFormat, and some drivers only use
DRI2GetBuffersWithFormat. Is mixing DRI2GetBuffersWithFormat and
DRI2GetBuffers going to cause problems or unexpected behavior changes?
> + if (buffers == NULL)
> + return 0;
> +
> + process_buffers(priv, buffers, i);
> + free(buffers);
> +
> + dri2XcbSwapBuffersComplete(priv);
> + }
> +
> + if (!priv->have_back)
> + return 0;
> +
> + for (i = 0; i < priv->bufferCount; i++) {
> + if (priv->buffers[i].attachment != __DRI_BUFFER_BACK_LEFT)
> + continue;
> +
> + if (priv->buffers[i].flags == 0)
> + continue;
> +
> + age = priv->last_swap_sbc - priv->buffers[i].flags + 1;
> + if (age < 0)
> + age = 0;
I was going to comment that this looked like it calculated age wrong
when the buffers had different ages. Then I realized that age should
only be calculated once. I think this would be more obvious if the body
of the loop were:
if (priv->buffers[i].attachment == __DRI_BUFFER_BACK_LEFT &&
priv->buffers[i].flags != 0) {
age = priv->last_swap_sbc - priv->buffers[i].flags + 1;
if (age < 0)
age = 0;
break;
}
I also just noticed that your patches are mixing tabs and spaces (use
spaces only) and are using a mix of 3-space and 8-space (maybe?) indents
(use 3 spaces only).
> + }
> +
> + return age;
> +}
> +
> +static int
> dri2SetSwapInterval(__GLXDRIdrawable *pdraw, int interval)
> {
> xcb_connection_t *c = XGetXCBConnection(pdraw->psc->dpy);
> @@ -1290,6 +1351,10 @@ dri2CreateScreen(int screen, struct glx_display * priv)
> psp->setSwapInterval = NULL;
> psp->getSwapInterval = NULL;
> psp->getBufferAge = NULL;
Blank line here.
> + if (dri2HasBufferAge(screen, priv)) {
> + psp->getBufferAge = dri2GetBufferAge;
> + __glXEnableDirectExtension(&psc->base, "GLX_EXT_buffer_age");
> + }
>
> if (pdp->driMinor >= 2) {
> psp->getDrawableMSC = dri2DrawableGetMSC;
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
