| Message ID | 1249694345-9228-1-git-send-email-eric@anholt.net (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Eric Anholt wrote: > diff --git a/src/i830_uxa.c b/src/i830_uxa.c > index 3a476a7..1087128 100644 > --- a/src/i830_uxa.c > +++ b/src/i830_uxa.c > @@ -615,6 +615,13 @@ i830_uxa_create_pixmap (ScreenPtr screen, int w, int h, int depth, unsigned usag > if (tiling == I915_TILING_NONE) { > size = stride * h; > } else { > + int aligned_h = h; > + if (tiling == I915_TILING_X) > + aligned_h = ALIGN(h, 8); > + else > + aligned_h = ALIGN(h, 16); > + assert(aligned_h >= h); > + Looking at the other patch, should the else case be 'else if (tiling == I915_TILING_Y)'? > stride = i830_get_fence_pitch(i830, stride, tiling); > /* Round the object up to the size of the fence it will live in > * if necessary. We could potentially make the kernel allocate -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkqDOloACgkQX1gOwKyEAw8T7wCeLC4PkjNrqTjGX8kK9E16oria sGIAn0C8lm9B6Zjo9LMjmGY8oQ7GkJSF =Qa2G -----END PGP SIGNATURE-----
On Wed, 2009-08-12 at 14:55 -0700, Ian Romanick wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Eric Anholt wrote: > > > diff --git a/src/i830_uxa.c b/src/i830_uxa.c > > index 3a476a7..1087128 100644 > > --- a/src/i830_uxa.c > > +++ b/src/i830_uxa.c > > @@ -615,6 +615,13 @@ i830_uxa_create_pixmap (ScreenPtr screen, int w, int h, int depth, unsigned usag > > if (tiling == I915_TILING_NONE) { > > size = stride * h; > > } else { > > + int aligned_h = h; > > + if (tiling == I915_TILING_X) > > + aligned_h = ALIGN(h, 8); > > + else > > + aligned_h = ALIGN(h, 16); > > + assert(aligned_h >= h); > > + > > Looking at the other patch, should the else case be 'else if (tiling == > I915_TILING_Y)'? Depends. Is there a secret undocumented tiling mode? ;-) In seriousness though, according to the documentation [Vol 1a, p135] even the NONE case should be ALIGN(h, 2). -ickle
On Wed, 2009-08-12 at 23:13 +0100, Chris Wilson wrote: > On Wed, 2009-08-12 at 14:55 -0700, Ian Romanick wrote: > > -----BEGIN PGP SIGNED MESSAGE----- > > Hash: SHA1 > > > > Eric Anholt wrote: > > > > > diff --git a/src/i830_uxa.c b/src/i830_uxa.c > > > index 3a476a7..1087128 100644 > > > --- a/src/i830_uxa.c > > > +++ b/src/i830_uxa.c > > > @@ -615,6 +615,13 @@ i830_uxa_create_pixmap (ScreenPtr screen, int w, int h, int depth, unsigned usag > > > if (tiling == I915_TILING_NONE) { > > > size = stride * h; > > > } else { > > > + int aligned_h = h; > > > + if (tiling == I915_TILING_X) > > > + aligned_h = ALIGN(h, 8); > > > + else > > > + aligned_h = ALIGN(h, 16); > > > + assert(aligned_h >= h); > > > + > > > > Looking at the other patch, should the else case be 'else if (tiling == > > I915_TILING_Y)'? > > Depends. Is there a secret undocumented tiling mode? ;-) > > In seriousness though, according to the documentation [Vol 1a, p135] > even the NONE case should be ALIGN(h, 2). Good point. That's actually the part of the docs I was looking at that sparked me thinking to produce that bugfix!
diff --git a/src/i830.h b/src/i830.h index 58afe76..b46eff1 100644 --- a/src/i830.h +++ b/src/i830.h @@ -619,6 +619,8 @@ typedef struct _I830Rec { #define I830PTR(p) ((I830Ptr)((p)->driverPrivate)) #define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0])) +#define ALIGN(i,m) (((i) + (m) - 1) & ~((m) - 1)) +#define MIN(a,b) ((a) < (b) ? (a) : (b)) #define I830_SELECT_FRONT 0 #define I830_SELECT_BACK 1 diff --git a/src/i830_uxa.c b/src/i830_uxa.c index 3a476a7..1087128 100644 --- a/src/i830_uxa.c +++ b/src/i830_uxa.c @@ -615,6 +615,13 @@ i830_uxa_create_pixmap (ScreenPtr screen, int w, int h, int depth, unsigned usag if (tiling == I915_TILING_NONE) { size = stride * h; } else { + int aligned_h = h; + if (tiling == I915_TILING_X) + aligned_h = ALIGN(h, 8); + else + aligned_h = ALIGN(h, 16); + assert(aligned_h >= h); + stride = i830_get_fence_pitch(i830, stride, tiling); /* Round the object up to the size of the fence it will live in * if necessary. We could potentially make the kernel allocate @@ -622,8 +629,8 @@ i830_uxa_create_pixmap (ScreenPtr screen, int w, int h, int depth, unsigned usag * but this is easier and also keeps us out of trouble (as much) * with drm_intel_bufmgr_check_aperture(). */ - size = i830_get_fence_size(i830, stride * h); - assert(size >= stride * h); + size = i830_get_fence_size(i830, stride * aligned_h); + assert(size >= stride * aligned_h); } /* Fail very large allocations on 32-bit systems. Large BOs will diff --git a/src/i965_render.c b/src/i965_render.c index eeb23e1..1a8075b 100644 --- a/src/i965_render.c +++ b/src/i965_render.c @@ -251,8 +251,6 @@ i965_check_composite(int op, PicturePtr pSrcPicture, PicturePtr pMaskPicture, } -#define ALIGN(i,m) (((i) + (m) - 1) & ~((m) - 1)) -#define MIN(a,b) ((a) < (b) ? (a) : (b)) #define BRW_GRF_BLOCKS(nreg) ((nreg + 15) / 16 - 1) /* Set up a default static partitioning of the URB, which is supposed to diff --git a/src/i965_video.c b/src/i965_video.c index 805b33f..46a461f 100644 --- a/src/i965_video.c +++ b/src/i965_video.c @@ -131,9 +131,6 @@ static const uint32_t ps_kernel_planar_static_gen5[][4] = { #include "exa_wm_write.g4b.gen5" }; -#define ALIGN(i,m) (((i) + (m) - 1) & ~((m) - 1)) -#define MIN(a,b) ((a) < (b) ? (a) : (b)) - static uint32_t float_to_uint (float f) { union {uint32_t i; float f;} x; x.f = f;