Allocate a correctly sized framebuffer when tiling by using libdrm's support.

When I made libdrm stop overallocating so much memory for the purpose
of bo caching, things started scribbling on the bottom of my
frontbuffer (and vice versa, leading to GPU hangs).  We had the usual
mistake of size = tiled_pitch * height instead of size = tiled_pitch *
tile_aligned_height.

4 files changed, 58 insertions, 200 deletions

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 3ea9b0fe..63ff9f38 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1257,28 +1257,22 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 	ScreenPtr   screen = screenInfo.screens[scrn->scrnIndex];
 	PixmapPtr   pixmap;
 	uint32_t    old_fb_id;
-	int	    i, w, pitch, old_width, old_height, old_pitch;
+	int	    i, old_width, old_height, old_pitch;
+	unsigned long pitch;
 
 	if (scrn->virtualX == width && scrn->virtualY == height)
 		return TRUE;
 
-	w = i830_pad_drawable_width(width);
-	i830_tiled_width(intel, &w, intel->cpp);
-	pitch = w * intel->cpp;
-	xf86DrvMsg(scrn->scrnIndex, X_INFO,
-		   "Allocate new frame buffer %dx%d stride %d\n",
-		   width, height, pitch);
-
 	old_width = scrn->virtualX;
 	old_height = scrn->virtualY;
 	old_pitch = scrn->displayWidth;
 	old_fb_id = drmmode->fb_id;
 	old_front = intel->front_buffer;
 
-	scrn->virtualX = width;
-	scrn->virtualY = height;
-	scrn->displayWidth = w;
-	intel->front_buffer = i830_allocate_framebuffer(scrn);
+	intel->front_buffer = i830_allocate_framebuffer(scrn,
+							width, height,
+							intel->cpp,
+							&pitch);
 	if (!intel->front_buffer)
 		goto fail;
 
@@ -1289,6 +1283,10 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 	if (ret)
 		goto fail;
 
+	scrn->virtualX = width;
+	scrn->virtualY = height;
+	scrn->displayWidth = pitch / intel->cpp;
+
 	pixmap = screen->GetScreenPixmap(screen);
 	screen->ModifyPixmapHeader(pixmap, width, height, -1, -1, pitch, NULL);
 	i830_set_pixmap_bo(pixmap, intel->front_buffer);
diff --git a/src/i830.h b/src/i830.h
index 4e24c779..09920883 100644
--- a/src/i830.h
+++ b/src/i830.h
@@ -464,18 +464,17 @@ extern Bool i830_crtc_on(xf86CrtcPtr crtc);
 extern int i830_crtc_to_pipe(xf86CrtcPtr crtc);
 extern Bool I830AccelInit(ScreenPtr pScreen);
 
-void i830_reset_allocations(ScrnInfoPtr scrn);
 void i830_init_bufmgr(ScrnInfoPtr scrn);
 
-Bool i830_tiled_width(intel_screen_private *intel, int *width, int cpp);
-
 /* i830_memory.c */
 unsigned long i830_get_fence_size(intel_screen_private *intel, unsigned long size);
 unsigned long i830_get_fence_pitch(intel_screen_private *intel, unsigned long pitch,
 				   uint32_t tiling_mode);
 void i830_set_gem_max_sizes(ScrnInfoPtr scrn);
 
-drm_intel_bo *i830_allocate_framebuffer(ScrnInfoPtr scrn);
+drm_intel_bo *i830_allocate_framebuffer(ScrnInfoPtr scrn,
+					int w, int h, int cpp,
+					unsigned long *pitch);
 
 /* i830_render.c */
 Bool i830_check_composite(int op,
diff --git a/src/i830_driver.c b/src/i830_driver.c
index a82ad0bf..eed755c4 100644
--- a/src/i830_driver.c
+++ b/src/i830_driver.c
@@ -348,49 +348,6 @@ static void PreInitCleanup(ScrnInfoPtr scrn)
 }
 
 /*
- * Adjust *width to allow for tiling if possible
- */
-Bool i830_tiled_width(intel_screen_private *intel, int *width, int cpp)
-{
-	Bool tiled = FALSE;
-
-	/*
-	 * Adjust the display width to allow for front buffer tiling if possible
-	 */
-	if (intel->tiling) {
-		if (IS_I965G(intel)) {
-			int tile_pixels = 512 / cpp;
-			*width = (*width + tile_pixels - 1) &
-			    ~(tile_pixels - 1);
-			tiled = TRUE;
-		} else {
-			/* Good pitches to allow tiling.  Don't care about pitches < 1024
-			 * pixels.
-			 */
-			static const int pitches[] = {
-				1024,
-				2048,
-				4096,
-				8192,
-				0
-			};
-			int pitch;
-			int i;
-
-			pitch = *width * cpp;
-			for (i = 0; pitches[i] != 0; i++) {
-				if (pitches[i] >= pitch) {
-					*width = pitches[i] / cpp;
-					tiled = TRUE;
-					break;
-				}
-			}
-		}
-	}
-	return tiled;
-}
-
-/*
  * DRM mode setting Linux only at this point... later on we could
  * add a wrapper here.
  */
@@ -972,66 +929,27 @@ static void i830_fixup_mtrrs(ScrnInfoPtr scrn)
 #endif
 }
 
-static Bool i830_try_memory_allocation(ScrnInfoPtr scrn)
+static Bool
+intel_init_initial_framebuffer(ScrnInfoPtr scrn)
 {
 	intel_screen_private *intel = intel_get_screen_private(scrn);
-	Bool tiled = intel->tiling;
+	unsigned long pitch;
 
-	xf86DrvMsg(scrn->scrnIndex, X_INFO,
-		   "Attempting memory allocation with %stiled buffers.\n",
-		   tiled ? "" : "un");
+	intel->front_buffer = i830_allocate_framebuffer(scrn,
+							scrn->virtualX,
+							scrn->virtualY,
+							intel->cpp,
+							&pitch);
 
-	intel->front_buffer = i830_allocate_framebuffer(scrn);
 	if (!intel->front_buffer) {
-		xf86DrvMsg(scrn->scrnIndex, X_INFO,
-			   "%siled allocation failed.\n",
-			   tiled ? "T" : "Unt");
+		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
+			   "Couldn't allocate initial framebuffer.\n");
 		return FALSE;
 	}
 
-	xf86DrvMsg(scrn->scrnIndex, X_INFO, "%siled allocation successful.\n",
-		   tiled ? "T" : "Unt");
-	return TRUE;
-}
-
-/*
- * Try to allocate memory in several ways:
- *  1) If direct rendering is enabled, try to allocate enough memory for tiled
- *     surfaces by rounding up the display width to a tileable one.
- *  2) If that fails or the allocations themselves fail, try again with untiled
- *     allocations (if this works DRI will stay enabled).
- *  3) And if all else fails, disable DRI and try just 2D allocations.
- *  4) Give up and fail ScreenInit.
- */
-static Bool i830_memory_init(ScrnInfoPtr scrn)
-{
-	intel_screen_private *intel = intel_get_screen_private(scrn);
-	int savedDisplayWidth = scrn->displayWidth;
-	Bool tiled = FALSE;
-
-	tiled = i830_tiled_width(intel, &scrn->displayWidth, intel->cpp);
-
-	xf86DrvMsg(scrn->scrnIndex,
-		   intel->pEnt->device->videoRam ? X_CONFIG : X_DEFAULT,
-		   "VideoRam: %d KB\n", scrn->videoRam);
-
-	/* Tiled first if we got a good displayWidth */
-	if (tiled) {
-		if (i830_try_memory_allocation(scrn))
-			return TRUE;
-		else {
-			i830_reset_allocations(scrn);
-			intel->tiling = FALSE;
-		}
-	}
-
-	/* If tiling fails we have to disable FBC */
-	scrn->displayWidth = savedDisplayWidth;
+	scrn->displayWidth = pitch / intel->cpp;
 
-	if (i830_try_memory_allocation(scrn))
-		return TRUE;
-
-	return FALSE;
+	return TRUE;
 }
 
 void i830_init_bufmgr(ScrnInfoPtr scrn)
@@ -1096,8 +1014,6 @@ I830ScreenInit(int scrnIndex, ScreenPtr screen, int argc, char **argv)
 	struct pci_device *const device = intel->PciInfo;
 	int fb_bar = IS_I9XX(intel) ? 2 : 0;
 
-	scrn->displayWidth = i830_pad_drawable_width(scrn->virtualX);
-
 	/*
 	 * The "VideoRam" config file parameter specifies the maximum amount of
 	 * memory that will be used/allocated.  When not present, we allow the
@@ -1169,11 +1085,12 @@ I830ScreenInit(int scrnIndex, ScreenPtr screen, int argc, char **argv)
 	 */
 	intel->XvEnabled = TRUE;
 
-	if (!i830_memory_init(scrn)) {
-		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
-			   "Couldn't allocate video memory\n");
+	xf86DrvMsg(scrn->scrnIndex,
+		   intel->pEnt->device->videoRam ? X_CONFIG : X_DEFAULT,
+		   "VideoRam: %d KB\n", scrn->videoRam);
+
+	if (!intel_init_initial_framebuffer(scrn))
 		return FALSE;
-	}
 
 	i830_fixup_mtrrs(scrn);
 
@@ -1192,9 +1109,6 @@ I830ScreenInit(int scrnIndex, ScreenPtr screen, int argc, char **argv)
 
 	DPRINTF(PFX, "assert( if(!I830EnterVT(scrnIndex, 0)) )\n");
 
