Bug 90363 - [nv50] HW state is not reset correctly when using a new GL context
Summary: [nv50] HW state is not reset correctly when using a new GL context
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: Nouveau Project
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-07 16:46 UTC by Matteo Bruni
Modified: 2015-05-09 17:56 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Hack (1.38 KB, text/plain)
2015-05-07 16:46 UTC, Matteo Bruni
Details
Ilia's attempted fix (3.81 KB, patch)
2015-05-07 16:51 UTC, Matteo Bruni
Details | Splinter Review
Improved fix (4.11 KB, patch)
2015-05-07 18:25 UTC, Matteo Bruni
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matteo Bruni 2015-05-07 16:46:53 UTC
Created attachment 115622 [details]
Hack

Hardware state is not reset correctly when a process makes use of a new GL context, after it has used and then destroyed the previous one.
This manifests in a (still uncommitted, https://www.winehq.org/pipermail/wine-patches/2015-April/138974.html) Wine test which fails on nv50. Actually commenting out stream_test(), which makes use of instanced draws with ARB_instanced_arrays, workarounds the issue.
A single process executes both tests, destroying and then recreating a GL context in between. AFAIU specifically the per-instance enable flags are never reset on the HW and that breaks the latter test.

I'm attaching a patch which "fixes" the issue for me but it's not the correct approach.
Comment 1 Matteo Bruni 2015-05-07 16:51:46 UTC
Created attachment 115623 [details] [review]
Ilia's attempted fix

I'm attaching Ilia's patch for this issue, from IRC. It doesn't work for me (yet) but this should be more like the correct fix.

BTW, I can try to make a standalone, GL testcase for this bug. It might take a while though.
Comment 2 Matteo Bruni 2015-05-07 18:25:01 UTC
Created attachment 115624 [details] [review]
Improved fix

The attached works for me. It's the patch from Ilia with an additional state copy in nv50_create.
Comment 3 Ilia Mirkin 2015-05-07 19:32:57 UTC
Ah yeah, that makes sense. Forgot that contexts liked to auto-attach themselves. cur_state seems like a dumb name, I'm going to change it to saved_state or something. Also that state needs to be initialized, so I'm going to move the state init bits from that state init function to the screen init. Will send a proper patch for testing tonight-ish.
Comment 4 Ilia Mirkin 2015-05-09 17:56:19 UTC
Just pushed out the fixes to make it work on nv50 and nvc0. nvc0 had the additional bug that 3d-engine-based blits use a vbo, and were also forgetting to overwrite these attributes, leading to a fail even when having the proper "state".

Thanks for reporting!


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.