summaryrefslogtreecommitdiffstats
path: root/src/intel
Commit message (Collapse)AuthorAgeFilesLines
* intel/decoder: Decode SFIXED values.Kenneth Graunke2018-08-231-3/+7
| | | | | | This lets us example SAMPLER_STATE's LOD Bias field, among other things. Reviewed-by: Lionel Landwerlin <[email protected]>
* configure: allow building with python3Emil Velikov2018-08-233-7/+7
| | | | | | | | | | | | Pretty much all of the scripts are python2+3 compatible. Check and allow using python3, while adjusting the PYTHON2 refs. Note: - python3.4 is used as it's the earliest supported version - python3 chosen prior to python2 Signed-off-by: Emil Velikov <[email protected]> Acked-by: Eric Engestrom <[email protected]>
* intel/compiler: Implement untyped atomic float min, max, and compare-swap ↵Ian Romanick2018-08-2214-1/+261
| | | | | | | | | | dataport messages v2: Split changes to the message type field to another patch. Suggested by Caio. Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Caio Marcelo de Oliveira Filho <[email protected]>
* intel/compiler: Expand untyped atomic message type field by a bitIan Romanick2018-08-223-4/+9
| | | | | | | | | | | This is necessary for a new Gen9 message type that will be added in the next patch. There are also Gen8 message types that need the extra bit (mostly for bindless). v2: Split off from the next patch. Suggested by Caio. Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Caio Marcelo de Oliveira Filho <[email protected]>
* intel/compiler: Silence unused parameter warningsIan Romanick2018-08-225-7/+5
| | | | | | | | | | | | | | | | | | | | | | | | src/intel/compiler/brw_disasm_info.c: In function ‘nir_print_instr’: src/intel/compiler/brw_disasm_info.c:30:61: warning: unused parameter ‘instr’ [-Wunused-parameter] __attribute__((weak)) void nir_print_instr(const nir_instr *instr, FILE *fp) {} ^~~~~ src/intel/compiler/brw_disasm_info.c:30:74: warning: unused parameter ‘fp’ [-Wunused-parameter] __attribute__((weak)) void nir_print_instr(const nir_instr *instr, FILE *fp) {} ^~ src/intel/compiler/brw_disasm.c: In function ‘src_ia1’: src/intel/compiler/brw_disasm.c:850:18: warning: unused parameter ‘_reg_file’ [-Wunused-parameter] unsigned _reg_file, ^~~~~~~~~ src/intel/compiler/brw_fs_surface_builder.cpp: In function ‘void brw::surface_access::emit_byte_scattered_write(const brw::fs_builder&, const fs_reg&, const fs_reg&, const fs_reg&, unsigned int, unsigned int, unsigned int, brw_predicate)’: src/intel/compiler/brw_fs_surface_builder.cpp:193:57: warning: unused parameter ‘size’ [-Wunused-parameter] unsigned dims, unsigned size, ^~~~ v2: Update commit message. brw_fs_generator.cpp warnings were already fixed by another patch. Noticed by Caio. Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Caio Marcelo de Oliveira Filho <[email protected]>
* intel/isl: Avoid tiling some 16K-wide render targetsNanley Chery2018-08-221-0/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix rendering issues on BDW and SKL. Fixes: 0288fe8d0417730bdd5b3477130dd1dc32bdbcd3 ("i965/miptree: Use the correct BLT pitch") Fixes the following regressions seen exclusively on SKL: * KHR-GL46.texture_barrier_ARB.disjoint-texels * KHR-GL46.texture_barrier_ARB.overlapping-texels * KHR-GL46.texture_barrier.disjoint-texels * KHR-GL46.texture_barrier.overlapping-texels and both on BDW and SKL: * GTF-GL46.gtf21.GL2FixedTests.buffer_corners.buffer_corners * GTF-GL46.gtf21.GL2FixedTests.stencil_plane_corners.stencil_plane_corners v2: Note the fixed tests (Andres). Don't cause failures with multisampled buffers (Andres). Don't hamper SKL GT4 (Ken). v3: Fix the Fixes tag (Dylan). Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107359 Cc: <[email protected]> Tested-by: Andres Gomez <[email protected]> Reviewed-by: Jason Ekstrand <[email protected]>
* intel/tools/aubwrite: Always use physical addresses for traces.Rafael Antognolli2018-08-222-11/+13
| | | | | | | | | | | | | | | It looks like we can't rely on the simulator to always translate virtual addresses to physical ones correctly. So let's use physical everywhere. Since our current GGTT maps virtual to physical addresses in a 1:1 way, no further changes are required. Additionally, we have other address spaces not in use right now. So let's make it easier to switch which one we are using but putting the default one into the aub_file struct. Cc: Lionel Landwerlin <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* intel/tools/aubwrite: Rename "legacy" to "Trace Block".Rafael Antognolli2018-08-221-1/+1
| | | | | | | Hopefully it's a little more descriptive, and more accurate. Cc: Lionel Landwerlin <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* intel: aubinator_viewer: add urb viewLionel Landwerlin2018-08-223-0/+172
| | | | | | | | | | This is available through a "Show URB" button on the 3DPRIMITIVE instructions. v2: Fix urb allocation end value in tooltip (Rafael) Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Rafael Antognolli <[email protected]>
* intel: aubinator_viewer: store urb state during decodingLionel Landwerlin2018-08-222-23/+153
| | | | | Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Rafael Antognolli <[email protected]>
* intel: tools: add aubinator viewerLionel Landwerlin2018-08-226-0/+2788
| | | | | | | | | | | | | | | | | A graphical user interface version of aubinator. Allows you to : - simultaneously look at multiple points in the aub file (using all the goodness of the existing decoding in aubinator) - edit an aub file v2: Switch from GLFW to GTK+3 v3: Fix warning when exiting Signed-off-by: Lionel Landwerlin <[email protected]> Acked-by: Rafael Antognolli <[email protected]> (v1)
* intel: tools: import ImGuiLionel Landwerlin2018-08-2219-2/+31693
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We want to add a new UI tool to decode aub files. This will use the Dear ImGui library to render its interface. The build of this UI toolkit is conditional to -Dwith_tools=intel-ui which superseeds -Dwith_tools=intel. The main way to use ImGui is to embed its source code at a particular revision. Most embedding projects have to do a bit of integration which is really specific to one's project. In our case the only modification is to include libepoxy. We also choose to use Gtk+3 for the window system integration. As oppose to the previous previous version of this patch using GLFW, Gtk+ is able to handle X11/Wayland session as well as property DPI scaling on retina monitors. The import was done at this commit (https://github.com/ocornut/imgui) : commit 6211f40f3d903dd9df961256e044029c49793aa3 Author: omar <[email protected]> Date: Fri Jul 27 12:29:33 2018 +0200 Internals: Drag and Drop: default drop preview use a narrower clipping rectangle (no effect here, but other branches uses a narrow clipping rectangle that was too small so this is a fix for it) + Comments v2: Switch from GLFW to GTK+ (Lionel) Signed-off-by: Lionel Landwerlin <[email protected]> Acked-by: Rafael Antognolli <[email protected]>
* intel: tools: aub_mem: reuse already mapped ppgtt buffersLionel Landwerlin2018-08-221-5/+11
| | | | | | | | | | | | | | | | | | | | When we map a PPGTT buffer into a continous address space of aubinator to be able to inspect it, we currently add it to the list of BOs to unmap once we're finished. An optimization we can apply it to look up that list before trying to remap PPGTT buffers again (we already do this for GGTT buffers). We need to take some care before doing this because the list also contains GGTT BOs. As GGTT & PPGTT are 2 different address spaces, we can have matching addresses in both that point to different physical locations. This changes adds a flag on the elements of the list of mapped BOs to differenciate between GGTT & PPGTT, which allows use to reuse that list when looking up both address spaces. Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Rafael Antognolli <[email protected]>
* intel: tools: aubmem: map gtt data to aub fileLionel Landwerlin2018-08-222-0/+35
| | | | | | | | This will allow the aubinator viewer tool to modify the aub data that was loaded at a particular gtt address. Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Rafael Antognolli <[email protected]>
* intel: tools: create libaubLionel Landwerlin2018-08-221-2/+12
| | | | Signed-off-by: Lionel Landwerlin <[email protected]>
* intel: tools: aubwrite: wrap function declarations for c++Lionel Landwerlin2018-08-221-0/+8
| | | | Reviewed-by: Rafael Antognolli <[email protected]>
* intel: tools: split memory management out of aubinatorLionel Landwerlin2018-08-225-353/+493
| | | | | Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Rafael Antognolli <[email protected]>
* intel: tools: split aub parsing from aubinatorLionel Landwerlin2018-08-225-279/+460
| | | | | | | v2: add parsing error callback (Lionel) Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Rafael Antognolli <[email protected]> (v1)
* anv: add VK_EXT_sampler_filter_minmax supportYunchao He2018-08-224-0/+43
| | | | | | | | | | | | | | | | | | This extension can be supported on SKL+. With this patch, all corresponding tests (6K+) in CTS can pass. No test fails. I verified CTS with the command below: deqp-vk --deqp-case=dEQP-VK.pipeline.sampler.view_type.*reduce* v2: 1) support all depth formats, not depth-only formats, 2) fix a wrong indention (Jason). v3: fix a few nits (Lionel). v4: fix failures in CI: disable sampler reduction when sampler reduction mode is not specified via this extension (Lionel). Reviewed-by: Lionel Landwerlin <[email protected]>
* anv/icl: Allow headerless sampler messages for pre-emptable contextsAnuj Phogat2018-08-212-0/+22
| | | | | | | | | It fixes simulator warnings in vulkancts tests complaining about missing support for headerless sampler messages for pre-emptable contexts. Bit 5 in SAMPLER MODE register is newly introduced for ICLLP. Signed-off-by: Anuj Phogat <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* anv/icl: Disable binding table prefetchingAnuj Phogat2018-08-211-6/+15
| | | | | | | | | | Gen 11 workarounds table #2056 WABTPPrefetchDisable suggests to disable prefetching of binding tables for ICLLP A0 and B0 steppings. We have a similar patch for i965 driver in Mesa commit a5889d70. Signed-off-by: Anuj Phogat <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* intel/genxml: minor python style fixEric Engestrom2018-08-211-1/+1
| | | | | Suggested-by: Dylan Baker <[email protected]> Signed-off-by: Eric Engestrom <[email protected]>
* intel/decoder: mark total_length as MAYBE_UNUSED in gen_spec_loadKai Wasserbäch2018-08-201-1/+2
| | | | | | | | | | | | | Only used, when asserts are enabled. Fixes an unused-variable warning with GCC 8: ../../../src/intel/common/gen_decoder.c: In function 'gen_spec_load': ../../../src/intel/common/gen_decoder.c:535:47: warning: variable 'total_length' set but not used [-Wunused-but-set-variable] uint32_t text_offset = 0, text_length = 0, total_length; ^~~~~~~~~~~~ Signed-off-by: Kai Wasserbäch <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* intel/tools: initialise bo_addr to 0 in mainKai Wasserbäch2018-08-201-1/+1
| | | | | | | Supresses a maybe-uninitialized warning with GCC 8. Signed-off-by: Kai Wasserbäch <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* intel: aubinator: mark ftruncate_res as MAYBE_UNUSED in ensure_phys_memKai Wasserbäch2018-08-201-1/+1
| | | | | | | | | | | | | Only used, when asserts are enabled. Fixes an unused-variable warning with GCC 8: ../../../src/intel/tools/aubinator.c: In function 'ensure_phys_mem': ../../../src/intel/tools/aubinator.c:209:11: warning: unused variable 'ftruncate_res' [-Wunused-variable] int ftruncate_res = ftruncate(mem_fd, mem_fd_len += 4096); ^~~~~~~~~~~~~ Signed-off-by: Kai Wasserbäch <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* intel/aubinator_error_decode: mark ret as MAYBE_UNUSED in mainKai Wasserbäch2018-08-201-1/+1
| | | | | | | | | | | | | Only used, when asserts are enabled. Fixes an unused-but-set-variable warning with GCC 8: ../../../src/intel/tools/aubinator_error_decode.c: In function 'main': ../../../src/intel/tools/aubinator_error_decode.c:759:11: warning: variable 'ret' set but not used [-Wunused-but-set-variable] int ret; ^~~ Signed-off-by: Kai Wasserbäch <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* anv/pipeline: Lower pipeline layouts etc. after linkingJason Ekstrand2018-08-171-30/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This allows us to use the link-optimized shader for determining binding table layouts and, more importantly, URB layouts. For apps running on DXVK, this is extremely important as DXVK likes to declare max-size inputs and outputs and this lets is massively shrink our URB space requirements. VkPipeline-db results (Batman pipelines only) on KBL: total instructions in shared programs: 820403 -> 790008 (-3.70%) instructions in affected programs: 273759 -> 243364 (-11.10%) helped: 622 HURT: 42 total spills in shared programs: 8449 -> 5212 (-38.31%) spills in affected programs: 3427 -> 190 (-94.46%) helped: 607 HURT: 2 total fills in shared programs: 11638 -> 6067 (-47.87%) fills in affected programs: 5879 -> 308 (-94.76%) helped: 606 HURT: 3 Looking at shaders by hand, it makes the URB between TCS and TES go from containing 32 per-vertex varyings per tessellation shader pair to a more reasonable 8-12. For a 3-vertex patch, that's at least half the URB space no matter how big the patch section is. Reviewed-by: Timothy Arceri <[email protected]>
* anv/pipeline: Set tess IO read/written key fields in compile_*Jason Ekstrand2018-08-171-9/+10
| | | | | | | | | We want these to be set as close to the final compile as possible so that they are guaranteed to happen after nir_shader_gather_info is called. The next commit is going to move nir_shader_gather_info to after the linking step which makes this necessary. Reviewed-by: Timothy Arceri <[email protected]>
* anv/pipeline: Use more fields from stage in compile_csJason Ekstrand2018-08-171-16/+21
| | | | | Reviewed-by: Timothy Arceri <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* anv/apply_pipeline_layout: Add to the bind map instead of replacing itJason Ekstrand2018-08-173-59/+21
| | | | | | | | | | | This commit makes three changes. One is to only walk the descriptors once and set bind map sizes at the same time as filling out the entries. The second is to make the pass additive so that we can put stuff in the bind map before applying the pipeline layout. Third, we switch to using designated initializers. Reviewed-by: Timothy Arceri <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* anv/lower_ycbcr: Use the binding array size for bounds checksJason Ekstrand2018-08-171-6/+4
| | | | | | | | | | | Because lower_ycbcr gets called before apply_pipeline_layout, the indices are all logical and the binding layout HW size is actually too big for the bounds check. We should just use the regular logical array size instead. Fixes: f3e91e78a33 "anv: add nir lowering pass for ycbcr textures" Reviewed-by: Timothy Arceri <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* anv: drop cast-to-void of used variableEric Engestrom2018-08-161-1/+0
| | | | | | | `device` is used 2 lines below, even visible in the diff context printed. Signed-off-by: Eric Engestrom <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* anv: use safer snprintf() to ensure NULL string-terminatorEric Engestrom2018-08-161-1/+1
| | | | | Signed-off-by: Eric Engestrom <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* intel/batch-decoder: replace local ARRAY_LENGTH() macro with global ARRAY_SIZE()Eric Engestrom2018-08-161-3/+2
| | | | | Signed-off-by: Eric Engestrom <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* intel: various python cleanupsEric Engestrom2018-08-165-26/+21
| | | | | Signed-off-by: Eric Engestrom <[email protected]> Reviewed-by: Emil Velikov <[email protected]>
* Revert "intel/nir: Call nir_lower_io_to_scalar_early"Jason Ekstrand2018-08-151-12/+5
| | | | | | | | | | | | | Commit 4434591bf56a6b0 caused substantially more URB messages in geometry and tessellation shaders. Before we can really enable this sort of optimization, We either need some way of combining them back together into vectors or we need to do cross-stage vector element elimination without splitting everything into scalars. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107510 Fixes: 4434591bf56a6 "intel/nir: Call nir_lower_io_to_scalar_early" Acked-by: Kenneth Graunke <[email protected]> Tested-by: Mark Janes <[email protected]>
* blorp: Properly handle Z24X8 blits.Kenneth Graunke2018-08-112-12/+11
| | | | | | | | | | | | | | | | | | | One of the reasons we didn't notice that R24_UNORM_X8_TYPELESS destinations were broken was that an earlier layer was swapping it out for B8G8R8A8_UNORM. That made Z24X8 -> Z24X8 blits work. However, R32_FLOAT -> R24_UNORM_X8_TYPELESS was still totally broken. The old code only considered one format at a time, without thinking that format conversion may need to occur. This patch moves the translation out to a place where it can consider both formats. If both are Z24X8, we continue using B8G8R8A8_UNORM to avoid having to do shader math workarounds. If we have a Z24X8 destination, but a non-matching source, we use our shader hacks to actually render to it properly. Fixes: 804856fa5735164cc0733ad0ea62adad39b00ae2 (intel/blorp: Handle more exotic destination formats) Reviewed-by: Jason Ekstrand <[email protected]>
* blorp: Don't try to use R32_UNORM for R24_UNORM_X8_TYPELESS rendering.Kenneth Graunke2018-08-111-5/+5
| | | | | | | | | | | | | The hardware doesn't support rendering to R24_UNORM_X8_TYPELESS, so Jason decided to fake it with a bit of shader math and R32_UNORM RTs. The only problem is that R32_UNORM isn't renderable either...so we've just traded one bad format for another. This patch makes us use R32_UINT instead. Fixes: 804856fa5735164cc0733ad0ea62adad39b00ae2 (intel/blorp: Handle more exotic destination formats) Reviewed-by: Jason Ekstrand <[email protected]>
* intel: Switch the order of the 2x MSAA sample positionsJason Ekstrand2018-08-112-5/+15
| | | | | | | | The Vulkan 1.1.82 spec flipped the order to better match D3D. Cc: [email protected] Reviewed-by: Iago Toral Quiroga <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* meson: Build with Python 3Mathieu Bridon2018-08-104-10/+10
| | | | | | | | | | | | Now that all the build scripts are compatible with both Python 2 and 3, we can flip the switch and tell Meson to use the latter. Since Meson already depends on Python 3 anyway, this means we don't need two different Python stacks to build Mesa. Signed-off-by: Mathieu Bridon <[email protected]> Reviewed-by: Eric Engestrom <[email protected]> Reviewed-by: Dylan Baker <[email protected]>
* intel: Fix SIMD16 unaligned payload GRF reads on Gen4-5.Kenneth Graunke2018-08-091-0/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the SIMD16 Gen4-5 fragment shader payload contains source depth (g2-3), destination stencil (g4), and destination depth (g5-6), the single register of stencil makes the destination depth unaligned. We were generating this instruction in the RT write payload setup: mov(16) m14<1>F g5<8,8,1>F { align1 compr }; which is illegal, instructions with a source region spanning more than one register need to be aligned to even registers. This is because the hardware implicitly does (nr | 1) instead of (nr + 1) when splitting the compressed instruction into two mov(8)'s. I believe this would cause the hardware to load g5 twice, replicating subspan 0-1's destination depth to subspan 2-3. This showed up as 2x2 artifact blocks in both TIS-100 and Reicast. Normally, we rely on the register allocator to even-align our virtual GRFs. But we don't control the payload, so we need to lower SIMD widths to make it work. To fix this, we teach lower_simd_width about the restriction, and then call it again after lower_load_payload (which is what generates the offending MOV). Fixes: 8aee87fe4cce0a883867df3546db0e0a36908086 (i965: Use SIMD16 instead of SIMD8 on Gen4 when possible.) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107212 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=13728 Reviewed-by: Jason Ekstrand <[email protected]> Tested-by: Diego Viola <[email protected]>
* anv: set error in all failure pathsEric Engestrom2018-08-091-1/+3
| | | | | | | | Cc: Jason Ekstrand <[email protected]> Fixes: 5b196f39bddc689742d3 "anv/pipeline: Compile to NIR in compile_graphics" Signed-off-by: Eric Engestrom <[email protected]> Reviewed-by: Tapani Pälli <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* intel/tools: add missing variable initialisationEric Engestrom2018-08-091-1/+1
| | | | | | Fixes: 6a60beba4089315685b8 "intel/tools: Add an error state to aub translator" Signed-off-by: Eric Engestrom <[email protected]> Reviewed-by: Lionel Landwerlin <[email protected]>
* python: Fix rich comparisonsMathieu Bridon2018-08-071-2/+3
| | | | | | | | | | | | | | Python 3 doesn't call objects __cmp__() methods any more to compare them. Instead, it requires implementing the rich comparison methods explicitly: __eq__(), __ne(), __lt__(), __le__(), __gt__() and __ge__(). Fortunately Python 2 also supports those. This commit only implements the comparison methods which are actually used by the build scripts. Signed-off-by: Mathieu Bridon <[email protected]> Reviewed-by: Dylan Baker <[email protected]>
* intel: don't build tools without -Dtools=intelLionel Landwerlin2018-08-072-15/+15
| | | | | | | Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107487 Fixes: 4334196ab325c6w ("intel: tools: simplify meson build") Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Eric Engestrom <[email protected]>
* anv: add more swapchain formatsTapani Pälli2018-08-061-5/+11
| | | | | | | | This change helps with some of the dEQP-VK.wsi.android.* tests that try to create swapchain with using such formats. Signed-off-by: Tapani Pälli <[email protected]> Reviewed-by: Chad Versace <[email protected]>
* intel: tools: simplify meson buildLionel Landwerlin2018-08-041-46/+50
| | | | | | | | Remove the if tools condition and just put it through the install: parameter. Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Eric Engestrom <[email protected]>
* intel: aubinator: simplify decodingLionel Landwerlin2018-08-041-10/+5
| | | | | | | | | | Since we don't support streaming an aub file, we can drop the decoding status enum. v2: include stdbool (Eric) Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Eric Engestrom <[email protected]>
* intel: common: add missing stdint includeLionel Landwerlin2018-08-041-0/+2
| | | | Reviewed-by: Eric Engestrom <[email protected]>
* intel: decoder: remove unused variableLionel Landwerlin2018-08-041-2/+0
| | | | | Signed-off-by: Lionel Landwerlin <[email protected]> Reviewed-by: Eric Engestrom <[email protected]>