[Mesa-dev] RFC: enforcing gallium resource bind flags

[Jose Fonseca]

----- Original Message -----
> On 01.03.2013 11:30, Jose Fonseca wrote:
> > ----- Original Message -----
> >> On Fri, Mar 1, 2013 at 12:31 AM, Roland Scheidegger <sroland at vmware.com>
> >> wrote:
> >>> Hi,
> >>>
> >>> there is some sloppy usage of bind flags in the opengl state tracker
> >>> (that is, resources get used for things which they didn't have the bind
> >>> flag set).
> >>> We'd really like to enforce these flags to be honored but it doesn't
> >>> really work (ok llvmpipe so far would only really care about sampler
> >>> view, color render target, depth/stencil - see also
> >>> c8eb2d0e829d0d2aea6a982620da0d3cfb5982e2).
> >>>
> >>> Currently it looks like there's at least two issues with those bind
> >>> flags in the opengl state tracker (for these bind flags only, there are
> >>> almost certainly more).
> >>> 1) for textures, the state tracker will always try to allocate resources
> >>> with both sampler_view and render_target (or depth/stencil) bind flags.
> >>> However it will drop these flags for resources where this isn't
> >>> supported. This is all right, however when we try to render to such
> >>> resources, the surface will be created regardless (but it won't get used
> >>> as it will fail framebuffer validation which checks the attachments and
> >>> specifically tests if the format is a renderable format). I guess this
> >>> could be fixed (seems a bit backward, it might be possible to just look
> >>> at the resource bind flags to decide if we create a surface or not, and
> >>> we shouldn't need to check the format later - if we've got the bind flag
> >>> we know we can create a surface and hence render to).
> >>>
> >>> 2) a far more difficult problem seem to be buffers. While piglit doesn't
> >>> hit it (I modified the tbo test to hit this) it is possible to create
> >>> buffers with any target and later bind to anything. So the state tracker
> >>> has no knowledge at all what a buffer will eventually get used for
> >>> (other than the hint when it was first created), and it seems
> >>> unreasonable to just set all possible bind flags all the time. But then
> >>> still enforcing bind flags later would require the state tracker to
> >>> recreate the resource (with more bind flags) and copy over the old
> >>> contents, which sounds very bad too.
> >>>
> >>> So any ideas?
> >> In my opinion, the bind flags are useless, because they cannot be
> >> determined for OpenGL resources exactly. The only exceptions are:
> >> - PIPE_BIND_CONSTANT_BUFFER, which is set correctly for the default
> >> non-UBO constant buffer.
> >> - PIPE_BIND_SCANOUT for the DDX and DRM state trackers.
> >> - PIPE_BIND_GLOBAL for OpenCL.
> >>
> >> The radeon drivers ignore the bind flags entirely except SCANOUT and
> >> GLOBAL, and r300g also checks for CONSTANT_BUFFER.
> >>
> >> The OpenGL buffer API doesn't have any bind flags. It only has binding
> >> points, and any buffer can be bound to any binding point. Textures are
> >> just as fun. You can create a texture or a renderbuffer, but if you
> >> use CopyTexSubImage, the roles are swapped - what was a texture is
> >> suddenly a renderbuffer and what was a renderbuffer is suddenly a
> >> texture.
> >>
> >> If we didn't need the 3 bind flags mentioned above, I would be for
> >> removing pipe_resource::bind, because it's not that useful.
> > API other than OpenGL have clear and stricter binding rules, which drivers
> > can rely upon to make real optimizations.
> >
> > I honestly don't see the what's the difficulty here, the semantics are
> > clear:
> >
> > - If Mesa state tracker doesn't care about BIND flags, that's fine, just
> > let Mesa request as much BIND flags as the driver advertises.
> >
> > - If a gallium driver doesn't care about BIND flags, that's fine, just
> > advertise ~0 bind flags.
> 
> You can still use the them as a hint for optimization. For example, if
> an application binds a buffer to GL_ARRAY_BUFFER on creation, it is
> rather likely to be used mainly as a vertex buffer.
> So I wouldn't want mesa/st to just simply set all of them and discard
> that bit of information about the user's intentions.
> Rather than setting all flags, I'd add an additional PIPE_BIND_UNKNOWN
> to signal that the binding specified should not be taken too seriously,
> and that the resource should be bindable to all points possible for the
> given resource.

Yes, that's a good idea too.  I'd prefer PIPE_BIND_HINT instead of unknwon, signifying that the other BIND flags are a merely an hint instead of a strict promise. Whenever a driver saw this flag it can expect the best, but should be prepared for the worst.

> We could also make it possible to add new bind flags as you go, via an
> additional gallium API function, where the driver can, for instance,
> migrate a resource if necessary.
> But I don't quite like that because the state tracker would have to keep
> checking the bind flags all the time which is rather ugly.

I don't it's necessary to check flags all the time, only a resource creation time -- the state tracker should not set bind flags that are unsupported by the driver (or if it can't be sure, it should set the new  PIPE_BIND_HINT/UNKNOWN, but still double check with is_format_supported that it is supported).  Furthermore, if something doesn't add up, it's better to fail at creation time, than to have random failures because the state tracker is binding resources in places the driver really wasn't expecting.  After creation it should be just a matter of asserting, or using galahad. 

I truly think that the the misuse of bind flags can hide honest bugs. Especially with GL 3.x/4.x where resources will be used in novel ways.

Jose