Re: [PATCH 3/4] drm/radeon: add DONT_SYNC flag to CS relocs

[Jerome Glisse <j.glisse@gmail.com>]

On Thu, Aug 14, 2014 at 06:12:04PM +0200, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
> 
> This allows userspace to explicitly don't sync submission to
> different rings as long as all uses stay in the same client.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/radeon/radeon.h     |  3 +++
>  drivers/gpu/drm/radeon/radeon_cs.c  | 23 ++++++++++++++++++++++-
>  drivers/gpu/drm/radeon/radeon_gem.c |  1 +
>  drivers/gpu/drm/radeon/radeon_ttm.c |  3 +++
>  include/uapi/drm/radeon_drm.h       |  2 ++
>  5 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index c0f7773..740a0b2 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -478,6 +478,8 @@ struct radeon_bo {
>  	u32				tiling_flags;
>  	u32				pitch;
>  	int				surface_reg;
> +	struct drm_file			*last_user;
> +	struct radeon_fence		*last_sync;
>  	struct radeon_fence		*written;
>  	/* list of all virtual address to which this bo
>  	 * is associated to
> @@ -1018,6 +1020,7 @@ struct radeon_cs_reloc {
>  	unsigned			allowed_domains;
>  	uint32_t			tiling_flags;
>  	uint32_t			handle;
> +	uint32_t			flags;
>  	bool				written;
>  };
>  
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
> index 3aa7e48..11e4789 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -166,6 +166,7 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
>  
>  		p->relocs[i].tv.bo = &p->relocs[i].robj->tbo;
>  		p->relocs[i].handle = r->handle;
> +		p->relocs[i].flags = r->flags;
>  		p->relocs[i].written = !!r->write_domain;
>  
>  		radeon_cs_buckets_add(&buckets, &p->relocs[i].tv.head,
> @@ -236,6 +237,14 @@ static void radeon_cs_sync_rings(struct radeon_cs_parser *p)
>  		if (!bo)
>  			continue;
>  
> +		/* always sync to the last operation
> +		   the clients doesn't know about */
> +		radeon_semaphore_sync_to(p->ib.presync, bo->last_sync);
> +
> +		if (bo->last_user == p->filp &&
> +		    reloc->flags & RADEON_RELOC_DONT_SYNC)
> +			continue;
> +
>  		fence = bo->tbo.sync_obj;
>  
>  		if (bo->written && radeon_fence_signaled(bo->written))
> @@ -421,7 +430,19 @@ static void radeon_cs_parser_fini(struct radeon_cs_parser *parser, int error, bo
>  			struct radeon_cs_reloc *reloc = &parser->relocs[i];
>  			struct radeon_bo *bo = reloc->robj;
>  
> -			if (!bo || !reloc->written)
> +			if (!bo)
> +				continue;
> +
> +			/* if the client changes remember that and always serialize
> +			   all operations from different clients */
> +			if (bo->last_user != parser->filp && bo->tbo.sync_obj) {
> +				struct radeon_fence *fence = bo->tbo.sync_obj;
> +				radeon_fence_unref(&bo->last_sync);
> +				bo->last_sync = radeon_fence_ref(fence);
> +			}
> +			bo->last_user = parser->filp;
> +
> +			if (!reloc->written)
>  				continue;
>  
>  			radeon_fence_unref(&bo->written);
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index bfd7e1b..c73dbc1 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -259,6 +259,7 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data,
>  		r = radeon_gem_handle_lockup(rdev, r);
>  		return r;
>  	}
> +	gem_to_radeon_bo(gobj)->last_user = filp;
>  	r = drm_gem_handle_create(filp, gobj, &handle);
>  	/* drop reference from allocate - handle holds it now */
>  	drm_gem_object_unreference_unlocked(gobj);
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 76be612..a4f964f 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -273,6 +273,9 @@ static int radeon_move_blit(struct ttm_buffer_object *bo,
>  			&fence);
>  
>  	if (!r) {
> +		rbo->last_user = NULL;
> +		radeon_fence_unref(&rbo->last_sync);
> +		rbo->last_sync = radeon_fence_ref(fence);
>  		radeon_fence_unref(&rbo->written);
>  		rbo->written = radeon_fence_ref(fence);
>  	}
> diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h
> index 509b2d7..5bd3f68 100644
> --- a/include/uapi/drm/radeon_drm.h
> +++ b/include/uapi/drm/radeon_drm.h
> @@ -944,6 +944,8 @@ struct drm_radeon_cs_chunk {
>  };
>  
>  /* drm_radeon_cs_reloc.flags */
> +#define RADEON_RELOC_PRIO_MASK		(0xf << 0)

RADEON_RELOC_PRIO_MASK not use anywhere, not explain by anything, so new API
with no justification, i would say NAK

> +#define RADEON_RELOC_DONT_SYNC		(1 << 4)
>  
>  struct drm_radeon_cs_reloc {
>  	uint32_t		handle;
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel