Bug 86747

Created attachment 110078 [details]
Screenshot of FM issue

Created attachment 110079 [details]
How it should look like

Still happens with the latest Mesa.  Valgrind doesn't show anything.  

INTEL_DEBUG=perf gives following kind of warnings (for apitrace trace) while this issue happens:
6202978: message: api performance issue 8: Stalling on the GPU for mapping a busy buffer object
6303147: message: api performance issue 9: Flushing before mapping a referenced bo.

Before the issue, there were a lot of warnings like:
6184232: message: api performance issue 3: CPU mapping a busy miptree BO stalled and took 0.052 ms.
6184249: message: major api performance issue 4: glGenerateMipmap() with MESA_FORMAT_RGBA_DXT3 format
6184249: message: api performance issue 3: CPU mapping a busy miptree BO stalled and took 0.054 ms
6189756: message: major api performance issue 4: glGenerateMipmap() with MESA_FORMAT_RGBA_DXT5 format

Texture targets / formats used by FM2014:
- GL_TEXTURE_2D / GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
  - max levels: 10
  - max size: 2048 kB (2048x2048)
- GL_TEXTURE_2D / GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
  - max levels: 9
  - max size: 4096 kB (2048x2048)
- GL_TEXTURE_2D / GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
  - max levels: 9
  - max size: 4096 kB (2048x2048)
- GL_TEXTURE_2D / GL_LUMINANCE8:
  - max levels: 0
  - max size: 768 kB (1024x768 GL_UNSIGNED_BYTE)
- GL_TEXTURE_2D / GL_R32F:
  - max levels: 0
  - max size: 32400 kB (3840x2160 GL_FLOAT)
- GL_TEXTURE_2D / GL_RG16F:
  - max levels: 0
  - max size: 4096 kB (1024x1024 GL_HALF_FLOAT)
- GL_TEXTURE_2D / GL_RGB8:
  - max levels: 0
  - max size: 32400 kB (3840x2160 GL_UNSIGNED_BYTE)
- GL_TEXTURE_2D / GL_RGBA8:
  - max levels: 0
  - max size: 32768 kB (4096x2048 GL_UNSIGNED_BYTE)

As to the minimum texture sizes, FM uses high mipmap levels even with small textures:
...
glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 7, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 4, height = 4, border = 0, imageSize = 8, data = blob(8))
glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 8, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 2, height = 2, border = 0, imageSize = 8, data = blob(8))
glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 9, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 1, height = 1, border = 0, imageSize = 8, data = blob(8))

DXT kernel is 4x4, so mipmap levels with 1x1 or 2x2 pixels seems a bit odd.

Tapani pointed at bug 76919 where he commented that:
---------------
EXT_texture_compression_s3tc specification states in "Implementation Note":

"If the width or height is not a multiple of four, there will be 4x4 blocks at the edge of the image that contain "extra" texels that are not part of the image."
---------------

When looking more at FM DXT texture sizes, at level 0, all are multiples of four.  Higher mipmap levels are not, some aren't even square, so there are things like these...

16 x 128:

6184235 glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 3, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 2, height = 16, border = 0, imageSize = 32, data = blob(32))
6184236 glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 4, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 1, height = 8, border = 0, imageSize = 16, data = blob(16))
6184237 glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 5, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 1, height = 4, border = 0, imageSize = 8, data = blob(8))
6184238 glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 6, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 1, height = 2, border = 0, imageSize = 8, data = blob(8))

256 x 32:

6184472 glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 4, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 16, height = 2, border = 0, imageSize = 32, data = blob(32))
6184473 glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 5, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 8, height = 1, border = 0, imageSize = 16, data = blob(16))
6184474 glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 6, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 4, height = 1, border = 0, imageSize = 8, data = blob(8))
6184475 glCompressedTexImage2DARB(target = GL_TEXTURE_2D, level = 7, internalformat = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, width = 2, height = 1, border = 0, imageSize = 8, data = blob(8))

