All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup.
@ 2012-07-27  9:06 zhigang.gong
  2012-07-27  9:10 ` Chris Wilson
  0 siblings, 1 reply; 4+ messages in thread
From: zhigang.gong @ 2012-07-27  9:06 UTC (permalink / raw)
  To: intel-gfx

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

The previous implementation is to create a new textured
pixmap based on the newly created pixmap's buffer object.

This is not efficient, as we already created it when we
call CreatePixmap. We can just exchange the underlying
texture/image buffers by calling intel_glamor_exchange_buffers().

And this commit seems also fix a weird rendering problem
when working with compiz/mutter.

Signed-off-by: Zhigang Gong <zhigang.gong@linux.intel.com>
---
 src/intel_dri.c |   29 ++++++++++-------------------
 1 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/src/intel_dri.c b/src/intel_dri.c
index d027a64..c168b78 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -110,6 +110,7 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
 {
 	ScreenPtr screen = drawable->pScreen;
 	ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
+	intel_screen_private *intel = intel_get_screen_private(scrn);
 	PixmapPtr old = get_drawable_pixmap(drawable);
 	struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
 	GCPtr gc;
@@ -139,28 +140,20 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
 	}
 
 	intel_set_pixmap_private(pixmap, NULL);
-	screen->DestroyPixmap(pixmap);
 
+	/* Exchange the underlying texture/image. */
+	intel_glamor_exchange_buffers(intel, old, 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(old,
 				   drawable->width,
 				   drawable->height,
 				   0, 0,
 				   priv->stride,
 				   NULL);
-
+	screen->DestroyPixmap(pixmap);
 	intel_get_screen_private(xf86ScreenToScrn(screen))->needs_flush = TRUE;
 	return old;
 }
@@ -194,10 +187,9 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int *attachments,
 		if (attachments[i] == DRI2BufferFrontLeft) {
 			pixmap = get_front_buffer(drawable);
 
-			if (pixmap && intel_get_pixmap_private(pixmap) == NULL) {
+			if (pixmap == NULL) {
+				drawable = &(get_drawable_pixmap(drawable)->drawable);
 				is_glamor_pixmap = TRUE;
-				drawable = &pixmap->drawable;
-				pixmap = NULL;
 			}
 		} else if (attachments[i] == DRI2BufferStencil && pDepthPixmap) {
 			pixmap = pDepthPixmap;
@@ -237,7 +229,7 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int *attachments,
 				goto unwind;
 			}
 
-			if (attachment == DRI2BufferFrontLeft)
+			if (attachment == DRI2BufferFrontLeft && is_glamor_pixmap)
 				pixmap = fixup_glamor(drawable, pixmap);
 		}
 
@@ -314,10 +306,9 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 	if (attachment == DRI2BufferFrontLeft) {
 		pixmap = get_front_buffer(drawable);
 
-		if (pixmap && intel_get_pixmap_private(pixmap) == NULL) {
+		if (pixmap == NULL) {
+			drawable = &(get_drawable_pixmap(drawable)->drawable);
 			is_glamor_pixmap = TRUE;
-			drawable = &pixmap->drawable;
-			pixmap = NULL;
 		}
 	}
 
@@ -391,7 +382,7 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 			free(buffer);
 			return NULL;
 		}
-		if (attachment == DRI2BufferFrontLeft)
+		if (attachment == DRI2BufferFrontLeft && is_glamor_pixmap)
 			pixmap = fixup_glamor(drawable, pixmap);
 	}
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup.
  2012-07-27  9:06 [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup zhigang.gong
@ 2012-07-27  9:10 ` Chris Wilson
  2012-07-27 10:12   ` Zhigang Gong
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2012-07-27  9:10 UTC (permalink / raw)
  To: zhigang.gong, intel-gfx

On Fri, 27 Jul 2012 17:06:21 +0800, zhigang.gong@linux.intel.com wrote:
> -		if (attachment == DRI2BufferFrontLeft)
> +		if (attachment == DRI2BufferFrontLeft && is_glamor_pixmap)
is_glamor_pixmap is only true for FrontLeft, mind resending with this
simplification?

Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup.
  2012-07-27  9:10 ` Chris Wilson
@ 2012-07-27 10:12   ` Zhigang Gong
  2012-07-27 10:33     ` Chris Wilson
  0 siblings, 1 reply; 4+ messages in thread
From: Zhigang Gong @ 2012-07-27 10:12 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Fri, Jul 27, 2012 at 10:10:32AM +0100, Chris Wilson wrote:
> On Fri, 27 Jul 2012 17:06:21 +0800, zhigang.gong@linux.intel.com wrote:
> > -		if (attachment == DRI2BufferFrontLeft)
> > +		if (attachment == DRI2BufferFrontLeft && is_glamor_pixmap)
> is_glamor_pixmap is only true for FrontLeft, mind resending with this
> simplification?

