[Intel-gfx] [PATCH v3] uxa/glamor: Create glamor pixmap by default.

zhigang.gong at linux.intel.com zhigang.gong at linux.intel.com
Thu Jan 12 08:30:15 CET 2012


From: Zhigang Gong <zhigang.gong at linux.intel.com>

Chris,

According to your previous comments. The X server will not die when
fail to get a dri buffer for a glamor pixmap, instead it will return
a BadAlloc. Your previous advice is to pass a BadMatch to the client,
and I traced into X's dri2 module and found current code can only
return a BadAlloc. IMHO, this should be ok. Right?

Another change is to disable the reuse of bo for glamor textured
pixmap.

The last change is reduce the performance gain. ;), I just found
another 10-20% is come from my latest glamor patchset. 

---
As create glamor pixmap by default will get much better performance
by using the textured-drm pixmap, this commit is to make that the
default behaviour when configure to use glamor. Another advantage
of this commit is that  we reduce the risk of encountering the
"incompatible region exists for this name" and the associated
render corruption. And since we now never intentionally allocate
a reusable pixmap we could just make all (intel_glamor) allocations
non-reusable without incurring too great an overhead.

A side effect is that for those glamor pixmaps, they don't have a
valid BO attached to it and thus it fails to get a DRI drawable. This
commit also fixes that problem by coping the fixup_shadow mechanism. I
tested this with mutter, and it works fine.

The performance gain to apply this patch is about 10% to 20% with
different workload.

Signed-off-by: Zhigang Gong <zhigang.gong at linux.intel.com>
---
 src/intel_dri.c    |   89 +++++++++++++++++++++++++++++++++++++++++++++++----
 src/intel_glamor.c |    2 +
 src/intel_uxa.c    |    6 +++
 3 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/src/intel_dri.c b/src/intel_dri.c
index df3338f..f0494c2 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -66,6 +66,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include "dri2.h"
 
+#include "intel_glamor.h"
+
 typedef struct {
 	int refcnt;
 	PixmapPtr pixmap;
@@ -125,6 +127,65 @@ static PixmapPtr get_front_buffer(DrawablePtr drawable)
 	return pixmap;
 }
 
+static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
+{
+	ScreenPtr screen = drawable->pScreen;
+	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
+	PixmapPtr old = get_drawable_pixmap(drawable);
+	struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
+	GCPtr gc;
+
+	/* With a glamor pixmap, 2D pixmaps are created in texture
+	 * and without a static BO attached to it. To support DRI,
+	 * we need to create a new textured-drm pixmap and
+	 * need to copy the original content to this new textured-drm
+	 * pixmap, and then convert the old pixmap to a coherent
+	 * textured-drm pixmap which has a valid BO attached to it
+	 * and also has a valid texture, thus both glamor and DRI2
+	 * can access it.
+	 *
+	 */
+
+	/* Copy the current contents of the pixmap to the bo. */
+	gc = GetScratchGC(drawable->depth, screen);
+	if (gc) {
+		ValidateGC(&pixmap->drawable, gc);
+		gc->ops->CopyArea(drawable, &pixmap->drawable,
+				  gc,
+				  0, 0,
+				  drawable->width,
+				  drawable->height,
+				  0, 0);
+		FreeScratchGC(gc);
+	}
+
+	intel_set_pixmap_private(pixmap, NULL);
+	screen->DestroyPixmap(pixmap);
+
+	/* And redirect the pixmap to the new bo (for 3D). */
+	intel_set_pixmap_private(old, priv);
+	old->refcnt++;
+
+	/* This creating should not fail, as we already created its
+	 * successfully. But if it happens, we put a warning indicator
+	 * here, and the old pixmap will still be a glamor pixmap, and
+	 * latter the pixmap_flink will get a 0 name, then the X server
+	 * will pass a BadAlloc to the client.*/
+	if (!intel_glamor_create_textured_pixmap(old))
+		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+			   "Failed to get DRI drawable for glamor pixmap.\n");
+
+	screen->ModifyPixmapHeader(pixmap,
+				   drawable->width,
+				   drawable->height,
+				   0, 0,
+				   priv->stride,
+				   NULL);
+
+	intel_get_screen_private(xf86Screens[screen->myNum])->needs_flush = TRUE;
+	return old;
+}
+
 static PixmapPtr fixup_shadow(DrawablePtr drawable, PixmapPtr pixmap)
 {
 	ScreenPtr screen = drawable->pScreen;
@@ -203,6 +264,7 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int *attachments,
 	I830DRI2BufferPrivatePtr privates;
 	PixmapPtr pixmap, pDepthPixmap;
 	int i;
+	int is_glamor_pixmap;
 
 	buffers = calloc(count, sizeof *buffers);
 	if (buffers == NULL)
@@ -222,7 +284,10 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int *attachments,
 			pixmap = pDepthPixmap;
 			pixmap->refcnt++;
 		}
-		if (pixmap == NULL) {
+
+		is_glamor_pixmap = pixmap && (intel_get_pixmap_private(pixmap) == NULL);
+
+		if (pixmap == NULL || is_glamor_pixmap) {
 			unsigned int hint = INTEL_CREATE_PIXMAP_DRI2;
 
 			if (intel->tiling & INTEL_TILING_3D) {
@@ -255,8 +320,12 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int *attachments,
 				goto unwind;
 			}
 
-			if (attachment == DRI2BufferFrontLeft)
-				pixmap = fixup_shadow(drawable, pixmap);
+			if (attachment == DRI2BufferFrontLeft) {
+				if (!is_glamor_pixmap)
+					pixmap = fixup_shadow(drawable, pixmap);
+				else
+					pixmap = fixup_glamor(drawable, pixmap);
+			}
 		}
 
 		if (attachments[i] == DRI2BufferDepth)
@@ -317,6 +386,7 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 	DRI2Buffer2Ptr buffer;
 	I830DRI2BufferPrivatePtr privates;
 	PixmapPtr pixmap;
+	int is_glamor_pixmap;
 
 	buffer = calloc(1, sizeof *buffer);
 	if (buffer == NULL)
@@ -330,7 +400,9 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 	pixmap = NULL;
 	if (attachment == DRI2BufferFrontLeft)
 		pixmap = get_front_buffer(drawable);
-	if (pixmap == NULL) {
+
+	is_glamor_pixmap = pixmap && (intel_get_pixmap_private(pixmap) == NULL);
+	if (pixmap == NULL || is_glamor_pixmap) {
 		unsigned int hint = INTEL_CREATE_PIXMAP_DRI2;
 		int pixmap_width = drawable->width;
 		int pixmap_height = drawable->height;
@@ -400,9 +472,12 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 			free(buffer);
 			return NULL;
 		}
-
-		if (attachment == DRI2BufferFrontLeft)
-			pixmap = fixup_shadow(drawable, pixmap);
+		if (attachment == DRI2BufferFrontLeft) {
+			if (!is_glamor_pixmap)
+				pixmap = fixup_shadow(drawable, pixmap);
+			else
+				pixmap = fixup_glamor(drawable, pixmap);
+		}
 	}
 
 	buffer->attachment = attachment;
diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 669b1b8..e96daa6 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -36,6 +36,7 @@
 #include <glamor.h>
 
 #include "intel.h"
+#include "i915_drm.h"
 #include "intel_glamor.h"
 #include "uxa.h"
 
@@ -98,6 +99,7 @@ intel_glamor_create_textured_pixmap(PixmapPtr pixmap)
 	priv = intel_get_pixmap_private(pixmap);
 	if (glamor_egl_create_textured_pixmap(pixmap, priv->bo->handle,
 					      priv->stride)) {
+		drm_intel_bo_disable_reuse(priv->bo);
 		priv->pinned = 1;
 		return TRUE;
 	} else
diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 9d74554..f04a2ef 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -1026,6 +1026,12 @@ intel_uxa_create_pixmap(ScreenPtr screen, int w, int h, int depth,
 	struct intel_pixmap *priv;
 	PixmapPtr pixmap, new_pixmap = NULL;
 
+	if (!(usage & INTEL_CREATE_PIXMAP_DRI2)) {
+		pixmap = intel_glamor_create_pixmap(screen, w, h, depth, usage);
+		if (pixmap)
+			return pixmap;
+	}
+
 	if (w > 32767 || h > 32767)
 		return NullPixmap;
 
-- 
1.7.4.4




More information about the Intel-gfx mailing list