xf86-video-amdgpu: Branch 'master' - 5 commits

Michel Dänzer daenzer at kemper.freedesktop.org
Thu Jun 16 06:31:29 UTC 2016


 src/amdgpu_bo_helper.c |   45 +++++++++++++++++++++------------------------
 src/amdgpu_glamor.c    |   11 +++++------
 src/amdgpu_glamor.h    |    2 +-
 src/amdgpu_kms.c       |    4 +++-
 src/amdgpu_pixmap.h    |   15 ++++++++-------
 src/drmmode_display.c  |   29 +++++++++++++++--------------
 6 files changed, 53 insertions(+), 53 deletions(-)

New commits:
commit 7f7f9825caf3983902491da27c16d14cd8bf9b7d
Author: Michel Dänzer <michel.daenzer at amd.com>
Date:   Mon Jun 13 18:58:49 2016 +0900

    Free priv in amdgpu_set_pixmap_bo also if priv->bo == NULL
    
    Fixes memory leak when destroying pixmaps with priv->bo == NULL.
    
    Reported-by: Qiang Yu <qiang.yu at amd.com>
    Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_pixmap.h b/src/amdgpu_pixmap.h
index 9c51067..a8de26a 100644
--- a/src/amdgpu_pixmap.h
+++ b/src/amdgpu_pixmap.h
@@ -63,10 +63,10 @@ static inline Bool amdgpu_set_pixmap_bo(PixmapPtr pPix, struct amdgpu_buffer *bo
 		return TRUE;
 
 	if (priv) {
-		if (priv->bo == bo)
-			return TRUE;
-
 		if (priv->bo) {
+			if (priv->bo == bo)
+				return TRUE;
+
 			amdgpu_bo_unref(&priv->bo);
 			priv->handle_valid = FALSE;
 		}
commit 397aedafee437c125b8ac1feafb1c3b466842aeb
Author: Michel Dänzer <michel.daenzer at amd.com>
Date:   Mon Jun 13 18:34:11 2016 +0900

    glamor: Fix leak of pixmap private when replacing BO
    
    Reported-by: Qiang Yu <qiang.yu at amd.com>
    Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_glamor.c b/src/amdgpu_glamor.c
index 9785ad7..62831d0 100644
--- a/src/amdgpu_glamor.c
+++ b/src/amdgpu_glamor.c
@@ -303,10 +303,9 @@ amdgpu_glamor_set_pixmap_bo(DrawablePtr drawable, PixmapPtr pixmap)
 		FreeScratchGC(gc);
 	}
 
-	amdgpu_set_pixmap_private(pixmap, NULL);
-
 	/* And redirect the pixmap to the new bo (for 3D). */
 	glamor_egl_exchange_buffers(old, pixmap);
+	amdgpu_set_pixmap_private(pixmap, amdgpu_get_pixmap_private(old));
 	amdgpu_set_pixmap_private(old, priv);
 
 	screen->ModifyPixmapHeader(old,
commit 5b4a8a7a6ed70a50be252fa9b34d3b3a17cdf91a
Author: Michel Dänzer <michel.daenzer at amd.com>
Date:   Tue Jun 14 19:00:18 2016 +0900

    Use amdgpu_set_pixmap_bo in amdgpu_set_shared_pixmap_backing
    
    Fixes leaking any existing pixmap private.
    
    While we're at it, also fix leaking the GBM BO if
    amdgpu_glamor_create_textured_pixmap fails.
    
    Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_bo_helper.c b/src/amdgpu_bo_helper.c
index 5482ff0..7acd005 100644
--- a/src/amdgpu_bo_helper.c
+++ b/src/amdgpu_bo_helper.c
@@ -387,7 +387,7 @@ Bool amdgpu_set_shared_pixmap_backing(PixmapPtr ppix, void *fd_handle)
 	Bool ret;
 
 	if (info->gbm) {
-		struct amdgpu_pixmap *priv;
+		struct amdgpu_buffer *bo;
 		struct gbm_import_fd_data data;
 		uint32_t bo_use = GBM_BO_USE_RENDERING;
 
@@ -396,16 +396,10 @@ Bool amdgpu_set_shared_pixmap_backing(PixmapPtr ppix, void *fd_handle)
 		if (data.format == ~0U)
 			return FALSE;
 
-		priv = calloc(1, sizeof(struct amdgpu_pixmap));
-		if (!priv)
+		bo = calloc(1, sizeof(struct amdgpu_buffer));
+		if (!bo)
 			return FALSE;
-
-		priv->bo = calloc(1, sizeof(struct amdgpu_buffer));
-		if (!priv->bo) {
-			free(priv);
-			return FALSE;
-		}
-		priv->bo->ref_count = 1;
+		bo->ref_count = 1;
 
 		data.fd = ihandle;
 		data.width = ppix->drawable.width;
@@ -415,27 +409,27 @@ Bool amdgpu_set_shared_pixmap_backing(PixmapPtr ppix, void *fd_handle)
 		if (ppix->drawable.bitsPerPixel == pScrn->bitsPerPixel)
 			bo_use |= GBM_BO_USE_SCANOUT;
 
-		priv->bo->bo.gbm = gbm_bo_import(info->gbm, GBM_BO_IMPORT_FD,
-						 &data, bo_use);
-		if (!priv->bo->bo.gbm) {
-			free(priv->bo);
-			free(priv);
+		bo->bo.gbm = gbm_bo_import(info->gbm, GBM_BO_IMPORT_FD, &data,
+					   bo_use);
+		if (!bo->bo.gbm) {
+			free(bo);
 			return FALSE;
 		}
 
-		priv->bo->flags |= AMDGPU_BO_FLAGS_GBM;
+		bo->flags |= AMDGPU_BO_FLAGS_GBM;
 
 #ifdef USE_GLAMOR
 		if (info->use_glamor &&
-		    !amdgpu_glamor_create_textured_pixmap(ppix, priv->bo)) {
-			free(priv->bo);
-			free(priv);
+		    !amdgpu_glamor_create_textured_pixmap(ppix, bo)) {
+			amdgpu_bo_unref(&bo);
 			return FALSE;
 		}
 #endif
 
-		amdgpu_set_pixmap_private(ppix, priv);
-		return TRUE;
+		ret = amdgpu_set_pixmap_bo(ppix, bo);
+		/* amdgpu_set_pixmap_bo increments ref_count if it succeeds */
+		amdgpu_bo_unref(&bo);
+		return ret;
 	}
 
 	pixmap_buffer = amdgpu_gem_bo_open_prime(pAMDGPUEnt->pDev, ihandle, size);
commit c315c00e44afc91a7c8e2eab5af836d9643ebb88
Author: Michel Dänzer <michel.daenzer at amd.com>
Date:   Wed Jun 15 17:20:36 2016 +0900

    Propagate failure from amdgpu_set_pixmap_bo
    
    Preparation for the following fixes.
    
    Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_bo_helper.c b/src/amdgpu_bo_helper.c
index f5a67e3..5482ff0 100644
--- a/src/amdgpu_bo_helper.c
+++ b/src/amdgpu_bo_helper.c
@@ -384,6 +384,7 @@ Bool amdgpu_set_shared_pixmap_backing(PixmapPtr ppix, void *fd_handle)
 	struct amdgpu_buffer *pixmap_buffer = NULL;
 	int ihandle = (int)(long)fd_handle;
 	uint32_t size = ppix->devKind * ppix->drawable.height;
+	Bool ret;
 
 	if (info->gbm) {
 		struct amdgpu_pixmap *priv;
@@ -442,13 +443,15 @@ Bool amdgpu_set_shared_pixmap_backing(PixmapPtr ppix, void *fd_handle)
 		return FALSE;
 	}
 
-	amdgpu_set_pixmap_bo(ppix, pixmap_buffer);
-
 	close(ihandle);
+
+	ret = amdgpu_set_pixmap_bo(ppix, pixmap_buffer);
+
 	/* we have a reference from the alloc and one from set pixmap bo,
 	   drop one */
 	amdgpu_bo_unref(&pixmap_buffer);
-	return TRUE;
+
+	return ret;
 }
 
 #endif /* AMDGPU_PIXMAP_SHARING */
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 3c53bc9..b34a223 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -205,7 +205,9 @@ static Bool AMDGPUCreateScreenResources_KMS(ScreenPtr pScreen)
 	if (info->dri2.enabled || info->use_glamor) {
 		if (info->front_buffer) {
 			PixmapPtr pPix = pScreen->GetScreenPixmap(pScreen);
-			amdgpu_set_pixmap_bo(pPix, info->front_buffer);
+
+			if (!amdgpu_set_pixmap_bo(pPix, info->front_buffer))
+				return FALSE;
 		}
 	}
 
diff --git a/src/amdgpu_pixmap.h b/src/amdgpu_pixmap.h
index ecdd74d..9c51067 100644
--- a/src/amdgpu_pixmap.h
+++ b/src/amdgpu_pixmap.h
@@ -54,17 +54,17 @@ static inline void amdgpu_set_pixmap_private(PixmapPtr pixmap,
 	dixSetPrivate(&pixmap->devPrivates, &amdgpu_pixmap_index, priv);
 }
 
-static inline void amdgpu_set_pixmap_bo(PixmapPtr pPix, struct amdgpu_buffer *bo)
+static inline Bool amdgpu_set_pixmap_bo(PixmapPtr pPix, struct amdgpu_buffer *bo)
 {
 	struct amdgpu_pixmap *priv;
 
 	priv = amdgpu_get_pixmap_private(pPix);
 	if (priv == NULL && bo == NULL)
-		return;
+		return TRUE;
 
 	if (priv) {
 		if (priv->bo == bo)
-			return;
+			return TRUE;
 
 		if (priv->bo) {
 			amdgpu_bo_unref(&priv->bo);
@@ -81,13 +81,14 @@ static inline void amdgpu_set_pixmap_bo(PixmapPtr pPix, struct amdgpu_buffer *bo
 		if (!priv) {
 			priv = calloc(1, sizeof(struct amdgpu_pixmap));
 			if (!priv)
-				goto out;
+				return FALSE;
 		}
 		amdgpu_bo_ref(bo);
 		priv->bo = bo;
 	}
-out:
+
 	amdgpu_set_pixmap_private(pPix, priv);
+	return TRUE;
 }
 
 static inline struct amdgpu_buffer *amdgpu_get_pixmap_bo(PixmapPtr pPix)
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index a675c73..3d76c88 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -111,18 +111,18 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn,
 		return NULL;
 
 	if (!(*pScreen->ModifyPixmapHeader) (pixmap, width, height,
-					     depth, bpp, pitch, NULL)) {
-		return NULL;
-	}
+					     depth, bpp, pitch, NULL))
+		goto fail;
 
-	amdgpu_set_pixmap_bo(pixmap, bo);
+	if (!amdgpu_glamor_create_textured_pixmap(pixmap, bo))
+		goto fail;
 
-	if (!amdgpu_glamor_create_textured_pixmap(pixmap, bo)) {
-		pScreen->DestroyPixmap(pixmap);
-		return NULL;
-	}
+	if (amdgpu_set_pixmap_bo(pixmap, bo))
+		return pixmap;
 
-	return pixmap;
+fail:
+	pScreen->DestroyPixmap(pixmap);
+	return NULL;
 }
 
 static void drmmode_destroy_bo_pixmap(PixmapPtr pixmap)
@@ -1944,9 +1944,14 @@ static Bool drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
 		goto fail;
 	}
 
