[PATCH xf86-video-amdgpu] Use reference counting for tracking KMS framebuffer lifetimes

Michel Dänzer michel at daenzer.net
Mon May 29 08:07:48 UTC 2017


From: Michel Dänzer <michel.daenzer at amd.com>

References are held by the pixmaps corresponding to the FBs (so
the same KMS FB can be reused as long as the pixmap exists) and by the
CRTCs scanning out from them (so a KMS FB is only destroyed once it's
not being scanned out anymore, preventing intermittent black screens and
worse issues due to a CRTC turning off when it should be on).

v2:
* Only increase reference count in drmmode_fb_reference if it was sane
  before
* Make drmmode_fb_reference's indentation match the rest of
  drmmode_display.h

(Ported from radeon commit 55e513b978b2afc52b7cafc5bfcb0d1dc78d75f6)

Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
---
 src/amdgpu_kms.c      |  70 +++++++++++++++++++---
 src/amdgpu_pixmap.h   |  58 +++++++++++++++++++
 src/amdgpu_present.c  |   9 ---
 src/drmmode_display.c | 157 ++++++++++++++++++++------------------------------
 src/drmmode_display.h |  42 ++++++++++++--
 5 files changed, 219 insertions(+), 117 deletions(-)

diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index a418cf9d3..69d61943d 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -675,6 +675,18 @@ amdgpu_prime_scanout_flip_abort(xf86CrtcPtr crtc, void *event_data)
 }
 
 static void
+amdgpu_prime_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc, uint64_t usec,
+				  void *event_data)
+{
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
+	drmmode_crtc_private_ptr drmmode_crtc = event_data;
+
+	drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
+			     drmmode_crtc->flip_pending);
+	amdgpu_prime_scanout_flip_abort(crtc, event_data);
+}
+
+static void
 amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
 {
 	ScreenPtr screen = ent->slave_dst->drawable.pScreen;
@@ -701,7 +713,8 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
 	drm_queue_seq = amdgpu_drm_queue_alloc(crtc,
 					       AMDGPU_DRM_QUEUE_CLIENT_DEFAULT,
 					       AMDGPU_DRM_QUEUE_ID_DEFAULT,
-					       drmmode_crtc, NULL,
+					       drmmode_crtc,
+					       amdgpu_prime_scanout_flip_handler,
 					       amdgpu_prime_scanout_flip_abort);
 	if (drm_queue_seq == AMDGPU_DRM_QUEUE_ERROR) {
 		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
@@ -709,8 +722,17 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
 		return;
 	}
 
+	drmmode_crtc->flip_pending =
+		amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
+	if (!drmmode_crtc->flip_pending) {
+		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+			   "Failed to get FB for PRIME flip.\n");
+		amdgpu_drm_abort_entry(drm_queue_seq);
+		return;
+	}
+
 	if (drmmode_page_flip_target_relative(pAMDGPUEnt, drmmode_crtc,
-					      drmmode_crtc->scanout[scanout_id].fb_id,
+					      drmmode_crtc->flip_pending->handle,
 					      0, drm_queue_seq, 0) != 0) {
 		xf86DrvMsg(scrn->scrnIndex, X_WARNING, "flip queue failed in %s: %s\n",
 			   __func__, strerror(errno));
@@ -720,7 +742,6 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
 
 	drmmode_crtc->scanout_id = scanout_id;
 	drmmode_crtc->scanout_update_pending = TRUE;
-	drmmode_crtc->flip_pending = TRUE;
 }
 
 static void
@@ -950,10 +971,14 @@ amdgpu_scanout_update(xf86CrtcPtr xf86_crtc)
 static void
 amdgpu_scanout_flip_abort(xf86CrtcPtr crtc, void *event_data)
 {
-	drmmode_crtc_private_ptr drmmode_crtc = event_data;
+	amdgpu_prime_scanout_flip_abort(crtc, event_data);
+}
 
