Attachment 36326 Details for Bug 28569 – Refactor put_image

[patch] Refactor put_image

0001-i830-Tidy-i830_uxa_put_image.patch (text/plain), 11.60 KB, created by Chris Wilson on 2010-06-17 03:44:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2010-06-17 03:44:36 UTC

Size: 11.60 KB

patch

obsolete

>From 6fc77e5041454c33cabbc93c59c7c4ce046c100c Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>Date: Tue, 15 Jun 2010 12:48:57 +0100
>Subject: [PATCH] i830: Tidy i830_uxa_put_image()
>
>Use a single code path to upload the image data after selecting the
>right bo, and take advantage of pwrite() when possible.
>
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> src/i830.h             |   22 +++-
> src/i830_batchbuffer.c |    1 +
> src/i830_batchbuffer.h |    2 +
> src/i830_uxa.c         |  257 +++++++++++++++++-------------------------------
> 4 files changed, 111 insertions(+), 171 deletions(-)
>
>diff --git a/src/i830.h b/src/i830.h
>index 02220ea..a5fee3d 100644
>--- a/src/i830.h
>+++ b/src/i830.h
>@@ -165,12 +165,17 @@ list_is_empty(struct list *head)
> 
> struct intel_pixmap {
> 	dri_bo *bo;
>-	uint32_t tiling, stride;
>-	uint32_t flush_write_domain;
>-	uint32_t flush_read_domains;
>-	uint32_t batch_write_domain;
>-	uint32_t batch_read_domains;
>+
> 	struct list flush, batch, in_flight;
>+
>+	int8_t busy;
>+	uint8_t tiling;
>+	uint16_t stride;
>+
>+	uint16_t flush_write_domain;
>+	uint16_t flush_read_domains;
>+	uint16_t batch_write_domain;
>+	uint16_t batch_read_domains;
> };
> 
> #if HAS_DEVPRIVATEKEYREC
>@@ -188,6 +193,13 @@ static inline struct intel_pixmap *i830_get_pixmap_intel(PixmapPtr pixmap)
> #endif
> }
> 
>+static inline Bool intel_pixmap_is_busy(struct intel_pixmap *priv)
>+{
>+	if (priv->busy == -1)
>+		priv->busy = drm_intel_bo_busy(priv->bo);
>+	return priv->busy;
>+}
>+
> static inline void i830_set_pixmap_intel(PixmapPtr pixmap, struct intel_pixmap *intel)
> {
> 	dixSetPrivate(&pixmap->devPrivates, &uxa_pixmap_index, intel);
>diff --git a/src/i830_batchbuffer.c b/src/i830_batchbuffer.c
>index ada5809..46c9d4f 100644
>--- a/src/i830_batchbuffer.c
>+++ b/src/i830_batchbuffer.c
>@@ -240,6 +240,7 @@ void intel_batch_submit(ScrnInfoPtr scrn)
> 					 batch);
> 
> 		entry->batch_read_domains = entry->batch_write_domain = 0;
>+		entry->busy = -1;
> 		list_del(&entry->batch);
> 	}
> 
>diff --git a/src/i830_batchbuffer.h b/src/i830_batchbuffer.h
>index 7508a9c..93f786a 100644
>--- a/src/i830_batchbuffer.h
>+++ b/src/i830_batchbuffer.h
>@@ -135,6 +135,8 @@ intel_batch_mark_pixmap_domains(intel_screen_private *intel,
> 		list_add(&priv->batch, &intel->batch_pixmaps);
> 	if (list_is_empty(&priv->flush))
> 		list_add(&priv->flush, &intel->flush_pixmaps);
>+
>+	priv->busy = 1;
> }
> 
> static inline void
>diff --git a/src/i830_uxa.c b/src/i830_uxa.c
>index bf364f8..b69d45e 100644
>--- a/src/i830_uxa.c
>+++ b/src/i830_uxa.c
>@@ -629,6 +629,7 @@ void i830_set_pixmap_bo(PixmapPtr pixmap, dri_bo * bo)
> 	}
> 
> 	if (bo != NULL) {
>+		uint32_t tiling;
> 		uint32_t swizzle_mode;
> 		int ret;
> 
>@@ -646,12 +647,15 @@ void i830_set_pixmap_bo(PixmapPtr pixmap, dri_bo * bo)
> 		priv->stride = i830_pixmap_pitch(pixmap);
> 
> 		ret = drm_intel_bo_get_tiling(bo,
>-					      &priv->tiling,
>+					      &tiling,
> 					      &swizzle_mode);
> 		if (ret != 0) {
> 			FatalError("Couldn't get tiling on bo %p: %s\n",
> 				   bo, strerror(-ret));
> 		}
>+
>+		priv->tiling = tiling;
>+		priv->busy = -1;
> 	} else {
> 		if (priv != NULL) {
> 			free(priv);
>@@ -696,6 +700,7 @@ static Bool i830_uxa_prepare_access(PixmapPtr pixmap, uxa_access_t access)
> 	}
> 
> 	pixmap->devPrivate.ptr = bo->virtual;
>+	priv->busy = 0;
> 
> 	return TRUE;
> }
>@@ -718,78 +723,33 @@ static void i830_uxa_finish_access(PixmapPtr pixmap)
> 	pixmap->devPrivate.ptr = NULL;
> }
> 
>-static Bool i830_bo_put_image(PixmapPtr pixmap, dri_bo *bo, char *src, int src_pitch, int w, int h)
>+static Bool i830_uxa_pixmap_put_image(PixmapPtr pixmap,
>+				      char *src, int src_pitch,
>+				      int x, int y, int w, int h)
> {
>+	struct intel_pixmap *priv = i830_get_pixmap_intel(pixmap);
> 	int stride = i830_pixmap_pitch(pixmap);
>-
>-	if (src_pitch == stride) {
>-		memcpy (bo->virtual, src, stride * h);
>-	} else {
>-		char *dst = bo->virtual;
>-		int row_length = w * pixmap->drawable.bitsPerPixel/8;
>+	int ret = FALSE;
>+
>+	if (src_pitch == stride && w == pixmap->drawable.width && priv->tiling == I915_TILING_NONE) {
>+		ret = drm_intel_bo_subdata(priv->bo, y * stride, stride * h, src) == 0;
>+	} else if (drm_intel_gem_bo_map_gtt(priv->bo) == 0) {
>+		char *dst = priv->bo->virtual;
>+		int cpp = pixmap->drawable.bitsPerPixel/8;
>+		int row_length = w * cpp;
> 		int num_rows = h;
>-		while (num_rows--) {
>+		if (row_length == src_pitch && src_pitch == stride)
>+			num_rows = 1, row_length *= h;
>+		dst += y * stride + x * cpp;
>+		do {
> 			memcpy (dst, src, row_length);
> 			src += src_pitch;
> 			dst += stride;
>-		}
>-	}
>-
>-	return TRUE;
>-}
>-
>-static Bool
>-i830_uxa_pixmap_swap_bo_with_image(PixmapPtr pixmap,
>-				   char *src, int src_pitch)
>-{
>-	ScrnInfoPtr scrn = xf86Screens[pixmap->drawable.pScreen->myNum];
>-	intel_screen_private *intel = intel_get_screen_private(scrn);
>-	struct intel_pixmap *priv;
>-	dri_bo *bo;
>-	uint32_t tiling = I915_TILING_X;
>-	int stride;
>-	int w = pixmap->drawable.width;
>-	int h = pixmap->drawable.height;
>-	Bool ret;
>-
>-	priv = i830_get_pixmap_intel(pixmap);
>-
>-	if (priv->batch_read_domains || drm_intel_bo_busy(priv->bo)) {
>-		unsigned int size;
>-
>-		size = i830_uxa_pixmap_compute_size (pixmap, w, h,
>-						     &tiling, &stride);
>-		if (size > intel->max_gtt_map_size)
>-			return FALSE;
>-
>-		bo = drm_intel_bo_alloc(intel->bufmgr, "pixmap", size, 0);
>-		if (bo == NULL)
>-			return FALSE;
>-
>-		if (tiling != I915_TILING_NONE)
>-			drm_intel_bo_set_tiling(bo, &tiling, stride);
>-
>-		pixmap->drawable.pScreen->ModifyPixmapHeader(pixmap,
>-							     w, h,
>-							     0, 0,
>-							     stride, NULL);
>-		i830_set_pixmap_bo(pixmap, bo);
>-		dri_bo_unreference(bo);
>-	} else {
>-		bo = priv->bo;
>-		stride = i830_pixmap_pitch(pixmap);
>-	}
>-
>-	if (drm_intel_gem_bo_map_gtt(bo)) {
>-		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
>-			   "%s: bo map failed\n", __FUNCTION__);
>-		return FALSE;
>+		} while (--num_rows);
>+		drm_intel_gem_bo_unmap_gtt(priv->bo);
>+		ret = TRUE;
> 	}
> 
>-	ret = i830_bo_put_image(pixmap, bo, src, src_pitch, w, h);
>-
>-	drm_intel_gem_bo_unmap_gtt(bo);
>-
> 	return ret;
> }
> 
>@@ -798,114 +758,77 @@ static Bool i830_uxa_put_image(PixmapPtr pixmap,
> 			       int w, int h,
> 			       char *src, int src_pitch)
> {
>-	ScreenPtr screen = pixmap->drawable.pScreen;
>-	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
>-	PixmapPtr scratch;
> 	struct intel_pixmap *priv;
>-	GCPtr gc;
>-	Bool ret;
>-
>-	if (x == 0 && y == 0 &&
>-	    w == pixmap->drawable.width &&
>-	    h == pixmap->drawable.height)
>-	{
>-		/* Replace GPU hot bo with new CPU data. */
>-		return i830_uxa_pixmap_swap_bo_with_image(pixmap,
>-							  src, src_pitch);
>-	}
> 
> 	priv = i830_get_pixmap_intel(pixmap);
>-	if (priv->batch_read_domains ||
>-	    drm_intel_bo_busy(priv->bo)) {
>-		dri_bo *bo;
>-
>-		/* Partial replacement, copy incoming image to a bo and blit. */
>-		scratch = (*screen->CreatePixmap)(screen, w, h,
>-						  pixmap->drawable.depth,
>-						  UXA_CREATE_PIXMAP_FOR_MAP);
>-		if (!scratch)
>-			return FALSE;
>-
>-		bo = i830_get_pixmap_bo(scratch);
>-		if (drm_intel_gem_bo_map_gtt(bo)) {
>-			(*screen->DestroyPixmap) (scratch);
>-			xf86DrvMsg(scrn->scrnIndex, X_WARNING,
>-				   "%s: bo map failed\n", __FUNCTION__);
>-			return FALSE;
>-		}
>-
>-		ret = i830_bo_put_image(scratch, bo, src, src_pitch, w, h);
>-
>-		drm_intel_gem_bo_unmap_gtt(bo);
>-
>-		if (ret) {
>-			gc = GetScratchGC(pixmap->drawable.depth, screen);
>-			if (gc) {
>-				ValidateGC(&pixmap->drawable, gc);
>-
>-				(*gc->ops->CopyArea)(&scratch->drawable,
>-						     &pixmap->drawable,
>-						     gc, 0, 0, w, h, x, y);
>-
>-				FreeScratchGC(gc);
>-			} else
>-				ret = FALSE;
>-		}
>-
>-		(*screen->DestroyPixmap)(scratch);
>+	if (!intel_pixmap_is_busy(priv)) {
>+		/* bo is not busy so can be replaced without a stall, upload in-place. */
>+		return i830_uxa_pixmap_put_image(pixmap, src, src_pitch, x, y, w, h);
> 	} else {
>-		int dst_pitch;
>+		ScreenPtr screen = pixmap->drawable.pScreen;
> 
>-		/* bo is not busy so can be mapped without a stall, upload in-place. */
>-		if (drm_intel_gem_bo_map_gtt(priv->bo)) {
>-			xf86DrvMsg(scrn->scrnIndex, X_WARNING,
>-				   "%s: bo map failed\n", __FUNCTION__);
>-			return FALSE;
>+		if (x == 0 && y == 0 &&
>+		    w == pixmap->drawable.width &&
>+		    h == pixmap->drawable.height)
>+		{
>+			intel_screen_private *intel = intel_get_screen_private(xf86Screens[screen->myNum]);
>+			uint32_t tiling = priv->tiling;
>+			int size, stride;
>+			dri_bo *bo;
>+
>+			/* Replace busy bo. */
>+			size = i830_uxa_pixmap_compute_size (pixmap, w, h,
>+							     &tiling, &stride);
>+			if (size > intel->max_gtt_map_size)
>+				return FALSE;
>+
>+			bo = drm_intel_bo_alloc(intel->bufmgr, "pixmap", size, 0);
>+			if (bo == NULL)
>+				return FALSE;
>+
>+			if (tiling != I915_TILING_NONE)
>+				drm_intel_bo_set_tiling(bo, &tiling, stride);
>+
>+			screen->ModifyPixmapHeader(pixmap,
>+						   w, h,
>+						   0, 0,
>+						   stride, NULL);
>+			i830_set_pixmap_bo(pixmap, bo);
>+			dri_bo_unreference(bo);
>+
>+			return i830_uxa_pixmap_put_image(pixmap, src, src_pitch, 0, 0, w, h);
> 		}
>-
>-		ret = TRUE;
>-
>-		/* Older version of pixman did not allow blt for <= 8bpp
>-		 * images, so if the blt fails fallback to using the fb.
>-		 */
>-		dst_pitch = i830_pixmap_pitch (pixmap);
>-		if (! pixman_blt((uint32_t *)src,
>-				 priv->bo->virtual,
>-				 src_pitch / sizeof(uint32_t),
>-				 dst_pitch / sizeof(uint32_t),
>-				 pixmap->drawable.bitsPerPixel,
>-				 pixmap->drawable.bitsPerPixel,
>-				 0, 0,
>-				 x, y,
>-				 w, h))
>+		else
> 		{
>-			ret = FALSE;
>-
>-			scratch = GetScratchPixmapHeader(screen,
>-							 w, h,
>-							 pixmap->drawable.depth,
>-							 pixmap->drawable.bitsPerPixel,
>-							 src_pitch,
>-							 (pointer) src);
>-			if (scratch) {
>-				gc = GetScratchGC(pixmap->drawable.depth, screen);
>+			PixmapPtr scratch;
>+			Bool ret;
>+
>+			/* Upload to a linear buffer and queue a blit.  */
>+			scratch = (*screen->CreatePixmap)(screen, w, h,
>+							  pixmap->drawable.depth,
>+							  UXA_CREATE_PIXMAP_FOR_MAP);
>+			if (!scratch)
>+				return FALSE;
>+
>+			ret = i830_uxa_pixmap_put_image(scratch, src, src_pitch, 0, 0, w, h);
>+			if (ret) {
>+				GCPtr gc = GetScratchGC(pixmap->drawable.depth, screen);
> 				if (gc) {
> 					ValidateGC(&pixmap->drawable, gc);
>-					pixmap->devPrivate.ptr = priv->bo->virtual;
>-					ret = !! fbCopyArea(&scratch->drawable, &pixmap->drawable, gc,
>-							    0, 0,
>-							    w, h,
>-							    x, y);
>+
>+					(*gc->ops->CopyArea)(&scratch->drawable,
>+							     &pixmap->drawable,
>+							     gc, 0, 0, w, h, x, y);
>+
> 					FreeScratchGC(gc);
>-				}
>-				FreeScratchPixmapHeader(scratch);
>+				} else
>+					ret = FALSE;
> 			}
>-		}
> 
>-		drm_intel_gem_bo_unmap_gtt(priv->bo);
>+			(*screen->DestroyPixmap)(scratch);
>+			return ret;
>+		}
> 	}
>-
>-	return ret;
> }
> 
> void i830_uxa_block_handler(ScreenPtr screen)
>@@ -1024,25 +947,26 @@ i830_uxa_create_pixmap(ScreenPtr screen, int w, int h, int depth,
> 			return NullPixmap;
> 		}
> 
>-		if (usage == UXA_CREATE_PIXMAP_FOR_MAP)
>+		if (usage == UXA_CREATE_PIXMAP_FOR_MAP) {
>+			priv->busy = 0;
> 			priv->bo = drm_intel_bo_alloc(intel->bufmgr,
> 						      "pixmap", size, 0);
>-		else
>+		} else {
>+			priv->busy = -1;
> 			priv->bo = drm_intel_bo_alloc_for_render(intel->bufmgr,
> 								 "pixmap",
> 								 size, 0);
>+		}
> 		if (!priv->bo) {
> 			free(priv);
> 			fbDestroyPixmap(pixmap);
> 			return NullPixmap;
> 		}
> 
>+		if (tiling != I915_TILING_NONE)
>+			drm_intel_bo_set_tiling(priv->bo, &tiling, stride);
> 		priv->stride = stride;
> 		priv->tiling = tiling;
>-		if (priv->tiling != I915_TILING_NONE)
>-			drm_intel_bo_set_tiling(priv->bo,
>-						&priv->tiling,
>-						stride);
> 
> 		screen->ModifyPixmapHeader(pixmap, w, h, 0, 0, stride, NULL);
> 
>@@ -1072,6 +996,7 @@ void i830_uxa_create_screen_resources(ScreenPtr screen)
> 	if (bo != NULL) {
> 		PixmapPtr pixmap = screen->GetScreenPixmap(screen);
> 		i830_set_pixmap_bo(pixmap, bo);
>+		i830_get_pixmap_intel(pixmap)->busy = 1;
> 	}
> }
> 
>-- 
>1.7.1
>

Actions: View | Splinter Review

Attachments on bug 28569: 36308 | 36326

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.