[Mesa-dev] [PATCH 2/2] gallium: add flag PIPE_TRANSFER_MAP_PERMANENTLY
Jose Fonseca
jfonseca at vmware.com
Tue Jan 10 03:29:14 PST 2012
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.
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.
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