-	if (scrn->virtualX > scrn->displayWidth)
-		scrn->displayWidth = scrn->virtualX;
-
 	DPRINTF(PFX, "assert( if(!fbScreenInit(screen, ...) )\n");
 	if (!fbScreenInit(screen, NULL,
 			  scrn->virtualX, scrn->virtualY,
@@ -1422,8 +1336,8 @@ static Bool I830CloseScreen(int scrnIndex, ScreenPtr screen)
 
 	xf86_cursors_fini(screen);
 
-	/* Free most of the allocations */
-	i830_reset_allocations(scrn);
+	drm_intel_bo_unreference(intel->front_buffer);
+	intel->front_buffer = NULL;
 
 	i965_free_video(scrn);
 
diff --git a/src/i830_memory.c b/src/i830_memory.c
index 611b5485..083ddf58 100644
--- a/src/i830_memory.c
+++ b/src/i830_memory.c
@@ -163,113 +163,60 @@ i830_check_display_stride(ScrnInfoPtr scrn, int stride, Bool tiling)
 		return FALSE;
 }
 
-/* Resets the state of the aperture allocator, freeing all memory that had
- * been allocated.
- */
-void i830_reset_allocations(ScrnInfoPtr scrn)
-{
-	intel_screen_private *intel = intel_get_screen_private(scrn);
-
-	/* Null out the pointers for all the allocations we just freed.  This is
-	 * kind of gross, but at least it's just one place now.
-	 */
-	drm_intel_bo_unreference(intel->front_buffer);
-	intel->front_buffer = NULL;
-}
-
-static Bool IsTileable(ScrnInfoPtr scrn, int pitch)
-{
-	intel_screen_private *intel = intel_get_screen_private(scrn);
-
-	if (IS_I965G(intel)) {
-		if (pitch / 512 * 512 == pitch && pitch <= KB(128))
-			return TRUE;
-		else
-			return FALSE;
-	}
-
-	/*
-	 * Allow tiling for pitches that are a power of 2 multiple of 128 bytes,
-	 * up to 64 * 128 (= 8192) bytes.
-	 */
-	switch (pitch) {
-	case 128:
-	case 256:
-		if (IS_I945G(intel) || IS_I945GM(intel) || IS_G33CLASS(intel))
-			return TRUE;
-		else
-			return FALSE;
-	case 512:
-	case KB(1):
-	case KB(2):
-	case KB(4):
-	case KB(8):
-		return TRUE;
-	default:
-		return FALSE;
-	}
-}
-
 /**
  * Allocates a framebuffer for a screen.
  *
  * Used once for each X screen, so once with RandR 1.2 and twice with classic
  * dualhead.
  */
-drm_intel_bo *i830_allocate_framebuffer(ScrnInfoPtr scrn)
+drm_intel_bo *i830_allocate_framebuffer(ScrnInfoPtr scrn,
+					int width, int height, int cpp,
+					unsigned long *out_pitch)
 {
 	intel_screen_private *intel = intel_get_screen_private(scrn);
 	drm_intel_bo *front_buffer;
-	long size, fb_height;
-	unsigned int pitch;
-	uint32_t tiling_mode, requested_tiling_mode;
-	int ret;
-
-	/* We'll allocate the fb such that the root window will fit regardless of
-	 * rotation.
-	 */
-	fb_height = scrn->virtualY;
-
-	pitch = scrn->displayWidth * intel->cpp;
-	size = ROUND_TO_PAGE(pitch * fb_height);
+	uint32_t tiling_mode;
+	unsigned long pitch;
 
-	if (intel->tiling && IsTileable(scrn, pitch))
+	if (intel->tiling)
 		tiling_mode = I915_TILING_X;
 	else
 		tiling_mode = I915_TILING_NONE;
 
-	if (!i830_check_display_stride(scrn, pitch,
-				       tiling_mode != I915_TILING_NONE)) {
+	width = i830_pad_drawable_width(width);
+
+	front_buffer = drm_intel_bo_alloc_tiled(intel->bufmgr, "front buffer",
+						width, height, intel->cpp,
+						&tiling_mode, &pitch, 0);
+	if (front_buffer == NULL) {
 		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
-			   "Front buffer stride %d kB "
-			   "exceed display limit\n", pitch / 1024);
+			   "Failed to allocate framebuffer.\n");
 		return NULL;
 	}
 
-	if (tiling_mode != I915_TILING_NONE) {
-		/* round to size necessary for the fence register to work */
-		size = i830_get_fence_size(intel, size);
-	}
-
-	front_buffer = drm_intel_bo_alloc(intel->bufmgr, "front buffer",
-					  size, GTT_PAGE_SIZE);
-	if (front_buffer == NULL) {
+	if (!i830_check_display_stride(scrn, pitch,
+				       tiling_mode != I915_TILING_NONE)) {
 		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
-			   "Failed to allocate framebuffer.\n");
+			   "Front buffer stride %ld kB "
+			   "exceeds display limit\n", pitch / 1024);
+		drm_intel_bo_unreference(front_buffer);
 		return NULL;
 	}
 
-	requested_tiling_mode = tiling_mode;
-	ret = drm_intel_bo_set_tiling(front_buffer, &tiling_mode, pitch);
-	if (ret != 0 || tiling_mode != requested_tiling_mode) {
+	if (intel->tiling && tiling_mode != I915_TILING_X) {
 		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
-			   "Failed to set tiling on frontbuffer: %s\n",
-			   ret == 0 ? "rejected by kernel" : strerror(-ret));
+			   "Failed to set tiling on frontbuffer.\n");
 	}
 
+	xf86DrvMsg(scrn->scrnIndex, X_INFO,
+		   "Allocated new frame buffer %dx%d stride %ld, %s\n",
+		   width, height, pitch,
+		   tiling_mode == I915_TILING_NONE ? "untiled" : "tiled");
+
 	drm_intel_bo_disable_reuse(front_buffer);
 
 	i830_set_gem_max_sizes(scrn);
+	*out_pitch = pitch;
 
 	return front_buffer;
 }