xf86-video-intel: 4 commits - src/intel_driver.c src/sna/kgem.c src/sna/sna_accel.c src/sna/sna_display.c src/sna/sna_driver.c src/sna/sna_video.c src/sna/sna_video_sprite.c

Chris Wilson ickle at kemper.freedesktop.org
Wed Sep 5 03:11:43 PDT 2012


 src/intel_driver.c         |    8 ++---
 src/sna/kgem.c             |    1 
 src/sna/sna_accel.c        |   64 +++++++++++++++++++++------------------------
 src/sna/sna_display.c      |    1 
 src/sna/sna_driver.c       |   14 +++------
 src/sna/sna_video.c        |    5 ---
 src/sna/sna_video_sprite.c |   13 ++++++---
 7 files changed, 50 insertions(+), 56 deletions(-)

New commits:
commit dff25e5ec4071a0404f82760e8deec3f99f4a0a9
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Wed Sep 5 11:05:28 2012 +0100

    sna: Drop master after discarding framebuffers
    
    As Imre Deak pointed out in the previous patch, drmModeRmFB only works
    when we hold the DRM master, therefore to prevent a leak of the
    framebuffer across server reset we need to defer dropping master until
    after we release our scanouts and modes.
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_driver.c b/src/sna/sna_driver.c
index a193607..b814e1f 100644
--- a/src/sna/sna_driver.c
+++ b/src/sna/sna_driver.c
@@ -701,15 +701,12 @@ static void sna_leave_vt(VT_FUNC_ARGS_DECL)
 {
 	SCRN_INFO_PTR(arg);
 	struct sna *sna = to_sna(scrn);
-	int ret;
 
 	DBG(("%s\n", __FUNCTION__));
 
-	xf86RotateFreeShadow(scrn);
 	xf86_hide_cursors(scrn);
 
-	ret = drmDropMaster(sna->kgem.fd);
-	if (ret)
+	if (drmDropMaster(sna->kgem.fd))
 		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
 			   "drmDropMaster failed: %s\n", strerror(errno));
 }
@@ -739,16 +736,14 @@ static Bool sna_early_close_screen(CLOSE_SCREEN_ARGS_DECL)
 	if (sna_mode_has_pending_events(sna))
 		sna_mode_wakeup(sna);
 
-	if (scrn->vtSema == TRUE) {
-		sna_leave_vt(VT_FUNC_ARGS(0));
-		scrn->vtSema = FALSE;
-	}
-
 	if (sna->dri_open) {
 		sna_dri_close(sna, screen);
 		sna->dri_open = false;
 	}
 
+	xf86_hide_cursors(scrn);
+	scrn->vtSema = FALSE;
+
 	xf86_cursors_fini(screen);
 
 	return TRUE;
@@ -769,6 +764,7 @@ static Bool sna_late_close_screen(CLOSE_SCREEN_ARGS_DECL)
 	}
 
 	sna_accel_close(sna);
+	drmDropMaster(sna->kgem.fd);
 
 	depths = screen->allowedDepths;
 	for (d = 0; d < screen->numDepths; d++)
commit 46981d01700c1159bfb6bc0aebc938ff1d447a0f
Author: Imre Deak <imre.deak at intel.com>
Date:   Fri Aug 31 14:31:51 2012 +0300

    uxa: fix leakage of FB when closing the screen
    
    Calling drmModeRmFB is only allowed in DRM master mode. Since leaving
    the VT also drops master mode we need to remove the FB before calling
    I830LeaveVT.
    
    This is only a real leak in case of a server reset, otherwise the server
    process will exit anyway and the kernel will clean up the FB.
    
    Signed-off-by: Imre Deak <imre.deak at intel.com>
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/intel_driver.c b/src/intel_driver.c
index 218b583..1b2c616 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -1136,10 +1136,6 @@ static Bool I830CloseScreen(CLOSE_SCREEN_ARGS_DECL)
 	I830UeventFini(scrn);
 #endif
 
-	if (scrn->vtSema == TRUE) {
-		I830LeaveVT(VT_FUNC_ARGS(0));
-	}
-
 	DeleteCallback(&FlushCallback, intel_flush_callback, scrn);
 
 	intel_glamor_close_screen(screen);
@@ -1169,6 +1165,10 @@ static Bool I830CloseScreen(CLOSE_SCREEN_ARGS_DECL)
 		intel->front_buffer = NULL;
 	}
 
+	if (scrn->vtSema == TRUE) {
+		I830LeaveVT(VT_FUNC_ARGS(0));
+	}
+
 	intel_batch_teardown(scrn);
 
 	if (INTEL_INFO(intel)->gen >= 40)
commit 55cef974a5dad3fda1922648fa27bcf5bb32ea03
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Wed Sep 5 09:38:47 2012 +0100

    sna: Review validity of damage when discarding CPU bo
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 3c0736e..41f53d4 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -1452,12 +1452,11 @@ skip_inplace_map:
 		priv->mapped = false;
 	}
 
-	if (priv->clear) {
-		if (priv->cpu_bo && !priv->cpu_bo->flush &&
-		    __kgem_bo_is_busy(&sna->kgem, priv->cpu_bo)) {
-			assert(!priv->shm);
-			sna_pixmap_free_cpu(sna, priv);
-		}
+	if (priv->clear && priv->cpu_bo && !priv->cpu_bo->flush &&
+	    __kgem_bo_is_busy(&sna->kgem, priv->cpu_bo)) {
+		assert(!priv->shm);
+		assert(DAMAGE_IS_ALL(priv->gpu_damage));
+		sna_pixmap_free_cpu(sna, priv);
 	}
 
 	if (pixmap->devPrivate.ptr == NULL &&
@@ -3462,36 +3461,33 @@ sna_put_zpixmap_blt(DrawablePtr drawable, GCPtr gc, RegionPtr region,
 		priv->gpu_bo = NULL;
 	}
 
-	if (priv->cpu_bo) {
-		/* If the GPU is currently accessing the CPU pixmap, then
-		 * we will need to wait for that to finish before we can
-		 * modify the memory.
-		 *
-		 * However, we can queue some writes to the GPU bo to avoid
-		 * the wait. Or we can try to replace the CPU bo.
-		 */
-		if (!priv->shm && __kgem_bo_is_busy(&sna->kgem, priv->cpu_bo)) {
-			assert(!priv->cpu_bo->flush);
-			DBG(("%s: cpu bo will stall, upload damage and discard\n",
-			     __FUNCTION__));
-			if (priv->cpu_damage) {
-				if (!region_subsumes_drawable(region, &pixmap->drawable)) {
-					sna_damage_subtract(&priv->cpu_damage, region);
-					if (!sna_pixmap_move_to_gpu(pixmap,
-								    MOVE_WRITE))
-						return false;
-				} else {
-					sna_damage_destroy(&priv->cpu_damage);
-					priv->undamaged = false;
-				}
-				assert(priv->cpu_damage == NULL);
+	/* If the GPU is currently accessing the CPU pixmap, then
+	 * we will need to wait for that to finish before we can
+	 * modify the memory.
+	 *
+	 * However, we can queue some writes to the GPU bo to avoid
+	 * the wait. Or we can try to replace the CPU bo.
+	 */
+	if (!priv->shm && priv->cpu_bo && __kgem_bo_is_busy(&sna->kgem, priv->cpu_bo)) {
+		assert(!priv->cpu_bo->flush);
+		DBG(("%s: cpu bo will stall, upload damage and discard\n",
+		     __FUNCTION__));
+		if (priv->cpu_damage) {
+			if (!region_subsumes_drawable(region, &pixmap->drawable)) {
+				sna_damage_subtract(&priv->cpu_damage, region);
+				if (!sna_pixmap_move_to_gpu(pixmap,
+							    MOVE_WRITE))
+					return false;
+			} else {
+				sna_damage_destroy(&priv->cpu_damage);
+				priv->undamaged = false;
 			}
-			if (!priv->undamaged)
-				sna_damage_all(&priv->gpu_damage,
-					       pixmap->drawable.width,
-					       pixmap->drawable.height);
-			sna_pixmap_free_cpu(sna, priv);
+			assert(priv->cpu_damage == NULL);
 		}
+		sna_damage_all(&priv->gpu_damage,
+			       pixmap->drawable.width,
+			       pixmap->drawable.height);
+		sna_pixmap_free_cpu(sna, priv);
 	}
 
 	if (priv->mapped) {
commit 587499bbf55b7eb0e1848822a792d535a8a3db1b
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Wed Sep 5 10:56:18 2012 +0100

    sna/video: Use the scanout flag and FB id for sprite framebuffers
    
    So that we can use the same teardown path as normal scanouts.
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index 7f687f7..de38f0a 100644
--- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -1331,6 +1331,7 @@ static void kgem_bo_clear_scanout(struct kgem *kgem, struct kgem_bo *bo)
 	DBG(("%s: handle=%d, fb=%d (reusable=%d)\n",
 	     __FUNCTION__, bo->handle, bo->delta, bo->reusable));
 	if (bo->delta) {
+		/* XXX will leak if we are not DRM_MASTER. *shrug* */
 		drmModeRmFB(kgem->fd, bo->delta);
 		bo->delta = 0;
 	}
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 4c260cd..59e128b 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -153,6 +153,7 @@ static unsigned get_fb(struct sna *sna, struct kgem_bo *bo,
 	arg.depth = scrn->depth;
 	arg.handle = bo->handle;
 
+	assert(sna->scrn->vtSema); /* must be master */
 	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_ADDFB, &arg)) {
 		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
 			   "%s: failed to add fb: %dx%d depth=%d, bpp=%d, pitch=%d: %d\n",
diff --git a/src/sna/sna_video.c b/src/sna/sna_video.c
index b8690ec..e7b335a 100644
--- a/src/sna/sna_video.c
+++ b/src/sna/sna_video.c
@@ -80,17 +80,12 @@ void sna_video_free_buffers(struct sna *sna, struct sna_video *video)
 
 	for (i = 0; i < ARRAY_SIZE(video->old_buf); i++) {
 		if (video->old_buf[i]) {
-			if (video->old_buf[i]->unique_id)
-				drmModeRmFB(sna->kgem.fd,
-						video->old_buf[i]->unique_id);
 			kgem_bo_destroy(&sna->kgem, video->old_buf[i]);
 			video->old_buf[i] = NULL;
 		}
 	}
 
 	if (video->buf) {
-		if (video->buf->unique_id)
-			drmModeRmFB(sna->kgem.fd, video->buf->unique_id);
 		kgem_bo_destroy(&sna->kgem, video->buf);
 		video->buf = NULL;
 	}
diff --git a/src/sna/sna_video_sprite.c b/src/sna/sna_video_sprite.c
index 87c5845..a912590 100644
--- a/src/sna/sna_video_sprite.c
+++ b/src/sna/sna_video_sprite.c
@@ -203,7 +203,7 @@ sna_video_sprite_show(struct sna *sna,
 	}
 #endif
 
-	if (frame->bo->unique_id == 0) {
+	if (frame->bo->delta == 0) {
 		uint32_t offsets[4], pitches[4], handles[4];
 		uint32_t pixel_format;
 
@@ -227,19 +227,24 @@ sna_video_sprite_show(struct sna *sna,
 		if (drmModeAddFB2(sna->kgem.fd,
 				  frame->width, frame->height, pixel_format,
 				  handles, pitches, offsets,
-				  &frame->bo->unique_id, 0)) {
+				  &frame->bo->delta, 0)) {
 			xf86DrvMsg(sna->scrn->scrnIndex,
 				   X_ERROR, "failed to add fb\n");
 			return false;
 		}
+
+		frame->bo->scanout = true;
 	}
 
 	DBG(("%s: updating plane=%d, handle=%d [fb %d], dst=(%d,%d)x(%d,%d)\n",
-	     __FUNCTION__, plane, frame->bo->handle, frame->bo->unique_id,
+	     __FUNCTION__, plane, frame->bo->handle, frame->bo->delta,
 	     dstBox->x1, dstBox->y1,
 	     dstBox->x2 - dstBox->x1, dstBox->y2 - dstBox->y1));
+	assert(frame->bo->scanout);
+	assert(frame->bo->delta);
+
 	if (drmModeSetPlane(sna->kgem.fd,
-			    plane, sna_crtc_id(crtc), frame->bo->unique_id, 0,
+			    plane, sna_crtc_id(crtc), frame->bo->delta, 0,
 			    dstBox->x1, dstBox->y1,
 			    dstBox->x2 - dstBox->x1, dstBox->y2 - dstBox->y1,
 			    0, 0, frame->width << 16, frame->height << 16))


More information about the xorg-commit mailing list