From d247615e0d67a7c8eaeea3fece837229c8c9658c Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Sat, 31 Jan 2015 20:04:55 +0200 Subject: [PATCH] i965: Fix PBO cache coherency issue after _mesa_meta_pbo_GetTexSubImage(). This problem can easily be reproduced with a number of ARB_shader_image_load_store piglit tests, which use a buffer object as PBO for a pixel transfer operation and later on bind the same buffer to the pipeline as shader image -- The problem is not exclusive to images though, and is likely to affect other kinds of buffer objects that can be bound to the 3D pipeline, including vertex, index, uniform, atomic counter buffers, etc. CC: 10.5 Reviewed-by: Jason Ekstrand Reviewed-by: Anuj Phogat --- src/mesa/drivers/dri/i965/intel_pixel_read.c | 24 +++++++++++++++++++- src/mesa/drivers/dri/i965/intel_tex_image.c | 9 +++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c b/src/mesa/drivers/dri/i965/intel_pixel_read.c index d3ca38b6ecd..30380570d62 100644 --- a/src/mesa/drivers/dri/i965/intel_pixel_read.c +++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c @@ -226,8 +226,30 @@ intelReadPixels(struct gl_context * ctx, if (_mesa_is_bufferobj(pack->BufferObj)) { if (_mesa_meta_pbo_GetTexSubImage(ctx, 2, NULL, x, y, 0, width, height, 1, - format, type, pixels, pack)) + format, type, pixels, pack)) { + /* _mesa_meta_pbo_GetTexSubImage() implements PBO transfers by + * binding the user-provided BO as a fake framebuffer and rendering + * to it. This breaks the invariant of the GL that nothing is able + * to render to a BO, causing nondeterministic corruption issues + * because the render cache is not coherent with a number of other + * caches that the BO could potentially be bound to afterwards. + * + * This could be solved in the same way that we guarantee texture + * coherency after a texture is attached to a framebuffer and + * rendered to, but that would involve checking *all* BOs bound to + * the pipeline for the case we need to emit a cache flush due to + * previous rendering to any of them -- Including vertex, index, + * uniform, atomic counter, shader image, transform feedback, + * indirect draw buffers, etc. + * + * That would increase the per-draw call overhead even though it's + * very unlikely that any of the BOs bound to the pipeline has been + * rendered to via a PBO at any point, so it seems better to just + * flush here unconditionally. + */ + intel_batchbuffer_emit_mi_flush(brw); return; + } perf_debug("%s: fallback to CPU mapping in PBO case\n", __func__); } diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c index 7952ee5ad88..85d3d04ecb3 100644 --- a/src/mesa/drivers/dri/i965/intel_tex_image.c +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c @@ -486,8 +486,15 @@ intel_get_tex_image(struct gl_context *ctx, if (_mesa_meta_pbo_GetTexSubImage(ctx, 3, texImage, 0, 0, 0, texImage->Width, texImage->Height, texImage->Depth, format, type, - pixels, &ctx->Pack)) + pixels, &ctx->Pack)) { + /* Flush to guarantee coherency between the render cache and other + * caches the PBO could potentially be bound to after this point. + * See the related comment in intelReadPixels() for a more detailed + * explanation. + */ + intel_batchbuffer_emit_mi_flush(brw); return; + } perf_debug("%s: fallback to CPU mapping in PBO case\n", __func__); } -- 2.30.2