summaryrefslogtreecommitdiffstats
path: root/src/glsl
Commit message (Collapse)AuthorAgeFilesLines
* glsl 1.30: Fix numerical instabilities in asinhPaul Berry2011-09-281-4/+36
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The formula we were previously using for asinh: asinh x = ln(x + sqrt(x * x + 1)) is numerically unstable: when x is a large negative value, the quantity x + sqrt(x * x + 1) is a small positive value (on the order of 1/(2|x|)). Since the logarithm function is very sensitive in this range, any error in the computation of the square root manifests as a large error in the result. This patch changes to the equivalent formula: asinh x = sign(x) * ln(abs(x) + sqrt(x * x + 1)) which is only slightly more expensive to compute, and is numerically stable for all x. Fixes piglit tests spec/glsl-1.30/execution/built-in-functions/[fv]s-asinh-*. Reviewed-by: Chad Versace <[email protected]> Acked-by: Kenneth Graunke <[email protected]> Reviewed-by: Eric Anholt <[email protected]>
* glsl: Add support for constant expression evaluation on trunc().Eric Anholt2011-09-281-0/+2
| | | | | | | Fixes the glsl-1.30/compiler/built-in-functions/trunc-* tests under 1.30. Reviewed-by: Chad Versace <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* glsl: Fix assertion checking types of constant bitshift expressions.Eric Anholt2011-09-281-1/+3
| | | | | | | | Bitshifts are one of the rare places that GLSL allows mixed base types without an implicit conversion occurring. Reviewed-by: Chad Versace <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* glsl: Fix Android buildChad Versace2011-09-241-0/+1
| | | | | | Add lower_clip_distance.cpp to list of source files. Signed-off-by: Chad Versace <[email protected]>
* glsl: Free all S-Expressions immediately after reading IR.Kenneth Graunke2011-09-231-2/+3
| | | | | | | | | For some reason I thought subexpressions were chained off the top-level one. This isn't the case, so just create a temporary context and free it. All of this memory would be eventually freed, but now is freed much sooner. Signed-off-by: Kenneth Graunke <[email protected]>
* glsl: Defer initialization of built-in functions until they're needed.Kenneth Graunke2011-09-234-3/+7
| | | | | | | | | | | | | | | Very simple shaders don't actually use GLSL built-ins. For example: - gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex; - gl_FragColor = vec4(0.0); Both of the shaders used by _mesa_meta_glsl_Clear() also qualify. By waiting to initialize the built-ins until the first time we need to look for a signature, we can avoid the overhead entirely in these cases. Makes piglit run roughly 18% faster (255 vs. 312 seconds). Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Eric Anholt <[email protected]>
* mesa: set up gl_vert_result and gl_frag_attrib values for gl_ClipDistance.Paul Berry2011-09-231-4/+4
| | | | | | | | | | | | This patch assigns enumerated values for gl_ClipDistance in the gl_vert_result and gl_frag_attrib enums, so that driver back-ends can assign gl_ClipDistance to the appropriate hardware registers. It also adjusts the functions _mesa_vert_result_to_frag_attrib() and _mesa_frag_attrib_to_vert_result() (which translate between the two enums) to correctly translate the new enumerated values. Reviewed-by: Eric Anholt <[email protected]> Tested-by: Brian Paul <[email protected]>
* mesa: Add a flag to indicate whether a program uses gl_ClipDistance.Paul Berry2011-09-231-1/+4
| | | | | | | | | | | | | | | | | | GLSL 1.30 requires us to use gl_ClipDistance for clipping if the vertex shader contains a static write to it, and otherwise use user-defined clipping planes. Since the driver needs to behave differently in these two cases, we need a flag to record whether the shader has written to gl_ClipDistance. The new flag is called UsesClipDistance. We initially store it in gl_shader_program (since that is the data structure that is available when we check to see whethe gl_ClipDistance was written to), and we later copy it to a flag with the same name in gl_vertex_program, since that is a more convenient place for the driver to access it (in i965, at least). Reviewed-by: Eric Anholt <[email protected]> Tested-by: Brian Paul <[email protected]>
* glsl: Implement a lowering pass for gl_ClipDistance.Paul Berry2011-09-235-0/+350
| | | | | | | | | | | | | | | | | | | In i965 GEN6+ (and I suspect most other hardware), gl_ClipDistance needs to be laid out as a pair of vec4's (the first containing clip distances 0-3, and the second containing clip distances 4-7). However, it is declared in GLSL as an array of 8 floats. This lowering pass acts at the GLSL level, modifying the declaration of gl_ClipDistance so that it is an array of vec4's rather than an array of floats, and renaming it to gl_ClipDistanceMESA. In addition, it modifies all accesses to the array so that they access the appropiate component of one of the vec4's. Since some hardware may not internally represent gl_ClipDistance as a pair of vec4's, this lowering pass is optional. To enable it, set the LowerClipDistance flag in gl_shader_compiler_options to true. Reviewed-by: Kenneth Graunke <[email protected]>
* glsl hierarchical visitor: Do not overwrite base_ir for parameter lists.Paul Berry2011-09-232-7/+17
| | | | | | | | | | | | | | This patch fixes a bug in ir_hirearchical_visitor: when traversing an exec_list representing the formal or actual parameters of a function, it modified base_ir to point to each parameter in turn, rather than leaving it as a pointer to the enclosing statement. This was a problem, since base_ir is used by visitor classes to locate the statement containing the node being visited (usually so that additional statements can be inserted before or after it). Without this fix, visitors might attempt to insert statements into parameter lists. Reviewed-by: Kenneth Graunke <[email protected]>
* make: Don't use builtin_stubs.cpp for standalone GLSL compiler.Kenneth Graunke2011-09-201-4/+4
| | | | | | | | | | | | | | | | builtin_stubs.cpp is only supposed to be used for builtin_compiler. It contains a stub version of _mesa_glsl_initialize_functions() that does nothing. libglsl.a already contains builtin_function.cpp, the generated file that contains a version of _mesa_glsl_initialize_functions() that actually initializes all the built-in functions. By mistakenly linking to builtin_stubs, glsl_compiler and glsl_test are unable to compile any shaders that use built-in functions. Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Paul Berry <[email protected]>
* glsl: Remove field array_lvalue from ir_variable.Paul Berry2011-09-144-32/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The array_lvalue field was attempting to enforce the restriction that whole arrays can't be used on the left-hand side of an assignment in GLSL 1.10 or GLSL ES, and can't be used as out or inout parameters in GLSL 1.10. However, it was buggy (it didn't work properly for built-in arrays), and it was clumsy (it unnecessarily kept track on a variable-by-variable basis, and it didn't cover the GLSL ES case). This patch removes the array_lvalue field completely in favor of explicit checks in ast_parameter_declarator::hir() (this check is added) and in do_assignment (this check was already present). This causes a benign behavioral change: when the user attempts to pass an array as an out or inout parameter of a function in GLSL 1.10, the error is now flagged at the time the function definition is encountered, rather than at the time of invocation. Previously we allowed such functions to be defined, and only flagged the error if they were invoked. Fixes Piglit tests spec/glsl-1.10/compiler/qualifiers/fn-{out,inout}-array-prohibited* and spec/glsl-1.20/compiler/assignment-operators/assign-builtin-array-allowed.vert. Reviewed-by: Ian Romanick <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* glsl: Silence "ast_to_hir.cpp:1984:25: warning: comparison of unsigned ↵Ian Romanick2011-09-091-1/+1
| | | | | | | | | | | expression >= 0 is always true" ast_type_qualifier::location should have been a signed integer from the beginning, and the giant comment in apply_type_qualifier_to_variable explains why. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40207 Reviewed-by: Kenneth Graunke <[email protected]>
* glsl: Silence several "warning: unused parameter"Ian Romanick2011-09-098-0/+16
|
* glsl: Don't do structure splitting until link time.Eric Anholt2011-09-081-1/+1
| | | | | | | | | We were splitting on each side of an unlinked program, and the two sides lost track of which variables they referenced, resulting in assertion failure during validation. Fixes piglit link-struct-uniform-usage. Reviewed-by: Ian Romanick <[email protected]>
* glsl: Clarify error message about whole-array assignment in GLSL 1.10.Eric Anholt2011-09-081-6/+25
| | | | | | | | | | | | | | | | | Previously, it would produce: Failed to compile FS: 0:6(7): error: non-lvalue in assignment and now it produces: Failed to compile FS: 0:5(7): error: whole array assignment is not allowed in GLSL 1.10 or GLSL ES 1.00. Also, add spec quotation to the two places we have code for array lvalues in GLSL 1.10. Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl: When assiging from a whole array, mark it as used.Eric Anholt2011-09-081-0/+1
| | | | | | Fixes piglit link-uniform-array-size. Reviewed-by: Ian Romanick <[email protected]>
* glsl: Fix setting of OutputsWritten for whole array dereference.Eric Anholt2011-09-081-4/+2
| | | | | | | | We just want to mark the whole thing used, not mark from each element the whole size in use. Fixes undefined URB entry writes on i965, which blew up with debugging enabled. Reviewed-by: Ian Romanick <[email protected]>
* glsl: Make sure gl_ClipDistance and gl_ClipVertex are not both written.Paul Berry2011-09-081-0/+19
| | | | | | | | | | | | | From section 7.1 (Vertex Shader Special Variables) of the GLSL 1.30 spec: "It is an error for a shader to statically write both gl_ClipVertex and gl_ClipDistance." Fixes piglit test mixing-clip-distance-and-clip-vertex-disallowed.c. Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl: Check that gl_ClipDistance[] is not sized too large.Paul Berry2011-09-081-0/+15
| | | | | | | | | Fixes piglit tests clip-distance-explicit-too-large-with-access.{frag,vert} and clip-distance-explicit-too-large.{frag,vert}. Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl: Rework oversize array check for gl_TexCoord.Paul Berry2011-09-081-12/+33
| | | | | | | | | | | | | The check now applies both when explicitly declaring the size of gl_TexCoord and when implicitly setting the size of gl_TexCoord by accessing it using integral constant expressions. This is prep work for adding similar size checks to gl_ClipDistance. Fixes piglit tests texcoord/implicit-access-max.{frag,vert}. Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl: Add constant gl_MaxClipDistances.Paul Berry2011-09-081-0/+15
| | | | | | | Fixes piglit tests {vs,fs}-clip-distance-sizeable-to-max.shader_test. Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl: Make gl_ClipDistance[] implicitly sized.Paul Berry2011-09-081-4/+24
| | | | | | | | | | | | | | | From the GLSL 1.30 spec, section 7.1 (Vertex Shader Special Variables): The gl_ClipDistance array is predeclared as unsized and must be sized by the shader either redeclaring it with a size or indexing it only with integral constant expressions. Fixes piglit tests clip-distance-implicit-length.vert, clip-distance-implicit-nonconst-access.vert, and {vs,fs}-clip-distance-explicitly-sized.shader_test. Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl: s/int/unsigned/ to silence warningBrian Paul2011-09-071-2/+2
|
* glsl/ir_reader: Make sure constants have the right number of components.Kenneth Graunke2011-09-071-0/+5
| | | | | | | | The list of numbers in (constant type (<numbers>)) needs to contain exactly type->components() numbers (16 for a mat4, 3 for a vec3, etc.) Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl/builtins: Fix invalid float constant in noise4 built-in.Kenneth Graunke2011-09-071-2/+2
| | | | | | | Throwing away the extra numbers ought to match the existing behavior. Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl/builtins: Fix invalid vecN constants in hyperbolic functions.Kenneth Graunke2011-09-075-21/+21
| | | | | | | | | | Each of these vecN constants only provided one component, which is illegal. The printed IR is meant to contain exactly as many components as are necessary; the IR reader does not splat single values. Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Paul Berry <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl: Use a separate div_to_mul_rcp lowering flag for integers.Bryan Cain2011-08-312-61/+77
| | | | | | | | | | | | | | Using multiply and reciprocal for integer division involves potentially lossy floating point conversions. This is okay for older GPUs that represent integers as floating point, but undesirable for GPUs with native integer division instructions. TGSI, for example, has UDIV/IDIV instructions for integer division, so it makes sense to handle this directly. Likewise for i965. Reviewed-by: Ian Romanick <[email protected]> Signed-off-by: Bryan Cain <[email protected]> Signed-off-by: Kenneth Graunke <[email protected]>
* glcpp: Add GL_ARB_conservative_depth #define.Kenneth Graunke2011-08-251-1/+3
| | | | | | | Forgotten in the patch that enabled the extension. Reviewed-by: Ian Romanick <[email protected]> Signed-off-by: Kenneth Graunke <[email protected]>
* glsl: fix crash when a const is passed to texelFetchOffsetDave Airlie2011-08-251-0/+1
| | | | | | | while debugging texelFetchOffset we kept hitting the assert. Signed-off-by: Dave Airlie <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* glsl: Bail after reporting an error for non-constant const_in parameters.Kenneth Graunke2011-08-251-0/+1
| | | | | | | | | | Otherwise we continue and hit the "Illegal formal parameter mode" assertion. Fixes negative compile test texelFetchOffset.frag in piglit. Signed-off-by: Kenneth Graunke <[email protected]> Signed-off-by: Dave Airlie <[email protected]>
* glsl: Implement the GL_ARB_conservative_depth extension.Kenneth Graunke2011-08-255-4/+7
| | | | | | | It's the same as GL_AMD_conservative_depth. The specs have slight differences in wording, but don't differ in content or behavior. Signed-off-by: Kenneth Graunke <[email protected]>
* glsl: Make sure that Extensions.dummy_true is set to trueIan Romanick2011-08-241-0/+2
| | | | | Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Paul Berry <[email protected]>
* glsl/builtins: Uncomment textureSize prototypes.Kenneth Graunke2011-08-232-4/+0
| | | | | | Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]> Reviewed-by: Dave Airlie <[email protected]>
* texture_builtins.py: Add support for textureSize (txs).Kenneth Graunke2011-08-231-14/+36
| | | | | Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* glsl: Add a new ir_txs (textureSize) opcode to ir_texture.Kenneth Graunke2011-08-238-35/+58
| | | | | | | | One unique aspect of TXS is that it doesn't have a coordinate. Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Ian Romanick <[email protected]> Reviewed-by: Dave Airlie <[email protected]>
* glsl: Make ir_validate actually visit ir_if nodes.Kenneth Graunke2011-08-221-2/+3
| | | | | | | | | | | | There is no ir_hierarchical_visitor::visit(ir_if *) method, since ir_if is not a leaf node. Instead, there are visit_enter and visit_leave methods. Use visit_enter arbitrarily (either would work fine, though visit_enter will catch errors sooner). Found thanks to a warning emitted by Clang. Reviewed-by: Ian Romanick <[email protected]> Signed-off-by: Kenneth Graunke <[email protected]>
* android: build glslChia-I Wu2011-08-212-0/+269
| | | | | | | This builds the static library libmesa_glsl and executable glsl_compiler from glsl. glsl_compiler is only installed for engineering build. Reviewed-by: Chad Versace <[email protected]>
* ralloc: include limits.h for SIZE_MAX on AndroidChia-I Wu2011-08-211-0/+5
| | | | | | | Android does not define SIZE_MAX in stdint.h. We have to include limits.h for it. Reviewed-by: Chad Versace <[email protected]>
* glsl: remove an unnecessary header includeChia-I Wu2011-08-211-1/+0
| | | | | | Reviewed-by: Brian Paul <[email protected]> Reviewed-by: Ian Romanick <[email protected]> Reviewed-by: Chad Versace <[email protected]>
* glsl: Remove unused variable.Kenneth Graunke2011-08-191-1/+0
|
* glsl: Fix type error when lowering integer divisionsPaul Berry2011-08-161-0/+4
| | | | | | | | | | | | | | | | | | | This patch fixes a bug when lowering an integer division: x/y to a multiplication by a reciprocal: int(float(x)*reciprocal(float(y))) If x was a plain int and y was an ivecN, the lowering pass incorrectly assigned the type of the product to be float, when in fact it should be vecN. This caused mesa to abort with an IR validation error. Fixes piglit tests {fs,vs}-op-div-int-ivec{2,3,4}. Reviewed-by: Kenneth Graunke <[email protected]>
* glsl: When assigning to a whole array, mark the array as accessed.Eric Anholt2011-08-151-10/+11
| | | | | | | | | | | | | The vs-varying-array-mat2-col-row-wr test writes a mat2[3] constant to a mat2[3] varying out array, and also statically accesses element 1 of it on the VS and FS sides. At link time it would get trimmed down to just 2 elements, and then codegen of the VS would end up generating assignments to the unallocated last entry of the array. On the new i965 VS backend, that happened to land on the vertex position. Some issues remain in this test on softpipe, i965/old-vs and i965/new-vs on visual inspection, but i965 is passing because only one green pixel is probed, not the whole split green/red quad.
* glsl: Add validations for ir_call.Paul Berry2011-08-151-0/+36
| | | | | | | | | | | | | | | This patch extends ir_validate.cpp to check the following characteristics of each ir_call: - The number of actual parameters must match the number of formal parameters in the signature. - The type of each actual parameter must match the type of the corresponding formal parameter in the signature. - Each "out" or "inout" actual parameter must be an lvalue. Reviewed-by: Chad Versace <[email protected]>
* glsl: Make is_lvalue() and variable_referenced() const.Paul Berry2011-08-152-11/+11
| | | | | | | | | | These functions don't modify the target instruction, so it makes sense to make them const. This allows these functions to be called from ir validation code (which uses const to ensure that it doesn't accidentally modify the IR being validated). Reviewed-by: Chad Versace <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* glsl: Perform implicit type conversions on function call out parameters.Paul Berry2011-08-151-5/+71
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When an out parameter undergoes an implicit type conversion, we need to store it in a temporary, and then after the call completes, convert the resulting value. In other words, we convert code like the following: void f(out int x); float value; f(value); Into IR that's equivalent to this: void f(out int x); float value; int out_parameter_conversion; f(out_parameter_conversion); value = float(out_parameter_conversion); This transformation needs to happen during ast-to-IR convertion (as opposed to, say, a lowering pass), because it is invalid IR for formal and actual parameters to have types that don't match. Fixes piglit tests spec/glsl-1.20/compiler/qualifiers/out-conversion-int-to-float.vert and spec/glsl-1.20/execution/qualifiers/vs-out-conversion-*.shader_test, and bug 39651. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=39651 Reviewed-by: Chad Versace <[email protected]>
* glsl: Modify strategy for accumulating conditions when lowering if-statementsIan Romanick2011-08-151-11/+55
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously if-statements were lowered from inner-most to outer-most (i.e., bottom-up). All assignments within an if-statement would have the condition of the if-statement appended to its existing condition. As a result the assignments from a deeply nested if-statement would have a very long and complex condition. Several shaders in the OpenGL ES2 conformance test suite contain non-constant array indexing that has been lowered by the shader writer. These tests usually look something like: if (i == 0) { value = array[0]; } else if (i == 1) { value = array[1]; } else ... The IR for the last assignment ends up as: (assign (expression bool && (expression bool ! (var_ref if_to_cond_assign_condition) ) (expression bool && (expression bool ! (var_ref if_to_cond_assign_condition@20) ) (expression bool && (expression bool ! (var_ref if_to_cond_assign_condition@22) ) (expression bool && (expression bool ! (var_ref if_to_cond_assign_condition@24) ) (var_ref if_to_cond_assign_condition@26) ) ) ) ) (x) (var_ref value) (array_ref (var_ref array) (constant int (5))) The Mesa IR that is generated from this is just as awesome as you might expect. Three changes are made to the way if-statements are lowered. 1. Two condition variables, if_to_cond_assign_then and if_to_cond_assign_else, are created for each if-then-else structure. The former contains the "positive" condition, and the later contains the "negative" condtion. This change was implemented in the previous patch. 2. Each condition variable is added to a hash-table when it is created. 3. When lowering an if-statement, assignments to existing condtion variables get the current condition anded. This ensures that nested condition variables are only set to true when the condition variable for all outer if-statements is also true. Changes #1 and #3 combine to ensure the correctness of the resulting code. 4. When a condition assignment is encountered with a condition that is a dereference of a previously added condition variable, the condition is not modified. Change #4 prevents the continuous accumulation of conditions on assignments. If the original if-statements were: if (x) { if (a && b && c && d && e) { ... } else { ... } } else { if (g && h && i && j && k) { ... } else { ... } } The lowered code will be if_to_cond_assign_then@1 = x; if_to_cond_assign_then@2 = a && b && c && d && e && if_to_cond_assign_then@1; ... if_to_cond_assign_else@2 = !if_to_cond_assign_then && if_to_cond_assign_then@1; ... if_to_cond_assign_else@1 = !if_to_cond_assign_then@1; if_to_cond_assign_then@3 = g && h && i && j; && if_to_cond_assign_else@1; ... if_to_cond_assign_else@3 = !if_to_cond_assign_then && if_to_cond_assign_else@1; ... Depending on how instructions are emitted, there may be an extra instruction due to the duplication of the '&& if_to_cond_assign_{then,else}@1' on the nested else conditions. In addition, this may cause some unnecessary register pressure since in the simple case (where the nested conditions are not complex) the nested then-condition variables are live longer than strictly necessary. Before this change, one of the shaders in the OpenGL ES2 conformance test suite's acos_float_frag_xvary generated 348 Mesa IR instructions. After this change it only generates 124. Many, but not all, of these instructions would have also been eliminated by CSE. Reviewed-by: Eric Anholt <[email protected]>
* glsl: Slight change to the code generated by if-flatteningIan Romanick2011-08-151-21/+39
| | | | | | | | | | | | | | | | | | | | | | Now the condition (for the then-clause) and the inverse condition (for the else-clause) get written to separate temporary variables. In the presence of complex conditions, this shouldn't result in more code being generated. If the original if-statement was if (a && b && c && d && e) { ... } else { ... } The lowered code will be if_to_cond_assign_then = a && b && c && d && e; ... if_to_cond_assign_else = !if_to_cond_assign_then; ... Reviewed-by: Eric Anholt <[email protected]>
* glsl: Replace foreach_iter with foreach_list_safeIan Romanick2011-08-151-2/+2
| | | | Reviewed-by: Eric Anholt <[email protected]>
* glsl: Make move_block_to_cond_assign not care which branch it's processingIan Romanick2011-08-151-27/+17
| | | | | | This will make some future changes a bit easier to digest. Reviewed-by: Eric Anholt <[email protected]>