Bug 34541

Summary: [ilk, wine] massive render corruption after recent patchset
Product: DRI Reporter: Tobias Jakobi <liquid.acid>
Component: DRM/IntelAssignee: Chris Wilson <chris>
Status: CLOSED FIXED QA Contact:
Severity: major    
Priority: medium CC: chris, jbarnes, Magnus.Kessler
Version: DRI git   
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
i915 platform: i915 features:

Description Tobias Jakobi 2011-02-21 10:23:04 UTC
Intel Corporation Arrandale Integrated Graphics Controller (rev 02)
libdrm, mesa, xf86-video-intel : git master tip

Just updated my git, which effectively pulled in the massive patchset by Chris Wilson, and now I experience massive render corruptions.

Especially wine seems to be affected. Some demos which show the problems:
Max Payne 2 (http://www.rockstargames.com/maxpayne2/mp2_downloads.html)
Splinter Cell (ftp://ftp.ubi.com/de/splintercell/splintercelldemo_english_us.exe)
FEAR (http://www.gamershell.com/download_10167.shtml)

Splinter Cell should even crash, which it didn't before the patchset. Max Payne 2 especially shows that vertex translation is somehow not working correctly.

My current wine version is 1.3.13, but I doubt the version matters that much.

Comment 1 Chris Wilson 2011-02-21 11:05:05 UTC
Obviously I didn't spot this in my testing, but the reason for it being a large patch set is to make bisection much easier...
Comment 2 Chris Wilson 2011-02-21 11:43:10 UTC
Found some under ut2004-demo, bisected to

commit 8a9e67b8df9836408270a4bc3660ec45b622ae56
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Feb 10 00:25:17 2011 +0000

    intel: Buffered upload
Comment 3 Chris Wilson 2011-02-21 12:59:13 UTC
This fixes ut2004-demo:

commit 5a1fbf0f70a1c2d444f61494f86e26ca866c31d5
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Feb 21 20:56:06 2011 +0000

    intel: Fix insufficient integer width for upload buffer offset
    I was being overly miserly and gave the offset of the buffer into the bo
    insufficient bits, distracted by the adjacency of the buffer[4096].
    Ref: https://bugs.freedesktop.org/show_bug.cgi?id=34541
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Does it do the trick for your wine apps?
Comment 4 Magnus Kessler 2011-02-21 14:23:54 UTC
The latest patch solves some of the corruption issues I was seeing with flightgear. However several issues still remain there and also in celestia.

In flightgear, with the standard Cessna c172p model, the normally semi-transparent spinning propeller disk is now opaque, and there are also some artefacts around solid objects.

In celestia, when zooming in on e.g. Jupiter or Saturn, at a close enough distance random coloured triangles start to flash.
Comment 5 Tobias Jakobi 2011-02-21 14:25:49 UTC
Not exactly. In fact I don't see any change to the errors with your patch.

I also went back to 8a9e67b8df9836408270a4bc3660ec45b622ae56 and checked there. I still get corruptions with this specific commit, but they're kinda small compared to the current state. Only some 'spiked' polygons here and there in Max Payne 2. FEAR is more affected, but it still looks better than with git master tip.

Going to check if applying 5a1fbf0f70a1c2d444f61494f86e26ca866c31d5 on top of 8a9e67b8df9836408270a4bc3660ec45b622ae56 is removing these 'minor' issues.

So probably we're seeing a multitude of separate issues here.
Comment 6 Tobias Jakobi 2011-02-21 14:33:54 UTC
(In reply to comment #5)
> Going to check if applying 5a1fbf0f70a1c2d444f61494f86e26ca866c31d5 on top of
> 8a9e67b8df9836408270a4bc3660ec45b622ae56 is removing these 'minor' issues.
This worked, the visuals are back to how they were before the patchset.

So the remaining problems are between f29606598e7703d830440a878673d98e7ce13218 and 41260a9bf63aa61f88f188053f1ed4dba3a852d2.
Comment 7 Tobias Jakobi 2011-02-21 15:40:35 UTC
The massive corruptions I noticed are introduced by this commit:

From c625aa19cb53ed27f91bfd16fea6ea727e9a5bbd Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Fri, 18 Feb 2011 10:37:43 +0000
Subject: intel: extend current vertex buffers

If the next vertex arrays are a (discontiguous) continuation of the
current arrays, such that the new vertices are simply offset from the
start of the current vertex buffer definitions we can reuse those
defintions and avoid the overhead of relocations and invalidations.
Comment 8 Chris Wilson 2011-02-21 16:01:14 UTC
Thanks Tobias, Eric just found a simple reproducer for it.
Comment 9 Tobias Jakobi 2011-02-22 01:09:32 UTC
Thanks Chris! Thanks Eric!

Looks like this commit fixed at least the render corruptions:

From 9e872a5865c66ed0a518dd1c6c54e72f3afa71f1 Mon Sep 17 00:00:00 2001
From: Eric Anholt <eric@anholt.net>
Date: Tue, 22 Feb 2011 00:24:41 +0000
Subject: i965: Fix VB packet reuse when offset for the new buffer isn't stride aligned.

Fixes regression in scissor-stencil-clear and 5 other tests.

I'm still getting the crash in Splinter Cell, but maybe that's unrelated. Going to do some more tests. If it turns out that the patchset introduces another problem, I'm going to open a new bug for that. For now this one looks FIXED!

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.