Markus Wick <[email protected]> writes: > Am 2014-03-09 05:07, schrieb Eric Anholt: >> diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c >> new file mode 100644 >> index 0000000..be2c2af >> --- /dev/null >> +++ b/glamor/glamor_vbo.c >> @@ -0,0 +1,138 @@ >> +/* >> + * Copyright © 2014 Intel Corporation >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a >> + * copy of this software and associated documentation files (the >> "Software"), >> + * to deal in the Software without restriction, including without >> limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom >> the >> + * Software is furnished to do so, subject to the following >> conditions: >> + * >> + * The above copyright notice and this permission notice (including >> the next >> + * paragraph) shall be included in all copies or substantial portions >> of the >> + * Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >> SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES >> OR OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> ARISING >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >> DEALINGS >> + * IN THE SOFTWARE. >> + */ >> + >> +/** >> + * @file glamor_vbo.c >> + * >> + * Helpers for managing streamed vertex bufffers used in glamor. >> + */ >> + >> +#include "glamor_priv.h" >> + >> +/** Default size of the VBO, in bytes. >> + * >> + * If a single request is larger than this size, we'll resize the VBO >> + * and return an appropriate mapping, but we'll resize back down after >> + * that to avoid hogging that memory forever. We don't anticipate >> + * normal usage actually requiring larger VBO sizes. >> + */ >> +#define GLAMOR_VBO_SIZE (64 * 1024) > > This is a bit too small imo. iirc glamor was used to split up renderings > to 64k vertices, not 64k bytes. > What is the cache implact on too big buffers? i965 must fall through to > LLC, so will it pollute the L1+L2 caches? > For non-coherent gpus, write combining also shouldn't pollute any > caches.
64k seems like plenty, but against a non-bufferstorage Mesa, 512k did
give 1.6% improvement, so I swapped to your value.
>> +
>> +/**
>> + * Returns a pointer to @size bytes of VBO storage, which should be
>> + * accessed by the GL using vbo_offset within the VBO.
>> + */
>> +void *
>> +glamor_get_vbo_space(ScreenPtr screen, unsigned size, char
>> **vbo_offset)
>> +{
>> + glamor_screen_private *glamor_priv =
>> glamor_get_screen_private(screen);
>> + void *data;
>> +
>> + glamor_get_context(glamor_priv);
>> +
>> + glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
>> +
>> + if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
>> + if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
>> + glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
>> + glamor_priv->vbo_offset = 0;
>> + glBufferData(GL_ARRAY_BUFFER,
>> + glamor_priv->vbo_size, NULL, GL_STREAM_DRAW);
>> + }
>> +
>> + data = glMapBufferRange(GL_ARRAY_BUFFER,
>> + glamor_priv->vbo_offset,
>> + size,
>> + GL_MAP_WRITE_BIT |
>> + GL_MAP_UNSYNCHRONIZED_BIT |
>> + GL_MAP_INVALIDATE_RANGE_BIT);
>> + assert(data != NULL);
>> + *vbo_offset = (void *)(uintptr_t)glamor_priv->vbo_offset;
>
> (char *) instead of (void *).
Changed.
>> + glamor_priv->vbo_offset += size;
>> + } else {
>> + /* Return a pointer to the statically allocated non-VBO
>> + * memory. We'll upload it through glBufferData() later.
>> + */
>> + if (glamor_priv->vbo_size < size) {
>> + glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
>> +
>> + glBufferData(GL_ARRAY_BUFFER,
>> + glamor_priv->vbo_size, NULL, GL_STREAM_DRAW);
>
> Is this call needed at all? As we upload by glBufferData, we will alloc
> the buffer then.
Nope, you're right.
>> +
>> + free(glamor_priv->vb);
>> + glamor_priv->vb = XNFalloc(size);
>> + }
>> + *vbo_offset = NULL;
>> + glamor_priv->vbo_offset = 0;
>
> This variable is used for the size on uploading. So it must be set to
> "size".
Good catch. Fixed (and commented).
>> +void
>> +glamor_put_vbo_space(ScreenPtr screen)
>> +{
>> + glamor_screen_private *glamor_priv =
>> glamor_get_screen_private(screen);
>> +
>> + glamor_get_context(glamor_priv);
>> +
>> + if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
>> + glUnmapBuffer(GL_ARRAY_BUFFER);
>> + } else {
>> + glBufferData(GL_ARRAY_BUFFER, glamor_priv->vbo_offset,
>> + glamor_priv->vb, GL_DYNAMIC_DRAW);
>> + }
>> +
>> + glBindBuffer(GL_ARRAY_BUFFER, 0);
>> +
>> + glamor_put_context(glamor_priv);
>> +}
>> +
>> +void
>> +glamor_init_vbo(ScreenPtr screen)
>> +{
>> + glamor_screen_private *glamor_priv =
>> glamor_get_screen_private(screen);
>> +
>> + glamor_get_context(glamor_priv);
>> +
>> + glGenBuffers(1, &glamor_priv->vbo);
>> +
>> + glamor_put_context(glamor_priv);
>> +}
>> +
>> +void
>> +glamor_fini_vbo(ScreenPtr screen)
>> +{
>> + glamor_screen_private *glamor_priv =
>> glamor_get_screen_private(screen);
>> +
>> + glamor_get_context(glamor_priv);
>> +
>> + glDeleteBuffers(1, &glamor_priv->vbo);
>> + if (glamor_priv->gl_flavor != GLAMOR_GL_DESKTOP)
>> + free(glamor_priv->vb);
>> +
>> + glamor_put_context(glamor_priv);
>> +}
>
> I don't like the idea to select the code by desktop vs gles as both
> don't have to support ARB_mbr.
Yeah, we should definitely extend this to GLES's ARB_mbr in the future.
> Do we have to care about allocing zero bytes? I don't think so, but it's
> broken right now.
I don't think so.
pgpGscOtvZBxJ.pgp
Description: PGP signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
