[Intel-gfx] [PATCH] Review i830_pad_drawable_width()

[Chris Wilson]

We appear to have a confusion of stride in terms of pixels, pitch in
terms of bytes and the actual width of the surface.
i830_pad_drawable_width() appears to be operating aligning *pixels* to a
64 pixel boundary and has never used the chars-per-pixel causing
considerable confusion in its callers. Remove the parameter and ensure
that the callers are expecting a value in pixels returned, multiplying
by cpp where necessary to get the pitch.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 src/drmmode_display.c |   18 +++++++++---------
 src/i830.h            |   11 +++++++++--
 src/i830_driver.c     |   17 +++++------------
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 1348e08..d8b158e 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -476,7 +476,7 @@ drmmode_crtc_shadow_allocate(xf86CrtcPtr crtc, int width, int height)
 	int size, ret;
 	unsigned long rotate_pitch;
 
-	width = i830_pad_drawable_width(width, drmmode->cpp);
+	width = i830_pad_drawable_width(width);
 	rotate_pitch = width * drmmode->cpp;
 	size = rotate_pitch * height;
 
@@ -523,8 +523,7 @@ drmmode_crtc_shadow_create(xf86CrtcPtr crtc, void *data, int width, int height)
 		}
 	}
 
-	rotate_pitch =
-		i830_pad_drawable_width(width, drmmode->cpp) * drmmode->cpp;
+	rotate_pitch = i830_pad_drawable_width(width) * drmmode->cpp;
 	rotate_pixmap = GetScratchPixmapHeader(scrn->pScreen,
 					       width, height,
 					       scrn->depth,
@@ -1257,13 +1256,14 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 	Bool	    ret;
 	ScreenPtr   screen = screenInfo.screens[scrn->scrnIndex];
 	uint32_t    old_fb_id;
-	int	    i, pitch, old_width, old_height, old_pitch;
+	int	    i, w, pitch, old_width, old_height, old_pitch;
 
 	if (scrn->virtualX == width && scrn->virtualY == height)
 		return TRUE;
 
-	pitch = i830_pad_drawable_width(width, intel->cpp);
-	i830_tiled_width(intel, &pitch, intel->cpp);
+	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);
@@ -1276,13 +1276,13 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 
 	scrn->virtualX = width;
 	scrn->virtualY = height;
-	scrn->displayWidth = pitch;
+	scrn->displayWidth = w;
 	intel->front_buffer = i830_allocate_framebuffer(scrn);
 	if (!intel->front_buffer)
 		goto fail;
 
 	ret = drmModeAddFB(drmmode->fd, width, height, scrn->depth,
-			   scrn->bitsPerPixel, pitch * intel->cpp,
+			   scrn->bitsPerPixel, pitch,
 			   intel->front_buffer->handle,
 			   &drmmode->fb_id);
 	if (ret)
@@ -1291,7 +1291,7 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height)
 	i830_set_pixmap_bo(screen->GetScreenPixmap(screen), intel->front_buffer);
 
 	screen->ModifyPixmapHeader(screen->GetScreenPixmap(screen),
-				   width, height, -1, -1, pitch * intel->cpp, NULL);
+				   width, height, -1, -1, pitch, NULL);
 
 	for (i = 0; i < xf86_config->num_crtc; i++) {
 		xf86CrtcPtr crtc = xf86_config->crtc[i];
diff --git a/src/i830.h b/src/i830.h
index 7593cde..43c5887 100644
--- a/src/i830.h
+++ b/src/i830.h
@@ -429,8 +429,6 @@ void i830_init_bufmgr(ScrnInfoPtr scrn);
 
 Bool i830_tiled_width(intel_screen_private *intel, int *width, int cpp);
 
-int i830_pad_drawable_width(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,
@@ -587,4 +585,13 @@ static inline Bool pixmap_is_scanout(PixmapPtr pixmap)
 	return pixmap == screen->GetScreenPixmap(screen);
 }
 
+/*
+ * Pad to accelerator requirement
+ */
+static inline int i830_pad_drawable_width(int width)
+{
+	return (width + 63) & ~63;
+}
+
+
 #endif /* _I830_H_ */
diff --git a/src/i830_driver.c b/src/i830_driver.c
index 22e8472..148675e 100644
--- a/src/i830_driver.c
+++ b/src/i830_driver.c
@@ -374,11 +374,13 @@ Bool i830_tiled_width(intel_screen_private *intel, int *width, int cpp)
 				8192,
 				0
 			};
+			int pitch;
 			int i;
 
+			pitch = *width * cpp;
 			for (i = 0; pitches[i] != 0; i++) {
-				if (pitches[i] >= *width) {
-					*width = pitches[i];
+				if (pitches[i] >= pitch) {
+					*width = pitches[i] / cpp;
 					tiled = TRUE;
 					break;
 				}
@@ -389,14 +391,6 @@ Bool i830_tiled_width(intel_screen_private *intel, int *width, int cpp)
 }
 
 /*
- * Pad to accelerator requirement
- */
-int i830_pad_drawable_width(int width, int cpp)
-{
-	return (width + 63) & ~63;
-}
-
-/*
  * DRM mode setting Linux only at this point... later on we could
  * add a wrapper here.
  */
@@ -1100,8 +1094,7 @@ 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, intel->cpp);
+	scrn->displayWidth = i830_pad_drawable_width(scrn->virtualX);
 
 	/*
 	 * The "VideoRam" config file parameter specifies the maximum amount of
-- 
1.7.0.3