Bug 26686 - Some textures are distorted with libdrm 2.4.18 in GTAVC&GTA3
Some textures are distorted with libdrm 2.4.18 in GTAVC&GTA3
Product: DRI
Classification: Unclassified
Component: libdrm
x86 (IA32) Linux (All)
: medium normal
Assigned To: Default DRI bug account
Depends on:
  Show dependency treegraph
Reported: 2010-02-21 12:10 UTC by Ruslan Kabatsayev
Modified: 2010-05-24 10:44 UTC (History)
1 user (show)

See Also:

GTA3 screenshot (268.05 KB, image/jpeg)
2010-02-21 12:11 UTC, Ruslan Kabatsayev
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ruslan Kabatsayev 2010-02-21 12:10:31 UTC
I just upgraded from libdrm 2.4.17 to 2.4.18, and now some textures in GTA3 & GTAVC (in WINE) are distorted as in attached screenshots.


$ lspci|grep VGA
00:02.0 VGA compatible controller: Intel Corporation 82915G/GV/910GL Integrated Graphics Controller (rev 04)
Comment 1 Ruslan Kabatsayev 2010-02-21 12:11:27 UTC
Created attachment 33475 [details]
GTA3 screenshot
Comment 2 Pauli 2010-02-21 13:45:35 UTC
Can you bisect the commit that broke it for you?

Comment 3 Ruslan Kabatsayev 2010-02-21 14:06:28 UTC
(In reply to comment #2)
> Can you bisect the commit that broke it for you?

OK, i did it using WINE bisect tutorial, and this is the final output:

4f0f871730b76730ca58209181d16725b0c40184 is first bad commit
commit 4f0f871730b76730ca58209181d16725b0c40184
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Feb 10 09:45:13 2010 +0000

    intel: Handle resetting of input params after EINTR during SET_TILING
    The SET_TILING is pernicious in that it overwrites the input arguments
    following an error in order to report the current tiling state of the
    buffer. This caught us by surprise as we then fed those arguments back
    into to the ioctl unmodified following an EINTR and so the kernel then
    reported success for the no-op. We interpreted this success as meaning
    that the tiling on the buffer had changed so updated our state and
    started using the buffer incorrectly in the new tiled/untiled manner.
    This lead to all sorts of random corruption and GPU hangs, even though
    the batch buffers would look sane (when the GPU had not wandered off
    into forbidden territory).
      Bug 25475 - [i915] Xorg crash / Execbuf while wedged
      Bug 25554 - i830_uxa_prepare_access: gtt bo map failed: Input/output error
    (And probably every other weird bug in the last few months.)
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

:040000 040000 0af4c96adb20514daff1251ec9fb5b542e8092f5 411ace5da42864017f0132efeb47c96778eca642 M      intel
Comment 4 Chris Wilson 2010-02-22 02:09:34 UTC
Extremely unlikely to be a direct result of that commit.
Comment 5 Ruslan Kabatsayev 2010-02-22 07:17:22 UTC
I tried to comment out this line:

bo_gem->tiling_mode = set_tiling.tiling_mode;

and now things seem to work. I.e., this statement:

>it overwrites the input arguments following an error in
>order to report the current tiling state of the buffer.

should be false (or, at least, not quite true). This is proved by the fact that if i undo this part of commit:

-	if (ret != 0) {
-		*tiling_mode = bo_gem->tiling_mode;
-		return -errno;
-	}

, i.e. leave 'if' statement in place (and uncomment saving of current tiling_mode), then things also work, because tiling_mode is not saved on error now. So, i can conclude that sometimes the kernel doesn't report current tiling state correctly on error.
Comment 6 Ruslan Kabatsayev 2010-02-22 08:10:42 UTC
I added perror() before 'return -errno;' to see what error doesn't report tiling mode, and got "Invalid argument", i.e. EINVAL. Then i found this in i915_gem_set_tiling() in the kernel:

if (!i915_tiling_ok(dev, args->stride, obj->size, args->tiling_mode)) {
        return -EINVAL;

As you can see, it doesn't change tiling_mode.

So, there should be a check for error after ioctl(), and tiling_mode shouldn't be considered current.
Comment 7 Ruslan Kabatsayev 2010-05-24 10:29:52 UTC
Still present with libdrm 2.4.20.
Comment 8 Chris Wilson 2010-05-24 10:42:56 UTC
commit fcf3e616eeeb289f96af1436d809f0a1a42bebb7
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon May 24 18:35:41 2010 +0100

    intel: Don't change tiling mode unless the kernel reports success.
      Bug 26686 - Some textures are distorted with libdrm 2.4.18 in GTAVC&GTA3
    This bug continues to haunt me. The kernel SET_TILING ioctl is
    inconsistent in its return values when reporting an error. If one of its
    sanity checks fail, then the input values are left unchanged. If the
    kernel later fails to change the tiling mode, then the input values are
    modified to match the current tiling on the object. In short, userspace
    cannot trust the return values upon error and so we must assume that
    upon error our current tiling mode matches reality and not update.
Comment 9 Chris Wilson 2010-05-24 10:44:27 UTC
Ruslan, it would be invaluable if you could add an assert(ret == 0); and track down the source of the EINVAL.