Bug 90363

Summary:	[nv50] HW state is not reset correctly when using a new GL context
Product:	Mesa	Reporter:	Matteo Bruni <matteo.mystral>
Component:	Drivers/DRI/nouveau	Assignee:	Nouveau Project <nouveau>
Status:	RESOLVED FIXED	QA Contact:	Nouveau Project <nouveau>
Severity:	normal
Priority:	medium
Version:	git
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Hack Ilia's attempted fix Improved fix

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.