Re: [PATCH 1/2] drm: rename null fence to stub fence in syncobj

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 22, 2018 at 04:38:56PM +0800, Chunming Zhou wrote:
> stub fence will be used by timeline syncobj as well.
> 
> Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285
> Signed-off-by: Chunming Zhou <david1.zhou@xxxxxxx>
> Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx>

Please don't expose stuff only used by the drm_syncobj implementation to
drivers. Gives us a very unclean driver interface. Imo this should all be
left within drm_syncobj.h.

See also my comments for patch 2, you leak all the implemenation details
to drivers. We need to fix that and have a clear interface.
-Daniel

> ---
>  drivers/gpu/drm/drm_syncobj.c | 28 ++--------------------------
>  include/drm/drm_syncobj.h     | 24 ++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index d4f4ce484529..70af32d0def1 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
>  }
>  EXPORT_SYMBOL(drm_syncobj_replace_fence);
>  
> -struct drm_syncobj_null_fence {
> -	struct dma_fence base;
> -	spinlock_t lock;
> -};
> -
> -static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence)
> -{
> -        return "syncobjnull";
> -}
> -
> -static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence)
> -{
> -    dma_fence_enable_sw_signaling(fence);
> -    return !dma_fence_is_signaled(fence);
> -}
> -
> -static const struct dma_fence_ops drm_syncobj_null_fence_ops = {
> -	.get_driver_name = drm_syncobj_null_fence_get_name,
> -	.get_timeline_name = drm_syncobj_null_fence_get_name,
> -	.enable_signaling = drm_syncobj_null_fence_enable_signaling,
> -	.wait = dma_fence_default_wait,
> -	.release = NULL,
> -};
> -
>  static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj)
>  {
> -	struct drm_syncobj_null_fence *fence;
> +	struct drm_syncobj_stub_fence *fence;
>  	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>  	if (fence == NULL)
>  		return -ENOMEM;
>  
>  	spin_lock_init(&fence->lock);
> -	dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops,
> +	dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops,
>  		       &fence->lock, 0, 0);
>  	dma_fence_signal(&fence->base);
>  
> diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
> index 3980602472c0..b04c492ddbb5 100644
> --- a/include/drm/drm_syncobj.h
> +++ b/include/drm/drm_syncobj.h
> @@ -30,6 +30,30 @@
>  
>  struct drm_syncobj_cb;
>  
> +struct drm_syncobj_stub_fence {
> +	struct dma_fence base;
> +	spinlock_t lock;
> +};
> +
> +const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence)
> +{
> +        return "syncobjstub";
> +}
> +
> +bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence)
> +{
> +    dma_fence_enable_sw_signaling(fence);
> +    return !dma_fence_is_signaled(fence);
> +}
> +
> +const struct dma_fence_ops drm_syncobj_stub_fence_ops = {
> +	.get_driver_name = drm_syncobj_stub_fence_get_name,
> +	.get_timeline_name = drm_syncobj_stub_fence_get_name,
> +	.enable_signaling = drm_syncobj_stub_fence_enable_signaling,
> +	.wait = dma_fence_default_wait,
> +	.release = NULL,
> +};
> +
>  /**
>   * struct drm_syncobj - sync object.
>   *
> -- 
> 2.14.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux