Re: [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

2014-08-04 Thread Eric Anholt 
Keith Packard kei...@keithp.com writes:

 Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 I don't see anything indicating that this code path is only used by
 glamor.

 True. It's a fix for DRI3 for either UXA or none. Mesa allocates a
 single page for a 1x1 texture, but this code thinks that should take two
 pages causing a texture-to-pixmap operation to fail.

OK, but isn't the problem with the code you're #if 0ing (which, really?
#if 0 in a patch being submitted for review?) that it's aligning to
2*height instead of height?


pgp8Ngnh1wzPp.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

2014-08-04 Thread Keith Packard 
Eric Anholt e...@anholt.net writes:

 OK, but isn't the problem with the code you're #if 0ing (which, really?
 #if 0 in a patch being submitted for review?) that it's aligning to
 2*height instead of height?

I went and did a bit of archaeology to figure out why it's using
2*height instead of just height. The initial change to pixmap
allocation, which was then copied to the BO validation logic, was here:

commit 736b89504a32239a0c7dfb5961c1b8292dd744bd
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Sun Dec 30 10:32:18 2012 +

uxa: Align surface allocations to even tile rows

Align surface sizes to an even number of tile rows to cater for 
sampler
prefetch. If we read beyond the last page we may catch the PTE in a
state of flux and trigger a GPU hang. Also detected by enabling 
invalid
PTE access checking.

References: https://bugs.freedesktop.org/show_bug.cgi?id=56916
References: https://bugs.freedesktop.org/show_bug.cgi?id=55984
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

diff --git a/src/intel_uxa.c b/src/intel_uxa.c
index f5ac0a6..2f14173 100644
--- a/src/intel_uxa.c
+++ b/src/intel_uxa.c
@@ -209,7 +209,7 @@ intel_uxa_pixmap_compute_size(PixmapPtr pixmap,
tile_height = 8;
else
tile_height = 32;
-   aligned_h = ALIGN(h, tile_height);
+   aligned_h = ALIGN(h, 2*tile_height);
 
*stride = intel_get_fence_pitch(intel,
ALIGN(pitch, 512),

Look at the referenced bugs, what I found was a long list of random GPU
hangs on ILK and SNB hardware that appear to have been caused by a
kernel change. Daniel and Chris created a number of scatter shot fixes
across the kernel, and this patch to the 2D driver. However, this patch
doesn't appear to have actually solved anything; Norbert Preining was
still crashing with this patch applied:

https://bugs.freedesktop.org/show_bug.cgi?id=55984#c129

Furthermore, SNA has some similar code, but it applies it conditionally
for hardware which doesn't have 'relaxed fencing', which is only
hardware older than i965 when running on a older kernel that doesn't
recognize the HAS_RELAXED_FENCING parameter (the current kernel always
returns 'true').

So, as near as I can tell, this fix should never be necessary as the
reported bug wasn't fixed by it, and SNA does not apply this rule to any
hardware on which either bug was reproduced.

If this fix is actually useful, wouldn't we want it in Mesa as well?

-- 
keith.pack...@intel.com


pgp4Ev3h5OZTD.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

2014-07-30 Thread Eric Anholt 
Keith Packard kei...@keithp.com writes:

I don't see anything indicating that this code path is only used by
glamor.

 ---
  src/uxa/intel_uxa.c | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/src/uxa/intel_uxa.c b/src/uxa/intel_uxa.c
 index b396188..717754f 100644
 --- a/src/uxa/intel_uxa.c
 +++ b/src/uxa/intel_uxa.c
 @@ -758,6 +758,7 @@ free_priv:
   goto free_priv;
   }
  
 +#if 0
   if (tiling != I915_TILING_NONE) {
   int height;
  
 @@ -780,6 +781,7 @@ free_priv:
   bo = NULL;
   goto free_priv;
   }
 +#endif
   }
  
BAIL:
 -- 
 2.0.1

 ___
 xorg-devel@lists.x.org: X.Org development
 Archives: http://lists.x.org/archives/xorg-devel
 Info: http://lists.x.org/mailman/listinfo/xorg-devel


pgpH9FiFdOcCs.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 01/12] Stop trying to out-guess mesa for BO allocation

2014-07-30 Thread Keith Packard 
Eric Anholt e...@anholt.net writes:

 Keith Packard kei...@keithp.com writes:

 I don't see anything indicating that this code path is only used by
 glamor.

True. It's a fix for DRI3 for either UXA or none. Mesa allocates a
single page for a 1x1 texture, but this code thinks that should take two
pages causing a texture-to-pixmap operation to fail.

In particular, gnome-shell fails without this patch.

It's useful independent of this series, which is why I stuck it first.

-- 
keith.pack...@intel.com


pgp8xiWTC4dRV.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 01/12] Stop trying to out-guess mesa for BO allocation

2014-07-24 Thread Keith Packard 
---
 src/uxa/intel_uxa.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/uxa/intel_uxa.c b/src/uxa/intel_uxa.c
index b396188..717754f 100644
--- a/src/uxa/intel_uxa.c
+++ b/src/uxa/intel_uxa.c
@@ -758,6 +758,7 @@ free_priv:
goto free_priv;
}
 
+#if 0
if (tiling != I915_TILING_NONE) {
int height;
 
@@ -780,6 +781,7 @@ free_priv:
bo = NULL;
goto free_priv;
}
+#endif
}
 
   BAIL:
-- 
2.0.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel