summaryrefslogtreecommitdiffstats
path: root/src/mesa
Commit message (Collapse)AuthorAgeFilesLines
* mesa: Use _aligned_malloc/free for MinGW too.Jose Fonseca2016-02-121-4/+4
| | | | | | | | We already use these for gallium in src/gallium/auxiliary/os/os_memory_stdc.h and it's always better to minimize divergences between MinGW and MSVC. Reviewed-by: Brian Paul <[email protected]>
* mesa: Remove support for MSVC2008.Jose Fonseca2016-02-121-3/+0
| | | | | | Spotted by Emil Velikov. Trivial.
* i965: Stop considering if msrt aux buffers need aux bufferTopi Pohjolainen2016-02-121-10/+10
| | | | | | | | | | | | | | | Auxiliary buffers are always created with sample number of zero which effectively prevents intel_miptree_create_layout() from trying to associate auxiliary buffers with auxiliary buffers. Now that there is more direct path available lets start using it instead and stop even checking for such (im)possibility. v2 (Ben): Do not signal msaa layout with explicit argument but using layout_flags instead. Signed-off-by: Topi Pohjolainen <[email protected]> Reviewed-by: Ben Widawsky <[email protected]>
* i965: Separate miptree creation from auxiliary buffer setupTopi Pohjolainen2016-02-121-17/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently the logic allocating and setting up miptrees is closely combined with decision making when to re-allocate buffers in X-tiled layout and when to associate colors with auxiliary buffers. These auxiliary buffers are in turn also represented as miptrees and are created by the same miptree creation logic calling itself recursively. This means considering in vain if the auxiliary buffers should be represented in X-tiled layout or if they should be associated with auxiliary buffers again. While this is somewhat unnecessary, this doesn't impose any problems currently. Miptrees for auxiliary buffers are created as simgle-sampled fusing the consideration for multi-sampled compression auxiliary buffers. The format in turn is such that is not applicable for single-sampled fast clears (that would require accompaning auxiliary buffer). But once the driver starts to support lossless compression of color buffers the auxiliary buffer will have a format that would itself be applicable for lossless compression. This would be rather difficult and ugly to detect in the current miptree creation logic, and therefore this patch seeks to separate the association logic from the general allocation and setup steps. v2 (Ben): - Do not reconsider for X-tiling in intel_miptree_create() as it was just forced to Y-tiling in miptree_create(). - Do not drop checks for allocation failures. Signed-off-by: Topi Pohjolainen <[email protected]>
* i965: Isolate aligned dimensions for stencil onlyTopi Pohjolainen2016-02-121-15/+14
| | | | | | | | This makes the logic a little more explicit and helps to keep subsequent patches easier to read. Suggested-by: Ben Widawsky <[email protected]> Signed-off-by: Topi Pohjolainen <[email protected]>
* i965: Restore vbo after color resolve during brw_try_draw_prims()Topi Pohjolainen2016-02-121-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Part of brw_try_draw_prims() is a check to validate textures (brw_validate_textures()). In case of textures that currently have only level zero but are marked for mipmap generation, i965 driver will decide to replace the underlying buffer with a larger one capable of holding also the additional levels. This results into blit from the original buffer to the newly allocated (see intel_miptree_copy_teximage()). This blit is currently handled with blitter engine and hence it won't effect the ongoing draw operation. However, this blit in turn may trigger color resolve on the source buffer. In principle, this should be possible with fast cleared buffers but I only started hitting it when I enabled lossless compression (that reguires similar resolve to fast cleared buffers). Now, the color resolve is a meta operation and uses the same drawing path we are already in middle of. After quite a bit of debugging I realized that the resolve will modify the current vbo setup but it won't restore it afterwards resulting in the original draw call using wrong vertex data. When brw_try_draw_prims() gets called, the vbo logic in the Mesa core (see vbo_draw_arrays()) has just bound the vbo (see vbo_bind_arrays() and recalculate_input_bindings()). Color resolve operation will overwrite the vbo setup by calling vbo_bind_arrays() against the resolve rectangle (see brw_draw_rectlist()). Once the color resolve is done the vbo setup is left to the resolve rectangle state and the original drawing call yields bogus results. This patch aims to restore the original state after the color resolve by calling vbo_bind_arrays() yet again after the vertex array state in the core context have been restored. Now having said all this, I'd also like to state that I'm quite uncomfortable with the nested meta operations. Ths original draw call in this case is in fact a meta operation itself. It is a blit from level zero to level one when generating the additional mipmap levels (see _mesa_meta_GenerateMipmap()). Imagine the complexity if the blit in the middle from buffer to another would go to meta path also instead of blitter. I would very tempted to try to move all the resolves to happen before a meta operation is started. Additionally I still feel that work I did earlier in the spring/ summer time moving meta operations to use direct state upload bypassing the core context would make sense. v2: Force input recalculation by setting the flag explicitly v3: Do not attempt to restore vbo for opengles1 which doesn't support vertex buffer objects. Signed-off-by: Topi Pohjolainen <[email protected]>
* i965: Validate textures before altering driver stateTopi Pohjolainen2016-02-121-9/+9
| | | | | | | | | | Validation may kick off copies and subsequently color resolves. Color resolves (and the copies themselves if ending up in meta path) will overwrite the internal driver state but are not prepared to restore it. Instead of adding that capability the validation can be simply performed before the state is updated. Signed-off-by: Topi Pohjolainen <[email protected]>
* i965: Make brw_clear_cache flag all the bits on both pipelines.Kenneth Graunke2016-02-111-2/+6
| | | | | | | | | | | | | | | | Setting brw->ctx.NewDriverState and brw->ctx.NewGLState affects the dirty bits for the current pipeline. But, we need to flag everything dirty on *both* pipelines, so that when we switch back, we'll realize our programs are stale and re-upload them. To accomplish this, flag the saved state for both pipelines. Only one of them should matter, but this way we don't have to check which we need to set. It's harmless to set the other. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93790 Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Jordan Justen <[email protected]> Tested-by: Ilia Mirkin <[email protected]>
* i965: Consider tessellation in get_pipeline_state_l3_weights.Kenneth Graunke2016-02-111-1/+6
| | | | | | | | I think this was just missed; Curro and I were probably writing code simultaneously and forgot to combine them at the end. Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Jordan Justen <[email protected]>
* i965: Split brw_upload_texture_surfaces into compute/render atoms.Kenneth Graunke2016-02-113-9/+34
| | | | | | | | | | | When uploading state for the compute pipeline, we don't want to look at VS/TCS/TES/GS/FS programs, as they might be stale, and aren't relevant anyway. Likewise, the render pipeline shouldn't look at CS. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93790 Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Jordan Justen <[email protected]>
* i965/gs: Pass VerticesIn though prog_dataJason Ekstrand2016-02-113-1/+5
| | | | Reviewed-by: Kenneth Graunke <[email protected]>
* i965/fs: Pass usage of depth, W, and sample mask through prog_dataJason Ekstrand2016-02-116-20/+30
| | | | | | | | | | We really need to stop pulling information directly out of shaders for state setup. For one thing, if we want any sort of an on-disk shader cache, having all of this metadata in one place is going to be crucial. Also, passing it all through prog_data cleans up the compiler <-> state setup API substantially. Reviewed-by: Kenneth Graunke <[email protected]>
* i965/fs: Refactor setup_payload_gen6 to assume FSJason Ekstrand2016-02-113-15/+15
| | | | | | | | | It's extremely FS specific so the fact that we have a stage check in the middle of it is rather bogus. While were here, we rename setup_payload_gen4 and setup_payload_gen6 to make it obvious that they are both FS specific. Reviewed-by: Kenneth Graunke <[email protected]>
* i965: ir: dump floats as %-g rather than %f, so we can see denormalsChris Forbes2016-02-111-1/+1
| | | | | Signed-off-by: Chris Forbes <[email protected]> Reviewed-by: Ben Widawsky <[email protected]>
* i965/gen7: Require kernel cmd_parser 5 for ARB_compute_shaderJordan Justen2016-02-111-1/+2
| | | | | | | | The indirect dispatch registers were whitelisted in command parser version 5. (Version 5 is available as of Linux 4.4) Signed-off-by: Jordan Justen <[email protected]> Reviewed-by: Kenneth Graunke <[email protected]>
* st/mesa: release GLSL IR in LinkShader after it's not neededMarek Olšák2016-02-111-0/+4
| | | | Reviewed-by: Brian Paul <[email protected]>
* mesa: call build_program_resource_list inside Driver.LinkShaderMarek Olšák2016-02-113-2/+7
| | | | | | to allow LinkShader to free the GLSL IR. Reviewed-by: Ian Romanick <[email protected]>
* st/mesa: use correct pipe functions to create tess shadersMarek Olšák2016-02-113-7/+27
| | | | | | | | | Broken by one of my cleanups. Spotted by luck. Radeonsi doesn't care, because all shader create callbacks go to the same function. Reviewed-by: Brian Paul <[email protected]>
* nir: Remove the const_offset from nir_tex_instrJason Ekstrand2016-02-102-25/+22
| | | | | | | | | | | When NIR was originally drafted, there was no easy way to determine if something was constant or not. The result was that we had lots of special-casing for constant values such as this. Now that load_const instructions are SSA-only, it's really easy to find constants and this isn't really needed anymore. Reviewed-by: Connor Abbott <[email protected]> Reviewed-by: Rob Clark <[email protected]>
* i965: Make sure we blit a full compressed blockBen Widawsky2016-02-101-0/+14
| | | | | | | | | | | | | | | | | | | | | | | | | This fixes an assertion failure in [at least] one of the Unreal Engine Linux demo/games that uses DXT1 compression. Specifically, the "Vehicle Game". At some point, the game ends up trying to blit mip level whose size is 2x2, which is smaller than a DXT1 block. As a result, the assertion in the blit path is triggered. It should be safe to simply make sure we align the width and height, which is sadly an example of compression being less efficient. NOTE: The demo seems to work fine without the assert, and therefore release builds of mesa wouldn't stumble over this. Perhaps there is some unnoticeable corruption, but I had trouble spotting it. Thanks to Jason for looking at my backtrace and figuring out what was going on. v2: Use NPOT alignment to make sure ASTC is handled properly (Ilia) Remove comment about how this doesn't fix other bugs, because it does. Cc: "11.0 11.1" <[email protected] Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93358 Signed-off-by: Ben Widawsky <[email protected]> Tested-by: Matt Turner <[email protected]> Reviewed-by: Jason Ekstrand <[email protected]>
* meta/decompress: Don't pollute the renderbuffer namespaceIan Romanick2016-02-101-6/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | tl;dr: For many types of GL object, we can *NEVER* use the Gen function. In OpenGL ES (all versions!) and OpenGL compatibility profile, applications don't have to call Gen functions. The GL spec is very clear about how you can mix-and-match generated names and non-generated names: you can use any name you want for a particular object type until you call the Gen function for that object type. Here's the problem scenario: - Application calls a meta function that generates a name. The first Gen will probably return 1. - Application decides to use the same name for an object of the same type without calling Gen. Many demo programs use names 1, 2, 3, etc. without calling Gen. - Application calls the meta function again, and the meta function replaces the data. The application's data is lost, and the app fails. Have fun debugging that. Fixes piglit 'object-namespace-pollution glGetTexImage-compressed renderbuffer' test. Signed-off-by: Ian Romanick <[email protected]> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92363 Reviewed-by: Anuj Phogat <[email protected]>
* meta: Use internal functions for renderbuffer accessIan Romanick2016-02-102-14/+8
| | | | | Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* meta/decompress: Track renderbuffer using gl_renderbuffer instead of GL API ↵Ian Romanick2016-02-102-5/+11
| | | | | | | object handle Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* i965/meta: Don't pollute the renderbuffer namespaceIan Romanick2016-02-103-16/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | tl;dr: For many types of GL object, we can *NEVER* use the Gen function. In OpenGL ES (all versions!) and OpenGL compatibility profile, applications don't have to call Gen functions. The GL spec is very clear about how you can mix-and-match generated names and non-generated names: you can use any name you want for a particular object type until you call the Gen function for that object type. Here's the problem scenario: - Application calls a meta function that generates a name. The first Gen will probably return 1. - Application decides to use the same name for an object of the same type without calling Gen. Many demo programs use names 1, 2, 3, etc. without calling Gen. - Application calls the meta function again, and the meta function replaces the data. The application's data is lost, and the app fails. Have fun debugging that. Signed-off-by: Ian Romanick <[email protected]> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92363 Reviewed-by: Anuj Phogat <[email protected]>
* i965/meta: Use internal functions for renderbuffer accessIan Romanick2016-02-103-11/+8
| | | | | Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* i965/meta: Return struct gl_renderbuffer* from brw_get_rb_for_slice instead ↵Ian Romanick2016-02-104-24/+30
| | | | | | | of GL API handle Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* meta: Don't save or restore the renderbuffer bindingIan Romanick2016-02-102-7/+1
| | | | | | | | Nothing left in meta does anything with the RBO binding, so we don't need to save or restore it. The FBO binding is still modified. Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* meta: Use _mesa_CreateRenderbuffers instead of _mesa_GenRenderbuffers and ↵Ian Romanick2016-02-101-5/+4
| | | | | | | | | | | | _mesa_BindRenderbuffer This has the advantage that it does not pollute the global binding state. It also enables later patches that will stop calling _mesa_GenRenderbuffers / _mesa_CreateRenderbuffers which pollute the renderbuffer namespace. Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* i965/meta: Use _mesa_CreateRenderbuffers instead of _mesa_GenRenderbuffers ↵Ian Romanick2016-02-101-4/+3
| | | | | | | | | | | | and _mesa_BindRenderbuffer This has the advantage that it does not pollute the global binding state. It also enables later patches that will stop calling _mesa_GenRenderbuffers / _mesa_CreateRenderbuffers which pollute the renderbuffer namespace. Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* mesa: Refactor renderbuffer_storage to make _mesa_renderbuffer_storageIan Romanick2016-02-102-39/+63
| | | | | | | | | Pulls the parts of renderbuffer_storage that aren't just parameter validation out into a function that can be called from other parts of Mesa (e.g., meta). Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* mesa: Refactor _mesa_framebuffer_renderbufferIan Romanick2016-02-102-18/+27
| | | | | | | | | | | This function previously was only used in fbobject.c and contained a bunch of API validation. Split the function into framebuffer_renderbuffer that is static and contains the validation, and _mesa_framebuffer_renderbuffer that is suitable for calling from elsewhere in Mesa (e.g., meta). Signed-off-by: Ian Romanick <[email protected]> Reviewed-by: Anuj Phogat <[email protected]>
* st/mesa: check ureg_create() retval in create_pbo_upload_vs()Samuel Pitoiset2016-02-101-0/+2
| | | | | | | | | | This avoids a possible NULL dereference because ureg_create() might return a NULL pointer. Spotted by coverity. Signed-off-by: Samuel Pitoiset <[email protected]> Reviewed-by: Nicolai Hähnle <[email protected]>
* i965/blorp: Fix hiz ops on MSAA surfacesChris Forbes2016-02-101-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Two things were broken here: - The depth/stencil surface dimensions were broken for MSAA. - Sample count was programmed incorrectly. Result was the depth resolve didn't work correctly on MSAA surfaces, and so sampling the surface later produced garbage. Fixes the new piglit test arb_texture_multisample-sample-depth, and various artifacts in 'tesseract' with msaa=4 glineardepth=0. Fixes freedesktop bug #76396. Not observed any piglit regressions on Haswell. v2: Just set brw_hiz_op_params::dst.num_samples rather than adding a helper function (Ken). Signed-off-by: Chris Forbes <[email protected]> v3: moved the alignment needed for hiz+msaa to brw_blorp.cpp, as suggested by Chad Versace (Alejandro Piñeiro on behalf of Chris Forbes) Signed-off-by: Alejandro Piñeiro <[email protected]> Reviewed-by: Ben Widawsky <[email protected]> Tested-by: Jordan Justen <[email protected]> Reviewed-by: Jordan Justen <[email protected]>
* i965/gen8: Remove dead assertionTopi Pohjolainen2016-02-101-6/+0
| | | | | | | | | | | | | | The assertion is inside a condition mandating num_samples > 1 and therefore the first half of the constraint is always met. The second half in turn would only be applicable for single sampled case and moreover it is trying to falsely check against surface type instead of format. Subsequent patches will introduce proper support for the lossless compression and dropping this here makes the patches a little simpler. Signed-off-by: Topi Pohjolainen <[email protected]> Reviewed-by: Ben Widawsky <[email protected]>
* i965: Use constant pointer when checking for compressionTopi Pohjolainen2016-02-102-2/+2
| | | | | Signed-off-by: Topi Pohjolainen <[email protected]> Reviewed-by: Ben Widawsky <[email protected]>
* mesa: fix trivial comment typo in dlist.cBrian Paul2016-02-091-1/+1
|
* i965/vec4: Drop support for ATTR as an instruction destination.Kenneth Graunke2016-02-091-16/+0
| | | | | | | | | This is no longer necessary...and it doesn't make much sense to have inputs as destinations. Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Matt Turner <[email protected]> Reviewed-by: Chris Forbes <[email protected]>
* i965/vec4/gs: Stop munging the ATTR containing gl_PointSize.Kenneth Graunke2016-02-092-23/+4
| | | | | | | | | | | | | | | | | | gl_PointSize is delivered in the .w component of the VUE header, while the language expects it to be a float (and thus in the .x component). Previously, we emitted MOVs to copy it over to the .x component. But this is silly - we can just use a .wwww swizzle and access it without copying anything or clobbering the value stored at .x (which admittedly is useless). Removes the last use of ATTR destinations. v2: Use BRW_SWIZZLE_WWWW, not SWIZZLE_WWWW (caught by GCC). Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Matt Turner <[email protected]> Reviewed-by: Chris Forbes <[email protected]>
* i965: Apply VS attribute workarounds in NIR.Kenneth Graunke2016-02-098-117/+202
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch re-implements the pre-Haswell VS attribute workarounds. Instead of emitting shader code in the vec4 backend, we now simply call a NIR pass to emit the necessary code. This simplifies the vec4 backend. Beyond deleting code, it removes the primary use of ATTR as a destination. It also eliminates the requirement that the vec4 VS backend express the ATTR file in terms of VERT_ATTRIB_* locations, giving us a bit more flexibility. This approach is a little different: rather than munging the attributes at the top, we emit code to fix them up when they're accessed. However, we run the optimizer afterwards, so CSE should eliminate the redundant math. It may even be able to fuse it with other calculations based on the input value. shader-db does not handle non-default NOS settings, so I have no statistics about this patch. Note that the scalar backend does not implement VS attribute workarounds, as they are unnecessary on hardware which allows SIMD8 VS. v2: Do one multiply for FIXED rescaling and select components from either the original or scaled copy, rather than multiplying each component separately (suggested by Matt Turner). Signed-off-by: Kenneth Graunke <[email protected]> Reviewed-by: Matt Turner <[email protected]> Reviewed-by: Chris Forbes <[email protected]>
* st/mesa: clarify some texture target code in st_cb_drawpix.cBrian Paul2016-02-091-3/+5
| | | | | | | | | | | Use st->internal_target instead of PIPE_TEXTURE_2D when choosing the texture format. Probably no real difference, but let's be consistent. Simplify a test when determining whether we need normalized texcoords. Add a new assertion. Reviewed-by: Roland Scheidegger <[email protected]>
* st/mesa: fix bitmap texture target code and simplify tex sampler stateBrian Paul2016-02-092-17/+17
| | | | | | | | | | | | | | | Bitmaps may be drawn with a PIPE_TEXTURE_2D or PIPE_TEXTURE_RECT resource as determined at context creation by checking if PIPE_CAP_NPOT_TEXTURES is supported. But many places in the bitmap code were hard-coded to use PIPE_TEXTURE_2D. Use st->internal_target instead. I think an older NV chip is the only case where a gallium driver does not support NPOT textures. Bitmap drawing was probably broken for that GPU. Also, we only need one sampler state with texcoord normalization set up according to st->internal_target. Reviewed-by: Roland Scheidegger <[email protected]>
* st/mesa: use MAX3() macro, as we do for sampler view code belowBrian Paul2016-02-091-1/+2
| | | | Reviewed-by: Roland Scheidegger <[email protected]>
* st/mesa: move some st_cb_drawpixels.c code, add commentsBrian Paul2016-02-091-17/+22
|
* mesa/readpix: Dedent former _mesa_readpixels() if blockNanley Chery2016-02-091-29/+29
| | | | | | | | Formatting patch split out for easy reviewing. Signed-off-by: Nanley Chery <[email protected]> Reviewed-by: Ian Romanick <[email protected]> Reviewed-by: Brian Paul <[email protected]>
* mesa/readpix: Don't clip in _mesa_readpixels()Nanley Chery2016-02-091-13/+7
| | | | | | | | The clipping is performed higher up in the call-chain. Signed-off-by: Nanley Chery <[email protected]> Reviewed-by: Ian Romanick <[email protected]> Reviewed-by: Brian Paul <[email protected]>
* mesa/readpix: Clip ReadPixels() area to the ReadBuffer'sNanley Chery2016-02-091-2/+5
| | | | | | | | | | | | | | | | | | | The fast path for Intel's ReadPixels() unintentionally omits clipping the specified area to a valid one. Rather than clip in various corner-cases, perform this operation in the API validation stage. The bug in intel_readpixels_tiled_memcpy() showed itself when the winsys ReadBuffer's height was smaller than the one specified by ReadPixels(). yoffset became negative, which was an invalid input for tiled_to_linear(). v2: Move clipping to validation stage (Jason) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92193 Reported-by: Marta Löfstedt <[email protected]> Cc: "11.0 11.1" <[email protected]> Signed-off-by: Nanley Chery <[email protected]> Reviewed-by: Ian Romanick <[email protected]> Reviewed-by: Brian Paul <[email protected]>
* mesa/image: Make _mesa_clip_readpixels() work with renderbuffersNanley Chery2016-02-091-5/+17
| | | | | | | | | v2: Use gl_renderbuffer::{Width,Height} (Jason) Cc: "11.0 11.1" <[email protected]> Signed-off-by: Nanley Chery <[email protected]> Reviewed-by: Ian Romanick <[email protected]> Reviewed-by: Brian Paul <[email protected]>
* i965/vec4: Plumb separate surfaces and samplers through from NIRJason Ekstrand2016-02-094-9/+23
| | | | Reviewed-by: Kenneth Graunke <[email protected]>
* i965/vec4: Separate the sampler from the surface in generate_texJason Ekstrand2016-02-091-5/+13
| | | | Reviewed-by: Kenneth Graunke <[email protected]>
* i965/fs: Plumb separate surfaces and samplers through from NIRJason Ekstrand2016-02-097-22/+46
| | | | Reviewed-by: Kenneth Graunke <[email protected]>