Sure, the simplified version is as below. Thanks.

>From 6224fb9839f417cdd412667f4e6f502df872e7aa Mon Sep 17 00:00:00 2001
From: Zhigang Gong <zhigang.gong@linux.intel.com>
Date: Fri, 27 Jul 2012 16:50:52 +0800
Subject: [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup.

The previous implementation is to create a new textured
pixmap based on the newly created pixmap's buffer object.

This is not efficient, as we already created it when we
call CreatePixmap. We can just exchange the underlying
texture/image buffers by calling intel_glamor_exchange_buffers().

And this commit seems also fix a weird rendering problem
when working with compiz/mutter.

Signed-off-by: Zhigang Gong <zhigang.gong@linux.intel.com>
---
 src/intel_dri.c |   29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/src/intel_dri.c b/src/intel_dri.c
index d027a64..fa1660c 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -110,6 +110,7 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
 {
 	ScreenPtr screen = drawable->pScreen;
 	ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
+	intel_screen_private *intel = intel_get_screen_private(scrn);
 	PixmapPtr old = get_drawable_pixmap(drawable);
 	struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
 	GCPtr gc;
@@ -139,28 +140,20 @@ static PixmapPtr fixup_glamor(DrawablePtr drawable, PixmapPtr pixmap)
 	}
 
 	intel_set_pixmap_private(pixmap, NULL);
-	screen->DestroyPixmap(pixmap);
 
+	/* Exchange the underlying texture/image. */
+	intel_glamor_exchange_buffers(intel, old, 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(old,
 				   drawable->width,
 				   drawable->height,
 				   0, 0,
 				   priv->stride,
 				   NULL);
-
+	screen->DestroyPixmap(pixmap);
 	intel_get_screen_private(xf86ScreenToScrn(screen))->needs_flush = TRUE;
 	return old;
 }
@@ -194,10 +187,9 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int *attachments,
 		if (attachments[i] == DRI2BufferFrontLeft) {
 			pixmap = get_front_buffer(drawable);
 
-			if (pixmap && intel_get_pixmap_private(pixmap) == NULL) {
+			if (pixmap == NULL) {
+				drawable = &(get_drawable_pixmap(drawable)->drawable);
 				is_glamor_pixmap = TRUE;
-				drawable = &pixmap->drawable;
-				pixmap = NULL;
 			}
 		} else if (attachments[i] == DRI2BufferStencil && pDepthPixmap) {
 			pixmap = pDepthPixmap;
@@ -237,7 +229,7 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int *attachments,
 				goto unwind;
 			}
 
-			if (attachment == DRI2BufferFrontLeft)
+			if (is_glamor_pixmap)
 				pixmap = fixup_glamor(drawable, pixmap);
 		}
 
@@ -314,10 +306,9 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 	if (attachment == DRI2BufferFrontLeft) {
 		pixmap = get_front_buffer(drawable);
 
-		if (pixmap && intel_get_pixmap_private(pixmap) == NULL) {
+		if (pixmap == NULL) {
+			drawable = &(get_drawable_pixmap(drawable)->drawable);
 			is_glamor_pixmap = TRUE;
-			drawable = &pixmap->drawable;
-			pixmap = NULL;
 		}
 	}
 
@@ -391,7 +382,7 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 			free(buffer);
 			return NULL;
 		}
-		if (attachment == DRI2BufferFrontLeft)
+		if (is_glamor_pixmap)
 			pixmap = fixup_glamor(drawable, pixmap);
 	}
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup.
  2012-07-27 10:12   ` Zhigang Gong
@ 2012-07-27 10:33     ` Chris Wilson
  0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2012-07-27 10:33 UTC (permalink / raw)
  To: Zhigang Gong; +Cc: intel-gfx

On Fri, 27 Jul 2012 18:12:26 +0800, Zhigang Gong <zhigang.gong@linux.intel.com> wrote:
> On Fri, Jul 27, 2012 at 10:10:32AM +0100, Chris Wilson wrote:
> > On Fri, 27 Jul 2012 17:06:21 +0800, zhigang.gong@linux.intel.com wrote:
> > > -		if (attachment == DRI2BufferFrontLeft)
> > > +		if (attachment == DRI2BufferFrontLeft && is_glamor_pixmap)
> > is_glamor_pixmap is only true for FrontLeft, mind resending with this
> > simplification?
> 
> Sure, the simplified version is as below. Thanks.

Thanks for the patch, pushed
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-07-27 10:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-27  9:06 [PATCH] uxa_glamor_dri: Use exchange buffer in glamor fixup zhigang.gong
2012-07-27  9:10 ` Chris Wilson
2012-07-27 10:12   ` Zhigang Gong
2012-07-27 10:33     ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.