All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
@ 2011-07-09  8:31 Chris Wilson
  2011-07-18 15:45 ` Daniel Vetter
  2011-07-18 16:17 ` Keith Packard
  0 siblings, 2 replies; 10+ messages in thread
From: Chris Wilson @ 2011-07-09  8:31 UTC (permalink / raw)
  To: intel-gfx

Align unfenced buffers on older hardware to the power-of-two object size.
The docs suggest that it should be possible to align only to a power-of-two
tile height, but using the already computed fence size is easier and
always correct. We also have to make sure that we unbind misaligned buffers
upon tiling changes.

Reported-and-tested-by: Sitosfe Wheeler <sitsofe@yahoo.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36326
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h        |    3 ++-
 drivers/gpu/drm/i915/i915_gem.c        |   30 +++++++++++++-----------------
 drivers/gpu/drm/i915/i915_gem_tiling.c |    3 ++-
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2798d27..a9492d9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1216,7 +1216,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_alignment(struct drm_i915_gem_object *obj);
+i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj,
+				    int tiling_mode);
 
 int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				    enum i915_cache_level cache_level);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e357c73..4fc9738 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1426,36 +1426,31 @@ i915_gem_get_gtt_alignment(struct drm_i915_gem_object *obj)
  * i915_gem_get_unfenced_gtt_alignment - return required GTT alignment for an
  *					 unfenced object
  * @obj: object to check
+ * @tiling_mode: tiling mode of the object
  *
  * Return the required GTT alignment for an object, only taking into account
  * unfenced tiled surface requirements.
  */
 uint32_t
-i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj)
+i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj,
+				    int tiling_mode)
 {
 	struct drm_device *dev = obj->base.dev;
-	int tile_height;
+
+	if (tiling_mode == I915_TILING_NONE)
+		return 4096;
 
 	/*
 	 * Minimum alignment is 4k (GTT page size) for sane hw.
 	 */
-	if (INTEL_INFO(dev)->gen >= 4 || IS_G33(dev) ||
-	    obj->tiling_mode == I915_TILING_NONE)
+	if (INTEL_INFO(dev)->gen >= 4 || IS_G33(dev))
 		return 4096;
 
-	/*
-	 * Older chips need unfenced tiled buffers to be aligned to the left
-	 * edge of an even tile row (where tile rows are counted as if the bo is
-	 * placed in a fenced gtt region).
+	/* Previous hardware however needs to be aligned to a power-of-two
+	 * tile height. The simplest method for determining this is to reuse
+	 * the power-of-tile object size.
 	 */
-	if (IS_GEN2(dev))
-		tile_height = 16;
-	else if (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 i915_gem_get_gtt_size(obj);
 }
 
 int
@@ -2781,7 +2776,8 @@ 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_alignment = i915_gem_get_unfenced_gtt_alignment(obj);
+	unfenced_alignment =
+		i915_gem_get_unfenced_gtt_alignment(obj, obj->tiling_mode);
 
 	if (alignment == 0)
 		alignment = map_and_fenceable ? fence_alignment :
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 82d70fd..8433b97 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -348,7 +348,8 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 		/* Rebind if we need a change of alignment */
 		if (!obj->map_and_fenceable) {
 			u32 unfenced_alignment =
-				i915_gem_get_unfenced_gtt_alignment(obj);
+				i915_gem_get_unfenced_gtt_alignment(obj,
+								    args->tiling_mode);
 			if (obj->gtt_offset & (unfenced_alignment - 1))
 				ret = i915_gem_object_unbind(obj);
 		}
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-09  8:31 [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware Chris Wilson
@ 2011-07-18 15:45 ` Daniel Vetter
  2011-07-18 16:17 ` Keith Packard
  1 sibling, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2011-07-18 15:45 UTC (permalink / raw)
  To: Chris Wilson, Keith Packard; +Cc: intel-gfx

It sucks a bit to see the relaxed fencing idea go down the toilet for pre-g33,
but bleh, hw sometimes just hates us.

Cc: stable@kernel.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwl.ch>
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-09  8:31 [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware Chris Wilson
  2011-07-18 15:45 ` Daniel Vetter
@ 2011-07-18 16:17 ` Keith Packard
  2011-07-18 16:25   ` Chris Wilson
                     ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Keith Packard @ 2011-07-18 16:17 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 589 bytes --]

On Sat,  9 Jul 2011 09:31:25 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:

>  uint32_t
> -i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj)
> +i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj,
> +				    int tiling_mode)
...
> +	return i915_gem_get_gtt_size(obj);

I think you want to pass the new tiling mode to this function rather
than using the object's existing tiling mode. Seems like most of the
issues could easily be explained by using the stale value when trying to
change tiling modes.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-18 16:17 ` Keith Packard
@ 2011-07-18 16:25   ` Chris Wilson
  2011-07-18 16:35   ` Keith Packard
  2011-07-18 16:41   ` Chris Wilson
  2 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2011-07-18 16:25 UTC (permalink / raw)
  To: Keith Packard, intel-gfx

On Mon, 18 Jul 2011 09:17:16 -0700, Keith Packard <keithp@keithp.com> wrote:
Non-text part: multipart/signed
> On Sat,  9 Jul 2011 09:31:25 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> >  uint32_t
> > -i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj)
> > +i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj,
> > +				    int tiling_mode)
> ...
> > +	return i915_gem_get_gtt_size(obj);
> 
> I think you want to pass the new tiling mode to this function rather
> than using the object's existing tiling mode. Seems like most of the
> issues could easily be explained by using the stale value when trying to
> change tiling modes.

So help me, I made that change at one time as Daniel pointed that out.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-18 16:17 ` Keith Packard
  2011-07-18 16:25   ` Chris Wilson
@ 2011-07-18 16:35   ` Keith Packard
  2011-07-18 16:57     ` Chris Wilson
  2011-07-18 16:41   ` Chris Wilson
  2 siblings, 1 reply; 10+ messages in thread
From: Keith Packard @ 2011-07-18 16:35 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 1217 bytes --]

On Mon, 18 Jul 2011 09:17:16 -0700, Keith Packard <keithp@keithp.com> wrote:
Non-text part: multipart/signed
> On Sat,  9 Jul 2011 09:31:25 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> >  uint32_t
> > -i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj)
> > +i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj,
> > +				    int tiling_mode)
> ...
> > +	return i915_gem_get_gtt_size(obj);
> 
> I think you want to pass the new tiling mode to this function rather
> than using the object's existing tiling mode. Seems like most of the
> issues could easily be explained by using the stale value when trying to
> change tiling modes.

Actually, given that the only thing you need from the object is the
size, it would be better to just create functions which take just the
size and tiling mode and computes the gtt size required to map
that. Like:

i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev, size_t size, unsigned tiling_mode)

i915_gem_get_gtt_size(struct drm_device *dev, size_t size, unsigned tiling_mode)

Then you can be sure you're using the correct tiling mode in all of the
computations.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-18 16:17 ` Keith Packard
  2011-07-18 16:25   ` Chris Wilson
  2011-07-18 16:35   ` Keith Packard
@ 2011-07-18 16:41   ` Chris Wilson
  2 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2011-07-18 16:41 UTC (permalink / raw)
  To: intel-gfx

Align unfenced buffers on older hardware to the power-of-two object size.
The docs suggest that it should be possible to align only to a power-of-two
tile height, but using the already computed fence size is easier and
always correct. We also have to make sure that we unbind misaligned buffers
upon tiling changes.

Reported-and-tested-by: Sitosfe Wheeler <sitsofe@yahoo.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36326
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---

Updated to pass tiling_mode to all the gtt size/alignment functions for
consistency and to prevent any more lurking bugs.

---
 drivers/gpu/drm/i915/i915_drv.h        |    3 +-
 drivers/gpu/drm/i915/i915_gem.c        |   47 +++++++++++++++-----------------
 drivers/gpu/drm/i915/i915_gem_tiling.c |    3 +-
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2798d27..a9492d9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1216,7 +1216,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_alignment(struct drm_i915_gem_object *obj);
+i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj,
+				    int tiling_mode);
 
 int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				    enum i915_cache_level cache_level);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2e0d891..4c4f6c3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1374,13 +1374,14 @@ i915_gem_free_mmap_offset(struct drm_i915_gem_object *obj)
 }
 
 static uint32_t
-i915_gem_get_gtt_size(struct drm_i915_gem_object *obj)
+i915_gem_get_gtt_size(struct drm_i915_gem_object *obj,
+		      int tiling_mode)
 {
 	struct drm_device *dev = obj->base.dev;
 	uint32_t size;
 
 	if (INTEL_INFO(dev)->gen >= 4 ||
-	    obj->tiling_mode == I915_TILING_NONE)
+	    tiling_mode == I915_TILING_NONE)
 		return obj->base.size;
 
 	/* Previous chips need a power-of-two fence region when tiling */
@@ -1403,7 +1404,8 @@ i915_gem_get_gtt_size(struct drm_i915_gem_object *obj)
  * potential fence register mapping.
  */
 static uint32_t
-i915_gem_get_gtt_alignment(struct drm_i915_gem_object *obj)
+i915_gem_get_gtt_alignment(struct drm_i915_gem_object *obj,
+			   int tiling_mode)
 {
 	struct drm_device *dev = obj->base.dev;
 
@@ -1411,51 +1413,45 @@ i915_gem_get_gtt_alignment(struct drm_i915_gem_object *obj)
 	 * Minimum alignment is 4k (GTT page size), but might be greater
 	 * if a fence register is needed for the object.
 	 */
-	if (INTEL_INFO(dev)->gen >= 4 ||
-	    obj->tiling_mode == I915_TILING_NONE)
+	if (INTEL_INFO(dev)->gen >= 4 || tiling_mode == I915_TILING_NONE)
 		return 4096;
 
 	/*
 	 * Previous chips need to be aligned to the size of the smallest
 	 * fence register that can contain the object.
 	 */
-	return i915_gem_get_gtt_size(obj);
+	return i915_gem_get_gtt_size(obj, tiling_mode);
 }
 
 /**
  * i915_gem_get_unfenced_gtt_alignment - return required GTT alignment for an
  *					 unfenced object
  * @obj: object to check
+ * @tiling_mode: tiling mode of the object
  *
  * Return the required GTT alignment for an object, only taking into account
  * unfenced tiled surface requirements.
  */
 uint32_t
-i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj)
+i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj,
+				    int tiling_mode)
 {
 	struct drm_device *dev = obj->base.dev;
-	int tile_height;
+
+	if (tiling_mode == I915_TILING_NONE)
+		return 4096;
 
 	/*
 	 * Minimum alignment is 4k (GTT page size) for sane hw.
 	 */
-	if (INTEL_INFO(dev)->gen >= 4 || IS_G33(dev) ||
-	    obj->tiling_mode == I915_TILING_NONE)
+	if (INTEL_INFO(dev)->gen >= 4 || IS_G33(dev))
 		return 4096;
 
-	/*
-	 * Older chips need unfenced tiled buffers to be aligned to the left
-	 * edge of an even tile row (where tile rows are counted as if the bo is
-	 * placed in a fenced gtt region).
+	/* Previous hardware however needs to be aligned to a power-of-two
+	 * tile height. The simplest method for determining this is to reuse
+	 * the power-of-tile object size.
 	 */
-	if (IS_GEN2(dev))
-		tile_height = 16;
-	else if (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 i915_gem_get_gtt_size(obj, tiling_mode);
 }
 
 int
@@ -2786,9 +2782,10 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 		return -EINVAL;
 	}
 
-	fence_size = i915_gem_get_gtt_size(obj);
-	fence_alignment = i915_gem_get_gtt_alignment(obj);
-	unfenced_alignment = i915_gem_get_unfenced_gtt_alignment(obj);
+	fence_size = i915_gem_get_gtt_size(obj, obj->tiling_mode);
+	fence_alignment = i915_gem_get_gtt_alignment(obj, obj->tiling_mode);
+	unfenced_alignment =
+		i915_gem_get_unfenced_gtt_alignment(obj, obj->tiling_mode);
 
 	if (alignment == 0)
 		alignment = map_and_fenceable ? fence_alignment :
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 82d70fd..8433b97 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -348,7 +348,8 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 		/* Rebind if we need a change of alignment */
 		if (!obj->map_and_fenceable) {
 			u32 unfenced_alignment =
-				i915_gem_get_unfenced_gtt_alignment(obj);
+				i915_gem_get_unfenced_gtt_alignment(obj,
+								    args->tiling_mode);
 			if (obj->gtt_offset & (unfenced_alignment - 1))
 				ret = i915_gem_object_unbind(obj);
 		}
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-18 16:35   ` Keith Packard
@ 2011-07-18 16:57     ` Chris Wilson
  2011-07-18 20:31       ` Keith Packard
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2011-07-18 16:57 UTC (permalink / raw)
  To: intel-gfx

Align unfenced buffers on older hardware to the power-of-two object size.
The docs suggest that it should be possible to align only to a power-of-two
tile height, but using the already computed fence size is easier and
always correct. We also have to make sure that we unbind misaligned buffers
upon tiling changes.

In order to prevent a repetition of this bug, we change the interface to
the alignment computation routines to force the caller to provide the
requested alignment and size of the GTT binding rather than assume the
current values on the object.

Reported-and-tested-by: Sitosfe Wheeler <sitsofe@yahoo.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36326
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---

Keith, apologies your missive arrived as I sent the previous patch. This
uses the inferface you proposed..

---
 drivers/gpu/drm/i915/i915_drv.h        |    4 +-
 drivers/gpu/drm/i915/i915_gem.c        |   75 ++++++++++++++++----------------
 drivers/gpu/drm/i915/i915_gem_tiling.c |    4 +-
 3 files changed, 43 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2798d27..b533ab8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1216,7 +1216,9 @@ 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_alignment(struct drm_i915_gem_object *obj);
+i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev,
+				    uint32_t size,
+				    int tiling_mode);
 
 int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				    enum i915_cache_level cache_level);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2e0d891..b9d9879 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1374,25 +1374,23 @@ i915_gem_free_mmap_offset(struct drm_i915_gem_object *obj)
 }
 
 static uint32_t
-i915_gem_get_gtt_size(struct drm_i915_gem_object *obj)
+i915_gem_get_gtt_size(struct drm_device *dev, uint32_t size, int tiling_mode)
 {
-	struct drm_device *dev = obj->base.dev;
-	uint32_t size;
+	uint32_t fence_size;
 
-	if (INTEL_INFO(dev)->gen >= 4 ||
-	    obj->tiling_mode == I915_TILING_NONE)
-		return obj->base.size;
+	if (INTEL_INFO(dev)->gen >= 4 || tiling_mode == I915_TILING_NONE)
+		return size;
 
 	/* Previous chips need a power-of-two fence region when tiling */
 	if (INTEL_INFO(dev)->gen == 3)
-		size = 1024*1024;
+		fence_size = 1024*1024;
 	else
-		size = 512*1024;
+		fence_size = 512*1024;
 
-	while (size < obj->base.size)
-		size <<= 1;
+	while (fence_size < size)
+		fence_size <<= 1;
 
-	return size;
+	return fence_size;
 }
 
 /**
@@ -1403,59 +1401,53 @@ i915_gem_get_gtt_size(struct drm_i915_gem_object *obj)
  * potential fence register mapping.
  */
 static uint32_t
-i915_gem_get_gtt_alignment(struct drm_i915_gem_object *obj)
+i915_gem_get_gtt_alignment(struct drm_device *dev,
+			   uint32_t size,
+			   int tiling_mode)
 {
-	struct drm_device *dev = obj->base.dev;
-
 	/*
 	 * Minimum alignment is 4k (GTT page size), but might be greater
 	 * if a fence register is needed for the object.
 	 */
-	if (INTEL_INFO(dev)->gen >= 4 ||
-	    obj->tiling_mode == I915_TILING_NONE)
+	if (INTEL_INFO(dev)->gen >= 4 || tiling_mode == I915_TILING_NONE)
 		return 4096;
 
 	/*
 	 * Previous chips need to be aligned to the size of the smallest
 	 * fence register that can contain the object.
 	 */
-	return i915_gem_get_gtt_size(obj);
+	return i915_gem_get_gtt_size(dev, size, tiling_mode);
 }
 
 /**
  * i915_gem_get_unfenced_gtt_alignment - return required GTT alignment for an
  *					 unfenced object
- * @obj: object to check
+ * @dev: the device
+ * @size: size of the object
+ * @tiling_mode: tiling mode of the object
  *
  * Return the required GTT alignment for an object, only taking into account
  * unfenced tiled surface requirements.
  */
 uint32_t
-i915_gem_get_unfenced_gtt_alignment(struct drm_i915_gem_object *obj)
+i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev,
+				    uint32_t size,
+				    int tiling_mode)
 {
-	struct drm_device *dev = obj->base.dev;
-	int tile_height;
+	if (tiling_mode == I915_TILING_NONE)
+		return 4096;
 
 	/*
 	 * Minimum alignment is 4k (GTT page size) for sane hw.
 	 */
-	if (INTEL_INFO(dev)->gen >= 4 || IS_G33(dev) ||
-	    obj->tiling_mode == I915_TILING_NONE)
+	if (INTEL_INFO(dev)->gen >= 4 || IS_G33(dev))
 		return 4096;
 
-	/*
-	 * Older chips need unfenced tiled buffers to be aligned to the left
-	 * edge of an even tile row (where tile rows are counted as if the bo is
-	 * placed in a fenced gtt region).
+	/* Previous hardware however needs to be aligned to a power-of-two
+	 * tile height. The simplest method for determining this is to reuse
+	 * the power-of-tile object size.
 	 */
-	if (IS_GEN2(dev))
-		tile_height = 16;
-	else if (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 i915_gem_get_gtt_size(dev, size, tiling_mode);
 }
 
 int
@@ -2786,9 +2778,16 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 		return -EINVAL;
 	}
 
-	fence_size = i915_gem_get_gtt_size(obj);
-	fence_alignment = i915_gem_get_gtt_alignment(obj);
-	unfenced_alignment = i915_gem_get_unfenced_gtt_alignment(obj);
+	fence_size = i915_gem_get_gtt_size(dev,
+					   obj->base.size,
+					   obj->tiling_mode);
+	fence_alignment = i915_gem_get_gtt_alignment(dev,
+						     obj->base.size,
+						     obj->tiling_mode);
+	unfenced_alignment =
+		i915_gem_get_unfenced_gtt_alignment(dev,
+						    obj->base.size,
+						    obj->tiling_mode);
 
 	if (alignment == 0)
 		alignment = map_and_fenceable ? fence_alignment :
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 82d70fd..99c4faa 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -348,7 +348,9 @@ i915_gem_set_tiling(struct drm_device *dev, void *data,
 		/* Rebind if we need a change of alignment */
 		if (!obj->map_and_fenceable) {
 			u32 unfenced_alignment =
-				i915_gem_get_unfenced_gtt_alignment(obj);
+				i915_gem_get_unfenced_gtt_alignment(dev,
+								    obj->base.size,
+								    args->tiling_mode);
 			if (obj->gtt_offset & (unfenced_alignment - 1))
 				ret = i915_gem_object_unbind(obj);
 		}
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-18 16:57     ` Chris Wilson
@ 2011-07-18 20:31       ` Keith Packard
  2011-07-18 22:48         ` Paul Menzel
  0 siblings, 1 reply; 10+ messages in thread
From: Keith Packard @ 2011-07-18 20:31 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 464 bytes --]

On Mon, 18 Jul 2011 17:57:52 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Keith, apologies your missive arrived as I sent the previous patch. This
> uses the inferface you proposed..

Thanks! I've reduced the diff a bit more to make it easier to review and
have pushed this patch onto drm-intel-fixes. I'd like to get a bit more
testing before I send it along, but that needs to be soon if we want
this in 3.0.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-18 20:31       ` Keith Packard
@ 2011-07-18 22:48         ` Paul Menzel
  2011-07-18 23:14           ` Keith Packard
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Menzel @ 2011-07-18 22:48 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sitsofe Wheeler


[-- Attachment #1.1: Type: text/plain, Size: 861 bytes --]

Am Montag, den 18.07.2011, 13:31 -0700 schrieb Keith Packard:
> On Mon, 18 Jul 2011 17:57:52 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > Keith, apologies your missive arrived as I sent the previous patch. This
> > uses the inferface you proposed..
> 
> Thanks! I've reduced the diff a bit more to make it easier to review and
> have pushed this patch onto drm-intel-fixes. I'd like to get a bit more
> testing before I send it along, but that needs to be soon if we want
> this in 3.0.

I have not been able to test this patch but if it fixes the issue it
should definitely be sent to stable as well since the bug has been there
since at least Linux 2.6.37 [1][2] in a lot distributions.


Thanks,

Paul


[1] https://bugzilla.redhat.com/show_bug.cgi?id=704959
[2] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=614296

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware
  2011-07-18 22:48         ` Paul Menzel
@ 2011-07-18 23:14           ` Keith Packard
  0 siblings, 0 replies; 10+ messages in thread
From: Keith Packard @ 2011-07-18 23:14 UTC (permalink / raw)
  To: Paul Menzel, intel-gfx; +Cc: Sitsofe Wheeler


[-- Attachment #1.1: Type: text/plain, Size: 666 bytes --]

On Tue, 19 Jul 2011 00:48:23 +0200, Paul Menzel <paulepanter@users.sourceforge.net> wrote:
Non-text part: multipart/mixed
Non-text part: multipart/signed
> Am Montag, den 18.07.2011, 13:31 -0700 schrieb Keith Packard:

> I have not been able to test this patch but if it fixes the issue it
> should definitely be sent to stable as well since the bug has been there
> since at least Linux 2.6.37 [1][2] in a lot distributions.

The patch is on drm-intel-fixes in my tree; please test what's there and
report back what you find out. That patch is marked with a Cc: to
stable, so it will end up in stable kernels eventually.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-07-18 23:16 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-09  8:31 [PATCH] drm/i915: Fix unfenced alignment on pre-G33 hardware Chris Wilson
2011-07-18 15:45 ` Daniel Vetter
2011-07-18 16:17 ` Keith Packard
2011-07-18 16:25   ` Chris Wilson
2011-07-18 16:35   ` Keith Packard
2011-07-18 16:57     ` Chris Wilson
2011-07-18 20:31       ` Keith Packard
2011-07-18 22:48         ` Paul Menzel
2011-07-18 23:14           ` Keith Packard
2011-07-18 16:41   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.