aboutsummaryrefslogtreecommitdiffstats
path: root/src/mesa/drivers/dri/intel
Commit message (Collapse)AuthorAgeFilesLines
* intel: Remove a never-taken debug print path.Eric Anholt2013-03-301-5/+0
| | | | | | | | Alessandro Pignotti noted when I added this code in commit 0e723b135bfd59868c92c3ae243f1adaedaec3a5 that it's in the else block for "if (busy)", so this debug print couldn't happen. Reviewed-by: Kenneth Graunke <[email protected]>
* i965: Enable ARB_texture_query_lod.Matt Turner2013-03-291-1/+3
| | | | | v2: Support Ironlake as well. Reviewed-by: Kenneth Graunke <[email protected]>
* i965: Add a driconf option to disable flush throttling.Paul Berry2013-03-213-2/+10
| | | | | | | | | | | | | | | Normally when submitting the first batch buffer after a flush, we check whether the GPU has completed processing of the first batch buffer of the previous frame. If it hasn't, we wait for it to finish before submitting any more batches. This prevents GPU-heavy and CPU-light applications from racing too far ahead of the current frame, but at the expense of possibly lower frame rates. Sometimes when benchmarking we want to disable this mechanism. This patch adds the driconf option "disable_throttling" to disable the throttling mechanism. Reviewed-by: Eric Anholt <[email protected]>
* i965/gen7: Align all depth miplevels to 8 in the X direction.Eric Anholt2013-03-201-1/+9
| | | | | | | | | | | | | On an INTEL_DEBUG=perf piglit run on IVB, reduces the instances of "HW workaround: blit" (the printouts from the misaligned-depth workaround blits) from 725 to 675. It doesn't totally eliminate the workaround blit, because we still have problems with Y offsets that we can't fix (since texturing can only align miplevels up to 2 or 4, not 8). No regressions on piglit/es3conform on IVB. Reviewed-by: Kenneth Graunke <[email protected]>
* i965: Avoid unnecessary copy when depthstencil workaround invoked by clear.Paul Berry2013-03-195-8/+19
| | | | | | | | | | | | | | | | | | | | | | | | | Since apps typically begin rendering with a call to glClear(), it is likely that when brw_workaround_depthstencil_alignment() moves a miplevel to a temporary buffer, it can avoid doing a blit, since the contents of the miplevel are about to be erased. This patch adds the necessary plumbing to determine when brw_workaround_depthstencil_alignment() is being called as a consequence of glClear(), and avoids the unnecessary blit when it is safe to do so. Reviewed-by: Chad Versace <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]> v2: Eliminate unnecessary call to _mesa_is_depthstencil_format(). Fix handling of depth buffer in depth/stencil format. v3: Use correct bitfields for clear_mask. Fix handling of depth buffer in depth/stencil format when hardware uses separate stencil. When invalidating, make sure we still reassociate the image to the new miptree. Reviewed-by: Eric Anholt <[email protected]>
* Add dri image entry point for creating image from fdKristian Høgsberg2013-03-183-4/+94
| | | | Reviewed-by: Ander Conselvan de Oliveira <[email protected]>
* i965/blorp: Add INTEL_DEBUG=blorp flag.Paul Berry2013-03-182-0/+2
| | | | | | | | | | | This debug flag prints out the native GEN assembly for a blitting shader produced using BLORP. Hopefully this should be useful in developing additional BLORP features. Reviewed-by: Matt Turner <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Anuj Phogat <[email protected]> Reviewed-by: Chad Versace <[email protected]>
* i965: Simplify separate stencil checkPaul Berry2013-03-161-1/+1
| | | | | | | | The only format returned by _mesa_get_format_base_format() that satisfies _mesa_is_depthstencil_format() is GL_DEPTH_STENCIL, so we can simplify the check. Reviewed-by: Eric Anholt <[email protected]>
* intel: Remove some unused debug flags.Eric Anholt2013-03-112-8/+0
| | | | | | | I was looking at the list to see what might be interesting to document for application developers, and it turns out some are completely dead. Reviewed-by: Kenneth Graunke <[email protected]>
* intel: Improve the matching (more formats!) for TexImage from PBOs.Eric Anholt2013-03-051-25/+3
| | | | | | | Mesa core is the place for encoding what format/type matches a mesa format, so rely on that. Reviewed-by: Chad Versace <[email protected]>
* intel: Improve the test for readpixels blit path format checking.Eric Anholt2013-03-053-37/+6
| | | | | | | | We were allowing things like copying RG1616 to a user's ARGB8888 format, while we were denying anything that wasn't ARGB8888 or RGB565. Reviewed-by: Chad Versace <[email protected]>
* intel: Fold intel_region_copy() into its one caller.Eric Anholt2013-03-053-53/+16
| | | | | | | | | This is similar code to intel_miptree_copy_slice, but the knobs are all set differently. v2: fix whitespace Reviewed-by: Chad Versace <[email protected]>
* intel: Transition intel_region_map() to being a miptree operation.Eric Anholt2013-03-055-91/+55
| | | | | | | | | | I'm trying to move us away from the region structure, and all the callers are currently dereferencing a miptree to get the region. In this change, the map_refcount is dropped. However, the bo->virtual is itself map refcounted, so that's already dealt with. Reviewed-by: Chad Versace <[email protected]>
* intel: Remove num_mapped_regions tracking.Eric Anholt2013-03-052-14/+0
| | | | | | | | The point of tracking the value was removed in February 2012 (65b096aeddd9b45ca038f44cc9adfff86c8c48b2), and this should have been removed at the same time. Reviewed-by: Chad Versace <[email protected]>
* intel: Remove the struct intel_region reuse hash table.Eric Anholt2013-03-054-39/+2
| | | | | | | | | | | | I don't see any reason for it -- it was introduced with the DRI2 invalidate work by krh in 2010 with no explanation. I suspect it was something about wanting the same drm_intel_bo struct underneath multiple openings of the BO within one process, but that's covered by libdrm at this point. As far as the struct region goes, it is not threadsafe, so multiple contexts sharing a region could have mixed up the map_count and assertion failed or worse. Reviewed-by: Chad Versace <[email protected]>
* intel: Add missing perf debug for a stall on mapping a BO.Eric Anholt2013-03-051-0/+2
| | | | | | | | I was testing the ARB_debug_output code and wrote an obvious sample that should have hit this, and got confused that my ARB_debug_output was broken. Reviewed-by: Jordan Justen <[email protected]>
* i965: Make perf_debug() output to GL_ARB_debug_output in a debug context.Eric Anholt2013-03-056-7/+21
| | | | | | | | I tried to ensure that performance in the non-debug case doesn't change (we still just check one condition up front), and I think the impact is small enough in the debug context case to warrant including all of it. Reviewed-by: Jordan Justen <[email protected]>
* intel: Finish renaming fallback_debug() to perf_debug().Eric Anholt2013-03-055-18/+13
| | | | | | | They're about to change to handle GL_ARB_debug_output, so just make one function. Reviewed-by: Jordan Justen <[email protected]>
* intel: Hook up the WARN_ONCE macro to GL_ARB_debug_output.Eric Anholt2013-03-051-0/+5
| | | | | | | | | | This doesn't provide detailed error type information, but it's important to get these relatively severe but rare error messages out to the developer through whatever mechanism they are using. v2: Rebase on new WARN_ONCE additions. Reviewed-by: Jordan Justen <[email protected]> (v1)
* i965: Fix Crystal Well PCI IDs.Kenneth Graunke2013-03-031-9/+9
| | | | | | | | | The second digit was off by one, which meant we accidentally treated GTn as GT(n-1). This also meant no support for GT1 at all. NOTE: This is a candidate for stable branches. Signed-off-by: Kenneth Graunke <[email protected]>
* i965: enable ARB_texture_multisample on Gen6+Chris Forbes2013-03-021-0/+1
| | | | | | | | V2: Works on Ivy Bridge now too, so this can be 6+. Signed-off-by: Chris Forbes <[email protected]> Reviewed-by: Paul Berry <[email protected]> Reviewed-by: Eric Anholt <[email protected]>
* i965: take the target into account for Gen7 MSAA modesChris Forbes2013-03-021-3/+19
| | | | | | | | | | | | | | | | | | | | | Gen7 has an erratum affecting the ld_mcs message, making it unsafe to use when the surface doesn't have an associated MCS. From the Ivy Bridge PRM, Vol4 Part1 p77 ("MCS Enable"): "If this field is disabled and the sampling engine <ld_mcs> message is issued on this surface, the MCS surface may be accessed. Software must ensure that the surface is defined to avoid GTT errors." To allow the shader to treat all surfaces uniformly, force UMS if the surface is to be used as a multisample texture, even if CMS would have been possible. V3: - Quoted erratum text Signed-off-by: Chris Forbes <[email protected]> Reviewed-by: Eric Anholt <[email protected]>
* i965: add support for multisample texturesChris Forbes2013-03-024-7/+52
| | | | | | | | | | | V2: - Fix for state moving from texobj to image - Rebased onto Paul's logical/physical cleanup - Fixed missing quantization of sample count - Fold in IMS renderbuffer wrapper fixes from later in the series - Use correct physical slice offset for UMS/CMS surfaces on Gen7 Signed-off-by: Chris Forbes <[email protected]> Reviewed-by: Paul Berry <[email protected]>
* intel: Use the new "ctx" local variable I just added some more.Eric Anholt2013-03-011-2/+2
| | | | Reviewed-and-tested-by: Ian Romanick <[email protected]>
* i965: Make sRGB-capable framebuffers by default.Eric Anholt2013-03-012-3/+63
| | | | | | | | | | | | | The GLX extension lets you expose visuals that explicitly guarantee you that the GL_FRAMEBUFFER_SRGB_CAPABLE flag will be set, but we can set the flag even while the visual doesn't provide the guarantee. This appears to be consistent with other implementations, as we've seen several apps now that don't require an srgb visual and assume sRGB will work without checking the GL_FRAMEBUFFER_SRGB_CAPABLE flag. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55783 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=60633 Reviewed-and-tested-by: Ian Romanick <[email protected]>
* intel: Fix software copying of miptree faces for weird formats.Eric Anholt2013-03-013-61/+77
| | | | | | | | | | | Now that we have W-tiled S8, we can't just region_map and poke at bits -- there has to be some swizzling. Rely on intel_miptree_map to get that job done. This should also get the highest performance path we know of for the mapping (interesting if I get around to finishing movntdqa some day). v2: Fix stale name of the bit in a comment. Reviewed-by: Chad Versace <[email protected]>
* intel: Add a flag for miptree mapping to disable transcoding.Eric Anholt2013-03-012-4/+17
| | | | | | | | I want to reuse intel_miptree_map() to replace some region mapping that's broken for separate stencil, but doing so would result in new demands on ETC transcode that we actually don't want to happen. Reviewed-by: Chad Versace <[email protected]>
* intel: Enable __DRI_API_OPENGL_CORE api with dri2 contextsJordan Justen2013-02-281-0/+2
| | | | | | | | | | | | | Without this set, dri_util.c:dri2CreateContextAttribs will reject requests to create a context with __DRI_API_OPENGL_CORE. This prevents a 3.2 core profile context from being created even when MESA_GL_OVERRIDE_VERSION=3.2 is used. Signed-off-by: Jordan Justen <[email protected]> Reviewed-by: Eric Anholt <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* intel: update max versions based on MESA_GL_VERSION_OVERRIDEJordan Justen2013-02-281-0/+10
| | | | | | | | | If the override is version is >= 3.1, then update the max_gl_core_version. Otherwise, update max_gl_compat_version. Signed-off-by: Jordan Justen <[email protected]> Reviewed-by: Eric Anholt <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* intel: Remove intel_mipmap_tree::wraps_etcChad Versace2013-02-282-21/+3
| | | | | | | | | | | | | | The field was equivalent to (etc_format != MESA_FORMAT_NONE), and therefore duplicate information. This patch removes field and replaces all references to it with `etc_format != MESA_FORMAT_NONE`. No Piglit ETC test regresses on Intel Sandybridge. Reviewed-by: Kenneth Graunke <[email protected]> Reviewed-by: Anuj Phogat <[email protected]> Signed-off-by: Chad Versace <[email protected]>
* i965: Enable OpenGL ES 3.0 on Sandy BridgeIan Romanick2013-02-221-1/+1
| | | | | | | | | | Regardless of what we put in the screen structure, all of the extensions that compute_version_es2 checks are present and 3.0 will be exposed anyway. NOTE: This is a candidate for the 9.1 branch. Signed-off-by: Ian Romanick <[email protected]>
* intel: Allow blit readpixels even when the pack alignment is set.Eric Anholt2013-02-131-9/+4
| | | | | | | | | | The default alignment is 4, so this fast path was rarely hit. Rather than introduce logic to handle alignment, just use the Mesa core function. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46632 Cc: [email protected] Reviewed-by: Kenneth Graunke <[email protected]>
* intel: Do not expose OES_compressed_ETC1_RGB8_texture or ↵Ian Romanick2013-02-081-2/+2
| | | | | | | | | | | | | | ARB_texture_rgb10_a2ui pre-GEN4 Older hardware cannot do ARB_texture_rgb10_a2ui, and the translation code for OES_compressed_ETC1_RGB8_texture was never implemented in the i915 driver. NOTE: This is a candidate for all stable branches. Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* intel: Ensure variable intel is used in i915 builds.Vinson Lee2013-02-081-1/+1
| | | | | | | | Fixes unused pointer value defect reported by Coverity. Signed-off-by: Vinson Lee <[email protected]> Reviewed-by: Brian Paul <[email protected]> Reviewed-by: Ian Romanick <[email protected]>
* Consolidate some redundant definitions of ARRAY_SIZE() macro.Paul Berry2013-02-082-2/+1
| | | | | | | | | | | | | | | | | | | | | Previous to this patch, there were 13 identical definitions of this macro in Mesa source. That's ridiculous. This patch consolidates 6 of them to a single definition in src/mesa/main/macros.h. Unfortunately, I wasn't able to eliminate the remaining definitions, since they occur in places that don't include src/mesa/main/macros.h: - include/pci_ids/pci_id_driver_map.h - src/egl/drivers/dri2/egl_dri2.h - src/egl/main/egldefines.h - src/gbm/main/backend.c - src/gbm/main/gbm.c - src/glx/glxclient.h - src/mapi/mapi/stub.c I'm open to suggestions as to how to deal with the remaining redundancy. Reviewed-by: Kenneth Graunke <[email protected]>
* intel/pre-gen6: Disable EXT_framebuffer_multisample.Paul Berry2013-02-083-12/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the i965 driver enabled EXT_framebuffer_multisample even on pre-gen6 chipsets. However, since we don't support multisampling on these chips, we set GL_MAX_SAMPLES=1 (the minimum allowed by EXT_framebuffer_multisample), and if the client ever requested a multisample buffer, we quietly supplied them with a single-sampled buffer instead. After some discussion on the mailing list (see thread "ext_framebuffer_multisample: check for num_samples<=1"), it's clear that this was the wrong approach. The correct approach is to only expose EXT_framebuffer_multisample when we truly support multisampling; that frees us to set a sensible value of GL_MAX_SAMPLES=0 on other chipsets, so that we never have to deal with a client requesting a multisample buffer when multisampling isn't supported. This change causes the following piglit tests to be skipped on chipsets prior to Gen6: - "ARB_framebuffer_sRGB/blit {renderbuffer,texture} {linear,linear_to_srgb,srgb,srgb_to_linear} {downsample,msaa,upsample} {disabled,enabled}" - EXT_framebuffer_multisample/blit-mismatched-formats - EXT_framebuffer_multisample/blit-mismatched-sizes - EXT_framebuffer_multisample/dlist - EXT_framebuffer_multisample/interpolation 0 * - EXT_framebuffer_multisample/minmax - EXT_framebuffer_multisample/negative-copypixels - EXT_framebuffer_multisample/negative-copyteximage - EXT_framebuffer_multisample/negative-max-samples - EXT_framebuffer_multisample/negative-mismatched-samples - EXT_framebuffer_multisample/negative-readpixels - EXT_framebuffer_multisample/renderbuffer-samples - EXT_framebuffer_multisample/renderbufferstorage-samples - EXT_framebuffer_multisample/samples This is expected, since the above tests exercise MSAA functionality, and shouldn't be run on systems prior to Gen6. Reviewed-by: Eric Anholt <[email protected]>
* i965: Implement CopyTexSubImage2D via BLORP (and use it by default).Kenneth Graunke2013-02-063-8/+58
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The BLT engine has many limitations. Currently, it can only blit X-tiled buffers (since we don't have a kernel API to whack the BLT tiling mode register), which means all depth/stencil operations get punted to meta code, which can be very CPU-intensive. Even if we used the BLT engine, it can't blit between buffers with different tiling modes, such as an X-tiled non-MSAA ARGB8888 texture and a Y-tiled CMS ARGB8888 renderbuffer. This is a fundamental limitation, and the only way around that is to use BLORP. Previously, BLORP only handled BlitFramebuffer. This patch adds an additional frontend for doing CopyTexSubImage. It also makes it the default. This is partly to increase testing and avoid hiding bugs, and partly because the BLORP path can already handle more cases. With trivial extensions, it should be able to handle everything the BLT can. This helps PlaneShift massively, which tries to CopyTexSubImage2D between depth buffers whenever a player casts a spell. Since these are Y-tiled, we hit meta and software ReadPixels paths, eating 99% CPU while delivering ~1 FPS. This is particularly bad in an MMO setting because people cast spells all the time. It also helps Xonotic in 4X MSAA mode. At default power management settings, I measured a 6.35138% +/- 0.672548% performance boost (n=5). (This data is from v1 of the patch.) No Piglit regressions on Ivybridge (v3) or Sandybridge (v2). v2: Create a fake intel_renderbuffer to wrap the destination texture image and then reuse do_blorp_blit rather than reimplementing most of it. Remove unnecessary clipping code and conditional rendering check. v3: Reuse formats_match() to centralize checks; delete temporary renderbuffers. Reorganize the code. v4: Actually copy stencil when dealing with separate stencil buffers but packed depth/stencil formats. Tested by a new Piglit test. NOTE: This is a candidate for the 9.1 branch. Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Paul Berry <[email protected]> [v4] Reviewed-by: Ian Romanick <[email protected]> [v3] Reviewed-and-tested-by: Carl Worth <[email protected]> [v2] Tested-by: Martin Steigerwald <[email protected]> [v3]
* mesa: fixup inconsistent naming of RG16 formatsMarek Olšák2013-02-061-1/+1
| | | | Reviewed-by: Brian Paul <[email protected]>
* intel: Fix regression in intel_create_image_from_name stride handlingTapani Pälli2013-02-041-1/+1
| | | | | | | | | Strangely, the DRIimage interface we have passes the pitch in pixels instead of bytes, which anholt missed in the change to using bytes for region pitch. Signed-off-by: Tapani Pälli <[email protected]> Reviewed-by: Eric Anholt <[email protected]>
* intel: make sure to setup image dimension in image_from_planar setupAbdiel Janulgue2013-02-041-0/+1
| | | | | | | | Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=60212 Tested-by: Scott Moreau <[email protected]> Tested-by: Tiago Vignatti <[email protected]> Reviewed-by: Chad Versace <[email protected]> Signed-off-by: Abdiel Janulgue <[email protected]>
* intel: implement create image from textureAbdiel Janulgue2013-02-011-21/+138
| | | | | | | | | | | | | | | | | Save miptree level info to DRIImage: - Appropriately-aligned base offset pointing to the image - Additional x/y adjustment offsets from above. v8: -Bump intelImageExtension version v9: -Don't use internal _eglError but implement error reporting in new DRI inteface instead. This fixes Android build problems based on feedback from Adrian M Negreanu and Chad Versace. -Move the non-tile-aligned check and error-reporting to intel_set_texture_image_region v10: -Don't #include "egl/main/eglcurrent.h". [chadv] Reviewed-by: Eric Anholt <[email protected]> (v6) Acked-by: Chad Versace <[email protected]> (v10) Signed-off-by: Abdiel Janulgue <[email protected]>
* intel: Account for mt->offset in intel_miptree_mapAbdiel Janulgue2013-02-011-2/+2
| | | | | | | | | | | We need to take account the offset from original bo when using glTexSubImage() and other functions that manipulate the subregion of an exported texture. Offsets are appended to mapped region address and when blitting from a source region. Reviewed-by: Eric Anholt <[email protected]> Reviewed-by: Chad Versace <[email protected]> Signed-off-by: Abdiel Janulgue <[email protected]>
* intel: Create a miptree using offsets in intel_set_texture_image_regionAbdiel Janulgue2013-02-011-7/+53
| | | | | | | | | | | | | | | | When binding a region to a texture image, re-create the miptree base-level considering the offset and dimension information exported by DRIImage. v8: - Move the alignment surface address checks from the image-from-texture code to the texture-from-image side. This allows the error reporting to conform to OES_EGL_Image and to prevent mixing up EGL and GL errors. Reported by Chad Versace. - Addressed an existing issue in renderbuffer case where there is a a possibility of creating EGL images out of depthstencil textures which isn't really possible. This was spotted by Eric earlier. Reviewed-by: Eric Anholt <[email protected]> (v6) Reviewed-by: Chad Versace <[email protected]> (v8) Signed-off-by: Abdiel Janulgue <[email protected]>
* intel: add pixel offset calculator for miptree levelsAbdiel Janulgue2013-02-012-0/+21
| | | | | | | | | Add helper to calculate fine-grained x and y adjustment pixels to an image within a miptree level for tiled regions. Reviewed-by: Eric Anholt <[email protected]> Reviewed-by: Chad Versace <[email protected]> Signed-off-by: Abdiel Janulgue <[email protected]>
* intel: Expose intel_miptree_create_internal as intel_miptree_create_layout.Abdiel Janulgue2013-02-012-14/+25
| | | | | | Reviewed-by: Eric Anholt <[email protected]> Reviewed-by: Chad Versace <[email protected]> Signed-off-by: Abdiel Janulgue <[email protected]>
* intel: expose dimensions and offsets of a miptree level in DRIImageAbdiel Janulgue2013-02-011-0/+7
| | | | | | | | v8: - Append has_depthstencil field in DRIImage structure. Reviewed-by: Eric Anholt <[email protected]> (v6) Reviewed-by: Chad Versace <[email protected]> (v8) Signed-off-by: Abdiel Janulgue <[email protected]>
* intel: Un-hardcode lengths from blitter commands.Kenneth Graunke2013-01-282-7/+7
| | | | | | | | | The packet length may change at some point in the future. Specifying it explicitly (rather than hardcoding it in the command #define) allows us to change it much more easily in the future. Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Eric Anholt <[email protected]>
* intel: Use a CPU map of the batch on LLC-sharing architectures.Eric Anholt2013-01-294-9/+24
| | | | | | | | | | | | | | | | Before, we were keeping a CPU-only buffer to accumulate the batchbuffer in, which was an improvement over mapping the batch through the GTT directly (since any readback or other failure to stream through write combining correctly would hurt). However, on LLC-sharing architectures we can do better by mapping the batch directly, which reduces the cache footprint of the application since we no longer have this extra copy of a batchbuffer around. Improves performance of GLBenchmark 2.1 offscreen on IVB by 3.5% +/- 0.4% (n=21). Improves Lightsmark performance by 1.1 +/- 0.1% (n=76). Improves cairo-gl performance by 1.9% +/- 1.4% (n=57). No statistically significant difference in GLB2.1 on SNB (n=37). Improves cairo-gl performance by 2.1% +/- 0.1% (n=278).
* intel: Typo fix: "pitsh" -> "pitch"Paul Berry2013-01-281-1/+1
| | | | Comment change only.
* i965: Enable ARB_shading_language_packingMatt Turner2013-01-251-0/+1
| | | | | Reviewed-by: Chad Versace <[email protected]> Reviewed-by: Paul Berry <[email protected]>