[Mesa-dev] [PATCH 2/2] r600g/compute: Decrement map_count when unmapping items

[Tom Stellard]

On Thu, Aug 07, 2014 at 12:14:24PM +0200, Bruno Jiménez wrote:
> This patch adds a new struct: r600_transfer_global. It will
> act as a wrapper around an r600_resource_global and an r600_transfer.
> 
> It will be used for calling r600_compute_global_transfer_unmap when
> transfer_unmap is called. And at the same time, keep all the transfer
> information, so we can call r600_buffer_transfer_unmap with the
> 'real' transfer.
> ---
>  src/gallium/drivers/r600/evergreen_compute.c | 46 +++++++++++++++++++++-------
>  src/gallium/drivers/r600/evergreen_compute.h |  5 +++
>  2 files changed, 40 insertions(+), 11 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/evergreen_compute.c b/src/gallium/drivers/r600/evergreen_compute.c
> index f50f94a..ac72256 100644
> --- a/src/gallium/drivers/r600/evergreen_compute.c
> +++ b/src/gallium/drivers/r600/evergreen_compute.c
> @@ -970,10 +970,16 @@ void *r600_compute_global_transfer_map(
>  	struct r600_resource_global* buffer =
>  		(struct r600_resource_global*)resource;
>  
> +	struct r600_transfer_global *trans = NULL;
> +	uint8_t *data;
> +
>  	struct compute_memory_item *item = buffer->chunk;
>  	struct pipe_resource *dst = NULL;
>  	unsigned offset = box->x;
>  
> +	trans = CALLOC(1, sizeof(struct r600_transfer_global));
> +	trans->resource = resource;
> +
>  	if (is_item_in_pool(item)) {
>  		compute_memory_demote_item(pool, item, ctx_);
>  	}
> @@ -1004,8 +1010,11 @@ void *r600_compute_global_transfer_map(
>  	assert(box->z == 0);
>  
>  	///TODO: do it better, mapping is not possible if the pool is too big
> -	return pipe_buffer_map_range(ctx_, dst,
> -			offset, box->width, usage, ptransfer);
> +	data = pipe_buffer_map_range(ctx_, dst,
> +			offset, box->width, usage, &trans->ptransfer);
> +
> +	*ptransfer = (struct pipe_transfer *)trans;
> +	return data;
>  }
>  
>  void r600_compute_global_transfer_unmap(
> @@ -1013,16 +1022,31 @@ void r600_compute_global_transfer_unmap(
>  	struct pipe_transfer* transfer)
>  {
>  	/* struct r600_resource_global are not real resources, they just map
> -	 * to an offset within the compute memory pool.  The function
> -	 * r600_compute_global_transfer_map() maps the memory pool
> -	 * resource rather than the struct r600_resource_global passed to
> -	 * it as an argument and then initalizes ptransfer->resource with
> -	 * the memory pool resource (via pipe_buffer_map_range).
> -	 * When transfer_unmap is called it uses the memory pool's
> -	 * vtable which calls r600_buffer_transfer_map() rather than
> -	 * this function.
> +	 * to an offset within the compute memory pool. The function
> +	 * r600_compute_global_transfer_map() creates a struct
> +	 * r600_transfer_global, which has as resource an r600_global_resource
> +	 * and an r600_transfer which will act as the 'real' pipe_transfer
> +	 * that will be passed to pipe_buffer_map_range.
> +	 *
> +	 * This allows us to use an r600_resource_global vtable when transfer_unmap
> +	 * is called, and still have the full information about the transfer,
> +	 * which will be used to actually unmap the resource.
>  	 */
> -	assert (!"This function should not be called");
> +
> +	struct r600_context *rctx = (struct r600_context *)ctx_;
> +	struct r600_transfer_global *trans =
> +		(struct r600_transfer_global *)transfer;
> +	struct r600_resource_global *buffer =
> +		(struct r600_resource_global *)trans->resource;
> +	struct compute_memory_item *item = buffer->chunk;
> +
> +	COMPUTE_DBG(rctx->screen, "* r600_compute_global_transfer_unmap()\n"
> +			"Unmaping Buffer: %u\n", item->id);
> +
> +	ctx_->transfer_unmap(ctx_, trans->ptransfer);
> +	item->map_count--;
> +
> +	FREE(trans);
>  }
>  
>  void r600_compute_global_transfer_flush_region(
> diff --git a/src/gallium/drivers/r600/evergreen_compute.h b/src/gallium/drivers/r600/evergreen_compute.h
> index 4fb53a1..842e5e4 100644
> --- a/src/gallium/drivers/r600/evergreen_compute.h
> +++ b/src/gallium/drivers/r600/evergreen_compute.h
> @@ -38,6 +38,11 @@ struct r600_resource_global {
>  	struct compute_memory_item *chunk;
>  };
>  
> +struct r600_transfer_global {
> +	struct pipe_resource *resource;
> +	struct pipe_transfer *ptransfer;

This still looks wrong. ptransfer should be the first member, and it
should not be a pointer.

-Tom

> +};
> +
>  void *evergreen_create_compute_state(struct pipe_context *ctx, const struct pipe_compute_state *cso);
>  void evergreen_delete_compute_state(struct pipe_context *ctx, void *state);
>  void evergreen_compute_upload_input(struct pipe_context *context, const uint *block_layout, const uint *grid_layout, const void *input);
> -- 
> 2.0.4
>