Tapani tested trace of this on AMD with fglrx driver and trace looks fine there.

The effect looks similar to old & fixed bug 78648 with KSP.  KSP has also non-square DXT1 & DXT5 textures with very high mip levels (width=8/height=1, width=1/height=4), but I don't see similar texture issue in it now.

(In reply to Eero Tamminen from comment #0)
> - No noise in textures, same as with default Mesa 10.3 in Ubuntu 14.10

So... it used to look correct with 10.3, but somewhere in 10.4 it changed?  If that's the case, could you bisect?

Bisect gave this as the commit breaking FM 2014:
------------------------------------------------
Author:     Jason Ekstrand <jason.ekstrand@intel.com>
AuthorDate: Fri Sep 12 16:17:37 2014 -0700
Commit:     Jason Ekstrand <jason.ekstrand@intel.com>
CommitDate: Tue Sep 30 10:29:15 2014 -0700

    i965/fs: Use the GRF for FB writes on gen >= 7
    
       On gen 7, the MRF was removed and we gained the ability to do send
       instructions directly from the GRF.  This commit enables that
       functinoality for FB writes.
    
       v2: Make handling of components more sane.
    
    i965/fs: Force a high register for the final FB write
    
       v2: Renamed the array for the range mappings and added a comment
    
    Signed-off-by: Jason Ekstrand <jason.ekstrand@intel.com>
    Reviewed-by: Matt Turner <mattst88@gmail.com>
------------------------------------------------

(In reply to Eero Tamminen from comment #8)
> Bisect gave this as the commit breaking FM 2014:
> ------------------------------------------------
> Author:     Jason Ekstrand <jason.ekstrand@intel.com>
> AuthorDate: Fri Sep 12 16:17:37 2014 -0700
> Commit:     Jason Ekstrand <jason.ekstrand@intel.com>
> CommitDate: Tue Sep 30 10:29:15 2014 -0700

This is commit 514fd1c55e617bb325979cbee4a89f0727c3b567.

Yes.  There have been some changes on the same code since it, so it doesn't revert as-is and manual fixing of revert issues still leaves build issues (there have been API changes).  Any other patches I could try out?

(In reply to Eero Tamminen from comment #10)
> Yes.  There have been some changes on the same code since it, so it doesn't
> revert as-is and manual fixing of revert issues still leaves build issues
> (there have been API changes).  Any other patches I could try out?

I'd be curious how Curro's load_payload cleanups affect it.  That's one possible source of bugs in that code-path.  The original load_payload code I wrote was not great and he's done quite a bit to clean it up and, quite possibly, make it more correct.

Payload changes Curro pushed in yesterday didn't affect FM issue.

*** Bug 89293 has been marked as a duplicate of this bug. ***

Could you please provide an API trace of FM that demonstrates the bug?  I don't have that game in my stream library.  I'm going to try and look into this this week.

We found the source of the bug.  Turns out Football Manager is using four render targets and only drawing to 3 of them resulting in junk in the other render target as well as (more importantly) the depth buffer.  Ken put together a patch to fix it which can be found here:

http://cgit.freedesktop.org/~kwg/mesa/log/?h=football

So, that patch was bogus - it fixed the bug, but broke Piglit horribly.

I've sent out a new and improved series that fixes the bug and passes Piglit:
http://lists.freedesktop.org/archives/mesa-dev/2015-February/077989.html
http://lists.freedesktop.org/archives/mesa-dev/2015-February/077988.html
http://lists.freedesktop.org/archives/mesa-dev/2015-February/077990.html
http://lists.freedesktop.org/archives/mesa-dev/2015-February/077991.html

Jason - thanks for doing the hard work to track this down!

I didn't test the latter patch series, but I can verify that the first patch fixes the issue in FM (in case you were using the apitrace output for checks).

Fixed by:

commit e95969cd9548033250ba12f2adf11740319b41e7
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Thu Feb 26 17:45:49 2015 -0800

    i965/fs: Don't issue FB writes for bound but unwritten color targets.

Verified.