[PATCH 0/2] drm/tilcdc: replace reference/unreference with get/put

[PATCH 0/2] drm/tilcdc: replace reference/unreference with get/put

[Aishwarya Pant]

This patchset introduces drm_dev_get() and drm_dev_put() functions that are
intented to be replacements for drm_dev_{ref/unref}.

Then all usages of ref/reference and unref/unreference suffixes are replaced by
get/put reference count functions in tilcdc. The following cocci script was used
to make the tilcdc patch:

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Aishwarya Pant (2):
  drm: introduce drm_dev_{get/put} functions
  drm/tilcdc: replace reference/unreference() with get/put

 drivers/gpu/drm/drm_drv.c            | 64 +++++++++++++++++++++++++-----------
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |  6 ++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  2 +-
 include/drm/drm_drv.h                |  4 ++-
 4 files changed, 52 insertions(+), 24 deletions(-)

-- 
2.7.4

[PATCH 0/2] drm/tilcdc: replace reference/unreference with get/put

[Aishwarya Pant]

This patchset introduces drm_dev_get() and drm_dev_put() functions that are
intented to be replacements for drm_dev_{ref/unref}.

Then all usages of ref/reference and unref/unreference suffixes are replaced by
get/put reference count functions in tilcdc. The following cocci script was used
to make the tilcdc patch:

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Aishwarya Pant (2):
  drm: introduce drm_dev_{get/put} functions
  drm/tilcdc: replace reference/unreference() with get/put

 drivers/gpu/drm/drm_drv.c            | 64 +++++++++++++++++++++++++-----------
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |  6 ++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  2 +-
 include/drm/drm_drv.h                |  4 ++-
 4 files changed, 52 insertions(+), 24 deletions(-)

-- 
2.7.4

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

[PATCH 1/2] drm: introduce drm_dev_{get/put} functions

[Aishwarya Pant]

Reference counting functions in the kernel typically use get/put suffixes. For
maintaining coding style consistency, introduce drm_dev_{get/put} functions
and let the old APIs (with ref/unref suffixes) remain for compatibility.

Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
---
 drivers/gpu/drm/drm_drv.c | 64 +++++++++++++++++++++++++++++++++--------------
 include/drm/drm_drv.h     |  4 ++-
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be38ac7..4592314 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -286,13 +286,13 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
 	spin_lock_irqsave(&drm_minor_lock, flags);
 	minor = idr_find(&drm_minors_idr, minor_id);
 	if (minor)
-		drm_dev_ref(minor->dev);
+		drm_dev_get(minor->dev);
 	spin_unlock_irqrestore(&drm_minor_lock, flags);
 
 	if (!minor) {
 		return ERR_PTR(-ENODEV);
 	} else if (drm_dev_is_unplugged(minor->dev)) {
-		drm_dev_unref(minor->dev);
+		drm_dev_put(minor->dev);
 		return ERR_PTR(-ENODEV);
 	}
 
@@ -301,7 +301,7 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
 
 void drm_minor_release(struct drm_minor *minor)
 {
-	drm_dev_unref(minor->dev);
+	drm_dev_put(minor->dev);
 }
 
 /**
@@ -326,11 +326,11 @@ void drm_minor_release(struct drm_minor *minor)
  * When cleaning up a device instance everything needs to be done in reverse:
  * First unpublish the device instance with drm_dev_unregister(). Then clean up
  * any other resources allocated at device initialization and drop the driver's
- * reference to &drm_device using drm_dev_unref().
+ * reference to &drm_device using drm_dev_put().
  *
  * Note that the lifetime rules for &drm_device instance has still a lot of
  * historical baggage. Hence use the reference counting provided by
- * drm_dev_ref() and drm_dev_unref() only carefully.
+ * drm_dev_get() and drm_dev_put() only carefully.
  *
  * It is recommended that drivers embed &struct drm_device into their own device
  * structure, which is supported through drm_dev_init().
@@ -345,7 +345,7 @@ void drm_minor_release(struct drm_minor *minor)
  * Cleans up all DRM device, calling drm_lastclose().
  *
  * Note: Use of this function is deprecated. It will eventually go away
- * completely.  Please use drm_dev_unregister() and drm_dev_unref() explicitly
+ * completely.  Please use drm_dev_unregister() and drm_dev_put() explicitly
  * instead to make sure that the device isn't userspace accessible any more
  * while teardown is in progress, ensuring that userspace can't access an
  * inconsistent state.
@@ -360,7 +360,7 @@ void drm_put_dev(struct drm_device *dev)
 	}
 
 	drm_dev_unregister(dev);
-	drm_dev_unref(dev);
+	drm_dev_put(dev);
 }
 EXPORT_SYMBOL(drm_put_dev);
 
@@ -386,7 +386,7 @@ void drm_dev_unplug(struct drm_device *dev)
 	mutex_lock(&drm_global_mutex);
 	drm_device_set_unplugged(dev);
 	if (dev->open_count == 0)
-		drm_dev_unref(dev);
+		drm_dev_put(dev);
 	mutex_unlock(&drm_global_mutex);
 }
 EXPORT_SYMBOL(drm_dev_unplug);
@@ -475,8 +475,8 @@ static void drm_fs_inode_free(struct inode *inode)
  * initialization sequence to make sure userspace can't access an inconsistent
  * state.
  *
- * The initial ref-count of the object is 1. Use drm_dev_ref() and
- * drm_dev_unref() to take and drop further ref-counts.
+ * The initial ref-count of the object is 1. Use drm_dev_get() and
+ * drm_dev_put() to take and drop further ref-counts.
  *
  * Note that for purely virtual devices @parent can be NULL.
  *
@@ -626,8 +626,8 @@ EXPORT_SYMBOL(drm_dev_fini);
  * initialization sequence to make sure userspace can't access an inconsistent
  * state.
  *
- * The initial ref-count of the object is 1. Use drm_dev_ref() and
- * drm_dev_unref() to take and drop further ref-counts.
+ * The initial ref-count of the object is 1. Use drm_dev_get() and
+ * drm_dev_put() to take and drop further ref-counts.
  *
  * Note that for purely virtual devices @parent can be NULL.
  *
@@ -670,36 +670,62 @@ static void drm_dev_release(struct kref *ref)
 }
 
 /**
- * drm_dev_ref - Take reference of a DRM device
+ * drm_dev_get - Take reference of a DRM device
  * @dev: device to take reference of or NULL
  *
  * This increases the ref-count of @dev by one. You *must* already own a
- * reference when calling this. Use drm_dev_unref() to drop this reference
+ * reference when calling this. Use drm_dev_put() to drop this reference
  * again.
  *
  * This function never fails. However, this function does not provide *any*
  * guarantee whether the device is alive or running. It only provides a
  * reference to the object and the memory associated with it.
  */
-void drm_dev_ref(struct drm_device *dev)
+void drm_dev_get(struct drm_device *dev)
 {
 	if (dev)
 		kref_get(&dev->ref);
 }
-EXPORT_SYMBOL(drm_dev_ref);
+EXPORT_SYMBOL(drm_dev_get);
 
 /**
- * drm_dev_unref - Drop reference of a DRM device
+ * drm_dev_put - Drop reference of a DRM device
  * @dev: device to drop reference of or NULL
  *
  * This decreases the ref-count of @dev by one. The device is destroyed if the
  * ref-count drops to zero.
  */
-void drm_dev_unref(struct drm_device *dev)
+void drm_dev_put(struct drm_device *dev)
 {
 	if (dev)
 		kref_put(&dev->ref, drm_dev_release);
 }
+EXPORT_SYMBOL(drm_dev_put);
+
+/**
+ * drm_dev_ref - Take reference of a DRM device
+ * @dev: device to take reference of or NULL
+ *
+ * This is a compatibility alias for drm_dev_get() and should not be used by new
+ * code.
+ */
+void drm_dev_ref(struct drm_device *dev)
+{
+	drm_dev_get(dev);
+}
+EXPORT_SYMBOL(drm_dev_ref);
+
+/**
+ * drm_dev_unref - Drop reference of a DRM device
+ * @dev: device to drop reference of or NULL
+ *
+ * This is a compatibility alias for drm_dev_put() and should not be used by new
+ * code.
+ */
+void drm_dev_unref(struct drm_device *dev)
+{
+	drm_dev_put(dev);
+}
 EXPORT_SYMBOL(drm_dev_unref);
 
 static int create_compat_control_link(struct drm_device *dev)
@@ -839,7 +865,7 @@ EXPORT_SYMBOL(drm_dev_register);
  *
  * Unregister the DRM device from the system. This does the reverse of
  * drm_dev_register() but does not deallocate the device. The caller must call
- * drm_dev_unref() to drop their final reference.
+ * drm_dev_put() to drop their final reference.
  *
  * A special form of unregistering for hotpluggable devices is drm_dev_unplug(),
  * which can be called while there are still open users of @dev.
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 71bbaae..e574fa0 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -155,7 +155,7 @@ struct drm_driver {
 	 * reverse order of the initialization.  Similarly to the load
 	 * hook, this handler is deprecated and its usage should be
 	 * dropped in favor of an open-coded teardown function at the
-	 * driver layer.  See drm_dev_unregister() and drm_dev_unref()
+	 * driver layer.  See drm_dev_unregister() and drm_dev_put()
 	 * for the proper way to remove a &struct drm_device.
 	 *
 	 * The unload() hook is called right after unregistering
@@ -611,6 +611,8 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 int drm_dev_register(struct drm_device *dev, unsigned long flags);
 void drm_dev_unregister(struct drm_device *dev);
 
+void drm_dev_get(struct drm_device *dev);
+void drm_dev_put(struct drm_device *dev);
 void drm_dev_ref(struct drm_device *dev);
 void drm_dev_unref(struct drm_device *dev);
 void drm_put_dev(struct drm_device *dev);
-- 
2.7.4

[PATCH 1/2] drm: introduce drm_dev_{get/put} functions

[Aishwarya Pant]

Reference counting functions in the kernel typically use get/put suffixes. For
maintaining coding style consistency, introduce drm_dev_{get/put} functions
and let the old APIs (with ref/unref suffixes) remain for compatibility.

Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
---
 drivers/gpu/drm/drm_drv.c | 64 +++++++++++++++++++++++++++++++++--------------
 include/drm/drm_drv.h     |  4 ++-
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be38ac7..4592314 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -286,13 +286,13 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
 	spin_lock_irqsave(&drm_minor_lock, flags);
 	minor = idr_find(&drm_minors_idr, minor_id);
 	if (minor)
-		drm_dev_ref(minor->dev);
+		drm_dev_get(minor->dev);
 	spin_unlock_irqrestore(&drm_minor_lock, flags);
 
 	if (!minor) {
 		return ERR_PTR(-ENODEV);
 	} else if (drm_dev_is_unplugged(minor->dev)) {
-		drm_dev_unref(minor->dev);
+		drm_dev_put(minor->dev);
 		return ERR_PTR(-ENODEV);
 	}
 
@@ -301,7 +301,7 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
 
 void drm_minor_release(struct drm_minor *minor)
 {
-	drm_dev_unref(minor->dev);
+	drm_dev_put(minor->dev);
 }
 
 /**
@@ -326,11 +326,11 @@ void drm_minor_release(struct drm_minor *minor)
  * When cleaning up a device instance everything needs to be done in reverse:
  * First unpublish the device instance with drm_dev_unregister(). Then clean up
  * any other resources allocated at device initialization and drop the driver's
- * reference to &drm_device using drm_dev_unref().
+ * reference to &drm_device using drm_dev_put().
  *
  * Note that the lifetime rules for &drm_device instance has still a lot of
  * historical baggage. Hence use the reference counting provided by
- * drm_dev_ref() and drm_dev_unref() only carefully.
+ * drm_dev_get() and drm_dev_put() only carefully.
  *
  * It is recommended that drivers embed &struct drm_device into their own device
  * structure, which is supported through drm_dev_init().
@@ -345,7 +345,7 @@ void drm_minor_release(struct drm_minor *minor)
  * Cleans up all DRM device, calling drm_lastclose().
  *
  * Note: Use of this function is deprecated. It will eventually go away
- * completely.  Please use drm_dev_unregister() and drm_dev_unref() explicitly
+ * completely.  Please use drm_dev_unregister() and drm_dev_put() explicitly
  * instead to make sure that the device isn't userspace accessible any more
  * while teardown is in progress, ensuring that userspace can't access an
  * inconsistent state.
@@ -360,7 +360,7 @@ void drm_put_dev(struct drm_device *dev)
 	}
 
 	drm_dev_unregister(dev);
-	drm_dev_unref(dev);
+	drm_dev_put(dev);
 }
 EXPORT_SYMBOL(drm_put_dev);
 
@@ -386,7 +386,7 @@ void drm_dev_unplug(struct drm_device *dev)
 	mutex_lock(&drm_global_mutex);
 	drm_device_set_unplugged(dev);
 	if (dev->open_count == 0)
-		drm_dev_unref(dev);
+		drm_dev_put(dev);
 	mutex_unlock(&drm_global_mutex);
 }
 EXPORT_SYMBOL(drm_dev_unplug);
@@ -475,8 +475,8 @@ static void drm_fs_inode_free(struct inode *inode)
  * initialization sequence to make sure userspace can't access an inconsistent
  * state.
  *
- * The initial ref-count of the object is 1. Use drm_dev_ref() and
- * drm_dev_unref() to take and drop further ref-counts.
+ * The initial ref-count of the object is 1. Use drm_dev_get() and
+ * drm_dev_put() to take and drop further ref-counts.
  *
  * Note that for purely virtual devices @parent can be NULL.
  *
@@ -626,8 +626,8 @@ EXPORT_SYMBOL(drm_dev_fini);
  * initialization sequence to make sure userspace can't access an inconsistent
  * state.
  *
- * The initial ref-count of the object is 1. Use drm_dev_ref() and
- * drm_dev_unref() to take and drop further ref-counts.
+ * The initial ref-count of the object is 1. Use drm_dev_get() and
+ * drm_dev_put() to take and drop further ref-counts.
  *
  * Note that for purely virtual devices @parent can be NULL.
  *
@@ -670,36 +670,62 @@ static void drm_dev_release(struct kref *ref)
 }
 
 /**
- * drm_dev_ref - Take reference of a DRM device
+ * drm_dev_get - Take reference of a DRM device
  * @dev: device to take reference of or NULL
  *
  * This increases the ref-count of @dev by one. You *must* already own a
- * reference when calling this. Use drm_dev_unref() to drop this reference
+ * reference when calling this. Use drm_dev_put() to drop this reference
  * again.
  *
  * This function never fails. However, this function does not provide *any*
  * guarantee whether the device is alive or running. It only provides a
  * reference to the object and the memory associated with it.
  */
-void drm_dev_ref(struct drm_device *dev)
+void drm_dev_get(struct drm_device *dev)
 {
 	if (dev)
 		kref_get(&dev->ref);
 }
-EXPORT_SYMBOL(drm_dev_ref);
+EXPORT_SYMBOL(drm_dev_get);
 
 /**
- * drm_dev_unref - Drop reference of a DRM device
+ * drm_dev_put - Drop reference of a DRM device
  * @dev: device to drop reference of or NULL
  *
  * This decreases the ref-count of @dev by one. The device is destroyed if the
  * ref-count drops to zero.
  */
-void drm_dev_unref(struct drm_device *dev)
+void drm_dev_put(struct drm_device *dev)
 {
 	if (dev)
 		kref_put(&dev->ref, drm_dev_release);
 }
+EXPORT_SYMBOL(drm_dev_put);
+
+/**
+ * drm_dev_ref - Take reference of a DRM device
+ * @dev: device to take reference of or NULL
+ *
+ * This is a compatibility alias for drm_dev_get() and should not be used by new
+ * code.
+ */
+void drm_dev_ref(struct drm_device *dev)
+{
+	drm_dev_get(dev);
+}
+EXPORT_SYMBOL(drm_dev_ref);
+
+/**
+ * drm_dev_unref - Drop reference of a DRM device
+ * @dev: device to drop reference of or NULL
+ *
+ * This is a compatibility alias for drm_dev_put() and should not be used by new
+ * code.
+ */
+void drm_dev_unref(struct drm_device *dev)
+{
+	drm_dev_put(dev);
+}
 EXPORT_SYMBOL(drm_dev_unref);
 
 static int create_compat_control_link(struct drm_device *dev)
@@ -839,7 +865,7 @@ EXPORT_SYMBOL(drm_dev_register);
  *
  * Unregister the DRM device from the system. This does the reverse of
  * drm_dev_register() but does not deallocate the device. The caller must call
- * drm_dev_unref() to drop their final reference.
+ * drm_dev_put() to drop their final reference.
  *
  * A special form of unregistering for hotpluggable devices is drm_dev_unplug(),
  * which can be called while there are still open users of @dev.
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 71bbaae..e574fa0 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -155,7 +155,7 @@ struct drm_driver {
 	 * reverse order of the initialization.  Similarly to the load
 	 * hook, this handler is deprecated and its usage should be
 	 * dropped in favor of an open-coded teardown function at the
-	 * driver layer.  See drm_dev_unregister() and drm_dev_unref()
+	 * driver layer.  See drm_dev_unregister() and drm_dev_put()
 	 * for the proper way to remove a &struct drm_device.
 	 *
 	 * The unload() hook is called right after unregistering
@@ -611,6 +611,8 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 int drm_dev_register(struct drm_device *dev, unsigned long flags);
 void drm_dev_unregister(struct drm_device *dev);
 
+void drm_dev_get(struct drm_device *dev);
+void drm_dev_put(struct drm_device *dev);
 void drm_dev_ref(struct drm_device *dev);
 void drm_dev_unref(struct drm_device *dev);
 void drm_put_dev(struct drm_device *dev);
-- 
2.7.4

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

Re: [Outreachy kernel] [PATCH 1/2] drm: introduce drm_dev_{get/put} functions

[Daniel Vetter]

On Fri, Sep 22, 2017 at 01:17:06AM +0530, Aishwarya Pant wrote:
> Reference counting functions in the kernel typically use get/put suffixes. For
> maintaining coding style consistency, introduce drm_dev_{get/put} functions
> and let the old APIs (with ref/unref suffixes) remain for compatibility.
> 
> Signed-off-by: Aishwarya Pant <aishpant@gmail.com>

Excellent idea to fill this gap, we totally overlooked this in the initial
work!

> ---
>  drivers/gpu/drm/drm_drv.c | 64 +++++++++++++++++++++++++++++++++--------------
>  include/drm/drm_drv.h     |  4 ++-

Please also update scripts/coccinelle/api/drm-get-put.cocci with this new
transformation.

>  2 files changed, 48 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index be38ac7..4592314 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -286,13 +286,13 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
>  	spin_lock_irqsave(&drm_minor_lock, flags);
>  	minor = idr_find(&drm_minors_idr, minor_id);
>  	if (minor)
> -		drm_dev_ref(minor->dev);
> +		drm_dev_get(minor->dev);
>  	spin_unlock_irqrestore(&drm_minor_lock, flags);
>  
>  	if (!minor) {
>  		return ERR_PTR(-ENODEV);
>  	} else if (drm_dev_is_unplugged(minor->dev)) {
> -		drm_dev_unref(minor->dev);
> +		drm_dev_put(minor->dev);
>  		return ERR_PTR(-ENODEV);
>  	}
>  
> @@ -301,7 +301,7 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
>  
>  void drm_minor_release(struct drm_minor *minor)
>  {
> -	drm_dev_unref(minor->dev);
> +	drm_dev_put(minor->dev);
>  }
>  
>  /**
> @@ -326,11 +326,11 @@ void drm_minor_release(struct drm_minor *minor)
>   * When cleaning up a device instance everything needs to be done in reverse:
>   * First unpublish the device instance with drm_dev_unregister(). Then clean up
>   * any other resources allocated at device initialization and drop the driver's
> - * reference to &drm_device using drm_dev_unref().
> + * reference to &drm_device using drm_dev_put().
>   *
>   * Note that the lifetime rules for &drm_device instance has still a lot of
>   * historical baggage. Hence use the reference counting provided by
> - * drm_dev_ref() and drm_dev_unref() only carefully.
> + * drm_dev_get() and drm_dev_put() only carefully.
>   *
>   * It is recommended that drivers embed &struct drm_device into their own device
>   * structure, which is supported through drm_dev_init().
> @@ -345,7 +345,7 @@ void drm_minor_release(struct drm_minor *minor)
>   * Cleans up all DRM device, calling drm_lastclose().
>   *
>   * Note: Use of this function is deprecated. It will eventually go away
> - * completely.  Please use drm_dev_unregister() and drm_dev_unref() explicitly
> + * completely.  Please use drm_dev_unregister() and drm_dev_put() explicitly
>   * instead to make sure that the device isn't userspace accessible any more
>   * while teardown is in progress, ensuring that userspace can't access an
>   * inconsistent state.
> @@ -360,7 +360,7 @@ void drm_put_dev(struct drm_device *dev)
>  	}
>  
>  	drm_dev_unregister(dev);
> -	drm_dev_unref(dev);
> +	drm_dev_put(dev);
>  }
>  EXPORT_SYMBOL(drm_put_dev);
>  
> @@ -386,7 +386,7 @@ void drm_dev_unplug(struct drm_device *dev)
>  	mutex_lock(&drm_global_mutex);
>  	drm_device_set_unplugged(dev);
>  	if (dev->open_count == 0)
> -		drm_dev_unref(dev);
> +		drm_dev_put(dev);
>  	mutex_unlock(&drm_global_mutex);
>  }
>  EXPORT_SYMBOL(drm_dev_unplug);
> @@ -475,8 +475,8 @@ static void drm_fs_inode_free(struct inode *inode)
>   * initialization sequence to make sure userspace can't access an inconsistent
>   * state.
>   *
> - * The initial ref-count of the object is 1. Use drm_dev_ref() and
> - * drm_dev_unref() to take and drop further ref-counts.
> + * The initial ref-count of the object is 1. Use drm_dev_get() and
> + * drm_dev_put() to take and drop further ref-counts.
>   *
>   * Note that for purely virtual devices @parent can be NULL.
>   *
> @@ -626,8 +626,8 @@ EXPORT_SYMBOL(drm_dev_fini);
>   * initialization sequence to make sure userspace can't access an inconsistent
>   * state.
>   *
> - * The initial ref-count of the object is 1. Use drm_dev_ref() and
> - * drm_dev_unref() to take and drop further ref-counts.
> + * The initial ref-count of the object is 1. Use drm_dev_get() and
> + * drm_dev_put() to take and drop further ref-counts.
>   *
>   * Note that for purely virtual devices @parent can be NULL.
>   *
> @@ -670,36 +670,62 @@ static void drm_dev_release(struct kref *ref)
>  }
>  
>  /**
> - * drm_dev_ref - Take reference of a DRM device
> + * drm_dev_get - Take reference of a DRM device
>   * @dev: device to take reference of or NULL
>   *
>   * This increases the ref-count of @dev by one. You *must* already own a
> - * reference when calling this. Use drm_dev_unref() to drop this reference
> + * reference when calling this. Use drm_dev_put() to drop this reference
>   * again.
>   *
>   * This function never fails. However, this function does not provide *any*
>   * guarantee whether the device is alive or running. It only provides a
>   * reference to the object and the memory associated with it.
>   */
> -void drm_dev_ref(struct drm_device *dev)
> +void drm_dev_get(struct drm_device *dev)
>  {
>  	if (dev)
>  		kref_get(&dev->ref);
>  }
> -EXPORT_SYMBOL(drm_dev_ref);
> +EXPORT_SYMBOL(drm_dev_get);
>  
>  /**
> - * drm_dev_unref - Drop reference of a DRM device
> + * drm_dev_put - Drop reference of a DRM device
>   * @dev: device to drop reference of or NULL
>   *
>   * This decreases the ref-count of @dev by one. The device is destroyed if the
>   * ref-count drops to zero.
>   */
> -void drm_dev_unref(struct drm_device *dev)
> +void drm_dev_put(struct drm_device *dev)
>  {
>  	if (dev)
>  		kref_put(&dev->ref, drm_dev_release);
>  }
> +EXPORT_SYMBOL(drm_dev_put);
> +
> +/**
> + * drm_dev_ref - Take reference of a DRM device
> + * @dev: device to take reference of or NULL
> + *
> + * This is a compatibility alias for drm_dev_get() and should not be used by new
> + * code.
> + */
> +void drm_dev_ref(struct drm_device *dev)
> +{
> +	drm_dev_get(dev);
> +}
> +EXPORT_SYMBOL(drm_dev_ref);

I think you've already converted all callers for drm_dev_ref() with this
patch here, so no need to add the compat function and risk getting new
callers. Once a conversion is done in upstream we should remove these.

Please remove.

With those 2 nits addressed I think your patch is ready for merging,
please resend with those changes.

Thanks, Daniel

> +
> +/**
> + * drm_dev_unref - Drop reference of a DRM device
> + * @dev: device to drop reference of or NULL
> + *
> + * This is a compatibility alias for drm_dev_put() and should not be used by new
> + * code.
> + */
> +void drm_dev_unref(struct drm_device *dev)
> +{
> +	drm_dev_put(dev);
> +}
>  EXPORT_SYMBOL(drm_dev_unref);
>  
>  static int create_compat_control_link(struct drm_device *dev)
> @@ -839,7 +865,7 @@ EXPORT_SYMBOL(drm_dev_register);
>   *
>   * Unregister the DRM device from the system. This does the reverse of
>   * drm_dev_register() but does not deallocate the device. The caller must call
> - * drm_dev_unref() to drop their final reference.
> + * drm_dev_put() to drop their final reference.
>   *
>   * A special form of unregistering for hotpluggable devices is drm_dev_unplug(),
>   * which can be called while there are still open users of @dev.
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 71bbaae..e574fa0 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -155,7 +155,7 @@ struct drm_driver {
>  	 * reverse order of the initialization.  Similarly to the load
>  	 * hook, this handler is deprecated and its usage should be
>  	 * dropped in favor of an open-coded teardown function at the
> -	 * driver layer.  See drm_dev_unregister() and drm_dev_unref()
> +	 * driver layer.  See drm_dev_unregister() and drm_dev_put()
>  	 * for the proper way to remove a &struct drm_device.
>  	 *
>  	 * The unload() hook is called right after unregistering
> @@ -611,6 +611,8 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
>  int drm_dev_register(struct drm_device *dev, unsigned long flags);
>  void drm_dev_unregister(struct drm_device *dev);
>  
> +void drm_dev_get(struct drm_device *dev);
> +void drm_dev_put(struct drm_device *dev);
>  void drm_dev_ref(struct drm_device *dev);
>  void drm_dev_unref(struct drm_device *dev);
>  void drm_put_dev(struct drm_device *dev);
> -- 
> 2.7.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/33c3c79daa7a8f9c52ee87f4d78f3a551ab96b54.1506023126.git.aishpant%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Outreachy kernel] [PATCH 1/2] drm: introduce drm_dev_{get/put} functions

[Daniel Vetter]

On Fri, Sep 22, 2017 at 01:17:06AM +0530, Aishwarya Pant wrote:
> Reference counting functions in the kernel typically use get/put suffixes. For
> maintaining coding style consistency, introduce drm_dev_{get/put} functions
> and let the old APIs (with ref/unref suffixes) remain for compatibility.
> 
> Signed-off-by: Aishwarya Pant <aishpant@gmail.com>

Excellent idea to fill this gap, we totally overlooked this in the initial
work!

> ---
>  drivers/gpu/drm/drm_drv.c | 64 +++++++++++++++++++++++++++++++++--------------
>  include/drm/drm_drv.h     |  4 ++-

Please also update scripts/coccinelle/api/drm-get-put.cocci with this new
transformation.

>  2 files changed, 48 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index be38ac7..4592314 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -286,13 +286,13 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
>  	spin_lock_irqsave(&drm_minor_lock, flags);
>  	minor = idr_find(&drm_minors_idr, minor_id);
>  	if (minor)
> -		drm_dev_ref(minor->dev);
> +		drm_dev_get(minor->dev);
>  	spin_unlock_irqrestore(&drm_minor_lock, flags);
>  
>  	if (!minor) {
>  		return ERR_PTR(-ENODEV);
>  	} else if (drm_dev_is_unplugged(minor->dev)) {
> -		drm_dev_unref(minor->dev);
> +		drm_dev_put(minor->dev);
>  		return ERR_PTR(-ENODEV);
>  	}
>  
> @@ -301,7 +301,7 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
>  
>  void drm_minor_release(struct drm_minor *minor)
>  {
> -	drm_dev_unref(minor->dev);
> +	drm_dev_put(minor->dev);
>  }
>  
>  /**
> @@ -326,11 +326,11 @@ void drm_minor_release(struct drm_minor *minor)
>   * When cleaning up a device instance everything needs to be done in reverse:
>   * First unpublish the device instance with drm_dev_unregister(). Then clean up
>   * any other resources allocated at device initialization and drop the driver's
> - * reference to &drm_device using drm_dev_unref().
> + * reference to &drm_device using drm_dev_put().
>   *
>   * Note that the lifetime rules for &drm_device instance has still a lot of
>   * historical baggage. Hence use the reference counting provided by
> - * drm_dev_ref() and drm_dev_unref() only carefully.
> + * drm_dev_get() and drm_dev_put() only carefully.
>   *
>   * It is recommended that drivers embed &struct drm_device into their own device
>   * structure, which is supported through drm_dev_init().
> @@ -345,7 +345,7 @@ void drm_minor_release(struct drm_minor *minor)
>   * Cleans up all DRM device, calling drm_lastclose().
>   *
>   * Note: Use of this function is deprecated. It will eventually go away
> - * completely.  Please use drm_dev_unregister() and drm_dev_unref() explicitly
> + * completely.  Please use drm_dev_unregister() and drm_dev_put() explicitly
>   * instead to make sure that the device isn't userspace accessible any more
>   * while teardown is in progress, ensuring that userspace can't access an
>   * inconsistent state.
> @@ -360,7 +360,7 @@ void drm_put_dev(struct drm_device *dev)
>  	}
>  
>  	drm_dev_unregister(dev);
> -	drm_dev_unref(dev);
> +	drm_dev_put(dev);
>  }
>  EXPORT_SYMBOL(drm_put_dev);
>  
> @@ -386,7 +386,7 @@ void drm_dev_unplug(struct drm_device *dev)
>  	mutex_lock(&drm_global_mutex);
>  	drm_device_set_unplugged(dev);
>  	if (dev->open_count == 0)
> -		drm_dev_unref(dev);
> +		drm_dev_put(dev);
>  	mutex_unlock(&drm_global_mutex);
>  }
>  EXPORT_SYMBOL(drm_dev_unplug);
> @@ -475,8 +475,8 @@ static void drm_fs_inode_free(struct inode *inode)
>   * initialization sequence to make sure userspace can't access an inconsistent
>   * state.
>   *
> - * The initial ref-count of the object is 1. Use drm_dev_ref() and
> - * drm_dev_unref() to take and drop further ref-counts.
> + * The initial ref-count of the object is 1. Use drm_dev_get() and
> + * drm_dev_put() to take and drop further ref-counts.
>   *
>   * Note that for purely virtual devices @parent can be NULL.
>   *
> @@ -626,8 +626,8 @@ EXPORT_SYMBOL(drm_dev_fini);
>   * initialization sequence to make sure userspace can't access an inconsistent
>   * state.
>   *
> - * The initial ref-count of the object is 1. Use drm_dev_ref() and
> - * drm_dev_unref() to take and drop further ref-counts.
> + * The initial ref-count of the object is 1. Use drm_dev_get() and
> + * drm_dev_put() to take and drop further ref-counts.
>   *
>   * Note that for purely virtual devices @parent can be NULL.
>   *
> @@ -670,36 +670,62 @@ static void drm_dev_release(struct kref *ref)
>  }
>  
>  /**
> - * drm_dev_ref - Take reference of a DRM device
> + * drm_dev_get - Take reference of a DRM device
>   * @dev: device to take reference of or NULL
>   *
>   * This increases the ref-count of @dev by one. You *must* already own a
> - * reference when calling this. Use drm_dev_unref() to drop this reference
> + * reference when calling this. Use drm_dev_put() to drop this reference
>   * again.
>   *
>   * This function never fails. However, this function does not provide *any*
>   * guarantee whether the device is alive or running. It only provides a
>   * reference to the object and the memory associated with it.
>   */
> -void drm_dev_ref(struct drm_device *dev)
> +void drm_dev_get(struct drm_device *dev)
>  {
>  	if (dev)
>  		kref_get(&dev->ref);
>  }
> -EXPORT_SYMBOL(drm_dev_ref);
> +EXPORT_SYMBOL(drm_dev_get);
>  
>  /**
> - * drm_dev_unref - Drop reference of a DRM device
> + * drm_dev_put - Drop reference of a DRM device
>   * @dev: device to drop reference of or NULL
>   *
>   * This decreases the ref-count of @dev by one. The device is destroyed if the
>   * ref-count drops to zero.
>   */
> -void drm_dev_unref(struct drm_device *dev)
> +void drm_dev_put(struct drm_device *dev)
>  {
>  	if (dev)
>  		kref_put(&dev->ref, drm_dev_release);
>  }
> +EXPORT_SYMBOL(drm_dev_put);
> +
> +/**
> + * drm_dev_ref - Take reference of a DRM device
> + * @dev: device to take reference of or NULL
> + *
> + * This is a compatibility alias for drm_dev_get() and should not be used by new
> + * code.
> + */
> +void drm_dev_ref(struct drm_device *dev)
> +{
> +	drm_dev_get(dev);
> +}
> +EXPORT_SYMBOL(drm_dev_ref);

I think you've already converted all callers for drm_dev_ref() with this
patch here, so no need to add the compat function and risk getting new
callers. Once a conversion is done in upstream we should remove these.

Please remove.

With those 2 nits addressed I think your patch is ready for merging,
please resend with those changes.

Thanks, Daniel

> +
> +/**
> + * drm_dev_unref - Drop reference of a DRM device
> + * @dev: device to drop reference of or NULL
> + *
> + * This is a compatibility alias for drm_dev_put() and should not be used by new
> + * code.
> + */
> +void drm_dev_unref(struct drm_device *dev)
> +{
> +	drm_dev_put(dev);
> +}
>  EXPORT_SYMBOL(drm_dev_unref);
>  
>  static int create_compat_control_link(struct drm_device *dev)
> @@ -839,7 +865,7 @@ EXPORT_SYMBOL(drm_dev_register);
>   *
>   * Unregister the DRM device from the system. This does the reverse of
>   * drm_dev_register() but does not deallocate the device. The caller must call
> - * drm_dev_unref() to drop their final reference.
> + * drm_dev_put() to drop their final reference.
>   *
>   * A special form of unregistering for hotpluggable devices is drm_dev_unplug(),
>   * which can be called while there are still open users of @dev.
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 71bbaae..e574fa0 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -155,7 +155,7 @@ struct drm_driver {
>  	 * reverse order of the initialization.  Similarly to the load
>  	 * hook, this handler is deprecated and its usage should be
>  	 * dropped in favor of an open-coded teardown function at the
> -	 * driver layer.  See drm_dev_unregister() and drm_dev_unref()
> +	 * driver layer.  See drm_dev_unregister() and drm_dev_put()
>  	 * for the proper way to remove a &struct drm_device.
>  	 *
>  	 * The unload() hook is called right after unregistering
> @@ -611,6 +611,8 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
>  int drm_dev_register(struct drm_device *dev, unsigned long flags);
>  void drm_dev_unregister(struct drm_device *dev);
>  
> +void drm_dev_get(struct drm_device *dev);
> +void drm_dev_put(struct drm_device *dev);
>  void drm_dev_ref(struct drm_device *dev);
>  void drm_dev_unref(struct drm_device *dev);
>  void drm_put_dev(struct drm_device *dev);
> -- 
> 2.7.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/33c3c79daa7a8f9c52ee87f4d78f3a551ab96b54.1506023126.git.aishpant%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.

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

[PATCH 2/2] drm/tilcdc: replace reference/unreference() with get/put

[Aishwarya Pant]

For maintaining consistency with kernel coding style replace
reference/unreference in ref counting functions with get/put.

The following cocci script was used to generate the tilcdc patch:

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 6 +++---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 406fe45..d2589f310 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -75,7 +75,7 @@ static void unref_worker(struct drm_flip_work *work, void *val)
 	struct drm_device *dev = tilcdc_crtc->base.dev;
 
 	mutex_lock(&dev->mode_config.mutex);
-	drm_framebuffer_unreference(val);
+	drm_framebuffer_put(val);
 	mutex_unlock(&dev->mode_config.mutex);
 }
 
@@ -456,7 +456,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
 
 	set_scanout(crtc, fb);
 
-	drm_framebuffer_reference(fb);
+	drm_framebuffer_get(fb);
 
 	crtc->hwmode = crtc->state->adjusted_mode;
 }
@@ -633,7 +633,7 @@ int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
 		return -EBUSY;
 	}
 
-	drm_framebuffer_reference(fb);
+	drm_framebuffer_get(fb);
 
 	crtc->primary->fb = fb;
 	tilcdc_crtc->event = event;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index b0d70f9..74276ef 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -225,7 +225,7 @@ static void tilcdc_fini(struct drm_device *dev)
 
 	pm_runtime_disable(dev->dev);
 
-	drm_dev_unref(dev);
+	drm_dev_put(dev);
 }
 
 static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
-- 
2.7.4

[PATCH 2/2] drm/tilcdc: replace reference/unreference() with get/put

[Aishwarya Pant]

For maintaining consistency with kernel coding style replace
reference/unreference in ref counting functions with get/put.

The following cocci script was used to generate the tilcdc patch:

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 6 +++---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 406fe45..d2589f310 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -75,7 +75,7 @@ static void unref_worker(struct drm_flip_work *work, void *val)
 	struct drm_device *dev = tilcdc_crtc->base.dev;
 
 	mutex_lock(&dev->mode_config.mutex);
-	drm_framebuffer_unreference(val);
+	drm_framebuffer_put(val);
 	mutex_unlock(&dev->mode_config.mutex);
 }
 
@@ -456,7 +456,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
 
 	set_scanout(crtc, fb);
 
-	drm_framebuffer_reference(fb);
+	drm_framebuffer_get(fb);
 
 	crtc->hwmode = crtc->state->adjusted_mode;
 }
@@ -633,7 +633,7 @@ int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
 		return -EBUSY;
 	}
 
-	drm_framebuffer_reference(fb);
+	drm_framebuffer_get(fb);
 
 	crtc->primary->fb = fb;
 	tilcdc_crtc->event = event;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index b0d70f9..74276ef 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -225,7 +225,7 @@ static void tilcdc_fini(struct drm_device *dev)
 
 	pm_runtime_disable(dev->dev);
 
-	drm_dev_unref(dev);
+	drm_dev_put(dev);
 }
 
 static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
-- 
2.7.4

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