Bug 90797 - [ALL bisected] Mesa change cause performance case manhattan fail.
Summary: [ALL bisected] Mesa change cause performance case manhattan fail.
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Tapani Pälli
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-01 03:33 UTC by Ding Heng
Modified: 2015-06-24 06:22 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
xorg log (18.73 KB, text/plain)
2015-06-01 03:33 UTC, Ding Heng
Details
patch to fix the issue (2.97 KB, patch)
2015-06-02 06:33 UTC, Tapani Pälli
Details | Splinter Review
fix using static values (3.24 KB, patch)
2015-06-04 06:10 UTC, Tapani Pälli
Details | Splinter Review
fix using static values (3.26 KB, patch)
2015-06-04 06:22 UTC, Tapani Pälli
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ding Heng 2015-06-01 03:33:18 UTC
Created attachment 116195 [details]
xorg log

System Environment:
--------------------------
Regression: yes
Platform: SKL

kernel: drm-intel-nightly 4b2be843fab906467eed62265f5066418c73649c(2015-05-30)

Mesa bisect result:

9b5e92f4ccc6ee1cb9caea947f6efaad2b391cf1 is the first bad commit
commit 9b5e92f4ccc6ee1cb9caea947f6efaad2b391cf1
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Wed Apr 29 16:12:40 2015 -0700

    mesa: Allow overriding the version of ES2+ contexts

    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Tapani Pälli <tapani.palli@intel.com>

:040000 040000 eb95bc313c993b4100c7d8f04f1e9c2a62c7605e 60fb49a51485c42da7c9060ac6462d0c3332c554 M   src


Bug detailed description:
-----------------------------

Run performance case manhattan will get following error message:

[INFO ]: width: 1920, height: 1080
[ERROR]: GLX: Failed to create context: GLXBadFBConfig


Reproduce steps:
---------------------------

Run manhattan manually and check the output.
Comment 1 Eero Tamminen 2015-06-01 09:27:04 UTC
Heng, Manhattan and Unigine Valley don't render correctly anymore in our tests (e.g. on BSW).  As both require overrides to render correctly, could you check whether that issue is also caused by the Mesa change in this bug?

PS. Please use --format=fuller when pasting commit info (this seems to be added to Mesa on 28th of April).
Comment 2 Tapani Pälli 2015-06-02 06:33:21 UTC
Created attachment 116229 [details] [review]
patch to fix the issue

this fixes the issue, will have to test with different override combinations
Comment 3 Ding Heng 2015-06-04 02:21:49 UTC
(In reply to Tapani Pälli from comment #2)
> Created attachment 116229 [details] [review] [review]
> patch to fix the issue
> 
> this fixes the issue, will have to test with different override combinations

This patch has fix this issue. Change state to resolved.
Comment 4 Matt Turner 2015-06-04 02:51:13 UTC
(In reply to Ding Heng from comment #3)
> (In reply to Tapani Pälli from comment #2)
> > Created attachment 116229 [details] [review] [review] [review]
> > patch to fix the issue
> > 
> > this fixes the issue, will have to test with different override combinations
> 
> This patch has fix this issue. Change state to resolved.

The patch is not upstream. The bug is not fixed until the patch is upstream.
Comment 5 Ian Romanick 2015-06-04 05:07:13 UTC
(In reply to Tapani Pälli from comment #2)
> Created attachment 116229 [details] [review] [review]
> patch to fix the issue
> 
> this fixes the issue, will have to test with different override combinations

Ugh... can we make it not do all the work every time a context is created?  That was the purpose of the static variables, etc.
Comment 6 Tapani Pälli 2015-06-04 05:15:43 UTC
(In reply to Ian Romanick from comment #5)
> (In reply to Tapani Pälli from comment #2)
> > Created attachment 116229 [details] [review] [review] [review]
> > patch to fix the issue
> > 
> > this fixes the issue, will have to test with different override combinations
> 
> Ugh... can we make it not do all the work every time a context is created? 
> That was the purpose of the static variables, etc.

It should be doable by having static array of versions for each gl_api, then it runs once for each. I can try if this works ok.
Comment 7 Tapani Pälli 2015-06-04 06:10:18 UTC
Created attachment 116276 [details] [review]
fix using static values

here's alternative fix with using static so that we initialize only once per api
Comment 8 Tapani Pälli 2015-06-04 06:22:56 UTC
Created attachment 116277 [details] [review]
fix using static values

sorry, there were some issues, here is a fixed version
Comment 9 Ding Heng 2015-06-08 07:37:32 UTC
(In reply to Tapani Pälli from comment #8)
> Created attachment 116277 [details] [review] [review]
> fix using static values
> 
> sorry, there were some issues, here is a fixed version

This patch has been test on latest mesa. The result is pass.
Comment 10 Tapani Pälli 2015-06-23 17:43:38 UTC
Fixed in master:

commit 7d88ab42b9dda825feddbae774a2a48ddf3cbec2
Author: Tapani Pälli <tapani.palli@intel.com>
Date:   Tue Jun 16 13:46:47 2015 +0300

    mesa: set override_version per api version override
    
    Before 9b5e92f get_gl_override was called only once, but now it is
    called for multiple APIs (GLES2, GL), version needs to be set always.
Comment 11 wendy.wang 2015-06-24 06:22:03 UTC
Bug has been verified as fixed with latest Mea master branch:

 Libdrm:		(master)libdrm-2.4.61-13-g97be70b45eccc37e98a1cecf360593f36956ea42
 Mesa:		(master)028590cbc758e877b963ba430f0a0cb49e882a6b
 Xserver:		(master)xorg-server-1.17.0-158-gfa12f2c150b2f50de9dac4a2b09265f13af353af
 Xf86_video_intel:		(master)2.99.917-366-g87b5ca2c3ed45a46ac037e71e96922b1a7f8c10f
 Cairo:		(master)2cf2d8e340a325adb205baf8e4bd64e1d1858008
 Libva:		(master)5d07b29687db6d17811b7ecf9b779377e9851a27
 Libva_intel_driver:		(master)0f6143e2e4bd70217887e1ad86a76200ad8b38a8
 Kernel:   (drm-intel-nightly)be39a7ed43b45f63aff1ad3b61008f67692db202


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.