[PATCH 0/4] uxa/glamor: performance tuning.

[PATCH 0/4] uxa/glamor: performance tuning.

[zhigang.gong]

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

According to your advice, split the previous 1 patch to 4.
And slightly changed the patch of create GC. The previous
verison will override the GC ops to glamor's GC ops which
is really not what I expected. If there are still some
other unexpected side effects, please help to point out
thanks. This patchset need to work with the latest glamor. 
I will move glamor to official repository this week and
gave an initial version 0.1.0 and will bump the version
number each time the API changed.

Signed-off-by: Zhigang Gong <zhigang.gong@linux.intel.com>
-

[PATCH 1/4] uxa/glamor: Remove dead code.

[zhigang.gong]

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

Signed-off-by: Zhigang Gong <zhigang.gong@linux.intel.com>
---
 src/intel_glamor.c |   13 -------------
 src/intel_glamor.h |    2 --
 2 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 8daa4b1..2b56ae5 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -192,19 +192,6 @@ intel_glamor_flush(intel_screen_private * intel)
 }
 
 Bool
-intel_glamor_create_screen_image(ScreenPtr screen, int handle, int stride)
-{
-	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
-	intel_screen_private *intel;
-
-	intel = intel_get_screen_private(scrn);
-	if ((intel->uxa_flags & UXA_USE_GLAMOR) == 0)
-		return TRUE;
-
-	return glamor_egl_create_textured_screen(screen, handle, stride);
-}
-
-Bool
 intel_glamor_close_screen(ScreenPtr screen)
 {
 	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
diff --git a/src/intel_glamor.h b/src/intel_glamor.h
index 1374588..3065132 100644
--- a/src/intel_glamor.h
+++ b/src/intel_glamor.h
@@ -35,7 +35,6 @@
 Bool intel_glamor_pre_init(ScrnInfoPtr scrn);
 Bool intel_glamor_init(ScreenPtr screen);
 Bool intel_glamor_create_screen_resources(ScreenPtr screen);
-Bool intel_glamor_create_screen_image(ScreenPtr screen, int handle, int stride);
 Bool intel_glamor_close_screen(ScreenPtr screen);
 void intel_glamor_free_screen(int scrnIndex, int flags);
 
@@ -51,7 +50,6 @@ PixmapPtr intel_glamor_create_pixmap(ScreenPtr screen, int w, int h,
 static inline Bool intel_glamor_pre_init(ScrnInfoPtr scrn) { return TRUE; }
 static inline Bool intel_glamor_init(ScreenPtr screen) { return TRUE; }
 static inline Bool intel_glamor_create_screen_resources(ScreenPtr screen) { return TRUE; }
-static inline Bool intel_glamor_create_screen_image(ScreenPtr screen, int handle, int stride) { return TRUE; }
 static inline Bool intel_glamor_close_screen(ScreenPtr screen) { return TRUE; }
 static inline void intel_glamor_free_screen(int scrnIndex, int flags) { }
 
-- 
1.7.4.4

[PATCH 2/4] uxa/glamor: Remove extraneous flush.

[zhigang.gong]

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

When glamor is enabled, a pixmap will not be accessed by the UXA's
accelerated function. Only un-accelerated funtion may access those
pixmap, and before each un-accelerated rendering, it calls
uxa_prepare_access which will do glFlush there.

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

diff --git a/src/intel_glamor.c b/src/intel_glamor.c
index 2b56ae5..669b1b8 100644
--- a/src/intel_glamor.c
+++ b/src/intel_glamor.c
@@ -135,7 +135,6 @@ intel_glamor_finish_access(PixmapPtr pixmap, uxa_access_t access)
 		break;
 	case UXA_GLAMOR_ACCESS_RW:
 		intel_glamor_need_flush(&pixmap->drawable);
-		glamor_block_handler(pixmap->drawable.pScreen);
 		break;
 	default:
 		ErrorF("Invalid access mode %d\n", access);
-- 
1.7.4.4

[PATCH 3/4] uxa/glamor: Let glamor do the GC validation.

[zhigang.gong]

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

If we are using GLAMOR, then a tile pixmap or stipple pixmap
may be pure glamor pixmap and thus UXA doesn't know how to
render to them, we should let glamor to do the validation.

Signed-off-by: Zhigang Gong <zhigang.gong@linux.intel.com>
---
 uxa/uxa-glamor.h |    1 +
 uxa/uxa.c        |   12 ++++++++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/uxa/uxa-glamor.h b/uxa/uxa-glamor.h
index 71a9c29..2b4b452 100644
--- a/uxa/uxa-glamor.h
+++ b/uxa/uxa-glamor.h
@@ -51,6 +51,7 @@
 #define glamor_trapezoids_nf(...)	FALSE
 #define glamor_triangles_nf(...)	FALSE
 #define glamor_add_traps_nf(...)	FALSE
+#define glamor_create_gc(...)		FALSE
 #endif
 
 #endif /* UXA_GLAMOR_H */
diff --git a/uxa/uxa.c b/uxa/uxa.c
index 5b3d709..56bc233 100644
--- a/uxa/uxa.c
+++ b/uxa/uxa.c
@@ -38,6 +38,7 @@
 #include <X11/fonts/fontstruct.h>
 #include "dixfontstr.h"
 #include "uxa.h"
+#include "uxa-glamor.h"
 
 #if HAS_DEVPRIVATEKEYREC
 DevPrivateKeyRec uxa_screen_index;
@@ -183,11 +184,21 @@ void uxa_finish_access(DrawablePtr pDrawable, uxa_access_t access)
 static void
 uxa_validate_gc(GCPtr pGC, unsigned long changes, DrawablePtr pDrawable)
 {
+	uxa_screen_t *uxa_screen = uxa_get_screen(pGC->pScreen);
 	/* fbValidateGC will do direct access to pixmaps if the tiling has
 	 * changed.
 	 * Preempt fbValidateGC by doing its work and masking the change out, so
 	 * that we can do the Prepare/finish_access.
 	 */
+
+	/* If we are using GLAMOR, then the tile or stipple pixmap
+	 * may be pure GLAMOR pixmap, then we should let the glamor
+	 * to do the validation.
+	 */
+	if (uxa_screen->info->flags & UXA_USE_GLAMOR) {
+		glamor_validate_gc(pGC, changes, pDrawable);
+		goto set_ops;
+	}
 #ifdef FB_24_32BIT
 	if ((changes & GCTile) && fbGetRotatedPixmap(pGC)) {
 		(*pGC->pScreen->DestroyPixmap) (fbGetRotatedPixmap(pGC));
@@ -256,6 +267,7 @@ uxa_validate_gc(GCPtr pGC, unsigned long changes, DrawablePtr pDrawable)
 		fbValidateGC(pGC, changes, pDrawable);
 	}
 
+set_ops:
 	pGC->ops = (GCOps *) & uxa_ops;
 }
 
-- 
1.7.4.4

[PATCH 4/4] uxa/glamor: Create glamor pixmap by default.

[zhigang.gong]

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

As a pure glamor pixmap has a local texture rather than
bind a pixmap to a external BO. This can avoid some
unecessary flush, and can achieve better performance.
The testing on my machine shows that aa10text/rgb10text
get about 20-30% performance improvement.

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

diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index 9d74554..ca3220a 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -1026,6 +1026,11 @@ 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

Re: [PATCH 4/4] uxa/glamor: Create glamor pixmap by default.

[Chris Wilson]

On Tue, 27 Dec 2011 17:09:18 +0800, zhigang.gong@linux.intel.com wrote:
> From: Zhigang Gong <zhigang.gong@linux.intel.com>
> 
> As a pure glamor pixmap has a local texture rather than
> bind a pixmap to a external BO. This can avoid some
> unecessary flush, and can achieve better performance.
> The testing on my machine shows that aa10text/rgb10text
> get about 20-30% performance improvement.

How do we get the name back for a pixmap created by glamor and then
attached to a DRI2Drawable? And then once that name is exposed, how do
you prevent GL from recreating the bo attached to the pixmap?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 4/4] uxa/glamor: Create glamor pixmap by default.

[Zhigang Gong]

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Wednesday, December 28, 2011 10:27 PM
> To: zhigang.gong@linux.intel.com
> Cc: intel-gfx@lists.freedesktop.org; zhigang.gong@gmail.com;
> zhigang.gong@linux.intel.com
> Subject: Re: [PATCH 4/4] uxa/glamor: Create glamor pixmap by default.
> 
> On Tue, 27 Dec 2011 17:09:18 +0800, zhigang.gong@linux.intel.com
> wrote:
> > From: Zhigang Gong <zhigang.gong@linux.intel.com>
> >
> > As a pure glamor pixmap has a local texture rather than bind a pixmap
> > to a external BO. This can avoid some unecessary flush, and can
> > achieve better performance.
> > The testing on my machine shows that aa10text/rgb10text get about
> > 20-30% performance improvement.
> 
> How do we get the name back for a pixmap created by glamor and then
> attached to a DRI2Drawable? And then once that name is exposed, how do
> you prevent GL from recreating the bo attached to the pixmap?

You are right, this patch will break the DRI2 usage.
And currently, one solution come to my mind:

At I830DRI2Create buffer, After we get a non-null pixmap at get_front_buffer

we check whether The pixmap has a valid bo, if it doesn't, we will think
this is
a pure glamor pixmap (maybe we can check more fields, such as zero
devPrivate.ptr)
Then we will create new a valid DRI2 pixmap, then we call fixup_glamor to
unbind
the bo from the new DRI2 pixmap and then bind it to the old pure glamor
pixmap
and call intel_create_textured_pixmap to create a coherent glamor-drm 
pixmap, the old pixmap's pointer will not change, only the type will be
changed
to a glamor-drm and has a valid bo attached to it.

Does this way work? And will it bring too much overhead.

Base on my current testing, change to use pure glamor pixmap for non-dri2 
pixmap really get much better performance then use coherent glamor-drm 
pixmap.

Any comments？

> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx