Created attachment 113657 [details] Unigine Valley Mesa-10.4 System Environment: -------------------------- chipset: HSW system architecture: x86-64 xf86-video-intel: 2.99.914-1 xserver: 1.16.0-1 mesa: 10.4-devel, git id: 7841a246b93d8d8aeb65df5805c0e9d05567c57e libdrm: 2.4.56-1 kernel: 3.16.0 OS: Ubuntu Linux 14.10 Bug detailed description: ------------------------- I run Unigine Valley benchmark in Windows 7 VM that works on Linux Workstation 11.0.0. After shifting to Mesa 10.4 Unigine Valley image become corrupted. I’ve found guilty commit, it is 7841a246b93d8d8aeb65df5805c0e9d05567c57e: 2014-08-31 7841a24 i965/vec4: Allow propagation of instructions with saturate flag to sel, Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Mesa built from previous commit 40aeb558ce8a7ffaaa6f81be16419b9b238c16d8 renders good picture.
Created attachment 113658 [details] Unigine Valley Mesa-10.4 Attached JPEG image with Unigine Valley incorrect rendering.
Checked with Mesa-10.5rc1. The issue is reproducible.
I found quick fix, but not sure it is completely correct: diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp index 6769310..2ed9ce1 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp @@ -47,6 +47,7 @@ is_direct_copy(vec4_instruction *inst) return (inst->opcode == BRW_OPCODE_MOV && !inst->predicate && inst->dst.file == GRF && + !inst->saturate && !inst->dst.reladdr && !inst->src[0].reladdr && inst->dst.type == inst->src[0].type);
Abdiel, can you look at this? It would be good if a fix for this could land in time for the 10.5 release.
Created attachment 113683 [details] [review] Valley patch for mesa-10.4 This patch looks more accurately.
I'll look into this.
(In reply to Andrey Sudnik from comment #5) > Created attachment 113683 [details] [review] [review] > Valley patch for mesa-10.4 > > This patch looks more accurately. I wrote a small vertex shader to reproduce this: in vec4 vertex; out vec4 color; uniform vec4 a, b; void main() { gl_Position = vertex; color.xyz = clamp(a.xyz, 0.0, 1.0); color.w = a.w; } Without your patch, we simply drop the saturate in copy propagation. I need to come up with a way of testing this other than inspecting the disassembly. I've sent your patch to the mailing list for review. Thanks!
Thanks for the patch. I've committed it as commit 0dfec59a2785cf7a87ee5128889ecebe810b611b Author: Andrey Sudnik <andrey.sudnik@intel.com> Date: Thu Mar 5 11:16:49 2015 -0800 i965/vec4: Don't lose the saturate modifier in copy propagation. Cc: 10.4, 10.5 <mesa-stable@lists.freedesktop.org> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89224 Reviewed-by: Matt Turner <mattst88@gmail.com> Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> It should end up in the 10.4.6 and 10.5.0 releases.
Thank you Matt!
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.