Re: [PATCH] Allocate a correctly sized framebuffer when tiling by using libdrm support.

[Chris Wilson <chris@chris-wilson.co.uk>]

On Sun,  6 Jun 2010 23:49:09 -0700, Eric Anholt <eric@anholt.net> wrote:
> When I made libdrm stop overallocating so much memory for the purpose
> +	front_buffer = drm_intel_bo_alloc_tiled(intel->bufmgr, "front buffer",
> +						scrn->displayWidth,
> +						scrn->virtualY, intel->cpp,
> +						&tiling_mode, &pitch, 0);

Using displayWidth here instead of virtualX. Makes xrandr much more
exciting than normal!

I think you can go much further with your cleanup, as using displayWidth
is now almost always a bug and we want to enable/disable frontbuffer
tiling depending upon the allocation size and so need to retry when
changing mode in case the frontbuffer can only be supported with an untiled
allocation.

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index df0282e..c61c6d9 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -318,9 +318,9 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	int i;
 	int fb_id;
 	drmModeModeInfo kmode;
-	unsigned int pitch = scrn->displayWidth * intel->cpp;
 
 	if (drmmode->fb_id == 0) {
+		unsigned int pitch = scrn->displayWidth * intel->cpp;
 		ret = drmModeAddFB(drmmode->fd,
 				   scrn->virtualX, scrn->virtualY,
 				   scrn->depth, scrn->bitsPerPixel,
@@ -1257,13 +1257,11 @@ 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, pitch, old_width, old_height, old_pitch;
 
 	if (scrn->virtualX == width && scrn->virtualY == height)
 		return TRUE;
 
-	w = i830_pad_drawable_width(width);
-
 	old_width = scrn->virtualX;
 	old_height = scrn->virtualY;
 	old_pitch = scrn->displayWidth;
@@ -1279,7 +1277,7 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 
 	xf86DrvMsg(scrn->scrnIndex, X_INFO,
 		   "Allocated new frame buffer %dx%d stride %d\n",
-		   width, height, scrn->displayWidth * intel->cpp);
+		   width, height, pitch);
 
 	ret = drmModeAddFB(drmmode->fd, width, height, scrn->depth,
 			   scrn->bitsPerPixel, pitch,
diff --git a/src/i830_driver.c b/src/i830_driver.c
index 016a781..accc547 100644
--- a/src/i830_driver.c
+++ b/src/i830_driver.c
@@ -929,28 +929,6 @@ static void i830_fixup_mtrrs(ScrnInfoPtr scrn)
 #endif
 }
 
-static Bool i830_try_memory_allocation(ScrnInfoPtr scrn)
-{
-	intel_screen_private *intel = intel_get_screen_private(scrn);
-	Bool tiled = intel->tiling;
-
-	xf86DrvMsg(scrn->scrnIndex, X_INFO,
-		   "Attempting memory allocation with %stiled buffers.\n",
-		   tiled ? "" : "un");
-
-	intel->front_buffer = i830_allocate_framebuffer(scrn);
-	if (!intel->front_buffer) {
-		xf86DrvMsg(scrn->scrnIndex, X_INFO,
-			   "%siled allocation failed.\n",
-			   tiled ? "T" : "Unt");
-		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
@@ -968,19 +946,8 @@ static Bool i830_memory_init(ScrnInfoPtr scrn)
 		   intel->pEnt->device->videoRam ? X_CONFIG : X_DEFAULT,
 		   "VideoRam: %d KB\n", scrn->videoRam);
 
-	if (i830_try_memory_allocation(scrn))
-		return TRUE;
-
-	/* Tiled first if we got a good displayWidth */
-	if (intel->tiling) {
-		intel->tiling = FALSE;
-		i830_reset_allocations(scrn);
-
-		if (i830_try_memory_allocation(scrn))
-			return TRUE;
-	}
-
-	return FALSE;
+	intel->front_buffer = i830_allocate_framebuffer(scrn);
+	return intel->front_buffer != NULL;
 }
 
 void i830_init_bufmgr(ScrnInfoPtr scrn)
@@ -1045,8 +1012,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
diff --git a/src/i830_memory.c b/src/i830_memory.c
index e71cbde..0101af2 100644
--- a/src/i830_memory.c
+++ b/src/i830_memory.c
@@ -195,9 +195,10 @@ drm_intel_bo *i830_allocate_framebuffer(ScrnInfoPtr scrn)
 	else
 		tiling_mode = I915_TILING_NONE;
 
+retry:
 	front_buffer = drm_intel_bo_alloc_tiled(intel->bufmgr, "front buffer",
-						scrn->displayWidth,
-						scrn->virtualY, intel->cpp,
+						scrn->virtualX, scrn->virtualY,
+						intel->cpp,
 						&tiling_mode, &pitch, 0);
 	if (front_buffer == NULL) {
 		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
@@ -207,10 +208,19 @@ drm_intel_bo *i830_allocate_framebuffer(ScrnInfoPtr scrn)
 
 	if (!i830_check_display_stride(scrn, pitch,
 				       tiling_mode != I915_TILING_NONE)) {
+		drm_intel_bo_unreference(front_buffer);
+
+		if (tiling_mode != I915_TILING_NONE) {
+			/* The older chips have larger support for
+			 * untiled surfaces, so try again without tiling.
+			 */
+			tiling_mode = I915_TILING_NONE;
+			goto retry;
+		}
+
 		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
 			   "Front buffer stride %ld kB "
 			   "exceeds display limit\n", pitch / 1024);
-		drm_intel_bo_unreference(front_buffer);
 		return NULL;
 	}
 

-- 
Chris Wilson, Intel Open Source Technology Centre