[Mesa-dev] [PATCH] i965: Perform an explicit flush after doing _mesa_meta_pbo_TexSubImage

[Francisco Jerez]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> On Fri, Sep 04, 2015 at 02:40:54PM -0700, Jason Ekstrand wrote:
>> On Fri, Sep 4, 2015 at 1:22 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>> > With the introduction of ARB_shader_image_load_store, shaders
>> > are able to perform random memory accesses to texture that
>> > are not easily tracked by the driver. Rather than see if this
>> > texture is bound to an ImageUnit when we perform a TexImage command
>> > via the GPU, and so its dirty state untracked by the normal implicit
>> > flush mechanism, insert an explicit flush before the next access.
>> 
>> This doesn't seem quite right to me.  What's to prevent a regular
>> render target from having the same problem?  We already do flushes
>> whenever we have a rendertarget -> texture transition, I think the
>> better thing to do would be to add them when we do a rendertarget ->
>> image transition.
>
Coincidentally I sent a patch to Jordan yesterday doing exactly that.
It should fix a superset of what this patch fixes and also handle the
case in which a buffer bound as image was fast-cleared and a resolve is
needed.  I didn't have time to do enough testing so I may not send it
for review until I'm back from vacation in three weeks.  In the meantime
I don't have any objection to this patch going in as temporary
workaround if this problem annoys you.

> Another argument is that it is a client bug not to call
> glMemoryBarrier() between setting up a texture and using it for the
> first time.
>
> The nearest discussion of this I could find was
>
> Issue (11) What amount of automatic synchronization is provided for image loads
>          and stores?  In particular, is the use of MemoryBarrier() required
>          to ensure consistent ordering relative to other GL operations?  Or is
>          some other mechanism (e.g., unbinding a texture from an image unit
>          and then binding it to a texture image unit) sufficient?
>
>       RESOLVED:  Use of MemoryBarrier is required, and there is no
>       automatic synchronization when images are bound or unbound.
>
>       Implicit synchronization is difficult, as it might require some
>       combination of:
>
>         - tracking which images might be written (randomly) in the shader
>           itself;
>
>         - assuming that if a shader that performs writes is executed, all
>           texels of all bound images could be modified and thus must be
>           treated as dirty;
>
>         - idling at the end of each primitive or draw call, so that the
>           results of all previous commands are complete.
>
>       Since normal OpenGL operation is pipelined, idling would result in a
>       significant performance impact since pipelining would otherwise allow
>       fragment shader execution for draw call N while simultaneously
>       performing vertex shader execution for draw call N+1.
>
>
> From that language, I understand it to mean that the client is
> responsible for ensuring that the texture is coherent on binding after a
> glTexImage*D.

Not really, ARB_shader_image_load_store does guarantee implicit
synchronization of image access done after any other GL command that
already guarantees implicit synchronization in unextended GL.
glMemoryBarrier is only required to order shader image memory access
with *subsequent* GL commands of the kind given by the bitset argument.

So an implicit flush is definitely needed at some point between
rendering and image access, either as in your or as in my patch.

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150905/068f1598/attachment.sig>