Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> <div class="gmail_quote">On May 12, 2015 7:55 AM, "Francisco Jerez" <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>> --- 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 d3ca38b..3038057 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 7952ee5..85d3d04 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.3.5 </blockquote></div>