Re: [Intel-gfx] [PATCH 0/2 xf86-video-intel] Two DRI3/Present bug fixes for UXA

[Keith Packard <keithp-aN4HjG94KOLQT0dZR+AlfA@public.gmane.org>]

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

Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org> writes:

> That extra alignment is due to gen2 and early gen3 (if
> (!intel->has_relaxed_fencing) covers them).

Here's the patch which changed the alignment requirment:

        commit 736b89504a32239a0c7dfb5961c1b8292dd744bd
        Author: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
        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-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>

Both of these bugs report regressions found past the 3.6 kernel, one on
965gm and the other on Ironlake. Are there additional bug reports on UXA
which actually relate to this patch as it affects gen2 and gen3
hardware?

Here's the patch that added the additional alignment restriction to SNA:

        commit 1b6c1a30723b1d13e9bd3df0b59a8d75639c89be
        Author: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
        Date:   Fri Nov 30 09:27:57 2012 +0000

            sna: Increase tiling alignment to an even tile

            Seems to help g4x.

            Signed-off-by: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>

Note that this does not reference gen2 or gen3 either.

From the above two patches, all that I can learn is that both of these
larger alignments were introduced to fix bugs on newer hardware, and
that the larger alignment is now specifically disabled on that same
hardware in the SNA code. Reading only this history, I felt reasonably
confident that changing UXA back to what libdrm does was the best plan.

If you have specific bug reports that were resolved by this patch, or
specific hardware documentation which indicates that this patch is
required, especially as it relates to gen2 and gen3 hardware, I'd love
to see them.

In any case, we've now got four versions of the pixmap alignment code
(libdrm, uxa and sna in two varieties). They're all subtly different;
one suspects that each one works on some set of problems and fails on
others...

Here's what the height alignment requirements are:

                libdrm  uxa     uxa     sna     sna
                      +keithp        >=2.6.38 <2.6.38

gen2 none         2      2       2       1       2
gen3 none         2      2       2       1       2
gen4+ none        2      2       2       1       1

gen2 X           16     16      32      16      32
gen3 X            8      8      16       8      16
gen4+ X           8      8      16       8       8

gen2 Y           16     16      32      16      32
i915 Y            8     32      64       8      16
i945 Y           32     32      64       8      16
gen4+ Y          32     32      64      32      32

It looks like the SNA alignment for untiled buffers is incorrect? 965
hardware is documented to read buffers in 2x2 chunks, so a failure to
height align allocations to 2 can result in reads off the end of the
buffer.

For uxa's intel_set_pixmap_bo, and sna's sna_dri3_pixmap_from_fd,
there's a clear requirement that the 2D driver impose no stricter
alignment than libdrm, so that, buffer passing from Mesa to X will work.

For pixmap allocations within the X server, we should ensure that the
alignment requirements are at least as strict as those in libdrm so that
pixmap passing from X to Mesa will work. Not that Mesa actually checks
the provided buffer size at all, although it probably should.x

It seems obvious to me that we should be doing this work in one place
and sharing the code across the 2D and 3D drivers, and yet we never
have.

-- 
keith.packard-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org

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

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

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel