[Mesa-dev] [PATCH 2/2] gallium: add flag PIPE_TRANSFER_MAP_PERMANENTLY
Marek Olšák
maraeo at gmail.com
Tue Jan 10 07:37:43 PST 2012
On Tue, Jan 10, 2012 at 12:29 PM, Jose Fonseca <jfonseca at vmware.com> wrote:
> Still catching up on email traffic during holidays...
>
> I agree that user buffer uploads should be moved out of drivers, but I don't think this is the way to go.
>
> This "PIPE_TRANSFER_MAP_PERMANENTLY" means the driver relinquishes the ability to transform this data in any way before reashing the GPU -- i.e., prevents any sort of workarounds -- something can't be always guaranteed. Flushing with map helps is also non-portable.
The flag is optional, it doesn't have to implemented by everybody. If
we do the uploads in the state tracker, we will also do any required
data transformation so that drivers don't have to do it at all.
>
>
> It looks to me that state trackers and/or drivers are not properly using PIPE_USAGE_STREAM flag. As all cases where PIPE_TRANSFER_MAP_PERMANENTLY would be used, the right way to do it would be for the state tracker to set PIPE_USAGE_STREAM, the pipe driver to recognize PIPE_USAGE_STREAM, and keep the mapping permanently internally, making mapping/unmapping of such buffers mere no-ops.
We were doing that already and it wasn't good enough. Avoiding
create+map+unmap+destroy *calls* have brought higher frame rates in
apps with lots of draw operations. It may not be the ideal
optimization, but so far it's the best we have.
Marek
>
>
> In summary, for me PIPE_TRANSFER_MAP_PERMANENTLY is premature/bad optmization. Before we are worried about saving a couple of map/unmap calls, we should ensure that PIPE_USAGE_STREAM/PIPE_USAGE_DYNAMIC code paths are fully optimal.
>
>
> Jose
>
> ----- Original Message -----
>> Please see the diff for further info.
>>
>> This paves the way for moving user buffer uploads out of drivers and
>> should
>> allow to clean up the mess in u_upload_mgr in the meantime.
>>
>> For now only allowed for buffers on r300 and r600.
>> ---
>> src/gallium/drivers/i915/i915_resource_buffer.c | 7 ++++++-
>> src/gallium/drivers/i915/i915_resource_texture.c | 7 ++++++-
>> src/gallium/drivers/llvmpipe/lp_texture.c | 4 ++++
>> src/gallium/drivers/nouveau/nouveau_buffer.c | 8 +++++++-
>> src/gallium/drivers/nv50/nv50_transfer.c | 2 +-
>> src/gallium/drivers/nvc0/nvc0_transfer.c | 2 +-
>> src/gallium/drivers/nvfx/nvfx_transfer.c | 3 +++
>> src/gallium/drivers/r300/r300_transfer.c | 4 ++++
>> src/gallium/drivers/r600/r600_texture.c | 4 ++++
>> src/gallium/drivers/svga/svga_resource_buffer.c | 4 ++++
>> src/gallium/drivers/svga/svga_resource_texture.c | 2 +-
>> src/gallium/include/pipe/p_defines.h | 16
>> ++++++++++++++++
>> 12 files changed, 57 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/gallium/drivers/i915/i915_resource_buffer.c
>> b/src/gallium/drivers/i915/i915_resource_buffer.c
>> index 77c0345..c54e481 100644
>> --- a/src/gallium/drivers/i915/i915_resource_buffer.c
>> +++ b/src/gallium/drivers/i915/i915_resource_buffer.c
>> @@ -68,8 +68,13 @@ i915_get_transfer(struct pipe_context *pipe,
>> const struct pipe_box *box)
>> {
>> struct i915_context *i915 = i915_context(pipe);
>> - struct pipe_transfer *transfer =
>> util_slab_alloc(&i915->transfer_pool);
>> + struct pipe_transfer *transfer;
>>
>> + if (usage & PIPE_TRANSFER_MAP_PERMANENTLY) {
>> + return NULL;
>> + }
>> +
>> + transfer = util_slab_alloc(&i915->transfer_pool);
>> if (transfer == NULL)
>> return NULL;
>>
>> diff --git a/src/gallium/drivers/i915/i915_resource_texture.c
>> b/src/gallium/drivers/i915/i915_resource_texture.c
>> index 8ff733a..64d071c 100644
>> --- a/src/gallium/drivers/i915/i915_resource_texture.c
>> +++ b/src/gallium/drivers/i915/i915_resource_texture.c
>> @@ -720,9 +720,14 @@ i915_texture_get_transfer(struct pipe_context
>> *pipe,
>> {
>> struct i915_context *i915 = i915_context(pipe);
>> struct i915_texture *tex = i915_texture(resource);
>> - struct i915_transfer *transfer =
>> util_slab_alloc(&i915->texture_transfer_pool);
>> + struct i915_transfer *transfer;
>> boolean use_staging_texture = FALSE;
>>
>> + if (usage & PIPE_TRANSFER_MAP_PERMANENTLY) {
>> + return NULL;
>> + }
>> +
>> + transfer = util_slab_alloc(&i915->texture_transfer_pool);
>> if (transfer == NULL)
>> return NULL;
>>
>> diff --git a/src/gallium/drivers/llvmpipe/lp_texture.c
>> b/src/gallium/drivers/llvmpipe/lp_texture.c
>> index ca38571..d86d493 100644
>> --- a/src/gallium/drivers/llvmpipe/lp_texture.c
>> +++ b/src/gallium/drivers/llvmpipe/lp_texture.c
>> @@ -587,6 +587,10 @@ llvmpipe_get_transfer(struct pipe_context *pipe,
>> assert(resource);
>> assert(level <= resource->last_level);
>>
>> + if (usage & PIPE_TRANSFER_MAP_PERMANENTLY) {
>> + return NULL;
>> + }
>> +
>> /*
>> * Transfers, like other pipe operations, must happen in order,
>> so flush the
>> * context if necessary.
>> diff --git a/src/gallium/drivers/nouveau/nouveau_buffer.c
>> b/src/gallium/drivers/nouveau/nouveau_buffer.c
>> index f822625..02186ba 100644
>> --- a/src/gallium/drivers/nouveau/nouveau_buffer.c
>> +++ b/src/gallium/drivers/nouveau/nouveau_buffer.c
>> @@ -172,7 +172,13 @@ nouveau_buffer_transfer_get(struct pipe_context
>> *pipe,
>> {
>> struct nv04_resource *buf = nv04_resource(resource);
>> struct nouveau_context *nv = nouveau_context(pipe);
>> - struct nouveau_transfer *xfr = CALLOC_STRUCT(nouveau_transfer);
>> + struct nouveau_transfer *xfr;
>> +
>> + if (usage & PIPE_TRANSFER_MAP_PERMANENTLY) {
>> + return NULL;
>> + }
>> +
>> + xfr = CALLOC_STRUCT(nouveau_transfer);
>> if (!xfr)
>> return NULL;
>>
>> diff --git a/src/gallium/drivers/nv50/nv50_transfer.c
>> b/src/gallium/drivers/nv50/nv50_transfer.c
>> index 6f860e7..8ddebeb 100644
>> --- a/src/gallium/drivers/nv50/nv50_transfer.c
>> +++ b/src/gallium/drivers/nv50/nv50_transfer.c
>> @@ -243,7 +243,7 @@ nv50_miptree_transfer_new(struct pipe_context
>> *pctx,
>> uint32_t size;
>> int ret;
>>
>> - if (usage & PIPE_TRANSFER_MAP_DIRECTLY)
>> + if (usage & (PIPE_TRANSFER_MAP_DIRECTLY |
>> PIPE_TRANSFER_MAP_PERMANENTLY))
>> return NULL;
>>
>> tx = CALLOC_STRUCT(nv50_transfer);
>> diff --git a/src/gallium/drivers/nvc0/nvc0_transfer.c
>> b/src/gallium/drivers/nvc0/nvc0_transfer.c
>> index f168637..c04f41f 100644
>> --- a/src/gallium/drivers/nvc0/nvc0_transfer.c
>> +++ b/src/gallium/drivers/nvc0/nvc0_transfer.c
>> @@ -243,7 +243,7 @@ nvc0_miptree_transfer_new(struct pipe_context
>> *pctx,
>> uint32_t size;
>> int ret;
>>
>> - if (usage & PIPE_TRANSFER_MAP_DIRECTLY)
>> + if (usage & (PIPE_TRANSFER_MAP_DIRECTLY |
>> PIPE_TRANSFER_MAP_PERMANENTLY))
>> return NULL;
>>
>> tx = CALLOC_STRUCT(nvc0_transfer);
>> diff --git a/src/gallium/drivers/nvfx/nvfx_transfer.c
>> b/src/gallium/drivers/nvfx/nvfx_transfer.c
>> index 7a218b1..605af8e 100644
>> --- a/src/gallium/drivers/nvfx/nvfx_transfer.c
>> +++ b/src/gallium/drivers/nvfx/nvfx_transfer.c
>> @@ -26,6 +26,9 @@ nvfx_transfer_new(struct pipe_context *pipe,
>> unsigned usage,
>> const struct pipe_box *box)
>> {
>> + if (usage & PIPE_TRANSFER_MAP_PERMANENTLY) {
>> + return NULL;
>> + }
>> if((usage & (PIPE_TRANSFER_UNSYNCHRONIZED |
>> PIPE_TRANSFER_DONTBLOCK)) == PIPE_TRANSFER_DONTBLOCK)
>> {
>> struct nouveau_bo* bo = ((struct
>> nvfx_resource*)pt)->bo;
>> diff --git a/src/gallium/drivers/r300/r300_transfer.c
>> b/src/gallium/drivers/r300/r300_transfer.c
>> index afdd530..91e861a 100644
>> --- a/src/gallium/drivers/r300/r300_transfer.c
>> +++ b/src/gallium/drivers/r300/r300_transfer.c
>> @@ -89,6 +89,10 @@ r300_texture_get_transfer(struct pipe_context
>> *ctx,
>> struct pipe_resource base;
>> boolean referenced_cs, referenced_hw;
>>
>> + if (usage & (PIPE_TRANSFER_MAP_DIRECTLY |
>> PIPE_TRANSFER_MAP_PERMANENTLY)) {
>> + return NULL;
>> + }
>> +
>> referenced_cs =
>> r300->rws->cs_is_buffer_referenced(r300->cs, tex->cs_buf);
>> if (referenced_cs) {
>> diff --git a/src/gallium/drivers/r600/r600_texture.c
>> b/src/gallium/drivers/r600/r600_texture.c
>> index 8fe54c8..decd941 100644
>> --- a/src/gallium/drivers/r600/r600_texture.c
>> +++ b/src/gallium/drivers/r600/r600_texture.c
>> @@ -630,6 +630,10 @@ struct pipe_transfer*
>> r600_texture_get_transfer(struct pipe_context *ctx,
>> int r;
>> boolean use_staging_texture = FALSE;
>>
>> + if (usage & PIPE_TRANSFER_MAP_PERMANENTLY) {
>> + return NULL;
>> + }
>> +
>> /* We cannot map a tiled texture directly because the data is
>> * in a different order, therefore we do detiling using a blit.
>> *
>> diff --git a/src/gallium/drivers/svga/svga_resource_buffer.c
>> b/src/gallium/drivers/svga/svga_resource_buffer.c
>> index fa713ee..57ec752 100644
>> --- a/src/gallium/drivers/svga/svga_resource_buffer.c
>> +++ b/src/gallium/drivers/svga/svga_resource_buffer.c
>> @@ -74,6 +74,10 @@ svga_buffer_get_transfer(struct pipe_context
>> *pipe,
>> struct svga_buffer *sbuf = svga_buffer(resource);
>> struct pipe_transfer *transfer;
>>
>> + if (usage & PIPE_TRANSFER_MAP_PERMANENTLY) {
>> + return NULL;
>> + }
>> +
>> transfer = CALLOC_STRUCT(pipe_transfer);
>> if (transfer == NULL) {
>> return NULL;
>> diff --git a/src/gallium/drivers/svga/svga_resource_texture.c
>> b/src/gallium/drivers/svga/svga_resource_texture.c
>> index 697c1d3..4eb1068 100644
>> --- a/src/gallium/drivers/svga/svga_resource_texture.c
>> +++ b/src/gallium/drivers/svga/svga_resource_texture.c
>> @@ -259,7 +259,7 @@ svga_texture_get_transfer(struct pipe_context
>> *pipe,
>> unsigned nblocksy = util_format_get_nblocksy(texture->format,
>> box->height);
>>
>> /* We can't map texture storage directly */
>> - if (usage & PIPE_TRANSFER_MAP_DIRECTLY)
>> + if (usage & (PIPE_TRANSFER_MAP_DIRECTLY |
>> PIPE_TRANSFER_MAP_PERMANENTLY))
>> return NULL;
>>
>> assert(box->depth == 1);
>> diff --git a/src/gallium/include/pipe/p_defines.h
>> b/src/gallium/include/pipe/p_defines.h
>> index 3b3940d..91d8b1e 100644
>> --- a/src/gallium/include/pipe/p_defines.h
>> +++ b/src/gallium/include/pipe/p_defines.h
>> @@ -226,6 +226,22 @@ enum pipe_transfer_usage {
>> PIPE_TRANSFER_MAP_DIRECTLY = (1 << 2),
>>
>> /**
>> + * The transfer should map the resource storage directly and the
>> GPU should
>> + * be able to see what the CPU has written. Such a storage may
>> stay mapped
>> + * while issuing draw commands which use it. The only allowed
>> usage is
>> + * non-overlapping writes which are suballocated out of a big
>> buffer.
>> + * The minimum allowed alignment of suballocations is 256 bytes
>> (this is
>> + * a subject to change).
>> + * The flag is intended to be used to avoid mapping and unmapping
>> + * resources repeatedly when doing uploads and draws intermixed.
>> + *
>> + * The driver may return NULL if that isn't possible, and the
>> state
>> + * tracker needs to cope with that and use an alternative path
>> + * without this flag.
>> + */
>> + PIPE_TRANSFER_MAP_PERMANENTLY = (1 << 3),
>> +
>> + /**
>> * Discards the memory within the mapped region.
>> *
>> * It should not be used with PIPE_TRANSFER_READ.
>> --
>> 1.7.4.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
More information about the mesa-dev
mailing list