[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-11 Thread Daniel Vetter
On Fri, Mar 11, 2011 at 02:08:46AM +0100, Indan Zupancic wrote:
> On Thu, March 10, 2011 14:31, Daniel Vetter wrote:
> > Am Do, 10.03.2011, 11:36 schrieb Indan Zupancic:
> >> Which versions fix this, just for reference?
> >
> > git master branch of libdrm and xf86-video-intel newer than 2011-02-22.
> 
> Thank you. If there will be no new releases of those packages within
> a couple weeks it might be better to temporarily add those checks back
> for gen 2 only, I think. Otherwise there will be a period where people
> who update regularly will have screen corruption, with no easy way of
> fixing it. I think this would avoid a few unnecessary bugreports. The
> check can be removed in 2.6.39-rc1, if you want I'll remind you about
> it too.

Well, libdrm is already released (2.4.24) and for the ddx there's an rc
(2.4.901) out there. So that should be sufficient.
-Daniel
-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48


[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-11 Thread Indan Zupancic
On Thu, March 10, 2011 14:31, Daniel Vetter wrote:
> Am Do, 10.03.2011, 11:36 schrieb Indan Zupancic:
>> Which versions fix this, just for reference?
>
> git master branch of libdrm and xf86-video-intel newer than 2011-02-22.

Thank you. If there will be no new releases of those packages within
a couple weeks it might be better to temporarily add those checks back
for gen 2 only, I think. Otherwise there will be a period where people
who update regularly will have screen corruption, with no easy way of
fixing it. I think this would avoid a few unnecessary bugreports. The
check can be removed in 2.6.39-rc1, if you want I'll remind you about
it too.

Greetings,

Indan




[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-10 Thread Daniel Vetter
Am Do, 10.03.2011, 11:36 schrieb Indan Zupancic:
> On Thu, March 10, 2011 08:52, Daniel Vetter wrote:
>> [Aside: This only happens if you have new enough userspace that supports
>> relaxed tiling. Old userspace and new kernels are _not_ broken.]
>
> Yes, I noticed it got reverted when digging into git log, but the
> commit message only said it broke gen4+, not how gen2 should be
> unbroken after the revert.
>
> Which versions fix this, just for reference?

git master branch of libdrm and xf86-video-intel newer than 2011-02-22.

-Daniel



[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-10 Thread Indan Zupancic
On Thu, March 10, 2011 08:52, Daniel Vetter wrote:
> Am Do, 10.03.2011, 06:06 schrieb Indan Zupancic:
>> This isn't in rc8, can someone make sure it gets into 2.6.38-rc9/2.6.38?
>
> The patch unfortunately broke gen4+ (more precisely: the unwritten abi
> guarantee that userspace can tile buffers that don't have a complete last
> tile row, as long as it promises not to touch it). Hence it got reverted.
> It was just a enforcement check to prevent broken userspace from fooling
> itself. The proper fix is to upgrade your userspace (libdrm +
> xf86-video-intel).
>
> [Aside: This only happens if you have new enough userspace that supports
> relaxed tiling. Old userspace and new kernels are _not_ broken.]

Yes, I noticed it got reverted when digging into git log, but the
commit message only said it broke gen4+, not how gen2 should be
unbroken after the revert.

Which versions fix this, just for reference?

I got libdrm 2.4.23 and xf86-video-intel 2.14.0.

(As a side note, do you have version checking on the kernel side?
If so, you could return 0 for the relaxed fencing feature check
if the driver is the wrong version.)

To keep things manageable I either upgrade the kernel, or userspace,
but never both at once. It seems that that doesn't work with graphics.

Thanks,

Indan




[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-10 Thread Daniel Vetter
Am Do, 10.03.2011, 06:06 schrieb Indan Zupancic:
> This isn't in rc8, can someone make sure it gets into 2.6.38-rc9/2.6.38?

The patch unfortunately broke gen4+ (more precisely: the unwritten abi
guarantee that userspace can tile buffers that don't have a complete last
tile row, as long as it promises not to touch it). Hence it got reverted.
It was just a enforcement check to prevent broken userspace from fooling
itself. The proper fix is to upgrade your userspace (libdrm +
xf86-video-intel).

[Aside: This only happens if you have new enough userspace that supports
relaxed tiling. Old userspace and new kernels are _not_ broken.]

Cheers, Daniel



[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-10 Thread Indan Zupancic
Hi,

On Wed, February 23, 2011 08:10, Daniel Vetter wrote:
> Am Mi, 23.02.2011, 07:59 schrieb Indan Zupancic:
>> On Tue, February 22, 2011 18:25, Daniel Vetter wrote:
>>> It looks like gen2 has a peculiar interleaved 2-row inter-tile
>>> layout. Probably inherited from i81x which had 2kb tiles (which
>>> naturally fit an even-number-of-tile-rows scheme to fit onto 4kb
>>> pages). There is no other mention of this in any docs (also not
>>> in the Intel internal documention according to Chris Wilson).
>>>
>>> Problem manifests itself in corruptions in the second half of the
>>> last tile row (if the bo has an odd number of tiles). Which can
>>> only happen with relaxed tiling (introduced in a00b10c360b35d6431a9).
>>>
>>> So reject set_tiling calls that don't satisfy this constrain to
>>> prevent broken userspace from causing havoc. While at it, also
>>> check the size for newer chipsets.
>>>
>>> LKML: https://lkml.org/lkml/2011/2/19/5
>>> Reported-by: Indan Zupancic 
>>> Signed-off-by: Daniel Vetter 
>>> ---
>>>  drivers/gpu/drm/i915/i915_gem_tiling.c |   16 +++-
>>>  1 files changed, 15 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
>>> b/drivers/gpu/drm/i915/i915_gem_tiling.c
>>> index 22a32b9..79a04fd 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
>>> @@ -184,7 +184,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device
>>> *dev)
>>>  static bool
>>>  i915_tiling_ok(struct drm_device *dev, int stride, int size, int
>>> tiling_mode)
>>>  {
>>> -   int tile_width;
>>> +   int tile_width, tile_height;
>>>
>>> /* Linear is always fine */
>>> if (tiling_mode == I915_TILING_NONE)
>>> @@ -215,6 +215,20 @@ i915_tiling_ok(struct drm_device *dev, int stride,
>>> int size, int
>>> tiling_mode)
>>> }
>>> }
>>>
>>> +   if (IS_GEN2(dev) ||
>>> +   (tiling_mode == I915_TILING_Y && HAS_128_BYTE_Y_TILING(dev)))
>>> +   tile_height = 32;
>>> +   else
>>> +   tile_height = 8;
>>> +   /* i8xx is strange: It has 2 interleaved rows of tiles, so needs an
>>> even
>>> +* number of tile rows. */
>>> +   if (IS_GEN2(dev))
>>> +   tile_height *= 2;
>>> +
>>> +   /* Size needs to be aligned to a full tile row */
>>> +   if (size & (tile_height * stride - 1))
>>> +   return false;
>>> +
>>> /* 965+ just needs multiples of tile width */
>>> if (INTEL_INFO(dev)->gen >= 4) {
>>> if (stride & (tile_width - 1))
>>
>> Tested-by: Indan Zupancic 
>>
>> I tested with this patch and without the other ones you send and the
>> corruption is indeed gone.
>>
>> Not sure why you dropped lkml from CC, now people who stuble upon it
>> don't see the ending...
>
> Random incoherency in my brain. Re-added to cc.

This isn't in rc8, can someone make sure it gets into 2.6.38-rc9/2.6.38?

Thanks,

Indan




Re: [PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-10 Thread Daniel Vetter
Am Do, 10.03.2011, 06:06 schrieb Indan Zupancic:
 This isn't in rc8, can someone make sure it gets into 2.6.38-rc9/2.6.38?

The patch unfortunately broke gen4+ (more precisely: the unwritten abi
guarantee that userspace can tile buffers that don't have a complete last
tile row, as long as it promises not to touch it). Hence it got reverted.
It was just a enforcement check to prevent broken userspace from fooling
itself. The proper fix is to upgrade your userspace (libdrm +
xf86-video-intel).

[Aside: This only happens if you have new enough userspace that supports
relaxed tiling. Old userspace and new kernels are _not_ broken.]

Cheers, Daniel

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-10 Thread Indan Zupancic
On Thu, March 10, 2011 08:52, Daniel Vetter wrote:
 Am Do, 10.03.2011, 06:06 schrieb Indan Zupancic:
 This isn't in rc8, can someone make sure it gets into 2.6.38-rc9/2.6.38?

 The patch unfortunately broke gen4+ (more precisely: the unwritten abi
 guarantee that userspace can tile buffers that don't have a complete last
 tile row, as long as it promises not to touch it). Hence it got reverted.
 It was just a enforcement check to prevent broken userspace from fooling
 itself. The proper fix is to upgrade your userspace (libdrm +
 xf86-video-intel).

 [Aside: This only happens if you have new enough userspace that supports
 relaxed tiling. Old userspace and new kernels are _not_ broken.]

Yes, I noticed it got reverted when digging into git log, but the
commit message only said it broke gen4+, not how gen2 should be
unbroken after the revert.

Which versions fix this, just for reference?

I got libdrm 2.4.23 and xf86-video-intel 2.14.0.

(As a side note, do you have version checking on the kernel side?
If so, you could return 0 for the relaxed fencing feature check
if the driver is the wrong version.)

To keep things manageable I either upgrade the kernel, or userspace,
but never both at once. It seems that that doesn't work with graphics.

Thanks,

Indan


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-10 Thread Daniel Vetter
Am Do, 10.03.2011, 11:36 schrieb Indan Zupancic:
 On Thu, March 10, 2011 08:52, Daniel Vetter wrote:
 [Aside: This only happens if you have new enough userspace that supports
 relaxed tiling. Old userspace and new kernels are _not_ broken.]

 Yes, I noticed it got reverted when digging into git log, but the
 commit message only said it broke gen4+, not how gen2 should be
 unbroken after the revert.

 Which versions fix this, just for reference?

git master branch of libdrm and xf86-video-intel newer than 2011-02-22.

-Daniel

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-03-09 Thread Indan Zupancic
Hi,

On Wed, February 23, 2011 08:10, Daniel Vetter wrote:
 Am Mi, 23.02.2011, 07:59 schrieb Indan Zupancic:
 On Tue, February 22, 2011 18:25, Daniel Vetter wrote:
 It looks like gen2 has a peculiar interleaved 2-row inter-tile
 layout. Probably inherited from i81x which had 2kb tiles (which
 naturally fit an even-number-of-tile-rows scheme to fit onto 4kb
 pages). There is no other mention of this in any docs (also not
 in the Intel internal documention according to Chris Wilson).

 Problem manifests itself in corruptions in the second half of the
 last tile row (if the bo has an odd number of tiles). Which can
 only happen with relaxed tiling (introduced in a00b10c360b35d6431a9).

 So reject set_tiling calls that don't satisfy this constrain to
 prevent broken userspace from causing havoc. While at it, also
 check the size for newer chipsets.

 LKML: https://lkml.org/lkml/2011/2/19/5
 Reported-by: Indan Zupancic in...@nul.nu
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_gem_tiling.c |   16 +++-
  1 files changed, 15 insertions(+), 1 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
 b/drivers/gpu/drm/i915/i915_gem_tiling.c
 index 22a32b9..79a04fd 100644
 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
 +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
 @@ -184,7 +184,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device
 *dev)
  static bool
  i915_tiling_ok(struct drm_device *dev, int stride, int size, int
 tiling_mode)
  {
 -   int tile_width;
 +   int tile_width, tile_height;

 /* Linear is always fine */
 if (tiling_mode == I915_TILING_NONE)
 @@ -215,6 +215,20 @@ i915_tiling_ok(struct drm_device *dev, int stride,
 int size, int
 tiling_mode)
 }
 }

 +   if (IS_GEN2(dev) ||
 +   (tiling_mode == I915_TILING_Y  HAS_128_BYTE_Y_TILING(dev)))
 +   tile_height = 32;
 +   else
 +   tile_height = 8;
 +   /* i8xx is strange: It has 2 interleaved rows of tiles, so needs an
 even
 +* number of tile rows. */
 +   if (IS_GEN2(dev))
 +   tile_height *= 2;
 +
 +   /* Size needs to be aligned to a full tile row */
 +   if (size  (tile_height * stride - 1))
 +   return false;
 +
 /* 965+ just needs multiples of tile width */
 if (INTEL_INFO(dev)-gen = 4) {
 if (stride  (tile_width - 1))

 Tested-by: Indan Zupancic in...@nul.nu

 I tested with this patch and without the other ones you send and the
 corruption is indeed gone.

 Not sure why you dropped lkml from CC, now people who stuble upon it
 don't see the ending...

 Random incoherency in my brain. Re-added to cc.

This isn't in rc8, can someone make sure it gets into 2.6.38-rc9/2.6.38?

Thanks,

Indan


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-02-23 Thread Daniel Vetter
Am Mi, 23.02.2011, 07:59 schrieb Indan Zupancic:
> On Tue, February 22, 2011 18:25, Daniel Vetter wrote:
>> It looks like gen2 has a peculiar interleaved 2-row inter-tile
>> layout. Probably inherited from i81x which had 2kb tiles (which
>> naturally fit an even-number-of-tile-rows scheme to fit onto 4kb
>> pages). There is no other mention of this in any docs (also not
>> in the Intel internal documention according to Chris Wilson).
>>
>> Problem manifests itself in corruptions in the second half of the
>> last tile row (if the bo has an odd number of tiles). Which can
>> only happen with relaxed tiling (introduced in a00b10c360b35d6431a9).
>>
>> So reject set_tiling calls that don't satisfy this constrain to
>> prevent broken userspace from causing havoc. While at it, also
>> check the size for newer chipsets.
>>
>> LKML: https://lkml.org/lkml/2011/2/19/5
>> Reported-by: Indan Zupancic 
>> Signed-off-by: Daniel Vetter 
>> ---
>>  drivers/gpu/drm/i915/i915_gem_tiling.c |   16 +++-
>>  1 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
>> b/drivers/gpu/drm/i915/i915_gem_tiling.c
>> index 22a32b9..79a04fd 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
>> @@ -184,7 +184,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device
>> *dev)
>>  static bool
>>  i915_tiling_ok(struct drm_device *dev, int stride, int size, int
>> tiling_mode)
>>  {
>> -int tile_width;
>> +int tile_width, tile_height;
>>
>>  /* Linear is always fine */
>>  if (tiling_mode == I915_TILING_NONE)
>> @@ -215,6 +215,20 @@ i915_tiling_ok(struct drm_device *dev, int stride,
>> int size, int
>> tiling_mode)
>>  }
>>  }
>>
>> +if (IS_GEN2(dev) ||
>> +(tiling_mode == I915_TILING_Y && HAS_128_BYTE_Y_TILING(dev)))
>> +tile_height = 32;
>> +else
>> +tile_height = 8;
>> +/* i8xx is strange: It has 2 interleaved rows of tiles, so needs an
>> even
>> + * number of tile rows. */
>> +if (IS_GEN2(dev))
>> +tile_height *= 2;
>> +
>> +/* Size needs to be aligned to a full tile row */
>> +if (size & (tile_height * stride - 1))
>> +return false;
>> +
>>  /* 965+ just needs multiples of tile width */
>>  if (INTEL_INFO(dev)->gen >= 4) {
>>  if (stride & (tile_width - 1))
>
> Tested-by: Indan Zupancic 
>
> I tested with this patch and without the other ones you send and the
> corruption
> is indeed gone.
>
> Not sure why you dropped lkml from CC, now people who stuble upon it don't
> see
> the ending...

Random incoherency in my brain. Re-added to cc.
- Daniel



[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-02-23 Thread Indan Zupancic
Hi,

On Tue, February 22, 2011 18:25, Daniel Vetter wrote:
> It looks like gen2 has a peculiar interleaved 2-row inter-tile
> layout. Probably inherited from i81x which had 2kb tiles (which
> naturally fit an even-number-of-tile-rows scheme to fit onto 4kb
> pages). There is no other mention of this in any docs (also not
> in the Intel internal documention according to Chris Wilson).
>
> Problem manifests itself in corruptions in the second half of the
> last tile row (if the bo has an odd number of tiles). Which can
> only happen with relaxed tiling (introduced in a00b10c360b35d6431a9).
>
> So reject set_tiling calls that don't satisfy this constrain to
> prevent broken userspace from causing havoc. While at it, also
> check the size for newer chipsets.
>
> LKML: https://lkml.org/lkml/2011/2/19/5
> Reported-by: Indan Zupancic 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/i915_gem_tiling.c |   16 +++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
> b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 22a32b9..79a04fd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -184,7 +184,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
>  static bool
>  i915_tiling_ok(struct drm_device *dev, int stride, int size, int tiling_mode)
>  {
> - int tile_width;
> + int tile_width, tile_height;
>
>   /* Linear is always fine */
>   if (tiling_mode == I915_TILING_NONE)
> @@ -215,6 +215,20 @@ i915_tiling_ok(struct drm_device *dev, int stride, int 
> size, int
> tiling_mode)
>   }
>   }
>
> + if (IS_GEN2(dev) ||
> + (tiling_mode == I915_TILING_Y && HAS_128_BYTE_Y_TILING(dev)))
> + tile_height = 32;
> + else
> + tile_height = 8;
> + /* i8xx is strange: It has 2 interleaved rows of tiles, so needs an even
> +  * number of tile rows. */
> + if (IS_GEN2(dev))
> + tile_height *= 2;
> +
> + /* Size needs to be aligned to a full tile row */
> + if (size & (tile_height * stride - 1))
> + return false;
> +
>   /* 965+ just needs multiples of tile width */
>   if (INTEL_INFO(dev)->gen >= 4) {
>   if (stride & (tile_width - 1))

Tested-by: Indan Zupancic 

I tested with this patch and without the other ones you send and the corruption
is indeed gone.

Not sure why you dropped lkml from CC, now people who stuble upon it don't see
the ending...

Greetings,

Indan




[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-02-22 Thread Daniel Vetter
On Tue, Feb 22, 2011 at 06:32:11PM +0100, Thierry Vignaud wrote:
> Giving that lack of documentation, could you put some more comments in the 
> code?
> so that nobody cleans out that "strange workaround" in 6 monthes...

It's not a workaround, it's how the hw works. If we'd add the explanation
from the changelog everytime there's a little and/or badly documented hw
oddity, you won't be able to read the code anymore.

Use git blame and dig into the history - drm/* is full of such stuff.
-Daniel
-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48


[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-02-22 Thread Thierry Vignaud
On 22 February 2011 18:25, Daniel Vetter  wrote:
> It looks like gen2 has a peculiar interleaved 2-row inter-tile
> layout. Probably inherited from i81x which had 2kb tiles (which
> naturally fit an even-number-of-tile-rows scheme to fit onto 4kb
> pages). There is no other mention of this in any docs (also not
> in the Intel internal documention according to Chris Wilson).

Giving that lack of documentation, could you put some more comments in the code?
so that nobody cleans out that "strange workaround" in 6 monthes...

> @@ -215,6 +215,20 @@ i915_tiling_ok(struct drm_device *dev, int stride, int 
> size, int tiling_mode)
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
>
> + ? ? ? if (IS_GEN2(dev) ||
> + ? ? ? ? ? (tiling_mode == I915_TILING_Y && HAS_128_BYTE_Y_TILING(dev)))
> + ? ? ? ? ? ? ? tile_height = 32;
> + ? ? ? else
> + ? ? ? ? ? ? ? tile_height = 8;
> + ? ? ? /* i8xx is strange: It has 2 interleaved rows of tiles, so needs an 
> even
> + ? ? ? ?* number of tile rows. */
> + ? ? ? if (IS_GEN2(dev))
> + ? ? ? ? ? ? ? tile_height *= 2;
> +
> + ? ? ? /* Size needs to be aligned to a full tile row */
> + ? ? ? if (size & (tile_height * stride - 1))
> + ? ? ? ? ? ? ? return false;
> +
> ? ? ? ?/* 965+ just needs multiples of tile width */
> ? ? ? ?if (INTEL_INFO(dev)->gen >= 4) {
> ? ? ? ? ? ? ? ?if (stride & (tile_width - 1))


[PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-02-22 Thread Daniel Vetter
It looks like gen2 has a peculiar interleaved 2-row inter-tile
layout. Probably inherited from i81x which had 2kb tiles (which
naturally fit an even-number-of-tile-rows scheme to fit onto 4kb
pages). There is no other mention of this in any docs (also not
in the Intel internal documention according to Chris Wilson).

Problem manifests itself in corruptions in the second half of the
last tile row (if the bo has an odd number of tiles). Which can
only happen with relaxed tiling (introduced in a00b10c360b35d6431a9).

So reject set_tiling calls that don't satisfy this constrain to
prevent broken userspace from causing havoc. While at it, also
check the size for newer chipsets.

LKML: https://lkml.org/lkml/2011/2/19/5
Reported-by: Indan Zupancic 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem_tiling.c |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c 
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 22a32b9..79a04fd 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -184,7 +184,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
 static bool
 i915_tiling_ok(struct drm_device *dev, int stride, int size, int tiling_mode)
 {
-   int tile_width;
+   int tile_width, tile_height;

/* Linear is always fine */
if (tiling_mode == I915_TILING_NONE)
@@ -215,6 +215,20 @@ i915_tiling_ok(struct drm_device *dev, int stride, int 
size, int tiling_mode)
}
}

+   if (IS_GEN2(dev) ||
+   (tiling_mode == I915_TILING_Y && HAS_128_BYTE_Y_TILING(dev)))
+   tile_height = 32;
+   else
+   tile_height = 8;
+   /* i8xx is strange: It has 2 interleaved rows of tiles, so needs an even
+* number of tile rows. */
+   if (IS_GEN2(dev))
+   tile_height *= 2;
+
+   /* Size needs to be aligned to a full tile row */
+   if (size & (tile_height * stride - 1))
+   return false;
+
/* 965+ just needs multiples of tile width */
if (INTEL_INFO(dev)->gen >= 4) {
if (stride & (tile_width - 1))
-- 
1.7.4.1



Re: [PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-02-22 Thread Indan Zupancic
Hi,

On Tue, February 22, 2011 18:25, Daniel Vetter wrote:
 It looks like gen2 has a peculiar interleaved 2-row inter-tile
 layout. Probably inherited from i81x which had 2kb tiles (which
 naturally fit an even-number-of-tile-rows scheme to fit onto 4kb
 pages). There is no other mention of this in any docs (also not
 in the Intel internal documention according to Chris Wilson).

 Problem manifests itself in corruptions in the second half of the
 last tile row (if the bo has an odd number of tiles). Which can
 only happen with relaxed tiling (introduced in a00b10c360b35d6431a9).

 So reject set_tiling calls that don't satisfy this constrain to
 prevent broken userspace from causing havoc. While at it, also
 check the size for newer chipsets.

 LKML: https://lkml.org/lkml/2011/2/19/5
 Reported-by: Indan Zupancic in...@nul.nu
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_gem_tiling.c |   16 +++-
  1 files changed, 15 insertions(+), 1 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
 b/drivers/gpu/drm/i915/i915_gem_tiling.c
 index 22a32b9..79a04fd 100644
 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
 +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
 @@ -184,7 +184,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
  static bool
  i915_tiling_ok(struct drm_device *dev, int stride, int size, int tiling_mode)
  {
 - int tile_width;
 + int tile_width, tile_height;

   /* Linear is always fine */
   if (tiling_mode == I915_TILING_NONE)
 @@ -215,6 +215,20 @@ i915_tiling_ok(struct drm_device *dev, int stride, int 
 size, int
 tiling_mode)
   }
   }

 + if (IS_GEN2(dev) ||
 + (tiling_mode == I915_TILING_Y  HAS_128_BYTE_Y_TILING(dev)))
 + tile_height = 32;
 + else
 + tile_height = 8;
 + /* i8xx is strange: It has 2 interleaved rows of tiles, so needs an even
 +  * number of tile rows. */
 + if (IS_GEN2(dev))
 + tile_height *= 2;
 +
 + /* Size needs to be aligned to a full tile row */
 + if (size  (tile_height * stride - 1))
 + return false;
 +
   /* 965+ just needs multiples of tile width */
   if (INTEL_INFO(dev)-gen = 4) {
   if (stride  (tile_width - 1))

Tested-by: Indan Zupancic in...@nul.nu

I tested with this patch and without the other ones you send and the corruption
is indeed gone.

Not sure why you dropped lkml from CC, now people who stuble upon it don't see
the ending...

Greetings,

Indan


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915: fix corruptions on i8xx due to relaxed fencing

2011-02-22 Thread Daniel Vetter
Am Mi, 23.02.2011, 07:59 schrieb Indan Zupancic:
 On Tue, February 22, 2011 18:25, Daniel Vetter wrote:
 It looks like gen2 has a peculiar interleaved 2-row inter-tile
 layout. Probably inherited from i81x which had 2kb tiles (which
 naturally fit an even-number-of-tile-rows scheme to fit onto 4kb
 pages). There is no other mention of this in any docs (also not
 in the Intel internal documention according to Chris Wilson).

 Problem manifests itself in corruptions in the second half of the
 last tile row (if the bo has an odd number of tiles). Which can
 only happen with relaxed tiling (introduced in a00b10c360b35d6431a9).

 So reject set_tiling calls that don't satisfy this constrain to
 prevent broken userspace from causing havoc. While at it, also
 check the size for newer chipsets.

 LKML: https://lkml.org/lkml/2011/2/19/5
 Reported-by: Indan Zupancic in...@nul.nu
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_gem_tiling.c |   16 +++-
  1 files changed, 15 insertions(+), 1 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
 b/drivers/gpu/drm/i915/i915_gem_tiling.c
 index 22a32b9..79a04fd 100644
 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
 +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
 @@ -184,7 +184,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device
 *dev)
  static bool
  i915_tiling_ok(struct drm_device *dev, int stride, int size, int
 tiling_mode)
  {
 -int tile_width;
 +int tile_width, tile_height;

  /* Linear is always fine */
  if (tiling_mode == I915_TILING_NONE)
 @@ -215,6 +215,20 @@ i915_tiling_ok(struct drm_device *dev, int stride,
 int size, int
 tiling_mode)
  }
  }

 +if (IS_GEN2(dev) ||
 +(tiling_mode == I915_TILING_Y  HAS_128_BYTE_Y_TILING(dev)))
 +tile_height = 32;
 +else
 +tile_height = 8;
 +/* i8xx is strange: It has 2 interleaved rows of tiles, so needs an
 even
 + * number of tile rows. */
 +if (IS_GEN2(dev))
 +tile_height *= 2;
 +
 +/* Size needs to be aligned to a full tile row */
 +if (size  (tile_height * stride - 1))
 +return false;
 +
  /* 965+ just needs multiples of tile width */
  if (INTEL_INFO(dev)-gen = 4) {
  if (stride  (tile_width - 1))

 Tested-by: Indan Zupancic in...@nul.nu

 I tested with this patch and without the other ones you send and the
 corruption
 is indeed gone.

 Not sure why you dropped lkml from CC, now people who stuble upon it don't
 see
 the ending...

Random incoherency in my brain. Re-added to cc.
- Daniel

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel