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

[Keith Packard <keithp@keithp.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2707 bytes --]

Eric Anholt <eric@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@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@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@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 810 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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