Re: [Intel-gfx] [PATCH] drm/i915: fix tiling limits for i915 class hw v2

2010-04-18 Thread Eric Anholt
On Sat, 17 Apr 2010 15:12:03 +0200, Daniel Vetter daniel.vet...@ffwll.ch 
wrote:
 Current code is definitely crap: Largest pitch allowed spills into
 the TILING_Y bit of the fence registers ... :(
 
 I've rewritten the limits check under the assumption that 3rd gen hw
 has a 3d pitch limit of 8kb (like 2nd gen). This is supported by an
 otherwise totally misleading XXX comment.
 
 This bug mostly resulted in tiling-corrupted pixmaps because the kernel
 allowed too wide buffers to be tiled. Bug brought to the light by the
 xf86-video-intel 2.11 release because that unconditionally enabled
 tiling for pixmaps, relying on the kernel to check things. Tiling for
 the framebuffer was not affected because the ddx does some additional
 checks there ensure the buffer is within hw-limits.
 
 v2: Instead of computing the value that would be written into the
 hw fence registers and then checking the limits simply check whether
 the stride is above the 8kb limit. To better document the hw, add
 some WARN_ONs in i915_write_fence_reg like I've done for the i830
 case (using the right limits).
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=27449
 Tested-by: Alexander Lam lambchop...@gmail.com
 Cc: sta...@kernel.org

Ignore previous response.  Here's exactly the patch I wanted.

Applied to for-linus.


pgpnCeEP0RkkC.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: fix tiling limits for i915 class hw v2

2010-04-17 Thread Daniel Vetter
Current code is definitely crap: Largest pitch allowed spills into
the TILING_Y bit of the fence registers ... :(

I've rewritten the limits check under the assumption that 3rd gen hw
has a 3d pitch limit of 8kb (like 2nd gen). This is supported by an
otherwise totally misleading XXX comment.

This bug mostly resulted in tiling-corrupted pixmaps because the kernel
allowed too wide buffers to be tiled. Bug brought to the light by the
xf86-video-intel 2.11 release because that unconditionally enabled
tiling for pixmaps, relying on the kernel to check things. Tiling for
the framebuffer was not affected because the ddx does some additional
checks there ensure the buffer is within hw-limits.

v2: Instead of computing the value that would be written into the
hw fence registers and then checking the limits simply check whether
the stride is above the 8kb limit. To better document the hw, add
some WARN_ONs in i915_write_fence_reg like I've done for the i830
case (using the right limits).

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=27449
Tested-by: Alexander Lam lambchop...@gmail.com
Cc: sta...@kernel.org
---
 drivers/gpu/drm/i915/i915_gem.c|6 ++
 drivers/gpu/drm/i915/i915_gem_tiling.c |   21 -
 drivers/gpu/drm/i915/i915_reg.h|2 +-
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6e71b5b..f302910 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2362,6 +2362,12 @@ static void i915_write_fence_reg(struct 
drm_i915_fence_reg *reg)
pitch_val = obj_priv-stride / tile_width;
pitch_val = ffs(pitch_val) - 1;
 
+   if (obj_priv-tiling_mode == I915_TILING_Y 
+   HAS_128_BYTE_Y_TILING(dev))
+   WARN_ON(pitch_val  I830_FENCE_MAX_PITCH_VAL);
+   else
+   WARN_ON(pitch_val  I915_FENCE_MAX_PITCH_VAL);
+
val = obj_priv-gtt_offset;
if (obj_priv-tiling_mode == I915_TILING_Y)
val |= 1  I830_FENCE_TILING_Y_SHIFT;
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c 
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 449157f..6393411 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -202,21 +202,16 @@ i915_tiling_ok(struct drm_device *dev, int stride, int 
size, int tiling_mode)
 * reg, so dont bother to check the size */
if (stride / 128  I965_FENCE_MAX_PITCH_VAL)
return false;
-   } else if (IS_I9XX(dev)) {
-   uint32_t pitch_val = ffs(stride / tile_width) - 1;
-
-   /* XXX: For Y tiling, FENCE_MAX_PITCH_VAL is actually 6 (8KB)
-* instead of 4 (2KB) on 945s.
-*/
-   if (pitch_val  I915_FENCE_MAX_PITCH_VAL ||
-   size  (I830_FENCE_MAX_SIZE_VAL  20))
+   } else if (IS_GEN3(dev) || IS_GEN2(dev)) {
+   if (stride  8192)
return false;
-   } else {
-   uint32_t pitch_val = ffs(stride / tile_width) - 1;
 
-   if (pitch_val  I830_FENCE_MAX_PITCH_VAL ||
-   size  (I830_FENCE_MAX_SIZE_VAL  19))
-   return false;
+   if (IS_GEN3(dev))
+   if (size  I830_FENCE_MAX_SIZE_VAL  20)
+   return false;
+   else
+   if (size  I830_FENCE_MAX_SIZE_VAL  19)
+   return false;
}
 
/* 965+ just needs multiples of tile width */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cbbf59f..773c1ad 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -241,7 +241,7 @@
 #define   I830_FENCE_SIZE_BITS(size)   ((ffs((size)  19) - 1)  8)
 #define   I830_FENCE_PITCH_SHIFT   4
 #define   I830_FENCE_REG_VALID (10)
-#define   I915_FENCE_MAX_PITCH_VAL 0x10
+#define   I915_FENCE_MAX_PITCH_VAL 4
 #define   I830_FENCE_MAX_PITCH_VAL 6
 #define   I830_FENCE_MAX_SIZE_VAL  (18)
 
-- 
1.6.6.1

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


Re: [Intel-gfx] [PATCH] drm/i915: fix tiling limits for i915 class hw

2010-04-15 Thread Chris Wilson
On Thu, 15 Apr 2010 09:08:16 +0200, Daniel Vetter daniel.vet...@ffwll.ch 
wrote:
 Current code is definitely crap: Largest pitch allowed spills into
 the TILING_Y bit of the fence registers ... :(
 
 I've rewritten the limits check under the assumption that 3rd gen hw
 has a 3d pitch limit of 8kb (like 2nd gen). This is supported by an
 otherwise totally misleading XXX comment.

Good catch Daniel! Hopefully we must be soon running out of tiling bugs.

It does - the fence pitch limit is 8KiB for both 512- and 128-wide tiles.
In the 128 byte case the max pitch value is 64, and correspondingly the
max pitch value is 16 for 512 byte wide tiles.
-ickle

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx