RE: [PATCH 4/7] drm/syncobj: use only a single stub fence

["Zhou, David(ChunMing)" <David1.Zhou@amd.com>]

> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Thursday, November 15, 2018 7:13 PM
> To: dri-devel@lists.freedesktop.org
> Cc: chris@chris-wilson.co.uk; daniel.vetter@ffwll.ch; eric@anholt.net; Zhou,
> David(ChunMing) <David1.Zhou@amd.com>
> Subject: [PATCH 4/7] drm/syncobj: use only a single stub fence
> 
> Extract of useful code from the timeline work. Let's use just a single stub
> fence instance instead of allocating a new one all the time.
> 
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> Signed-off-by: Christian König <christian.koenig@amd.com>

It is a good conclusion during previous review, there already is my Sined-off, I cannot give RB on that, need other people take an action.

-David
> ---
>  drivers/gpu/drm/drm_syncobj.c | 67 ++++++++++++++++++++++------------
> ---------
>  1 file changed, 35 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c
> b/drivers/gpu/drm/drm_syncobj.c index f190414511ae..4c45acb326b9
> 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -56,10 +56,8 @@
>  #include "drm_internal.h"
>  #include <drm/drm_syncobj.h>
> 
> -struct drm_syncobj_stub_fence {
> -	struct dma_fence base;
> -	spinlock_t lock;
> -};
> +static DEFINE_SPINLOCK(stub_fence_lock); static struct dma_fence
> +stub_fence;
> 
>  static const char *drm_syncobj_stub_fence_get_name(struct dma_fence
> *fence)  { @@ -71,6 +69,25 @@ static const struct dma_fence_ops
> drm_syncobj_stub_fence_ops = {
>  	.get_timeline_name = drm_syncobj_stub_fence_get_name,  };
> 
> +/**
> + * drm_syncobj_get_stub_fence - return a signaled fence
> + *
> + * Return a stub fence which is already signaled.
> + */
> +static struct dma_fence *drm_syncobj_get_stub_fence(void) {
> +	spin_lock(&stub_fence_lock);
> +	if (!stub_fence.ops) {
> +		dma_fence_init(&stub_fence,
> +			       &drm_syncobj_stub_fence_ops,
> +			       &stub_fence_lock,
> +			       0, 0);
> +		dma_fence_signal_locked(&stub_fence);
> +	}
> +	spin_unlock(&stub_fence_lock);
> +
> +	return dma_fence_get(&stub_fence);
> +}
> 
>  /**
>   * drm_syncobj_find - lookup and reference a sync object.
> @@ -190,23 +207,18 @@ void drm_syncobj_replace_fence(struct
> drm_syncobj *syncobj,  }  EXPORT_SYMBOL(drm_syncobj_replace_fence);
> 
> -static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj)
> +/**
> + * drm_syncobj_assign_null_handle - assign a stub fence to the sync
> +object
> + * @syncobj: sync object to assign the fence on
> + *
> + * Assign a already signaled stub fence to the sync object.
> + */
> +static void drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj)
>  {
> -	struct drm_syncobj_stub_fence *fence;
> -	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
> -	if (fence == NULL)
> -		return -ENOMEM;
> +	struct dma_fence *fence = drm_syncobj_get_stub_fence();
> 
> -	spin_lock_init(&fence->lock);
> -	dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops,
> -		       &fence->lock, 0, 0);
> -	dma_fence_signal(&fence->base);
> -
> -	drm_syncobj_replace_fence(syncobj, &fence->base);
> -
> -	dma_fence_put(&fence->base);
> -
> -	return 0;
> +	drm_syncobj_replace_fence(syncobj, fence);
> +	dma_fence_put(fence);
>  }
> 
>  /**
> @@ -273,7 +285,6 @@ EXPORT_SYMBOL(drm_syncobj_free);  int
> drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
>  		       struct dma_fence *fence)
>  {
> -	int ret;
>  	struct drm_syncobj *syncobj;
> 
>  	syncobj = kzalloc(sizeof(struct drm_syncobj), GFP_KERNEL); @@ -
> 284,13 +295,8 @@ int drm_syncobj_create(struct drm_syncobj
> **out_syncobj, uint32_t flags,
>  	INIT_LIST_HEAD(&syncobj->cb_list);
>  	spin_lock_init(&syncobj->lock);
> 
> -	if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) {
> -		ret = drm_syncobj_assign_null_handle(syncobj);
> -		if (ret < 0) {
> -			drm_syncobj_put(syncobj);
> -			return ret;
> -		}
> -	}
> +	if (flags & DRM_SYNCOBJ_CREATE_SIGNALED)
> +		drm_syncobj_assign_null_handle(syncobj);
> 
>  	if (fence)
>  		drm_syncobj_replace_fence(syncobj, fence); @@ -984,11
> +990,8 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
>  	if (ret < 0)
>  		return ret;
> 
> -	for (i = 0; i < args->count_handles; i++) {
> -		ret = drm_syncobj_assign_null_handle(syncobjs[i]);
> -		if (ret < 0)
> -			break;
> -	}
> +	for (i = 0; i < args->count_handles; i++)
> +		drm_syncobj_assign_null_handle(syncobjs[i]);
> 
>  	drm_syncobj_array_free(syncobjs, args->count_handles);
> 
> --
> 2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel