[Intel-gfx] [PATCH] xf86-video-intel: support swapbuffers using page flipping

Jesse Barnes jbarnes at virtuousgeek.org
Fri Feb 27 02:07:34 CET 2009


On Thursday, February 26, 2009 1:36:26 pm Jesse Barnes wrote:
> Support the new swapbuffers request using the new page flipping ioctl
> if possible.
>
> This patch still needs some work; there's a bug in the no-flip case that
> causes us to lose track of pixmaps, and the pipe is still hardcoded to 1,
> but that should be easy to fix.
>
> The code is pretty ugly too; it seems like getbuffers and swapbuffers could
> probably share more code, but we need to copy all the buffers in
> swapbuffers to return them...

This version is a little better, and with the X server fix I just posted it
no longer crashes with current compiz.  There are two open issues left:
  1) how to deal with pinning the new front & unpinning the old back
  2) figure out why mixed SwapBuffers/CopyRegion code (e.g. current compiz)
     render right; the front buffer doesn't get any rendering

Problem (2) is probably something simple I'm missing in the flip code, but
(1) is a little trickier.  We need to keep the new front buffer pinned until
the next swap, but the old buffer has to be kept pinned until the swap
completes; one ugly option would be to pass both buffers into the flip ioctl
and let the kernel take care of it.  Any opinions?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

 i830.h         |    1
 i830_display.c |    5 +
 i830_dri.c     |  188 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 182 insertions(+), 12 deletions(-)

diff --git a/src/i830.h b/src/i830.h
index 7904b9f..5c65ce8 100644
--- a/src/i830.h
+++ b/src/i830.h
@@ -452,6 +452,7 @@ typedef struct _I830Rec {
 #endif
 
    XF86ModReqInfo shadowReq; /* to test for later libshadow */
+   Bool shadow_present;
    Rotation rotation;
    void (*PointerMoved)(int, int, int);
    CreateScreenResourcesProcPtr    CreateScreenResources;
diff --git a/src/i830_display.c b/src/i830_display.c
index 8a5cf24..692349e 100644
--- a/src/i830_display.c
+++ b/src/i830_display.c
@@ -1669,6 +1669,9 @@ i830_crtc_shadow_create(xf86CrtcPtr crtc, void *data, int width, int height)
     }
     if (intel_crtc->rotate_mem && intel_crtc->rotate_mem->bo)
 	i830_set_pixmap_bo(rotate_pixmap, intel_crtc->rotate_mem->bo);
+
+    pI830->shadow_present = TRUE;
+
     return rotate_pixmap;
 }
 
@@ -1677,6 +1680,7 @@ i830_crtc_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr rotate_pixmap, void *data)
 {
     ScrnInfoPtr pScrn = crtc->scrn;
     I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
+    I830Ptr pI830 = I830PTR(pScrn);
 
     if (rotate_pixmap)
 	FreeScratchPixmapHeader(rotate_pixmap);
@@ -1687,6 +1691,7 @@ i830_crtc_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr rotate_pixmap, void *data)
 	i830_free_memory(pScrn, intel_crtc->rotate_mem);
 	intel_crtc->rotate_mem = NULL;
     }
+    pI830->shadow_present = FALSE;
 }
 
 #if RANDR_13_INTERFACE
diff --git a/src/i830_dri.c b/src/i830_dri.c
index f03be43..1c25b81 100644
--- a/src/i830_dri.c
+++ b/src/i830_dri.c
@@ -70,6 +70,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include <errno.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <sys/time.h>
+#include <time.h>
 
 #include "xf86.h"
 #include "xf86_OSproc.h"
@@ -1537,18 +1539,13 @@ I830DRI2CreateBuffers(DrawablePtr pDraw, unsigned int *attachments, int count)
     I830Ptr pI830 = I830PTR(pScrn);
     DRI2BufferPtr buffers;
     dri_bo *bo;
-    int i;
-    I830DRI2BufferPrivatePtr privates;
+    int i, j;
+    I830DRI2BufferPrivatePtr private;
     PixmapPtr pPixmap, pDepthPixmap;
 
     buffers = xcalloc(count, sizeof *buffers);
     if (buffers == NULL)
 	return NULL;
-    privates = xcalloc(count, sizeof *privates);
-    if (privates == NULL) {
-	xfree(buffers);
-	return NULL;
-    }
 
     pDepthPixmap = NULL;
     for (i = 0; i < count; i++) {
@@ -1597,12 +1594,21 @@ I830DRI2CreateBuffers(DrawablePtr pDraw, unsigned int *attachments, int count)
 	if (attachments[i] == DRI2BufferDepth)
 	    pDepthPixmap = pPixmap;
 
+	private = xcalloc(1, sizeof *private);
+	if (!private) {
+	    for (j = 0; j < i; j++)
+		xfree(buffers[j].driverPrivate);
+	    xfree(buffers);
+	    return NULL;
+	}
+
+
 	buffers[i].attachment = attachments[i];
 	buffers[i].pitch = pPixmap->devKind;
 	buffers[i].cpp = pPixmap->drawable.bitsPerPixel / 8;
-	buffers[i].driverPrivate = &privates[i];
+	buffers[i].driverPrivate = private;
 	buffers[i].flags = 0; /* not tiled */
-	privates[i].pPixmap = pPixmap;
+	private->pPixmap = pPixmap;
 
 	bo = i830_get_pixmap_bo (pPixmap);
 	if (dri_bo_flink(bo, &buffers[i].name) != 0) {
@@ -1625,11 +1631,11 @@ I830DRI2DestroyBuffers(DrawablePtr pDraw, DRI2BufferPtr buffers, int count)
     {
 	private = buffers[i].driverPrivate;
 	(*pScreen->DestroyPixmap)(private->pPixmap);
+	xfree(buffers[i].driverPrivate);
+	buffers[i].driverPrivate = NULL;
     }
 
-    if (buffers)
-    {
-	xfree(buffers[0].driverPrivate);
+    if (buffers) {
 	xfree(buffers);
     }
 }
@@ -1672,6 +1678,163 @@ I830DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion,
 
 }
 
+/*
+ * At flip time we need to:
+ *  - update X screen pixmap with the new front buffer info
+ *  - update new back buffer info with old front buffer info
+ *  - queue the flip
+ *  - queue a wait so we don't clobber rendering
+ *  - return the new front & back buffer info
+ */
+static Bool
+i830_do_pageflip(DrawablePtr pDraw, DRI2BufferPtr front, DRI2BufferPtr back)
+{
+    ScreenPtr pScreen = pDraw->pScreen;
+    ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
+    I830Ptr pI830 = I830PTR(pScrn);
+    I830DRI2BufferPrivatePtr front_priv, back_priv;
+    dri_bo *front_bo, *back_bo;
+    struct drm_i915_gem_page_flip flip;
+    int ret, tmp_name;
+
+    front_priv = front->driverPrivate;
+    back_priv = back->driverPrivate;
+
+    front_bo = i830_get_pixmap_bo(front_priv->pPixmap);
+    back_bo = i830_get_pixmap_bo(back_priv->pPixmap);
+
+    i830_set_pixmap_bo(front_priv->pPixmap, back_bo);
+    i830_set_pixmap_bo(back_priv->pPixmap, front_bo);
+
+    tmp_name = front->name;
+    front->name = back->name;
+    back->name = tmp_name;
+
+    dri_bo_pin(back_bo, 0);
+
+    pScrn->fbOffset = back_bo->offset;
+    /* If we're in charge of the front buffer, we can flip */
+    if (!pI830->shadow_present) {
+	flip.handle = back_bo->handle;
+	flip.pipe = 1;
+	flip.x = pScrn->virtualX;
+	flip.y = pScrn->virtualY;
+	flip.flags = 0;
+	flip.offset = 0;
+
+	ret = drmCommandWrite(pI830->drmSubFD, DRM_I915_GEM_PAGE_FLIP, &flip,
+			      sizeof(flip));
+	if (ret) {
+	    xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "Page flip failed: %s\n",
+		       strerror(errno));
+	    return FALSE;
+	}
+    }
+
+    return TRUE;
+}
+
+/* Check various flip constraints (drawable parameters vs screen params) */
+static Bool
+i830_flip_ok(DrawablePtr pDraw)
+{
+    ScreenPtr pScreen = pDraw->pScreen;
+    ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
+
+    if (pDraw->width != pScrn->virtualX)
+	return FALSE;
+    if (pDraw->height != pScrn->virtualY)
+	return FALSE;
+    if (pDraw->depth != pScrn->depth)
+	return FALSE;
+
+    return TRUE;
+}
+
+/*
+ * DRI2SwapBuffers should try to do a buffer swap if possible, however:
+ *   - if we're swapping buffers smaller than the screen, we have to blit
+ *   - if the back buffer doesn't match the screen depth, we have to blit
+ *   - otherwise we try to swap, and return to the caller the new front
+ *     and back buffers
+ */
+static DRI2BufferPtr
+I830DRI2SwapBuffers(DrawablePtr pDraw, DRI2BufferPtr buffers, int count)
+{
+    ScreenPtr pScreen = pDraw->pScreen;
+    ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
+    DRI2BufferPtr new_buffers;
+    I830DRI2BufferPrivatePtr private, old_priv;
+    PixmapPtr pPixmap, pDepthPixmap = NULL;
+    int i, j, front = 0, back = 1;
+    dri_bo *bo;
+
+    if (!i830_flip_ok(pDraw))
+	return NULL;
+
+    new_buffers = xcalloc(count, sizeof *buffers);
+    if (new_buffers == NULL)
+	return NULL;
+
+    for (i = 0; i < count; i++) {
+	old_priv = buffers[i].driverPrivate;
+
+	if (buffers[i].attachment == DRI2BufferFrontLeft) {
+	    if (pDraw->type == DRAWABLE_PIXMAP)
+		pPixmap = (PixmapPtr) pDraw;
+	    else
+		pPixmap = (*pScreen->GetWindowPixmap)((WindowPtr)pDraw);
+	    pPixmap->refcnt++;
+	} else if (buffers[i].attachment == DRI2BufferStencil && pDepthPixmap) {
+	    pPixmap = pDepthPixmap;
+	    pPixmap->refcnt++;
+	} else {
+	    pPixmap = (*pScreen->CreatePixmap)(pScreen, 0, 0, pDraw->depth, 0);
+	    (*pScreen->ModifyPixmapHeader)(pPixmap, pDraw->width, pDraw->height,
+					   0, 0, buffers[i].pitch, NULL);
+	}
+
+	if (buffers[i].attachment == DRI2BufferDepth)
+	    pDepthPixmap = pPixmap;
+
+	private = xcalloc(1, sizeof *private);
+	if (!private) {
+	    for (j = 0; j < i; j++)
+		xfree(new_buffers[j].driverPrivate);
+	    xfree(new_buffers);
+	    return NULL;
+	}
+
+	new_buffers[i].attachment = buffers[i].attachment;
+	new_buffers[i].pitch = buffers[i].pitch;
+	new_buffers[i].cpp = buffers[i].cpp;
+	new_buffers[i].driverPrivate = private;
+	new_buffers[i].name = buffers[i].name;
+	new_buffers[i].flags = 0; /* not tiled */
+	private->pPixmap = pPixmap;
+
+	bo = i830_get_pixmap_bo(old_priv->pPixmap);
+	dri_bo_reference(bo);
+	i830_set_pixmap_bo(pPixmap, bo);
+
+	if (buffers[i].attachment == DRI2BufferFrontLeft)
+	    front = i;
+	if (buffers[i].attachment == DRI2BufferBackLeft)
+	    back = i;
+    }
+
+    /* Page flip the full screen buffer */
+    I830Sync(pScrn);
+    if (!i830_do_pageflip(pDraw, &new_buffers[front], &new_buffers[back])) {
+	for (i = 0; i < count; i++)
+	    xfree(new_buffers[i].driverPrivate);
+	xfree(new_buffers);
+	return NULL;
+    }
+
+    return new_buffers;
+}
+
 Bool I830DRI2ScreenInit(ScreenPtr pScreen)
 {
     ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
@@ -1742,6 +1905,7 @@ Bool I830DRI2ScreenInit(ScreenPtr pScreen)
     info.CreateBuffers = I830DRI2CreateBuffers;
     info.DestroyBuffers = I830DRI2DestroyBuffers;
     info.CopyRegion = I830DRI2CopyRegion;
+    info.SwapBuffers = I830DRI2SwapBuffers;
 
     pI830->drmSubFD = info.fd;
 




More information about the Intel-gfx mailing list