-	drmmode_crtc->scanout_update_pending = FALSE;
-	drmmode_clear_pending_flip(crtc);
+static void
+amdgpu_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc, uint64_t usec,
+			    void *event_data)
+{
+	amdgpu_prime_scanout_flip_handler(crtc, msc, usec, event_data);
 }
 
 static void
@@ -977,7 +1002,8 @@ amdgpu_scanout_flip(ScreenPtr pScreen, AMDGPUInfoPtr info,
 	drm_queue_seq = amdgpu_drm_queue_alloc(xf86_crtc,
 					       AMDGPU_DRM_QUEUE_CLIENT_DEFAULT,
 					       AMDGPU_DRM_QUEUE_ID_DEFAULT,
-					       drmmode_crtc, NULL,
+					       drmmode_crtc,
+					       amdgpu_scanout_flip_handler,
 					       amdgpu_scanout_flip_abort);
 	if (drm_queue_seq == AMDGPU_DRM_QUEUE_ERROR) {
 		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
@@ -985,8 +1011,17 @@ amdgpu_scanout_flip(ScreenPtr pScreen, AMDGPUInfoPtr info,
 		return;
 	}
 
+	drmmode_crtc->flip_pending =
+		amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
+	if (!drmmode_crtc->flip_pending) {
+		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+			   "Failed to get FB for scanout flip.\n");
+		amdgpu_drm_abort_entry(drm_queue_seq);
+		return;
+	}
+
 	if (drmmode_page_flip_target_relative(pAMDGPUEnt, drmmode_crtc,
-					      drmmode_crtc->scanout[scanout_id].fb_id,
+					      drmmode_crtc->flip_pending->handle,
 					      0, drm_queue_seq, 0) != 0) {
 		xf86DrvMsg(scrn->scrnIndex, X_WARNING, "flip queue failed in %s: %s\n",
 			   __func__, strerror(errno));
@@ -996,13 +1031,13 @@ amdgpu_scanout_flip(ScreenPtr pScreen, AMDGPUInfoPtr info,
 
 	drmmode_crtc->scanout_id = scanout_id;
 	drmmode_crtc->scanout_update_pending = TRUE;
-	drmmode_crtc->flip_pending = TRUE;
 }
 
 static void AMDGPUBlockHandler_KMS(BLOCKHANDLER_ARGS_DECL)
 {
 	SCREEN_PTR(arg);
 	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
 	xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
 	int c;
@@ -1011,6 +1046,23 @@ static void AMDGPUBlockHandler_KMS(BLOCKHANDLER_ARGS_DECL)
 	(*pScreen->BlockHandler) (BLOCKHANDLER_ARGS);
 	pScreen->BlockHandler = AMDGPUBlockHandler_KMS;
 
+	if (!pScrn->vtSema) {
+#if XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,19,0,0,0)
+		if (info->use_glamor)
+			amdgpu_glamor_flush(pScrn);
+#endif
+
+		for (c = 0; c < xf86_config->num_crtc; c++) {
+			drmmode_crtc_private_ptr drmmode_crtc =
+				xf86_config->crtc[c]->driver_private;
+
+			drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
+					     NULL);
+		}
+
+		return;
+	}
+
 	if (!amdgpu_is_gpu_screen(pScreen))
 	{
 		for (c = 0; c < xf86_config->num_crtc; c++) {
diff --git a/src/amdgpu_pixmap.h b/src/amdgpu_pixmap.h
index a8de26a93..00fb5bf05 100644
--- a/src/amdgpu_pixmap.h
+++ b/src/amdgpu_pixmap.h
@@ -35,6 +35,7 @@ struct amdgpu_pixmap {
 	uint64_t tiling_info;
 
 	struct amdgpu_buffer *bo;
+	struct drmmode_fb *fb;
 
 	/* GEM handle for pixmaps shared via DRI2/3 */
 	Bool handle_valid;
@@ -56,6 +57,8 @@ static inline void amdgpu_set_pixmap_private(PixmapPtr pixmap,
 
 static inline Bool amdgpu_set_pixmap_bo(PixmapPtr pPix, struct amdgpu_buffer *bo)
 {
+	ScrnInfoPtr scrn = xf86ScreenToScrn(pPix->drawable.pScreen);
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
 	struct amdgpu_pixmap *priv;
 
 	priv = amdgpu_get_pixmap_private(pPix);
@@ -71,6 +74,8 @@ static inline Bool amdgpu_set_pixmap_bo(PixmapPtr pPix, struct amdgpu_buffer *bo
 			priv->handle_valid = FALSE;
 		}
 
+		drmmode_fb_reference(pAMDGPUEnt->fd, &priv->fb, NULL);
+
 		if (!bo) {
 			free(priv);
 			priv = NULL;
@@ -98,6 +103,59 @@ static inline struct amdgpu_buffer *amdgpu_get_pixmap_bo(PixmapPtr pPix)
 	return priv ? priv->bo : NULL;
 }
 
+static inline struct drmmode_fb*
+amdgpu_fb_create(int drm_fd, uint32_t width, uint32_t height, uint8_t depth,
+		 uint8_t bpp, uint32_t pitch, uint32_t handle)
+{
+	struct drmmode_fb *fb  = malloc(sizeof(*fb));
+
+	if (!fb)
+		return NULL;
+
+	fb->refcnt = 1;
+	if (drmModeAddFB(drm_fd, width, height, depth, bpp, pitch, handle,
+			 &fb->handle) == 0)
+		return fb;
+
+	free(fb);
+	return NULL;
+}
+
+static inline struct drmmode_fb*
+amdgpu_pixmap_create_fb(int drm_fd, PixmapPtr pix)
+{
+	uint32_t handle;
+
+	if (!amdgpu_pixmap_get_handle(pix, &handle))
+		return NULL;
+
+	return amdgpu_fb_create(drm_fd, pix->drawable.width, pix->drawable.height,
+				pix->drawable.depth, pix->drawable.bitsPerPixel,
+				pix->devKind, handle);
+}
+
+static inline struct drmmode_fb*
+amdgpu_pixmap_get_fb(PixmapPtr pix)
+{
+	ScrnInfoPtr scrn = xf86ScreenToScrn(pix->drawable.pScreen);
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
+	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
+
+	if (info->use_glamor) {
+		struct amdgpu_pixmap *priv = amdgpu_get_pixmap_private(pix);
+
+		if (!priv)
+			return NULL;
+
+		if (!priv->fb)
+			priv->fb = amdgpu_pixmap_create_fb(pAMDGPUEnt->fd, pix);
+
+		return priv->fb;
+	}
+
+	return NULL;
+}
+
 enum {
 	AMDGPU_CREATE_PIXMAP_DRI2    = 0x08000000,
 	AMDGPU_CREATE_PIXMAP_LINEAR  = 0x04000000,
diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
index 3c4bc5259..86b095cd1 100644
--- a/src/amdgpu_present.c
+++ b/src/amdgpu_present.c
@@ -350,7 +350,6 @@ static void
 amdgpu_present_unflip(ScreenPtr screen, uint64_t event_id)
 {
 	ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
 	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
 	xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
 	struct amdgpu_present_vblank_event *event;
@@ -358,7 +357,6 @@ amdgpu_present_unflip(ScreenPtr screen, uint64_t event_id)
 	enum drmmode_flip_sync flip_sync =
 		(amdgpu_present_screen_info.capabilities & PresentCapabilityAsync) ?
 		FLIP_ASYNC : FLIP_VSYNC;
-	int old_fb_id;
 	int i;
 
 	if (!amdgpu_present_check_unflip(scrn))
@@ -380,12 +378,6 @@ amdgpu_present_unflip(ScreenPtr screen, uint64_t event_id)
 		return;
 
 modeset:
-	/* info->drmmode.fb_id still points to the FB for the last flipped BO.
-	 * Clear it, drmmode_set_mode_major will re-create it
-	 */
-	old_fb_id = info->drmmode.fb_id;
-	info->drmmode.fb_id = 0;
-
 	amdgpu_glamor_finish(scrn);
 	for (i = 0; i < config->num_crtc; i++) {
 		xf86CrtcPtr crtc = config->crtc[i];
@@ -401,7 +393,6 @@ modeset:
 			drmmode_crtc->need_modeset = TRUE;
 	}
 
-	drmModeRmFB(pAMDGPUEnt->fd, old_fb_id);
 	present_event_notify(event_id, 0, 0);
 	info->drmmode.present_flipping = FALSE;
 }
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index add8287a0..f4bea0ce2 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -322,6 +322,7 @@ drmmode_crtc_dpms(xf86CrtcPtr crtc, int mode)
 
 		drmModeSetCrtc(pAMDGPUEnt->fd, drmmode_crtc->mode_crtc->crtc_id,
 			       0, 0, 0, NULL, 0, NULL);
+		drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb, NULL);
 	} else if (drmmode_crtc->dpms_mode != DPMSModeOn)
 		crtc->funcs->set_mode_major(crtc, &crtc->mode, crtc->rotation,
 					    crtc->x, crtc->y);
@@ -390,8 +391,9 @@ void drmmode_copy_fb(ScrnInfoPtr pScrn, drmmode_ptr drmmode)
 {
 	xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
-	PixmapPtr src, dst;
 	ScreenPtr pScreen = pScrn->pScreen;
+	PixmapPtr src, dst = pScreen->GetScreenPixmap(pScreen);
+	struct drmmode_fb *fb = amdgpu_pixmap_get_fb(dst);
 	int fbcon_id = 0;
 	GCPtr gc;
 	int i;
@@ -406,7 +408,7 @@ void drmmode_copy_fb(ScrnInfoPtr pScrn, drmmode_ptr drmmode)
 	if (!fbcon_id)
 		return;
 
-	if (fbcon_id == drmmode->fb_id) {
+	if (fbcon_id == fb->handle) {
 		/* in some rare case there might be no fbcon and we might already
 		 * be the one with the current fb to avoid a false deadlck in
 		 * kernel ttm code just do nothing as anyway there is nothing
@@ -419,8 +421,6 @@ void drmmode_copy_fb(ScrnInfoPtr pScrn, drmmode_ptr drmmode)
 	if (!src)
 		return;
 
-	dst = pScreen->GetScreenPixmap(pScreen);
-
 	gc = GetScratchGC(pScrn->depth, pScreen);
 	ValidateGC(&dst->drawable, gc);
 
@@ -449,10 +449,6 @@ drmmode_crtc_scanout_destroy(drmmode_ptr drmmode,
 	}
 
 	if (scanout->bo) {
-		AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(drmmode->scrn);
-
-		drmModeRmFB(pAMDGPUEnt->fd, scanout->fb_id);
-		scanout->fb_id = 0;
 		amdgpu_bo_unref(&scanout->bo);
 		scanout->bo = NULL;
 	}
@@ -497,10 +493,8 @@ drmmode_crtc_scanout_create(xf86CrtcPtr crtc, struct drmmode_scanout *scanout,
 			    int width, int height)
 {
 	ScrnInfoPtr pScrn = crtc->scrn;
-	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
 	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 	drmmode_ptr drmmode = drmmode_crtc->drmmode;
-	union gbm_bo_handle bo_handle;
 	int pitch;
 
 	if (scanout->pixmap) {
@@ -516,15 +510,7 @@ drmmode_crtc_scanout_create(xf86CrtcPtr crtc, struct drmmode_scanout *scanout,
 	if (!scanout->bo) {
 		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
 			   "Failed to allocate scanout buffer memory\n");
-		goto error;
-	}
-
-	bo_handle = gbm_bo_get_handle(scanout->bo->bo.gbm);
-	if (drmModeAddFB(pAMDGPUEnt->fd, width, height, pScrn->depth,
-			 pScrn->bitsPerPixel, pitch,
-			 bo_handle.u32, &scanout->fb_id) != 0) {
-		ErrorF("failed to add scanout fb\n");
-		goto error;
+		return NULL;
 	}
 
 	scanout->pixmap = drmmode_create_bo_pixmap(pScrn,
@@ -532,13 +518,17 @@ drmmode_crtc_scanout_create(xf86CrtcPtr crtc, struct drmmode_scanout *scanout,
 						 pScrn->depth,
 						 pScrn->bitsPerPixel,
 						 pitch, scanout->bo);
-	if (scanout->pixmap) {
+	if (!scanout->pixmap) {
+		ErrorF("failed to create CRTC scanout pixmap\n");
+		goto error;
+	}
+
+	if (amdgpu_pixmap_get_fb(scanout->pixmap)) {
 		scanout->width = width;
 		scanout->height = height;
 	} else {
-		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
-			   "Couldn't allocate scanout pixmap for CRTC\n");
-error:
+		ErrorF("failed to create CRTC scanout FB\n");
+error:		
 		drmmode_crtc_scanout_destroy(drmmode, scanout);
 	}
 
@@ -651,8 +641,8 @@ drmmode_handle_transform(xf86CrtcPtr crtc)
 
 static void
 drmmode_crtc_prime_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode,
-				  unsigned scanout_id, int *fb_id, int *x,
-				  int *y)
+				  unsigned scanout_id, struct drmmode_fb **fb,
+				  int *x, int *y)
 {
 	ScrnInfoPtr scrn = crtc->scrn;
 	ScreenPtr screen = scrn->pScreen;
@@ -701,7 +691,7 @@ drmmode_crtc_prime_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode,
 		}
 	}
 
-	*fb_id = drmmode_crtc->scanout[scanout_id].fb_id;
+	*fb = amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
 	*x = *y = 0;
 	drmmode_crtc->scanout_id = scanout_id;
 }
@@ -710,7 +700,8 @@ drmmode_crtc_prime_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode,
 
 static void
 drmmode_crtc_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode,
-			    unsigned scanout_id, int *fb_id, int *x, int *y)
+			    unsigned scanout_id, struct drmmode_fb **fb, int *x,
+			    int *y)
 {
 	ScrnInfoPtr scrn = crtc->scrn;
 	ScreenPtr screen = scrn->pScreen;
@@ -746,7 +737,7 @@ drmmode_crtc_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode,
 		box->x2 = max(box->x2, scrn->virtualX);
 		box->y2 = max(box->y2, scrn->virtualY);
 
-		*fb_id = drmmode_crtc->scanout[scanout_id].fb_id;
+		*fb = amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
 		*x = *y = 0;
 
 		amdgpu_scanout_do_update(crtc, scanout_id);
@@ -784,9 +775,8 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	int output_count = 0;
 	Bool ret = FALSE;
 	int i;
-	int fb_id;
+	struct drmmode_fb *fb = NULL;
 	drmModeModeInfo kmode;
-	uint32_t bo_handle;
 
 	/* The root window contents may be undefined before the WindowExposures
 	 * hook is called for it, so bail if we get here before that
@@ -834,15 +824,14 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 
 		drmmode_ConvertToKMode(crtc->scrn, &kmode, mode);
 
-		fb_id = drmmode->fb_id;
 #ifdef AMDGPU_PIXMAP_SHARING
 		if (drmmode_crtc->prime_scanout_pixmap) {
 			drmmode_crtc_prime_scanout_update(crtc, mode, scanout_id,
-							  &fb_id, &x, &y);
+							  &fb, &x, &y);
 		} else
 #endif
-		if (drmmode_crtc->rotate.fb_id) {
-			fb_id = drmmode_crtc->rotate.fb_id;
+		if (drmmode_crtc->rotate.pixmap) {
+			fb = amdgpu_pixmap_get_fb(drmmode_crtc->rotate.pixmap);
 			x = y = 0;
 
 		} else if (!amdgpu_is_gpu_screen(pScreen) &&
@@ -852,26 +841,27 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 #endif
 			    info->shadow_primary)) {
 			drmmode_crtc_scanout_update(crtc, mode, scanout_id,
-						    &fb_id, &x, &y);
+						    &fb, &x, &y);
 		}
 
-		if (fb_id == 0) {
-			if (!amdgpu_bo_get_handle(info->front_buffer, &bo_handle)) {
-				ErrorF("failed to get BO handle for FB\n");
-				goto done;
-			}
-
-			if (drmModeAddFB(pAMDGPUEnt->fd,
-				   pScrn->virtualX,
-				   pScrn->virtualY,
-				   pScrn->depth, pScrn->bitsPerPixel,
-				   pScrn->displayWidth * info->pixel_bytes,
-				   bo_handle, &drmmode->fb_id) < 0) {
-				ErrorF("failed to add fb\n");
-				goto done;
-			}
-
-			fb_id = drmmode->fb_id;
+		if (!fb)
+			fb = amdgpu_pixmap_get_fb(pScreen->GetWindowPixmap(pScreen->root));
+		if (!fb) {
+			union gbm_bo_handle bo_handle;
+
+			bo_handle = gbm_bo_get_handle(info->front_buffer->bo.gbm);
+			fb = amdgpu_fb_create(pAMDGPUEnt->fd, pScrn->virtualX,
+					      pScrn->virtualY, pScrn->depth,
+					      pScrn->bitsPerPixel,
+					      pScrn->displayWidth * info->pixel_bytes,
+					      bo_handle.u32);
+			/* Prevent refcnt of ad-hoc FBs from reaching 2 */
+			drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb, NULL);
+			drmmode_crtc->fb = fb;
+		}
+		if (!fb) {
+			ErrorF("failed to add FB for modeset\n");
+			goto done;
 		}
 
 		/* Wait for any pending flip to finish */
@@ -881,13 +871,15 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 
 		if (drmModeSetCrtc(pAMDGPUEnt->fd,
 				   drmmode_crtc->mode_crtc->crtc_id,
-				   fb_id, x, y, output_ids,
+				   fb->handle, x, y, output_ids,
 				   output_count, &kmode) != 0) {
 			xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR,
 				   "failed to set mode: %s\n", strerror(errno));
 			goto done;
-		} else
+		} else {
 			ret = TRUE;
+			drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb, fb);
+		}
 
 		if (pScreen)
 			xf86CrtcSetScreenSubpixelOrder(pScreen);
@@ -933,7 +925,9 @@ done:
 	} else {
 		crtc->active = TRUE;
 
-		if (fb_id != drmmode_crtc->scanout[scanout_id].fb_id)
+		if (drmmode_crtc->scanout[scanout_id].pixmap &&
+		    fb != amdgpu_pixmap_get_fb(drmmode_crtc->
+					       scanout[scanout_id].pixmap))
 			drmmode_crtc_scanout_free(drmmode_crtc);
 		else if (!drmmode_crtc->tear_free) {
 			drmmode_crtc_scanout_destroy(drmmode,
@@ -2079,14 +2073,9 @@ int drmmode_get_pitch_align(ScrnInfoPtr scrn, int bpe)
 static Bool drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
 {
 	xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
-	drmmode_crtc_private_ptr
-	    drmmode_crtc = xf86_config->crtc[0]->driver_private;
-	drmmode_ptr drmmode = drmmode_crtc->drmmode;
 	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
-	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
 	struct amdgpu_buffer *old_front = NULL;
 	ScreenPtr screen = xf86ScrnToScreen(scrn);
-	uint32_t old_fb_id;
 	int i, pitch, old_width, old_height, old_pitch;
 	int cpp = info->pixel_bytes;
 	PixmapPtr ppix = screen->GetScreenPixmap(screen);
@@ -2109,8 +2098,6 @@ static Bool drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
 	old_width = scrn->virtualX;
 	old_height = scrn->virtualY;
 	old_pitch = scrn->displayWidth;
-	old_fb_id = drmmode->fb_id;
-	drmmode->fb_id = 0;
 	old_front = info->front_buffer;
 
 	scrn->virtualX = width;
@@ -2182,8 +2169,6 @@ static Bool drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
 				       crtc->rotation, crtc->x, crtc->y);
 	}
 
-	if (old_fb_id)
-		drmModeRmFB(pAMDGPUEnt->fd, old_fb_id);
 	if (old_front) {
 		amdgpu_bo_unref(&old_front);
 	}
@@ -2198,7 +2183,6 @@ fail:
 	scrn->virtualX = old_width;
 	scrn->virtualY = old_height;
 	scrn->displayWidth = old_pitch;
-	drmmode->fb_id = old_fb_id;
 
 	return FALSE;
 }
@@ -2212,7 +2196,7 @@ drmmode_clear_pending_flip(xf86CrtcPtr crtc)
 {
 	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 
-	drmmode_crtc->flip_pending = FALSE;
+	drmmode_crtc->flip_pending = NULL;
 
 	if (!crtc->enabled ||
 	    (drmmode_crtc->pending_dpms_mode != DPMSModeOn &&
@@ -2257,6 +2241,7 @@ static void
 drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *event_data)
 {
 	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 	drmmode_flipdata_ptr flipdata = event_data;
 
 	/* Is this the event whose info shall be delivered to higher level? */
@@ -2276,12 +2261,11 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
 		else
 			flipdata->handler(crtc, frame, usec, flipdata->event_data);
 
-		/* Release framebuffer */
-		drmModeRmFB(pAMDGPUEnt->fd, flipdata->old_fb_id);
-
 		free(flipdata);
 	}
 
+	drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
+			     drmmode_crtc->flip_pending);
 	drmmode_clear_pending_flip(crtc);
 }
 
@@ -2514,6 +2498,8 @@ Bool drmmode_set_desired_modes(ScrnInfoPtr pScrn, drmmode_ptr drmmode,
 				drmModeSetCrtc(pAMDGPUEnt->fd,
 					       drmmode_crtc->mode_crtc->crtc_id,
 					       0, 0, 0, NULL, 0, NULL);
+				drmmode_fb_reference(pAMDGPUEnt->fd,
+						     &drmmode_crtc->fb, NULL);
 			}
 			continue;
 		}
@@ -2773,18 +2759,11 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 	xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
 	xf86CrtcPtr crtc = NULL;
 	drmmode_crtc_private_ptr drmmode_crtc = config->crtc[0]->driver_private;
-	drmmode_ptr drmmode = drmmode_crtc->drmmode;
 	int i;
 	uint32_t flip_flags = flip_sync == FLIP_ASYNC ? DRM_MODE_PAGE_FLIP_ASYNC : 0;
 	drmmode_flipdata_ptr flipdata;
 	uintptr_t drm_queue_seq = 0;
-	uint32_t new_front_handle;
-
-	if (!amdgpu_pixmap_get_handle(new_front, &new_front_handle)) {
-		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
-			   "flip queue: data alloc failed.\n");
-		return FALSE;
-	}
+	struct drmmode_fb *fb;
 
 	flipdata = calloc(1, sizeof(drmmode_flipdata_rec));
 	if (!flipdata) {
@@ -2793,15 +2772,11 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 		goto error;
 	}
 
-	/*
-	 * Create a new handle for the back buffer
-	 */
-	flipdata->old_fb_id = drmmode->fb_id;
-	if (drmModeAddFB(pAMDGPUEnt->fd, new_front->drawable.width,
-			 new_front->drawable.height, scrn->depth,
-			 scrn->bitsPerPixel, new_front->devKind,
-			 new_front_handle, &drmmode->fb_id))
+	fb = amdgpu_pixmap_get_fb(new_front);
+	if (!fb) {
+		ErrorF("Failed to get FB for flip\n");
 		goto error;
+	}
 
 	/*
 	 * Queue flips on all enabled CRTCs
@@ -2845,7 +2820,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 		if (drmmode_crtc->hw_id == ref_crtc_hw_id) {
 			if (drmmode_page_flip_target_absolute(pAMDGPUEnt,
 							      drmmode_crtc,
-							      drmmode->fb_id,
+							      fb->handle,
 							      flip_flags,
 							      drm_queue_seq,
 							      target_msc) != 0)
@@ -2853,13 +2828,13 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 		} else {
 			if (drmmode_page_flip_target_relative(pAMDGPUEnt,
 							      drmmode_crtc,
-							      drmmode->fb_id,
+							      fb->handle,
 							      flip_flags,
 							      drm_queue_seq, 0) != 0)
 				goto flip_error;
 		}
 
-		drmmode_crtc->flip_pending = TRUE;
+		drmmode_crtc->flip_pending = fb;
 		drm_queue_seq = 0;
 	}
 
@@ -2871,12 +2846,6 @@ flip_error:
 		   strerror(errno));
 
 error:
-	if (flipdata && flipdata->flip_count <= 1 &&
-	    drmmode->fb_id != flipdata->old_fb_id) {
-		drmModeRmFB(pAMDGPUEnt->fd, drmmode->fb_id);
-		drmmode->fb_id = flipdata->old_fb_id;
-	}
-
 	if (drm_queue_seq)
 		amdgpu_drm_abort_entry(drm_queue_seq);
 	else if (crtc)
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 6a57fd23b..b64a938cd 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -37,7 +37,6 @@
 #include "amdgpu.h"
 
 typedef struct {
-	unsigned fb_id;
 	ScrnInfoPtr scrn;
 #ifdef HAVE_LIBUDEV
 	struct udev_monitor *uevent_monitor;
@@ -53,7 +52,6 @@ typedef struct {
 } drmmode_rec, *drmmode_ptr;
 
 typedef struct {
-	unsigned old_fb_id;
 	int flip_count;
 	void *event_data;
 	unsigned int fe_frame;
@@ -63,10 +61,14 @@ typedef struct {
 	amdgpu_drm_abort_proc abort;
 } drmmode_flipdata_rec, *drmmode_flipdata_ptr;
 
+struct drmmode_fb {
+	int refcnt;
+	uint32_t handle;
+};
+
 struct drmmode_scanout {
 	struct amdgpu_buffer *bo;
 	PixmapPtr pixmap;
-	unsigned fb_id;
 	int width, height;
 };
 
@@ -96,8 +98,10 @@ typedef struct {
 
 	/* Modeset needed for DPMS on */
 	Bool need_modeset;
-	/* A flip is pending for this CRTC */
-	Bool flip_pending;
+	/* A flip to this FB is pending for this CRTC */
+	struct drmmode_fb *flip_pending;
+	/* The FB currently being scanned out by this CRTC, if any */
+	struct drmmode_fb *fb;
 } drmmode_crtc_private_rec, *drmmode_crtc_private_ptr;
 
 typedef struct {
@@ -128,6 +132,34 @@ enum drmmode_flip_sync {
 };
 
 
+static inline void
+drmmode_fb_reference(int drm_fd, struct drmmode_fb **old, struct drmmode_fb *new)
+{
+	if (new) {
+		if (new->refcnt <= 0) {
+			ErrorF("New FB's refcnt was %d in %s\n", new->refcnt,
+			       __func__);
+		} else {
+			new->refcnt++;
+		}
+	}
+
+	if (*old) {
+		if ((*old)->refcnt <= 0) {
+			ErrorF("Old FB's refcnt was %d in %s\n",
+			       (*old)->refcnt, __func__);
+		} else {
+			if (--(*old)->refcnt == 0) {
+				drmModeRmFB(drm_fd, (*old)->handle);
+				free(*old);
+			}
+		}
+	}
+
+	*old = new;
+}
+
+
 extern int drmmode_page_flip_target_absolute(AMDGPUEntPtr pAMDGPUEnt,
 					     drmmode_crtc_private_ptr drmmode_crtc,
 					     int fb_id, uint32_t flags,
-- 
2.11.0



More information about the amd-gfx mailing list