Bug 91895 - [nve7] Shadow Warrior: black gun
Summary: [nve7] Shadow Warrior: black gun
Status: RESOLVED MOVED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/nouveau (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Nouveau Project
QA Contact: Nouveau Project
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-06 16:09 UTC by Marcin Slusarz
Modified: 2019-09-18 20:40 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Shadow Warrior on Nouveau (2.21 MB, image/png)
2015-09-06 16:09 UTC, Marcin Slusarz
Details
Shadow Warrior on Intel (1.94 MB, image/png)
2015-09-06 16:10 UTC, Marcin Slusarz
Details
vertex shader that fails to compile (6.84 KB, text/plain)
2015-09-06 19:09 UTC, Ilia Mirkin
Details

Description Marcin Slusarz 2015-09-06 16:09:31 UTC
Created attachment 118102 [details]
Shadow Warrior on Nouveau

Screenshots comparing intel and nouveau attached.
Apitrace: http://people.freedesktop.org/~mslusarz/ShadowWarrior.bin.x86.trace.xz (174MB)

This is on mesa-git (3c6c4d4f298ec81fe57992790a68aaab2e573519) with the patch from bug#91890.
Comment 1 Marcin Slusarz 2015-09-06 16:10:35 UTC
Created attachment 118103 [details]
Shadow Warrior on Intel
Comment 2 Ilia Mirkin 2015-09-06 17:21:25 UTC
GlennK reproduced the same issue on r600, so it's most likely a st/mesa bug.
Comment 3 Ilia Mirkin 2015-09-06 17:28:24 UTC
(In reply to Ilia Mirkin from comment #2)
> GlennK reproduced the same issue on r600, so it's most likely a st/mesa bug.

Errr... two issues. One is the lighting issue, which also happens on r600. Another is the blacked out bits, which is nouveau-specific.
Comment 4 Ilia Mirkin 2015-09-06 18:58:22 UTC
Probably relevant to the nouveau-specific issue:

ERROR: no viable spill candidates left

I bet the end result is black, which explains those spots. Haven't done more debugging on that yet.
Comment 5 Ilia Mirkin 2015-09-06 19:09:37 UTC
Created attachment 118107 [details]
vertex shader that fails to compile

Don't know if this is *the* issue, but it's certainly *an* issue. A little surprising that such a simple shader dies -- no control flow, no nothing. Fails to find spill candidates.

Perhaps the function calls somehow mess it up.
Comment 6 Marcin Slusarz 2015-09-06 19:35:26 UTC
Hmm, I'm not sure anymore which lighting is the correct one...

One more trace with better view:
http://people.freedesktop.org/~mslusarz/ShadowWarrior.bin.x86.2.trace.xz (181 MB)
Comment 7 Ilia Mirkin 2015-09-06 21:13:32 UTC
OK, I think I see what's happening... the sequence goes something like this:

  3: mov u32 %r438 0x3f800000 (0)

 16: mov u32 $r0 %r445 (0)
 17: mov u32 $r1 %r452 (0)
 18: call abs BUILTIN:1 (0)
 19: mov u32 %r455 $r1 (0)
 20: nop - { $r0 $r2d } (0)
 21: nop - $p0q (0)

337: mov u32 %r1227 %r1012 (0)
338: mov u32 %r1228 %r1014 (0)
339: mov u32 %r1229 %r954 (0)
340: merge b128 %r1222q %r1227 %r1228 %r1229 %r438 (0)
341: export b128 # o[0x10] %r1222q (0)

Now the resulting "code" looks like:

  1: mov u32 $r2 0x3f800000 (8)
  7: mov u32 $r10 0x00000004 (8)

263: mov u32 $r1073741823 $r10 (8)
264: mov u32 $r0 $r11 (8)
265: mov u32 $r1 $r16 (8)
266: export b128 # o[0x10] %r1222q (8)

So it must be trying to stick this stuff into r0..r3. However there's a clobber of $r2 between the initial move and the desire to use it, without any constraint move being inserted. So the RA fails because it doesn't understand the situation properly. I *think*.

Not sure why the first 3 args of the merge have a mov but the 4th one doesn't. Perhaps something eliminates it for some reason? Also worth noting that this works fine on GK110, presumably due to slightly different RA constraints. However we don't get anywhere close to using up the 64 registers here either.
Comment 8 Ilia Mirkin 2015-09-06 21:14:58 UTC
This patch should get it going, but I don't think I want to commit it as-is. This gets me the same overly-lit rendering as r600, so any remaining issues are in the st. Should probably investigate what the "real" rendering looks like, neither lighting matches any screenshots of the game I've found online.


diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
index 0cd21cf..268ea0b 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
@@ -2121,8 +2121,10 @@ RegAlloc::InsertConstraintsPass::insertConstraintMoves()
 
             Instruction *defi = cst->getSrc(s)->defs.front()->getInsn();
             // catch some cases where don't really need MOVs
+/*
             if (cst->getSrc(s)->refCount() == 1 && !defi->constrainedDefs())
                continue;
+*/
 
             LValue *lval = new_LValue(func, cst->src(s).getFile());
             lval->reg.size = size;
Comment 9 Marcin Slusarz 2015-09-06 21:56:25 UTC
Yes, this fixes gun rendering.
Comment 10 Ilia Mirkin 2015-09-08 21:52:05 UTC
This trace can't be used on i965:

62714 @0 glLinkProgram(program = 4038)
62714: warning: error: Uniform block MaterialCB too big (32768/16384)

Their UBO size is too small I guess.

GL_MAX_UNIFORM_BLOCK_SIZE = 16384

vs

GL_MAX_UNIFORM_BLOCK_SIZE = 65536

Although I bet that's a mistake...
Comment 11 Marcin Slusarz 2015-09-09 06:46:37 UTC
The game behaves exactly the same WRT lighting under Intel driver. I can upload a trace if you want.
Comment 12 Ilia Mirkin 2015-09-09 06:49:59 UTC
(In reply to Marcin Slusarz from comment #11)
> The game behaves exactly the same WRT lighting under Intel driver. I can
> upload a trace if you want.

No, that's fine. The game probably doesn't check for UBO sizes and just assumes it'll work. I hacked it to expose a 64K buffer and it was fine. Ken sent a patch to list which does this for real.
Comment 13 Marcin Slusarz 2015-09-09 18:28:07 UTC
Fun, with bumped MAX_UNIFORM_BLOCK_SIZE lighting looks exactly the same on Intel and Nouveau.
Comment 14 Ilia Mirkin 2016-01-07 03:16:02 UTC
This patch fixes the trace: http://patchwork.freedesktop.org/patch/69589/

However it needs to get a lot more testing before I'll be ready to push it... all this RA stuff is a little over my head.
Comment 15 GitLab Migration User 2019-09-18 20:40:57 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/1082.


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.