[Mesa-dev] [PATCH 1/3] r600g/compute: Allow compute_memory_move_item to move items between resources

[Bruno Jimenez]

On Wed, 2014-07-23 at 10:46 -0400, Tom Stellard wrote:
> On Sat, Jul 19, 2014 at 07:35:49PM +0200, Bruno Jiménez wrote:
> > ---
> >  src/gallium/drivers/r600/compute_memory_pool.c | 43 ++++++++++++++------------
> >  src/gallium/drivers/r600/compute_memory_pool.h |  1 +
> >  2 files changed, 25 insertions(+), 19 deletions(-)
> > 
> > diff --git a/src/gallium/drivers/r600/compute_memory_pool.c b/src/gallium/drivers/r600/compute_memory_pool.c
> > index 254c1d7..1ad77ad 100644
> > --- a/src/gallium/drivers/r600/compute_memory_pool.c
> > +++ b/src/gallium/drivers/r600/compute_memory_pool.c
> > @@ -331,6 +331,7 @@ void compute_memory_defrag(struct compute_memory_pool *pool,
> >  	struct pipe_context *pipe)
> >  {
> >  	struct compute_memory_item *item;
> > +	struct pipe_resource *src = (struct pipe_resource *)pool->bo;
> >  	int64_t last_pos;
> >  
> >  	COMPUTE_DBG(pool->screen, "* compute_memory_defrag()\n");
> > @@ -340,7 +341,8 @@ void compute_memory_defrag(struct compute_memory_pool *pool,
> >  		if (item->start_in_dw != last_pos) {
> >  			assert(last_pos < item->start_in_dw);
> >  
> > -			compute_memory_move_item(pool, item, last_pos, pipe);
> > +			compute_memory_move_item(pool, src, src,
> > +					item, last_pos, pipe);
> >  		}
> >  
> >  		last_pos += align(item->size_in_dw, ITEM_ALIGNMENT);
> > @@ -431,7 +433,8 @@ void compute_memory_demote_item(struct compute_memory_pool *pool,
> >  }
> >  
> >  /**
> > - * Moves the item \a item forward in the pool to \a new_start_in_dw
> > + * Moves the item \a item forward from the resource \a src to the
> > + * resource \a dst at \a new_start_in_dw
> >   *
> >   * This function assumes two things:
> >   * 1) The item is \b only moved forward
> > @@ -442,13 +445,14 @@ void compute_memory_demote_item(struct compute_memory_pool *pool,
> >   * \see compute_memory_defrag
> >   */
> >  void compute_memory_move_item(struct compute_memory_pool *pool,
> > +	struct pipe_resource *src, struct pipe_resource *dst,
> >  	struct compute_memory_item *item, uint64_t new_start_in_dw,
> >  	struct pipe_context *pipe)
> >  {
> >  	struct pipe_screen *screen = (struct pipe_screen *)pool->screen;
> >  	struct r600_context *rctx = (struct r600_context *)pipe;
> > -	struct pipe_resource *src = (struct pipe_resource *)pool->bo;
> > -	struct pipe_resource *dst;
> > +	struct pipe_resource *src_ = src;
> > +	struct pipe_resource *dst_;
> 
> I think it is confusing to have variables named _src and src.  Could you
> rename one of them to something more descriptive.

Hi,

Sure, It was a case of 'what do I call these two now?' I have changed it
so we don't need these two variables.

Thanks!
Bruno.

> 
> >  	struct pipe_box box;
> >  
> >  	struct compute_memory_item *prev;
> > @@ -465,34 +469,35 @@ void compute_memory_move_item(struct compute_memory_pool *pool,
> >  
> >  	u_box_1d(item->start_in_dw * 4, item->size_in_dw * 4, &box);
> >  
> > -	/* If the ranges don't overlap, we can just copy the item directly */
> > -	if (new_start_in_dw + item->size_in_dw <= item->start_in_dw) {
> > -		dst = (struct pipe_resource *)pool->bo;
> > +	/* If the ranges don't overlap, or we are copying from one resource
> > +	 * to another, we can just copy the item directly */
> > +	if (src != dst || new_start_in_dw + item->size_in_dw <= item->start_in_dw) {
> > +		dst_ = dst;
> >  
> >  		rctx->b.b.resource_copy_region(pipe,
> > -			dst, 0, new_start_in_dw * 4, 0, 0,
> > -			src, 0, &box);
> > +			dst_, 0, new_start_in_dw * 4, 0, 0,
> > +			src_, 0, &box);
> >  	} else {
> >  		/* The ranges overlap, we will try first to use an intermediate
> >  		 * resource to move the item */
> > -		dst = (struct pipe_resource *)r600_compute_buffer_alloc_vram(
> > +		dst_ = (struct pipe_resource *)r600_compute_buffer_alloc_vram(
> >  				pool->screen, item->size_in_dw * 4);
> >  
> > -		if (dst != NULL) {
> > +		if (dst_ != NULL) {
> >  			rctx->b.b.resource_copy_region(pipe,
> > -				dst, 0, 0, 0, 0,
> > -				src, 0, &box);
> > +				dst_, 0, 0, 0, 0,
> > +				src_, 0, &box);
> >  
> > -			src = dst;
> > -			dst = (struct pipe_resource *)pool->bo;
> > +			src_ = dst_;
> > +			dst_ = dst;
> >  
> >  			box.x = 0;
> >  
> >  			rctx->b.b.resource_copy_region(pipe,
> > -				dst, 0, new_start_in_dw * 4, 0, 0,
> > -				src, 0, &box);
> > +				dst_, 0, new_start_in_dw * 4, 0, 0,
> > +				src_, 0, &box);
> >  
> > -			pool->screen->b.b.resource_destroy(screen, src);
> > +			pool->screen->b.b.resource_destroy(screen, src_);
> >  
> >  		} else {
> >  			/* The allocation of the temporary resource failed,
> > @@ -505,7 +510,7 @@ void compute_memory_move_item(struct compute_memory_pool *pool,
> >  
> >  			u_box_1d(new_start_in_dw * 4, (offset + item->size_in_dw) * 4, &box);
> >  
> > -			map = pipe->transfer_map(pipe, src, 0, PIPE_TRANSFER_READ_WRITE,
> > +			map = pipe->transfer_map(pipe, src_, 0, PIPE_TRANSFER_READ_WRITE,
> >  				&box, &trans);
> >  
> >  			assert(map);
> > diff --git a/src/gallium/drivers/r600/compute_memory_pool.h b/src/gallium/drivers/r600/compute_memory_pool.h
> > index 5a1b33b..822bfbe 100644
> > --- a/src/gallium/drivers/r600/compute_memory_pool.h
> > +++ b/src/gallium/drivers/r600/compute_memory_pool.h
> > @@ -101,6 +101,7 @@ void compute_memory_demote_item(struct compute_memory_pool *pool,
> >  	struct compute_memory_item *item, struct pipe_context *pipe);
> >  
> >  void compute_memory_move_item(struct compute_memory_pool *pool,
> > +	struct pipe_resource *src, struct pipe_resource *dst,
> >  	struct compute_memory_item *item, uint64_t new_start_in_dw,
> >  	struct pipe_context *pipe);
> >  
> > -- 
> > 2.0.2
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev