[Mesa-dev] i965 GL_ARB_buffer_storage

[Kenneth Graunke]

On 03/14/2014 11:35 AM, Eric Anholt wrote:
> Kenneth Graunke <kenneth at 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.
> 

Seems reasonable to me.

--Ken

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140314/c9272aaf/attachment-0001.pgp>