Re: [PATCH 1/5] drm: introduce sync objects (v2)

2017-05-12 Thread Sean Paul 
On Fri, May 12, 2017 at 10:34:53AM +1000, Dave Airlie wrote:
> From: Dave Airlie 
> 
> Sync objects are new toplevel drm object, that contain a
> pointer to a fence. This fence can be updated via command
> submission ioctls via drivers.
> 
> There is also a generic wait obj API modelled on the vulkan
> wait API (with code modelled on some amdgpu code).
> 
> These objects can be converted to an opaque fd that can be
> passes between processes.
> 
> v2: rename reference/unreference to put/get (Chris)
> fix leaked reference (David Zhou)
> drop mutex in favour of cmpxchg (Chris)
> document drm_syncobj_fence_get
> use ENOENT for syncobj lookup.
> 
> Signed-off-by: Dave Airlie 

With Daniel's comments addressed,

Reviewed-by: Sean Paul 

> 
> fixup
> ---
>  Documentation/gpu/drm-internals.rst |   3 +
>  Documentation/gpu/drm-mm.rst|   6 +
>  drivers/gpu/drm/Makefile|   2 +-
>  drivers/gpu/drm/drm_fops.c  |   8 +
>  drivers/gpu/drm/drm_internal.h  |  13 ++
>  drivers/gpu/drm/drm_ioctl.c |  12 ++
>  drivers/gpu/drm/drm_syncobj.c   | 385 
> 
>  include/drm/drmP.h  |   5 +
>  include/drm/drm_drv.h   |   1 +
>  include/drm/drm_syncobj.h   |  87 
>  include/uapi/drm/drm.h  |  25 +++
>  11 files changed, 546 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_syncobj.c
>  create mode 100644 include/drm/drm_syncobj.h
> 
> diff --git a/Documentation/gpu/drm-internals.rst 
> b/Documentation/gpu/drm-internals.rst
> index e35920d..2ea3bce 100644
> --- a/Documentation/gpu/drm-internals.rst
> +++ b/Documentation/gpu/drm-internals.rst
> @@ -98,6 +98,9 @@ DRIVER_ATOMIC
>  implement appropriate obj->atomic_get_property() vfuncs for any
>  modeset objects with driver specific properties.
>  
> +DRIVER_SYNCOBJ
> +Driver support drm sync objects.
> +
>  Major, Minor and Patchlevel
>  ~~~
>  
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index f5760b1..28aebe8 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -483,3 +483,9 @@ DRM Cache Handling
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_cache.c
> :export:
> +
> +DRM Sync Objects
> +===
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
> +   :export:
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 3ee9579..b5e565c 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -16,7 +16,7 @@ drm-y   :=  drm_auth.o drm_bufs.o drm_cache.o \
>   drm_framebuffer.o drm_connector.o drm_blend.o \
>   drm_encoder.o drm_mode_object.o drm_property.o \
>   drm_plane.o drm_color_mgmt.o drm_print.o \
> - drm_dumb_buffers.o drm_mode_config.o
> + drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o
>  
>  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>  drm-$(CONFIG_DRM_VM) += drm_vm.o
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index afdf5b1..9a61df2 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct 
> drm_minor *minor)
>   if (drm_core_check_feature(dev, DRIVER_GEM))
>   drm_gem_open(dev, priv);
>  
> + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> + drm_syncobj_open(priv);
> +
>   if (drm_core_check_feature(dev, DRIVER_PRIME))
>   drm_prime_init_file_private(>prime);
>  
> @@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct 
> drm_minor *minor)
>  out_prime_destroy:
>   if (drm_core_check_feature(dev, DRIVER_PRIME))
>   drm_prime_destroy_file_private(>prime);
> + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> + drm_syncobj_release(priv);
>   if (drm_core_check_feature(dev, DRIVER_GEM))
>   drm_gem_release(dev, priv);
>   put_pid(priv->pid);
> @@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)
>   drm_property_destroy_user_blobs(dev, file_priv);
>   }
>  
> + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> + drm_syncobj_release(file_priv);
> +
>   if (drm_core_check_feature(dev, DRIVER_GEM))
>   drm_gem_release(dev, file_priv);
>  
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index f37388c..44ef903 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct 
> drm_crtc *crtc)
>  {
>   return 0;
>  }
> +
>  #endif
> +
> +/* drm_syncobj.c */
> +void drm_syncobj_open(struct drm_file *file_private);
> +void drm_syncobj_release(struct drm_file *file_private);
> +int

Re: [PATCH 1/5] drm: introduce sync objects (v2)

2017-05-12 Thread Daniel Vetter 
On Fri, May 12, 2017 at 10:34:53AM +1000, Dave Airlie wrote:
> From: Dave Airlie 
> 
> Sync objects are new toplevel drm object, that contain a
> pointer to a fence. This fence can be updated via command
> submission ioctls via drivers.
> 
> There is also a generic wait obj API modelled on the vulkan
> wait API (with code modelled on some amdgpu code).
> 
> These objects can be converted to an opaque fd that can be
> passes between processes.
> 
> v2: rename reference/unreference to put/get (Chris)
> fix leaked reference (David Zhou)
> drop mutex in favour of cmpxchg (Chris)
> document drm_syncobj_fence_get
> use ENOENT for syncobj lookup.
> 
> Signed-off-by: Dave Airlie 
> 
> fixup

Found a few ioctl and other nits, with those addressed

Acked-by: Daniel Vetter 
> ---
>  Documentation/gpu/drm-internals.rst |   3 +
>  Documentation/gpu/drm-mm.rst|   6 +
>  drivers/gpu/drm/Makefile|   2 +-
>  drivers/gpu/drm/drm_fops.c  |   8 +
>  drivers/gpu/drm/drm_internal.h  |  13 ++
>  drivers/gpu/drm/drm_ioctl.c |  12 ++
>  drivers/gpu/drm/drm_syncobj.c   | 385 
> 
>  include/drm/drmP.h  |   5 +
>  include/drm/drm_drv.h   |   1 +
>  include/drm/drm_syncobj.h   |  87 
>  include/uapi/drm/drm.h  |  25 +++
>  11 files changed, 546 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_syncobj.c
>  create mode 100644 include/drm/drm_syncobj.h
> 
> diff --git a/Documentation/gpu/drm-internals.rst 
> b/Documentation/gpu/drm-internals.rst
> index e35920d..2ea3bce 100644
> --- a/Documentation/gpu/drm-internals.rst
> +++ b/Documentation/gpu/drm-internals.rst
> @@ -98,6 +98,9 @@ DRIVER_ATOMIC
>  implement appropriate obj->atomic_get_property() vfuncs for any
>  modeset objects with driver specific properties.
>  
> +DRIVER_SYNCOBJ
> +Driver support drm sync objects.
> +
>  Major, Minor and Patchlevel
>  ~~~
>  
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index f5760b1..28aebe8 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -483,3 +483,9 @@ DRM Cache Handling
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_cache.c
> :export:
> +
> +DRM Sync Objects
> +===
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
> +   :export:

You're missing parts of your pretty kernel-doc!

.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
   :doc: Overview

.. kernel-doc:: include/drm/drm_syncobj.h
   :export:

.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
   :export:

> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 3ee9579..b5e565c 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -16,7 +16,7 @@ drm-y   :=  drm_auth.o drm_bufs.o drm_cache.o \
>   drm_framebuffer.o drm_connector.o drm_blend.o \
>   drm_encoder.o drm_mode_object.o drm_property.o \
>   drm_plane.o drm_color_mgmt.o drm_print.o \
> - drm_dumb_buffers.o drm_mode_config.o
> + drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o
>  
>  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>  drm-$(CONFIG_DRM_VM) += drm_vm.o
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index afdf5b1..9a61df2 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct 
> drm_minor *minor)
>   if (drm_core_check_feature(dev, DRIVER_GEM))
>   drm_gem_open(dev, priv);
>  
> + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> + drm_syncobj_open(priv);
> +
>   if (drm_core_check_feature(dev, DRIVER_PRIME))
>   drm_prime_init_file_private(>prime);
>  
> @@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct 
> drm_minor *minor)
>  out_prime_destroy:
>   if (drm_core_check_feature(dev, DRIVER_PRIME))
>   drm_prime_destroy_file_private(>prime);
> + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> + drm_syncobj_release(priv);
>   if (drm_core_check_feature(dev, DRIVER_GEM))
>   drm_gem_release(dev, priv);
>   put_pid(priv->pid);
> @@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)
>   drm_property_destroy_user_blobs(dev, file_priv);
>   }
>  
> + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> + drm_syncobj_release(file_priv);
> +
>   if (drm_core_check_feature(dev, DRIVER_GEM))
>   drm_gem_release(dev, file_priv);
>  
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index f37388c..44ef903 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -142,4 +142,17 @@ static inline int

[PATCH 1/5] drm: introduce sync objects (v2)

2017-05-11 Thread Dave Airlie 
From: Dave Airlie 

Sync objects are new toplevel drm object, that contain a
pointer to a fence. This fence can be updated via command
submission ioctls via drivers.

There is also a generic wait obj API modelled on the vulkan
wait API (with code modelled on some amdgpu code).

These objects can be converted to an opaque fd that can be
passes between processes.

v2: rename reference/unreference to put/get (Chris)
fix leaked reference (David Zhou)
drop mutex in favour of cmpxchg (Chris)
document drm_syncobj_fence_get
use ENOENT for syncobj lookup.

Signed-off-by: Dave Airlie 

fixup
---
 Documentation/gpu/drm-internals.rst |   3 +
 Documentation/gpu/drm-mm.rst|   6 +
 drivers/gpu/drm/Makefile|   2 +-
 drivers/gpu/drm/drm_fops.c  |   8 +
 drivers/gpu/drm/drm_internal.h  |  13 ++
 drivers/gpu/drm/drm_ioctl.c |  12 ++
 drivers/gpu/drm/drm_syncobj.c   | 385 
 include/drm/drmP.h  |   5 +
 include/drm/drm_drv.h   |   1 +
 include/drm/drm_syncobj.h   |  87 
 include/uapi/drm/drm.h  |  25 +++
 11 files changed, 546 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_syncobj.c
 create mode 100644 include/drm/drm_syncobj.h

diff --git a/Documentation/gpu/drm-internals.rst 
b/Documentation/gpu/drm-internals.rst
index e35920d..2ea3bce 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -98,6 +98,9 @@ DRIVER_ATOMIC
 implement appropriate obj->atomic_get_property() vfuncs for any
 modeset objects with driver specific properties.
 
+DRIVER_SYNCOBJ
+Driver support drm sync objects.
+
 Major, Minor and Patchlevel
 ~~~
 
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index f5760b1..28aebe8 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -483,3 +483,9 @@ DRM Cache Handling
 
 .. kernel-doc:: drivers/gpu/drm/drm_cache.c
:export:
+
+DRM Sync Objects
+===
+
+.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
+   :export:
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 3ee9579..b5e565c 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,7 +16,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_framebuffer.o drm_connector.o drm_blend.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
-   drm_dumb_buffers.o drm_mode_config.o
+   drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o
 
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index afdf5b1..9a61df2 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct 
drm_minor *minor)
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_open(dev, priv);
 
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_open(priv);
+
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_init_file_private(>prime);
 
@@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct 
drm_minor *minor)
 out_prime_destroy:
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_destroy_file_private(>prime);
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_release(priv);
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, priv);
put_pid(priv->pid);
@@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)
drm_property_destroy_user_blobs(dev, file_priv);
}
 
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_release(file_priv);
+
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, file_priv);
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index f37388c..44ef903 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc 
*crtc)
 {
return 0;
 }
+
 #endif
+
+/* drm_syncobj.c */
+void drm_syncobj_open(struct drm_file *file_private);
+void drm_syncobj_release(struct drm_file *file_private);
+int drm_syncobj_create_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file_private);
+int drm_syncobj_destroy_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
+  struct