+	if (!amdgpu_glamor_create_screen_resources(scrn->pScreen))
+		goto fail;
+
 	if (info->use_glamor ||
 	    (info->front_buffer->flags & AMDGPU_BO_FLAGS_GBM)) {
-		amdgpu_set_pixmap_bo(ppix, info->front_buffer);
+		if (!amdgpu_set_pixmap_bo(ppix, info->front_buffer))
+			goto fail;
+
 		screen->ModifyPixmapHeader(ppix,
 					   width, height, -1, -1, pitch, info->front_buffer->cpu_ptr);
 	} else {
@@ -1963,9 +1968,6 @@ static Bool drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
 	scrn->pixmapPrivate.ptr = ppix->devPrivate.ptr;
 #endif
 
-	if (info->use_glamor)
-		amdgpu_glamor_create_screen_resources(scrn->pScreen);
-
 	/* Clear new buffer */
 	gc = GetScratchGC(ppix->drawable.depth, scrn->pScreen);
 	ValidateGC(&ppix->drawable, gc);
commit 74602c4221e3c84949fd69f690cbc66dcae384ea
Author: Michel Dänzer <michel.daenzer at amd.com>
Date:   Tue Jun 14 18:53:34 2016 +0900

    glamor: Make amdgpu_glamor_create_textured_pixmap take amdgpu_buffer*
    
    Preparation for the following fixes.
    
    Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_bo_helper.c b/src/amdgpu_bo_helper.c
index 783347b..f5a67e3 100644
--- a/src/amdgpu_bo_helper.c
+++ b/src/amdgpu_bo_helper.c
@@ -426,7 +426,7 @@ Bool amdgpu_set_shared_pixmap_backing(PixmapPtr ppix, void *fd_handle)
 
 #ifdef USE_GLAMOR
 		if (info->use_glamor &&
-		    !amdgpu_glamor_create_textured_pixmap(ppix, priv)) {
+		    !amdgpu_glamor_create_textured_pixmap(ppix, priv->bo)) {
 			free(priv->bo);
 			free(priv);
 			return FALSE;
diff --git a/src/amdgpu_glamor.c b/src/amdgpu_glamor.c
index 3f4b1af..9785ad7 100644
--- a/src/amdgpu_glamor.c
+++ b/src/amdgpu_glamor.c
@@ -127,7 +127,7 @@ Bool amdgpu_glamor_pre_init(ScrnInfoPtr scrn)
 }
 
 Bool
-amdgpu_glamor_create_textured_pixmap(PixmapPtr pixmap, struct amdgpu_pixmap *priv)
+amdgpu_glamor_create_textured_pixmap(PixmapPtr pixmap, struct amdgpu_buffer *bo)
 {
 	ScrnInfoPtr scrn = xf86ScreenToScrn(pixmap->drawable.pScreen);
 	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
@@ -136,7 +136,7 @@ amdgpu_glamor_create_textured_pixmap(PixmapPtr pixmap, struct amdgpu_pixmap *pri
 	if ((info->use_glamor) == 0)
 		return TRUE;
 
-	if (!amdgpu_bo_get_handle(priv->bo, &bo_handle))
+	if (!amdgpu_bo_get_handle(bo, &bo_handle))
 		return FALSE;
 
 	return glamor_egl_create_textured_pixmap(pixmap, bo_handle,
@@ -233,7 +233,7 @@ amdgpu_glamor_create_pixmap(ScreenPtr screen, int w, int h, int depth,
 
 		pixmap->devPrivate.ptr = NULL;
 
-		if (!amdgpu_glamor_create_textured_pixmap(pixmap, priv))
+		if (!amdgpu_glamor_create_textured_pixmap(pixmap, priv->bo))
 			goto fallback_glamor;
 	}
 
@@ -372,7 +372,7 @@ amdgpu_glamor_set_shared_pixmap_backing(PixmapPtr pixmap, void *handle)
 
 	priv = amdgpu_get_pixmap_private(pixmap);
 
-	if (!amdgpu_glamor_create_textured_pixmap(pixmap, priv)) {
+	if (!amdgpu_glamor_create_textured_pixmap(pixmap, priv->bo)) {
 		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
 			   "Failed to get PRIME drawable for glamor pixmap.\n");
 		return FALSE;
diff --git a/src/amdgpu_glamor.h b/src/amdgpu_glamor.h
index 05ca5cf..4473495 100644
--- a/src/amdgpu_glamor.h
+++ b/src/amdgpu_glamor.h
@@ -69,7 +69,7 @@ void amdgpu_glamor_flush(ScrnInfoPtr pScrn);
 void amdgpu_glamor_finish(ScrnInfoPtr pScrn);
 
 Bool
-amdgpu_glamor_create_textured_pixmap(PixmapPtr pixmap, struct amdgpu_pixmap *priv);
+amdgpu_glamor_create_textured_pixmap(PixmapPtr pixmap, struct amdgpu_buffer *bo);
 void amdgpu_glamor_exchange_buffers(PixmapPtr src, PixmapPtr dst);
 PixmapPtr amdgpu_glamor_set_pixmap_bo(DrawablePtr drawable, PixmapPtr pixmap);
 
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 7b326bb..a675c73 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -117,8 +117,7 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn,
 
 	amdgpu_set_pixmap_bo(pixmap, bo);
 
-	if (!amdgpu_glamor_create_textured_pixmap(pixmap,
-						  amdgpu_get_pixmap_private(pixmap))) {
+	if (!amdgpu_glamor_create_textured_pixmap(pixmap, bo)) {
 		pScreen->DestroyPixmap(pixmap);
 		return NULL;
 	}


More information about the xorg-commit mailing list