On Fri, Mar 14, 2014 at 7:35 PM, Eric Anholt <e...@anholt.net> wrote: > Kenneth Graunke <kenn...@whitecape.org> writes: > >> On 02/27/2014 02:52 PM, Eric Anholt wrote: >>> One thing I noticed while working on this was that we only reallocate buffer >>> storage for INVALIDATE_BUFFER_BIT when UNSYNCHRONIZED_BIT is unset. The >>> ARB_mbr spec says that the contents "may be discarded", not "must be >>> discarded". However, while writing the glamor code I happened to type this >>> for the wraparound case: >>> >>> glamor_priv->vb = glMapBufferRange(GL_ARRAY_BUFFER, >>> 0, size, >>> GL_MAP_WRITE_BIT | >>> GL_MAP_INVALIDATE_BUFFER_BIT >>> | >>> GL_MAP_UNSYNCHRONIZED_BIT | >>> GL_MAP_PERSISTENT_BIT | >>> GL_MAP_COHERENT_BIT); >>> >>> intending that the buffer storage get reallocated, and that we not worry >>> about >>> any synchronization after that. My code would have been broken on the i965 >>> driver. I'm wondering if this is the intended behavior of the spec, or if >>> we >>> want to treat the "may" as a "must". >> >> I don't understand. Your code here sets GL_MAP_INVALIDATE_BUFFER_BIT >> and GL_MAP_UNSYNCHRONIZED_BIT, which would have reallocated the storage, >> as you intended. Was this a mispaste - you originally lacked the >> GL_MAP_UNSYNCHRONIZED_BIT, were surprised, and eventually added it? > > Here's the only i965 code referencing MAP_INVALIDATE_BUFFER_BIT: > > if (!(access & GL_MAP_UNSYNCHRONIZED_BIT)) { > if (drm_intel_bo_references(brw->batch.bo, intel_obj->buffer)) { > if (access & GL_MAP_INVALIDATE_BUFFER_BIT) { > drm_intel_bo_unreference(intel_obj->buffer); > intel_bufferobj_alloc_buffer(brw, intel_obj); > } else { > perf_debug("Stalling on the GPU for mapping a busy buffer " > "object\n"); > intel_batchbuffer_flush(brw); > } > } else if (drm_intel_bo_busy(intel_obj->buffer) && > (access & GL_MAP_INVALIDATE_BUFFER_BIT)) { > drm_intel_bo_unreference(intel_obj->buffer); > intel_bufferobj_alloc_buffer(brw, intel_obj); > } > } > > so for that glamor call we wouldn't have thrown out the storage, even > though it was referenced by the batch or the GPU. I think we should > pull the INVALIDATE_BUFFER test out and just always invalidate when it's > set.
You can do that, but there are also drivers which either do not implement the INVALIDATE_BUFFER flag or ignore the flag if other flags are set, which complies with the wording "may be discarded" in the spec. However, if I had written the spec, I would have added an INVALID_OPERATION error if both INVALIDATE and UNSYNCHRONIZED flags are set, because they really are mutually-exclusive. Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev