| Summary: | [HSW Regression] Synmark2_v6 Multithread performance case FPS reduced by 36% | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | wendy.wang |
| Component: | Drivers/DRI/i965 | Assignee: | Ian Romanick <idr> |
| Status: | VERIFIED FIXED | QA Contact: | Intel 3D Bugs Mailing List <intel-3d-bugs> |
| Severity: | normal | ||
| Priority: | medium | CC: | currojerez |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
Xorg.0.log info
drop_image_unit_validation.patch |
||
|
Description
wendy.wang
2015-08-28 08:07:56 UTC
Created attachment 117961 [details]
Xorg.0.log info
I'm bisecting it to commit 3569742ec458c0a881857d9deb782c1e11f195d8 Author: Francisco Jerez <currojerez@riseup.net> Date: Fri Nov 22 16:00:33 2013 -0800 i965: Define implementation constants for ARB_shader_image_load_store. Reviewed-by: Paul Berry <stereotype441@gmail.com> v2: Drop VS support pre-Gen8, drop GS support. Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> Specifically, commenting out the assignment of ctx->Const.MaxImageUnits brings performance back to previous levels. Running strings on synmark indeed shows "GL_ARB_shader_image_load_store", so the benchmark may be doing additional work with the extension enabled. Except that the extension isn't enabled at that point - it's turned on several commits later. I'm pretty sure only tests with "CS" in their name use ARB_shader_image_load_store, and this isn't one of them. Yeah, apparently the regression is caused by the additional validation done for each image unit when _NEW_TEXTURE is flagged. I would expect validation for a single disabled image unit to be cheap, little more than a couple of pointer dereferences and a branch, but still it seems to add up to a measurable overhead due to the huge number of image units we expose. The attached patch removes the _NEW_TEXTURE-dependent state (actually just the _Valid field) from the image unit struct so this per-image unit validation is no longer necessary. Instead the driver is required to call a function to calculate the same value when needed, what may make per-image state updates in the driver a little more expensive but is likely to be a net win because the overall complexity is reduced since now only image units which are actually bound to a shader need to be validated. Created attachment 117980 [details] [review] drop_image_unit_validation.patch Tested on the latest mesa with the patch, this problem does not exists. (In reply to ye.tian from comment #6) > Tested on the latest mesa with the patch, this problem does not exists. Thanks, I've formatted the change as a patch series and sent it for review with your tested-by. [1] [1] http://lists.freedesktop.org/archives/mesa-dev/2015-September/093362.html I've just landed the series that fixes this regression, marking as resolved. Thanks Curro |
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.