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

[Keith Packard]

Eric Anholt <eric at 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 <chris at chris-wilson.co.uk>
	Date:   Sun Dec 30 10:32:18 2012 +0000
	
	    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 <chris at 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.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20140804/a2fed968/attachment.sig>