[Glamor] [PATCH 1/1 v2] Refine CloseScreen and FreeScreen processes.
Li, Peng
peng.li at intel.com
Tue Feb 7 21:20:21 PST 2012
Zhigang
Please add #if GLAMOR_HAS_GBM before below piece of code. Otherwise it won't build success if no gbm available.
Besides that, The patch works well for me, you can add my name in Test-By.
+ if (glamor_egl->gbm)
+ gbm_device_destroy(glamor_egl->gbm);
+
Thanks
Peng
-----Original Message-----
From: glamor-bounces+peng.li=intel.com at lists.freedesktop.org [mailto:glamor-bounces+peng.li=intel.com at lists.freedesktop.org] On Behalf Of zhigang.gong at linux.intel.com
Sent: Wednesday, February 01, 2012 7:46 PM
To: glamor at lists.freedesktop.org
Cc: zhigang.gong at linux.intel.com
Subject: [Glamor] [PATCH 1/1 v2] Refine CloseScreen and FreeScreen processes.
From: Zhigang Gong <zhigang.gong at linux.intel.com>
This commit move the calling to glamor_close_screen from
glamor_egl_free_screen to glamor_egl_close_screen, as this
is the right place to do this.
We should detach screen fbo and destroy the corresponding
KHR image at glamor_egl_close_screen stage. As latter
DDX driver will call DestroyPixmap to destroy screen pixmap,
if the fbo and image are still there but glamor screen private
data pointer has been freed, then it causes segfault.
This commit also introduces a new flag GLAMOR_USE_EGL_SCREEN.
if DDX driver is using EGL layer then should set this bit
when call to glamor_init and then both glamor_close_screen
and glamor_egl_close_screen will be registered correctly,
DDX layer will not need to call these two functions manually.
This way is also the preferred method within Xorg domain.
As interfaces changed, bump the version to 0.3.0.
Signed-off-by: Zhigang Gong <zhigang.gong at linux.intel.com>
---
configure.ac | 2 +-
src/glamor.c | 24 +++++++----
src/glamor.h | 73 +++++++++++++++++----------------
src/glamor_egl.c | 117 ++++++++++++++++++++++++++++++++++-------------------
src/glamor_fbo.c | 2 +-
src/glamor_priv.h | 1 +
6 files changed, 131 insertions(+), 88 deletions(-)
diff --git a/configure.ac b/configure.ac
index f283d4f..1d19534 100644
--- a/configure.ac
+++ b/configure.ac
@@ -24,7 +24,7 @@
# Initialize Autoconf
AC_PREREQ([2.63])
AC_INIT([glamor-egl],
- [0.2.0],
+ [0.3.0],
[https://bugs.freedesktop.org/enter_bug.cgi?product=glamor],
[glamor-egl])
diff --git a/src/glamor.c b/src/glamor.c
index 80434e9..e900a91 100644
--- a/src/glamor.c
+++ b/src/glamor.c
@@ -318,6 +318,13 @@ glamor_init(ScreenPtr screen, unsigned int flags)
#else
glamor_priv->gl_flavor = GLAMOR_GL_DESKTOP;
#endif
+ /* If we are using egl screen, call egl screen init to
+ * register correct close screen function. */
+ if (flags & GLAMOR_USE_EGL_SCREEN)
+ glamor_egl_screen_init(screen);
+
+ glamor_priv->saved_procs.close_screen = screen->CloseScreen;
+ screen->CloseScreen = glamor_close_screen;
if (flags & GLAMOR_USE_SCREEN) {
if (!RegisterBlockAndWakeupHandlers(_glamor_block_handler,
@@ -327,9 +334,6 @@ glamor_init(ScreenPtr screen, unsigned int flags)
goto fail;
}
- glamor_priv->saved_procs.close_screen = screen->CloseScreen;
- screen->CloseScreen = glamor_close_screen;
-
glamor_priv->saved_procs.create_gc = screen->CreateGC;
screen->CreateGC = glamor_create_gc;
@@ -427,6 +431,9 @@ Bool
glamor_close_screen(int idx, ScreenPtr screen)
{
glamor_screen_private *glamor_priv;
+ PixmapPtr screen_pixmap;
+ glamor_pixmap_private *screen_pixmap_priv;
+ glamor_pixmap_fbo *fbo;
int flags;
glamor_priv = glamor_get_screen_private(screen);
@@ -435,9 +442,9 @@ glamor_close_screen(int idx, ScreenPtr screen)
PictureScreenPtr ps = GetPictureScreenIfSet(screen);
#endif
glamor_glyphs_fini(screen);
+ screen->CloseScreen = glamor_priv->saved_procs.close_screen;
if (flags & GLAMOR_USE_SCREEN) {
- screen->CloseScreen = glamor_priv->saved_procs.close_screen;
screen->CreateGC = glamor_priv->saved_procs.create_gc;
screen->CreatePixmap = glamor_priv->saved_procs.create_pixmap;
screen->DestroyPixmap = glamor_priv->saved_procs.destroy_pixmap;
@@ -457,12 +464,13 @@ glamor_close_screen(int idx, ScreenPtr screen)
ps->CreatePicture = glamor_priv->saved_procs.create_picture;
}
#endif
+ screen_pixmap = screen->GetScreenPixmap(screen);
+ screen_pixmap_priv = glamor_get_pixmap_private(screen_pixmap);
+ fbo = glamor_pixmap_detach_fbo(screen_pixmap_priv);
+ glamor_purge_fbo(fbo);
glamor_release_screen_priv(screen);
- if (flags & GLAMOR_USE_SCREEN)
- return screen->CloseScreen(idx, screen);
- else
- return TRUE;
+ return screen->CloseScreen(idx, screen);
}
diff --git a/src/glamor.h b/src/glamor.h
index fe15700..e29a3e4 100644
--- a/src/glamor.h
+++ b/src/glamor.h
@@ -41,36 +41,6 @@
#endif /* GLAMOR_H */
-/* @GLAMOR_INVERTED_Y_AXIS:
- * set 1 means the GL env's origin (0,0) is at top-left.
- * EGL/DRM platform is an example need to set this bit.
- * glx platform's origin is at bottom-left thus need to
- * clear this bit.*/
-
-#define GLAMOR_INVERTED_Y_AXIS 1
-
-/* @GLAMOR_USE_SCREEN:
- * If want to let glamor to do everything including the
- * create/destroy pixmap and handle the gc ops. need to
- * set this bit. Standalone glamor DDX driver need to set
- * this bit.
- * Otherwise, need to clear this bit, as the intel video
- * driver with glamor enabled.
- * */
-#define GLAMOR_USE_SCREEN 2
-/* @GLAMOR_USE_PICTURE_SCREEN:
- * If want to let glamor to do all the composition related
- * things, need to set this bit. Just as standalone glamor
- * DDX driver.
- * Otherwise, need to clear this bit, as the intel video
- * driver with glamor enabled.
- */
-#define GLAMOR_USE_PICTURE_SCREEN 4
-
-#define GLAMOR_VALID_FLAGS (GLAMOR_INVERTED_Y_AXIS \
- | GLAMOR_USE_SCREEN \
- | GLAMOR_USE_PICTURE_SCREEN)
-
/*
* glamor_pixmap_type : glamor pixmap's type.
* @MEMORY: pixmap is in memory.
@@ -90,15 +60,49 @@ typedef enum glamor_pixmap_type {
} glamor_pixmap_type_t;
#define GLAMOR_EGL_EXTERNAL_BUFFER 3
+#define GLAMOR_INVERTED_Y_AXIS 1
+#define GLAMOR_USE_SCREEN (1 << 1)
+#define GLAMOR_USE_PICTURE_SCREEN (1 << 2)
+#define GLAMOR_USE_EGL_SCREEN (1 << 3)
+#define GLAMOR_VALID_FLAGS (GLAMOR_INVERTED_Y_AXIS \
+ | GLAMOR_USE_SCREEN \
+ | GLAMOR_USE_PICTURE_SCREEN \
+ | GLAMOR_USE_EGL_SCREEN)
+
/* @glamor_init: Initialize glamor internal data structure.
*
* @screen: Current screen pointer.
* @flags: Please refer the flags description above.
*
+ * @GLAMOR_INVERTED_Y_AXIS:
+ * set 1 means the GL env's origin (0,0) is at top-left.
+ * EGL/DRM platform is an example need to set this bit.
+ * glx platform's origin is at bottom-left thus need to
+ * clear this bit.
+ *
+ * @GLAMOR_USE_SCREEN:
+ * If running in an pre-existing X environment, and the
+ * gl context is GLX, then you should set this bit and
+ * let the glamor to handle all the screen related
+ * functions such as GC ops and CreatePixmap/DestroyPixmap.
+ *
+ * @GLAMOR_USE_PICTURE_SCREEN:
+ * If don't use any other underlying DDX driver to handle
+ * the picture related rendering functions, please set this
+ * bit on. Otherwise, clear this bit. And then it is the DDX
+ * driver's responsibility to determine how/when to jump to
+ * glamor's picture compositing path.
+ *
+ * @GLAMOR_USE_EGL_SCREEN:
+ * If you are using EGL layer, then please set this bit
+ * on, otherwise, clear it.
+ *
* This function initializes necessary internal data structure
* for glamor. And before calling into this function, the OpenGL
* environment should be ready. Should be called before any real
- * glamor rendering or texture allocation functions.
+ * glamor rendering or texture allocation functions. And should
+ * be called after the DDX's screen initialization or at the last
+ * step of the DDX's screen initialization.
*/
extern _X_EXPORT Bool glamor_init(ScreenPtr screen, unsigned int flags);
extern _X_EXPORT void glamor_fini(ScreenPtr screen);
@@ -139,6 +143,8 @@ extern _X_EXPORT void glamor_block_handler(ScreenPtr screen);
extern _X_EXPORT PixmapPtr glamor_create_pixmap(ScreenPtr screen, int w, int h, int depth,
unsigned int usage);
+extern _X_EXPORT void glamor_egl_screen_init(ScreenPtr screen);
+
#ifdef GLAMOR_FOR_XORG
/* @glamor_egl_init: Initialize EGL environment.
*
@@ -173,7 +179,6 @@ extern _X_EXPORT Bool glamor_egl_init_textured_pixmap(ScreenPtr screen);
extern _X_EXPORT Bool glamor_egl_create_textured_screen(ScreenPtr screen,
int handle,
int stride);
-
/*
* @glamor_egl_create_textured_pixmap: Try to create a textured pixmap from
* a BO handle.
@@ -191,10 +196,6 @@ extern _X_EXPORT Bool glamor_egl_create_textured_pixmap(PixmapPtr pixmap,
int stride);
extern _X_EXPORT void glamor_egl_destroy_textured_pixmap(PixmapPtr pixmap);
-
-extern _X_EXPORT Bool glamor_egl_close_screen(ScreenPtr screen);
-extern _X_EXPORT void glamor_egl_free_screen(int scrnIndex, int flags);
-
#endif
extern _X_EXPORT int glamor_create_gc(GCPtr gc);
diff --git a/src/glamor_egl.c b/src/glamor_egl.c
index 71121bc..ed98aad 100644
--- a/src/glamor_egl.c
+++ b/src/glamor_egl.c
@@ -84,7 +84,7 @@ struct glamor_egl_screen_private {
CreateScreenResourcesProcPtr CreateScreenResources;
CloseScreenProcPtr CloseScreen;
int fd;
- int front_buffer_handle;
+ EGLImageKHR front_image;
int cpp;
#ifdef GLAMOR_HAS_GBM
struct gbm_device *gbm;
@@ -95,6 +95,8 @@ struct glamor_egl_screen_private {
PFNEGLDESTROYIMAGEKHRPROC egl_destroy_image_khr;
PFNGLEGLIMAGETARGETTEXTURE2DOESPROC egl_image_target_texture2d_oes;
struct glamor_gl_dispatch *dispatch;
+ CloseScreenProcPtr saved_close_screen;
+ xf86FreeScreenProc *saved_free_screen;
};
int xf86GlamorEGLPrivateIndex = -1;
@@ -185,6 +187,8 @@ glamor_egl_create_textured_screen(ScreenPtr screen, int handle, int stride)
return FALSE;
}
+ glamor_egl->front_image = dixLookupPrivate(&screen_pixmap->devPrivates,
+ glamor_egl_pixmap_private_key);
glamor_set_screen_pixmap(screen_pixmap);
return TRUE;
}
@@ -242,37 +246,51 @@ glamor_egl_create_textured_pixmap(PixmapPtr pixmap, int handle, int stride)
return TRUE;
}
-void
-glamor_egl_destroy_textured_pixmap(PixmapPtr pixmap)
+static void
+_glamor_egl_destroy_pixmap_image(PixmapPtr pixmap)
{
- EGLImageKHR image;
ScrnInfoPtr scrn = xf86Screens[pixmap->drawable.pScreen->myNum];
+ EGLImageKHR image;
struct glamor_egl_screen_private *glamor_egl =
glamor_egl_get_screen_private(scrn);
- if (pixmap->refcnt == 1) {
- image = dixLookupPrivate(&pixmap->devPrivates,
- glamor_egl_pixmap_private_key);
- if (image != EGL_NO_IMAGE_KHR && image != NULL) {
- /* Before destroy an image which was attached to
- * a texture. we must call glFlush to make sure the
- * operation on that texture has been done.*/
- glamor_block_handler(pixmap->drawable.pScreen);
- glamor_egl->egl_destroy_image_khr(glamor_egl->display, image);
- }
+ image = dixLookupPrivate(&pixmap->devPrivates,
+ glamor_egl_pixmap_private_key);
+ if (image != EGL_NO_IMAGE_KHR && image != NULL) {
+ /* Before destroy an image which was attached to
+ * a texture. we must call glFlush to make sure the
+ * operation on that texture has been done.*/
+ glamor_block_handler(pixmap->drawable.pScreen);
+ glamor_egl->egl_destroy_image_khr(glamor_egl->display, image);
+ dixSetPrivate(&pixmap->devPrivates, glamor_egl_pixmap_private_key, NULL);
}
+}
+
+void
+glamor_egl_destroy_textured_pixmap(PixmapPtr pixmap)
+{
+ if (pixmap->refcnt == 1)
+ _glamor_egl_destroy_pixmap_image(pixmap);
glamor_destroy_textured_pixmap(pixmap);
}
-Bool
-glamor_egl_close_screen(ScreenPtr screen)
+static Bool
+glamor_egl_close_screen(int idx, ScreenPtr screen)
{
- ScrnInfoPtr scrn = xf86Screens[screen->myNum];
- struct glamor_egl_screen_private *glamor_egl =
- glamor_egl_get_screen_private(scrn);
- glamor_fini(screen);
+ ScrnInfoPtr scrn;
+ struct glamor_egl_screen_private *glamor_egl;
+ PixmapPtr screen_pixmap;
- return TRUE;
+ scrn = xf86Screens[screen->myNum];
+ glamor_egl = glamor_egl_get_screen_private(scrn);
+ screen_pixmap = screen->GetScreenPixmap(screen);
+ glamor_egl->egl_destroy_image_khr(glamor_egl->display, glamor_egl->front_image);
+ dixSetPrivate(&screen_pixmap->devPrivates, glamor_egl_pixmap_private_key, NULL);
+ glamor_egl->front_image = NULL;
+
+ screen->CloseScreen = glamor_egl->saved_close_screen;
+
+ return screen->CloseScreen(idx, screen);
}
static Bool
@@ -299,6 +317,40 @@ glamor_egl_has_extension(struct glamor_egl_screen_private *glamor_egl,
return FALSE;
}
+void
+glamor_egl_screen_init(ScreenPtr screen)
+{
+ ScrnInfoPtr scrn = xf86Screens[screen->myNum];
+ struct glamor_egl_screen_private *glamor_egl =
+ glamor_egl_get_screen_private(scrn);
+
+ glamor_egl->saved_close_screen = screen->CloseScreen;
+ screen->CloseScreen = glamor_egl_close_screen;
+}
+
+static void
+glamor_egl_free_screen(int scrnIndex, int flags)
+{
+ ScrnInfoPtr scrn = xf86Screens[scrnIndex];
+ struct glamor_egl_screen_private *glamor_egl;
+
+ ErrorF("free egl screen resources\n");
+ glamor_egl = glamor_egl_get_screen_private(scrn);
+ if (glamor_egl != NULL) {
+
+ eglMakeCurrent(glamor_egl->display,
+ EGL_NO_SURFACE, EGL_NO_SURFACE,
+ EGL_NO_CONTEXT);
+
+ if (glamor_egl->gbm)
+ gbm_device_destroy(glamor_egl->gbm);
+
+ scrn->FreeScreen = glamor_egl->saved_free_screen;
+ free(glamor_egl);
+ scrn->FreeScreen(scrnIndex, flags);
+ }
+}
+
Bool
glamor_egl_init(ScrnInfoPtr scrn, int fd)
{
@@ -396,6 +448,8 @@ glamor_egl_init(ScrnInfoPtr scrn, int fd)
return FALSE;
}
+ glamor_egl->saved_free_screen = scrn->FreeScreen;
+ scrn->FreeScreen = glamor_egl_free_screen;
return TRUE;
}
@@ -412,27 +466,6 @@ glamor_egl_init_textured_pixmap(ScreenPtr screen)
return TRUE;
}
-void
-glamor_egl_free_screen(int scrnIndex, int flags)
-{
- ScrnInfoPtr scrn = xf86Screens[scrnIndex];
- struct glamor_egl_screen_private *glamor_egl =
- glamor_egl_get_screen_private(scrn);
-
- if (glamor_egl != NULL) {
- if (!(flags & GLAMOR_EGL_EXTERNAL_BUFFER)) {
- eglMakeCurrent(glamor_egl->display,
- EGL_NO_SURFACE, EGL_NO_SURFACE,
- EGL_NO_CONTEXT);
-
- eglTerminate(glamor_egl->display);
- }
- free(glamor_egl);
- }
-
- glamor_close_screen(scrn->scrnIndex, scrn->pScreen);
-}
-
Bool
glamor_gl_dispatch_init(ScreenPtr screen,
struct glamor_gl_dispatch *dispatch,
diff --git a/src/glamor_fbo.c b/src/glamor_fbo.c
index ffda04a..2243564 100644
--- a/src/glamor_fbo.c
+++ b/src/glamor_fbo.c
@@ -123,7 +123,7 @@ glamor_pixmap_fbo_cache_get(glamor_screen_private *glamor_priv,
return NULL;
}
-static void
+void
glamor_purge_fbo(glamor_pixmap_fbo *fbo)
{
glamor_gl_dispatch *dispatch = &fbo->glamor_priv->dispatch;
diff --git a/src/glamor_priv.h b/src/glamor_priv.h
index 9c2881c..d8168a5 100644
--- a/src/glamor_priv.h
+++ b/src/glamor_priv.h
@@ -346,6 +346,7 @@ glamor_pixmap_fbo * glamor_create_fbo_from_tex(glamor_screen_private *glamor_pri
glamor_pixmap_fbo * glamor_create_fbo(glamor_screen_private *glamor_priv,
int w, int h, int depth, int flag);
void glamor_destroy_fbo(glamor_pixmap_fbo *fbo);
+void glamor_purge_fbo(glamor_pixmap_fbo *fbo);
void glamor_init_pixmap_fbo(ScreenPtr screen);
void glamor_fini_pixmap_fbo(ScreenPtr screen);
--
1.7.4.4
_______________________________________________
Glamor mailing list
Glamor at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/glamor
More information about the Glamor
mailing list