intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH] drm/i915: Round-up GTT allocations for unfenced surfaces to the next tile row
Date: Fri, 25 Mar 2011 12:16:25 +0000	[thread overview]
Message-ID: <1301055385-4844-1-git-send-email-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <1301045779-8076-1-git-send-email-chris@chris-wilson.co.uk>

The kernel has always been lax in enforcing size restrictions upon the
buffers it is handed to by userspace, and userspace has always tried to
overallocate its buffers.

However, userspace made a mistake and failed to follow the undocumented
requirement that gen2 hardware requires its allocations to be rounded up
to the next even tile row. Rampant corruption on the bottom on images on
i8xx devices ensued. Userspace was fixed not to do that again, and now
we teach the kernel not to be so trusting of userspace.

We ensure that an allocated region within the GTT matches the proposed
usage restrictions. For fenced buffers on old hardware, this means
rounding up the allocation to the next power of two size and aligning it
to that size. For unfenced buffers, we need to ensure that the start and
end of the allocation is aligned to an even tile row.

v2: Apply the allocation fixup to all devices

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reported-by: Everyone with an i8xx device
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h        |    2 +
 drivers/gpu/drm/i915/i915_gem.c        |   66 ++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/i915_gem_tiling.c |    5 ++-
 3 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 787c969..359ddce 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1183,6 +1183,8 @@ void i915_gem_free_all_phys_object(struct drm_device *dev);
 void i915_gem_release(struct drm_device *dev, struct drm_file *file);
 
 uint32_t
+i915_gem_get_unfenced_gtt_size(struct drm_i915_gem_object *obj);
+uint32_t
 i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj);
 
 /* i915_gem_gtt.c */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0012061..9144ff6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1426,6 +1426,55 @@ i915_gem_get_gtt_alignment(struct drm_i915_gem_object *obj)
 	return i915_gem_get_gtt_size(obj);
 }
 
+static u32 i915_gem_object_tiled_row_size(struct drm_i915_gem_object *obj)
+{
+	struct drm_device *dev = obj->base.dev;
+	int tile_height;
+
+	if (IS_GEN2(dev) ||
+	    (obj->tiling_mode == I915_TILING_Y && HAS_128_BYTE_Y_TILING(dev)))
+		tile_height = 32;
+	else
+		tile_height = 8;
+
+	return tile_height * obj->stride;
+}
+
+/**
+ * i915_gem_get_unfenced_gtt_size - return required GTT size for an
+ *				    unfenced object
+ * @obj: object to check
+ *
+ * Return the required GTT size for an object, only taking into account
+ * unfenced tiled surface requirements.
+ */
+uint32_t
+i915_gem_get_unfenced_gtt_size(struct drm_i915_gem_object *obj)
+{
+	u32 unfenced_alignment;
+
+	if (obj->tiling_mode == I915_TILING_NONE)
+		return obj->base.size;
+
+	/*
+	 * Current userspace will attempt to overallocate a bo so that it
+	 * can be reused with another surface and so its size is unlikely
+	 * to be an exact number of tile rows - but it promised never to
+	 * access beyond the end of the last complete row.
+	 *
+	 * gen2 has a further restriction, in that it requires an even number
+	 * of tiles rows. Userspace was not aware of this until recently
+	 * and so violated its promise to always allocate enough pages
+	 * for the hardware. In reply, we now always round up the GTT
+	 * allocation to the next [even] tile row.
+	 */
+	unfenced_alignment = i915_gem_object_tiled_row_size(obj);
+	if (IS_GEN2(obj->base.dev))
+		unfenced_alignment *= 2;
+
+	return ALIGN(obj->base.size, unfenced_alignment);
+}
+
 /**
  * i915_gem_get_unfenced_gtt_alignment - return required GTT alignment for an
  *					 unfenced object
@@ -1438,7 +1487,6 @@ uint32_t
 i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
-	int tile_height;
 
 	/*
 	 * Minimum alignment is 4k (GTT page size) for sane hw.
@@ -1452,13 +1500,7 @@ i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj)
 	 * edge of an even tile row (where tile rows are counted as if the bo is
 	 * placed in a fenced gtt region).
 	 */
-	if (IS_GEN2(dev) ||
-	    (obj->tiling_mode == I915_TILING_Y && HAS_128_BYTE_Y_TILING(dev)))
-		tile_height = 32;
-	else
-		tile_height = 8;
-
-	return tile_height * obj->stride * 2;
+	return 2 * i915_gem_object_tiled_row_size(obj);
 }
 
 int
@@ -2744,7 +2786,8 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_mm_node *free_space;
 	gfp_t gfpmask = __GFP_NORETRY | __GFP_NOWARN;
-	u32 size, fence_size, fence_alignment, unfenced_alignment;
+	u32 size, fence_size, fence_alignment;
+	u32 unfenced_size, unfenced_alignment;
 	bool mappable, fenceable;
 	int ret;
 
@@ -2755,6 +2798,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 
 	fence_size = i915_gem_get_gtt_size(obj);
 	fence_alignment = i915_gem_get_gtt_alignment(obj);
+	unfenced_size = i915_gem_get_unfenced_gtt_size(obj);
 	unfenced_alignment = i915_gem_get_unfenced_gtt_alignment(obj);
 
 	if (alignment == 0)
@@ -2765,12 +2809,12 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 		return -EINVAL;
 	}
 
-	size = map_and_fenceable ? fence_size : obj->base.size;
+	size = map_and_fenceable ? fence_size : unfenced_size;
 
 	/* If the object is bigger than the entire aperture, reject it early
 	 * before evicting everything in a vain attempt to find space.
 	 */
-	if (obj->base.size >
+	if (size >
 	    (map_and_fenceable ? dev_priv->mm.gtt_mappable_end : dev_priv->mm.gtt_total)) {
 		DRM_ERROR("Attempting to bind an object larger than the aperture\n");
 		return -E2BIG;
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 281ad3d..e4d5c58 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -349,7 +349,10 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 		if (!obj->map_and_fenceable) {
 			u32 unfenced_alignment =
 				i915_gem_get_unfenced_gtt_alignment(obj);
-			if (obj->gtt_offset & (unfenced_alignment - 1))
+			u32 unfenced_size =
+				i915_gem_get_unfenced_gtt_size(obj);
+			if (obj->gtt_space->size < unfenced_size ||
+			    obj->gtt_offset & (unfenced_alignment - 1))
 				ret = i915_gem_object_unbind(obj);
 		}
 
-- 
1.7.4.1

  reply	other threads:[~2011-03-25 12:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-25  9:36 [PATCH] drm/i915: Round-up GTT allocations for unfenced surfaces to the next tile row Chris Wilson
2011-03-25 12:16 ` Chris Wilson [this message]
2011-03-26  8:52   ` Chris Wilson
2011-03-26  9:20     ` Daniel Vetter
2011-03-26  9:43       ` Chris Wilson
2011-03-26 14:22         ` [PATCH] drm/i915: fix relaxed tiling on gen2 Daniel Vetter
2011-03-26 18:23           ` Chris Wilson
2011-03-26 19:55             ` [PATCH] drm/i915: fix relaxed tiling on gen2 v2 Daniel Vetter
2011-03-26 20:44               ` Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1301055385-4844-1-git-send-email-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).