Bug 70212 - glxinfo triggers assert in cso_release_all after 3f0627c2ad6
Summary: glxinfo triggers assert in cso_release_all after 3f0627c2ad6
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/nouveau (show other bugs)
Version: git
Hardware: All All
: medium normal
Assignee: Nouveau Project
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-06 23:48 UTC by Aaron Watry
Modified: 2013-10-23 19:43 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
limit samplers returned to PIPE_MAX_SAMPLERS (589 bytes, patch)
2013-10-23 15:47 UTC, Brian Paul
Details | Splinter Review

Description Aaron Watry 2013-10-06 23:48:20 UTC
I just switched my GF9400M from Nvidia's blob to nouveau with mesa/drm from git.

When executing glxinfo with git master, I get the following assert after the list of core profile extensions are printed:

cso_cache/cso_context.c:315:cso_release_all: Assertion `max <= 16' failed.
Trace/breakpoint trap (core dumped)


I've bisected the regression to this commit:
3f0627c2ad605b006737312c478907859411ffa8 is the first bad commit
commit 3f0627c2ad605b006737312c478907859411ffa8
Author: Brian Paul <brianp@vmware.com>
Date:   Thu Sep 12 18:09:33 2013 -0600

    nouveau: implement pipe_context::bind_sampler_states()

:040000 040000 b09183c220eb383907e212299a3ed5d66bd26c18 cbfe533aeb6e5ff8648ac8efdf60a28b86ffe5e0 M	src
Comment 1 Brian Paul 2013-10-07 16:15:21 UTC
I can't test nouveau here.  Can you debug this a bit further?  What's the value of 'max' and 'sh'?
Looks like the driver is returning an unexpected value for the PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS query.
Comment 2 Emil Velikov 2013-10-07 18:08:15 UTC
Hi Brian

The nv50 driver returns max=32 for sh=PIPE_SHADER_{VERTEX,FRAGMENT,GEOMETRY} and max=0 otherwise.

Another interesting point is that with the bind_*_sampler_states hooks the drivers where left to handle cases of 0 samplers on their own - with nouveau and radeon doing additional work (state tracking afaics). Whereas with the unified bind_sampler_states() in case of 0 samplers the driver hook is not executed. Is that change intentional ?
Comment 3 Emil Velikov 2013-10-12 18:52:24 UTC
(In reply to comment #2)
> Another interesting point is that with the bind_*_sampler_states hooks the
> drivers where left to handle cases of 0 samplers on their own - with nouveau
> and radeon doing additional work (state tracking afaics). Whereas with the
> unified bind_sampler_states() in case of 0 samplers the driver hook is not
> executed. Is that change intentional ?

Scratch that
Comment 4 exc 2013-10-20 14:52:12 UTC
It's because value returned by nv50_screen_get_shader_param with param=PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS is greater than PIPE_MAX_SAMPLERS. Setting either of them to another fixes the problem, dunno which is right. 

git blame outputs:

% git blame src/gallium/drivers/nouveau/nv50/nv50_screen.c -L 251,252
f5bfe54a src/gallium/drivers/nv50/nv50_screen.c (Marek Olšák 2011-09-27 22:22:06 +0200 251)    case PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS:
f5bfe54a src/gallium/drivers/nv50/nv50_screen.c (Marek Olšák 2011-09-27 22:22:06 +0200 252)       return 32;

% git blame src/gallium/include/pipe/p_state.h -L 60,60
da893403 (Brian 2008-04-08 21:43:36 -0600 60) #define PIPE_MAX_SAMPLERS         16
Comment 5 Alen Skondro 2013-10-23 14:19:22 UTC
I had the same problem with nv50. 
Now that I own an NVE4 the issue is gone, the assert passes.

in src/gallium/drivers/nvc0/nvc0_screen.c

 case PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS:
254       return 16; /* would be 32 in linked (OpenGL-style) mode */
255       /*
256    case PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLER_VIEWS:
257       return 32;
258       */
Comment 6 Brian Paul 2013-10-23 15:47:28 UTC
Created attachment 88040 [details] [review]
limit samplers returned to PIPE_MAX_SAMPLERS

Can someone verify this patch to nv50?
Comment 7 Aaron Watry 2013-10-23 18:42:16 UTC
(In reply to comment #6)
> Created attachment 88040 [details] [review] [review]
> limit samplers returned to PIPE_MAX_SAMPLERS
> 
> Can someone verify this patch to nv50?

I can confirm that this patch avoids the assert in glxinfo on the original hardware which encountered the bug (NVAC, GF9400M).

Thanks,
Aaron
Comment 8 Brian Paul 2013-10-23 19:43:50 UTC
Fixed w/ commit c1345720c86639bb570f3495b834916849d50e37


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.