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!


bug/show.html.tmpl processed on Mar 25, 2017 at 09:45:45.
(provided by the Example extension).