[Mesa-dev] [PATCH 6/6] radeonsi: always unmap texture CPU mappings on 32-bit CPU architectures

[Axel Davy]

Hi Marek,

That seems a good idea.
Several 32bits games have virtual address space issues as well with both 
Nine and Wine (but Nine seems a bit more affected because more libs are 
loaded).

Maybe the patch could go a little bit further by doing the same for 
buffers the first time they si_buffer_transfer_unmap is called for them ?

I've seen several nine games use a few big buffers (in DEFAULT pool) to 
store quite a lot of vertices,
in addition to the buffers frequently written to for rendering.

Unmapping those big buffers, that we are never going to write to again, 
could save a bit of that precious virtual space.

The safest test I believe, is to look only at buffers that are written 
to only once, thus always unmapping on first unmap.
For the buffers often written to, the cost of the first unmap will be 
negligible.

What do you think ?

Axel

On 14/12/2018 22:24, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> Team Fortress 2 32-bit version runs out of the CPU address space.
> ---
>   src/gallium/drivers/radeonsi/si_texture.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_texture.c b/src/gallium/drivers/radeonsi/si_texture.c
> index 95f1e8c9693..39869445b0f 100644
> --- a/src/gallium/drivers/radeonsi/si_texture.c
> +++ b/src/gallium/drivers/radeonsi/si_texture.c
> @@ -1791,20 +1791,26 @@ static void *si_texture_transfer_map(struct pipe_context *ctx,
>   
>   		buf = trans->staging;
>   	} else {
>   		/* the resource is mapped directly */
>   		offset = si_texture_get_offset(sctx->screen, tex, level, box,
>   						 &trans->b.b.stride,
>   						 &trans->b.b.layer_stride);
>   		buf = &tex->buffer;
>   	}
>   
> +	/* Always unmap texture CPU mappings on 32-bit architectures, so that
> +	 * we don't run out of the CPU address space.
> +	 */
> +	if (sizeof(void*) == 4)
> +		usage |= RADEON_TRANSFER_TEMPORARY;
> +
>   	if (!(map = si_buffer_map_sync_with_rings(sctx, buf, usage)))
>   		goto fail_trans;
>   
>   	*ptransfer = &trans->b.b;
>   	return map + offset;
>   
>   fail_trans:
>   	r600_resource_reference(&trans->staging, NULL);
>   	pipe_resource_reference(&trans->b.b.resource, NULL);
>   	FREE(trans);
> @@ -1812,20 +1818,30 @@ fail_trans:
>   }
>   
>   static void si_texture_transfer_unmap(struct pipe_context *ctx,
>   				      struct pipe_transfer* transfer)
>   {
>   	struct si_context *sctx = (struct si_context*)ctx;
>   	struct si_transfer *stransfer = (struct si_transfer*)transfer;
>   	struct pipe_resource *texture = transfer->resource;
>   	struct si_texture *tex = (struct si_texture*)texture;
>   
> +	/* Always unmap texture CPU mappings on 32-bit architectures, so that
> +	 * we don't run out of the CPU address space.
> +	 */
> +	if (sizeof(void*) == 4) {
> +		struct r600_resource *buf =
> +			stransfer->staging ? stransfer->staging : &tex->buffer;
> +
> +		sctx->ws->buffer_unmap(buf->buf);
> +	}
> +
>   	if ((transfer->usage & PIPE_TRANSFER_WRITE) && stransfer->staging) {
>   		if (tex->is_depth && tex->buffer.b.b.nr_samples <= 1) {
>   			ctx->resource_copy_region(ctx, texture, transfer->level,
>   						  transfer->box.x, transfer->box.y, transfer->box.z,
>   						  &stransfer->staging->b.b, transfer->level,
>   						  &transfer->box);
>   		} else {
>   			si_copy_from_staging_texture(ctx, stransfer);
>   		}
>   	}