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
