[Mesa-dev] [PATCH 2/2] st/mesa: Handle GL_MAP_INVALIDATE_BUFFER_BIT in st_bufferobj_map_range().

[Marek Olšák]

We can't do try-map + create + map + dereference internally in a
driver. Creating a new buffer and replacing a pointer to the old one
may lead to the following issue. If a buffer pointer is replaced, it
doesn't necessarily update all the states the buffer is set in in all
existing contexts. Such states and contexts would still use the old
buffer and wouldn't see the change until the old buffer is unbound.

I think the only correct way to implement the DISCARD flags in drivers
is through a temporary (staging) resource and doing an on-gpu copy to
the original one (i.e. what we do for texture transfers in rX00g).

Marek


On Mon, Jul 18, 2011 at 2:09 AM, Jose Fonseca <jfonseca at vmware.com> wrote:
> PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE was added recently, and is the preferred interface, because:
> - it can give better performance when the kernel memory manager supports; it requires less kernel roundtrips than try-map + create + map + dereference.
> - it also allows the drivers to do more fancy implementations when kernel doesn't, e.g., keep a round robin of buffers, to avoid consst, which can be freed when memory is low
>
> Preferably, all drivers should implement PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE -- they can simply internally do the try-map + create + map + dereference if there's no way pass that flag to the kernel map ioctl.
>
> If there are strong reasons to not support PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE in certain drivers, then a new PIPE CAP should be added, so that drivers that do support don't limited to the lowest common denominator.
>
> Note that this can happen in other places as well:
> - glBufferData
> - glTexImage of single level textures.
>
> Jose
>
>
>
> ----- Original Message -----
>> Alternatively, individual drivers could actually implement
>> PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE. As far as I can tell only svga
>> currently implements that, and st_bufferobj_map_range() seems to be
>> the main
>> user. I wonder if in general PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE is
>> something
>> that should just be handled by the state trackers.
>>
>> As for the actual implementation, we could also try a map with
>> PIPE_TRANSFER_DONTBLOCK first, and avoid invalidating
>> _NEW_BUFFER_OBJECT in
>> some cases. I'm not sure if that's worth it without doing more
>> benchmarking
>> though, since in the typical case GL_MAP_INVALIDATE_BUFFER_BIT will
>> probably
>> imply that the buffer is in use.
>>
>> Signed-off-by: Henri Verbeet <hverbeet at gmail.com>
>> ---
>>  src/mesa/state_tracker/st_cb_bufferobjects.c |   15 +++++++++++++++
>>  1 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c
>> b/src/mesa/state_tracker/st_cb_bufferobjects.c
>> index 7374bb0..7aa859e 100644
>> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
>> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
>> @@ -332,6 +332,21 @@ st_bufferobj_map_range(struct gl_context *ctx,
>> GLenum target,
>>        obj->Pointer = &st_bufferobj_zero_length;
>>     }
>>     else {
>> +      if (flags & PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE) {
>> +         struct pipe_resource *buffer;
>> +
>> +         buffer = pipe_buffer_create(pipe->screen,
>> +                                     st_obj->buffer->bind,
>> +                                     st_obj->buffer->usage,
>> +                                     st_obj->buffer->width0);
>> +         if (buffer) {
>> +            st_invalidate_state(ctx, _NEW_BUFFER_OBJECT);
>> +            pipe_resource_reference(&st_obj->buffer, NULL);
>> +            st_obj->buffer = buffer;
>> +            flags &= ~PIPE_TRANSFER_DISCARD_WHOLE_RESOURCE;
>> +         }
>> +      }
>> +
>>        obj->Pointer = pipe_buffer_map_range(pipe,
>>                                             st_obj->buffer,
>>                                             offset, length,
>> --
>> 1.7.2.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>