On Tue, May 12, 2015 at 7:55 AM, Francisco Jerez <curroje...@riseup.net> wrote:
> 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 <mesa-sta...@lists.freedesktop.org>
> ---
> 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
>
> _______________________________________________
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
