* [PATCH 0/6] drm: Introduce consistent reference counting APIs
@ 2017-02-08 18:24 Thierry Reding
2017-02-08 18:24 ` [PATCH 1/6] drm: Rename drm_mode_object_get() Thierry Reding
` (7 more replies)
0 siblings, 8 replies; 25+ messages in thread
From: Thierry Reding @ 2017-02-08 18:24 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
From: Thierry Reding <treding@nvidia.com>
This series introduces DRM reference counting APIs that are consistent
with other reference counting APIs in the kernel. They are also much
shorter. Compatibility aliases are added to keep existing code working
and will stay in place until all users of the old APIs are gone.
All occurrences of the old APIs in the core are already replaced by
this series, and a semantic patch is provided that allows drivers to
be converted automatically. I plan on generating patches against the
drivers once the new functions have been merged and send them out to
their respective maintainers.
Oh, and this fixes one of the items in our recently added TODO list.
Thanks,
Thierry
Thierry Reding (6):
drm: Rename drm_mode_object_get()
drm: Introduce drm_mode_object_{get,put}()
drm: Introduce drm_connector_{get,put}()
drm: Introduce drm_framebuffer_{get,put}()
drm: Introduce drm_gem_object_{get,put}()
drm: Introduce drm_property_blob_{get,put}()
Documentation/gpu/drm-mm.rst | 14 +++--
drivers/gpu/drm/drm_atomic.c | 44 +++++++--------
drivers/gpu/drm/drm_atomic_helper.c | 26 ++++-----
drivers/gpu/drm/drm_connector.c | 16 +++---
drivers/gpu/drm/drm_crtc.c | 12 ++---
drivers/gpu/drm/drm_crtc_helper.c | 6 +--
drivers/gpu/drm/drm_crtc_internal.h | 12 ++---
drivers/gpu/drm/drm_encoder.c | 2 +-
drivers/gpu/drm/drm_fb_cma_helper.c | 16 +++---
drivers/gpu/drm/drm_fb_helper.c | 12 ++---
drivers/gpu/drm/drm_framebuffer.c | 38 ++++++-------
drivers/gpu/drm/drm_gem.c | 44 +++++++--------
drivers/gpu/drm/drm_gem_cma_helper.c | 10 ++--
drivers/gpu/drm/drm_mode_config.c | 4 +-
drivers/gpu/drm/drm_mode_object.c | 44 +++++++--------
drivers/gpu/drm/drm_modes.c | 2 +-
drivers/gpu/drm/drm_plane.c | 14 ++---
drivers/gpu/drm/drm_prime.c | 10 ++--
drivers/gpu/drm/drm_property.c | 52 +++++++++---------
include/drm/drm_connector.h | 41 +++++++++++---
include/drm/drm_framebuffer.h | 49 ++++++++++++-----
include/drm/drm_gem.h | 80 +++++++++++++++++++++------
include/drm/drm_mode_object.h | 36 ++++++++++---
include/drm/drm_property.h | 35 ++++++++++--
scripts/coccinelle/api/drm-get-put.cocci | 92 ++++++++++++++++++++++++++++++++
25 files changed, 471 insertions(+), 240 deletions(-)
create mode 100644 scripts/coccinelle/api/drm-get-put.cocci
--
2.11.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/6] drm: Rename drm_mode_object_get()
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
@ 2017-02-08 18:24 ` Thierry Reding
2017-02-08 19:23 ` Sean Paul
2017-02-08 18:24 ` [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}() Thierry Reding
` (6 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thierry Reding @ 2017-02-08 18:24 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
From: Thierry Reding <treding@nvidia.com>
Subsequent patches will introduce reference counting APIs that are more
consistent with similar APIs throughout the Linux kernel. These APIs use
the _get() and _put() suffixes and will collide with this existing
function.
Rename the function to drm_mode_object_add() which is a slightly more
accurate description of what it does. Also the kerneldoc for this
function gives an indication that it's badly named because it doesn't
actually acquire a reference to anything.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_connector.c | 6 +++---
drivers/gpu/drm/drm_crtc.c | 2 +-
drivers/gpu/drm/drm_crtc_internal.h | 12 +++++-------
drivers/gpu/drm/drm_encoder.c | 2 +-
drivers/gpu/drm/drm_framebuffer.c | 4 ++--
drivers/gpu/drm/drm_mode_object.c | 18 +++++++-----------
drivers/gpu/drm/drm_modes.c | 2 +-
drivers/gpu/drm/drm_plane.c | 2 +-
drivers/gpu/drm/drm_property.c | 6 +++---
9 files changed, 24 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 45464c8b797d..0616062b055a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -189,9 +189,9 @@ int drm_connector_init(struct drm_device *dev,
struct ida *connector_ida =
&drm_connector_enum_list[connector_type].ida;
- ret = drm_mode_object_get_reg(dev, &connector->base,
- DRM_MODE_OBJECT_CONNECTOR,
- false, drm_connector_free);
+ ret = __drm_mode_object_add(dev, &connector->base,
+ DRM_MODE_OBJECT_CONNECTOR,
+ false, drm_connector_free);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 6915f897bd8e..e2284539f82c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -282,7 +282,7 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
spin_lock_init(&crtc->commit_lock);
drm_modeset_lock_init(&crtc->mutex);
- ret = drm_mode_object_get(dev, &crtc->base, DRM_MODE_OBJECT_CRTC);
+ ret = drm_mode_object_add(dev, &crtc->base, DRM_MODE_OBJECT_CRTC);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 955c5690bf64..43bbf48ee129 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -98,15 +98,13 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv);
/* drm_mode_object.c */
-int drm_mode_object_get_reg(struct drm_device *dev,
- struct drm_mode_object *obj,
- uint32_t obj_type,
- bool register_obj,
- void (*obj_free_cb)(struct kref *kref));
+int __drm_mode_object_add(struct drm_device *dev, struct drm_mode_object *obj,
+ uint32_t obj_type, bool register_obj,
+ void (*obj_free_cb)(struct kref *kref));
+int drm_mode_object_add(struct drm_device *dev, struct drm_mode_object *obj,
+ uint32_t obj_type);
void drm_mode_object_register(struct drm_device *dev,
struct drm_mode_object *obj);
-int drm_mode_object_get(struct drm_device *dev,
- struct drm_mode_object *obj, uint32_t obj_type);
struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
uint32_t id, uint32_t type);
void drm_mode_object_unregister(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 129450713bb7..634ae0244ea9 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -110,7 +110,7 @@ int drm_encoder_init(struct drm_device *dev,
{
int ret;
- ret = drm_mode_object_get(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
+ ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 28a0108a1ab8..11daa24692aa 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -638,8 +638,8 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
fb->funcs = funcs;
- ret = drm_mode_object_get_reg(dev, &fb->base, DRM_MODE_OBJECT_FB,
- false, drm_framebuffer_free);
+ ret = __drm_mode_object_add(dev, &fb->base, DRM_MODE_OBJECT_FB,
+ false, drm_framebuffer_free);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 220a6c1f4ab9..3b405dbf1b8d 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -31,11 +31,9 @@
* Internal function to assign a slot in the object idr and optionally
* register the object into the idr.
*/
-int drm_mode_object_get_reg(struct drm_device *dev,
- struct drm_mode_object *obj,
- uint32_t obj_type,
- bool register_obj,
- void (*obj_free_cb)(struct kref *kref))
+int __drm_mode_object_add(struct drm_device *dev, struct drm_mode_object *obj,
+ uint32_t obj_type, bool register_obj,
+ void (*obj_free_cb)(struct kref *kref))
{
int ret;
@@ -59,23 +57,21 @@ int drm_mode_object_get_reg(struct drm_device *dev,
}
/**
- * drm_mode_object_get - allocate a new modeset identifier
+ * drm_mode_object_add - allocate a new modeset identifier
* @dev: DRM device
* @obj: object pointer, used to generate unique ID
* @obj_type: object type
*
* Create a unique identifier based on @ptr in @dev's identifier space. Used
- * for tracking modes, CRTCs and connectors. Note that despite the _get postfix
- * modeset identifiers are _not_ reference counted. Hence don't use this for
- * reference counted modeset objects like framebuffers.
+ * for tracking modes, CRTCs and connectors.
*
* Returns:
* Zero on success, error code on failure.
*/
-int drm_mode_object_get(struct drm_device *dev,
+int drm_mode_object_add(struct drm_device *dev,
struct drm_mode_object *obj, uint32_t obj_type)
{
- return drm_mode_object_get_reg(dev, obj, obj_type, true, NULL);
+ return __drm_mode_object_add(dev, obj, obj_type, true, NULL);
}
void drm_mode_object_register(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index fd22c1c891bf..f2493b9b82e6 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -71,7 +71,7 @@ struct drm_display_mode *drm_mode_create(struct drm_device *dev)
if (!nmode)
return NULL;
- if (drm_mode_object_get(dev, &nmode->base, DRM_MODE_OBJECT_MODE)) {
+ if (drm_mode_object_add(dev, &nmode->base, DRM_MODE_OBJECT_MODE)) {
kfree(nmode);
return NULL;
}
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index c464fc4a874d..f42590049a3a 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -88,7 +88,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
struct drm_mode_config *config = &dev->mode_config;
int ret;
- ret = drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
+ ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 7fc070f3e49e..411e470369c0 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -91,7 +91,7 @@ struct drm_property *drm_property_create(struct drm_device *dev, int flags,
goto fail;
}
- ret = drm_mode_object_get(dev, &property->base, DRM_MODE_OBJECT_PROPERTY);
+ ret = drm_mode_object_add(dev, &property->base, DRM_MODE_OBJECT_PROPERTY);
if (ret)
goto fail;
@@ -570,8 +570,8 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
if (data)
memcpy(blob->data, data, length);
- ret = drm_mode_object_get_reg(dev, &blob->base, DRM_MODE_OBJECT_BLOB,
- true, drm_property_free_blob);
+ ret = __drm_mode_object_add(dev, &blob->base, DRM_MODE_OBJECT_BLOB,
+ true, drm_property_free_blob);
if (ret) {
kfree(blob);
return ERR_PTR(-EINVAL);
--
2.11.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
2017-02-08 18:24 ` [PATCH 1/6] drm: Rename drm_mode_object_get() Thierry Reding
@ 2017-02-08 18:24 ` Thierry Reding
2017-02-08 19:28 ` Sean Paul
2017-02-08 18:24 ` [PATCH 3/6] drm: Introduce drm_connector_{get,put}() Thierry Reding
` (5 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thierry Reding @ 2017-02-08 18:24 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
From: Thierry Reding <treding@nvidia.com>
For consistency with other reference counting APIs in the kernel, add
drm_mode_object_get() and drm_mode_object_put() to reference count DRM
mode objects.
Compatibility aliases are added to keep existing code working. To help
speed up the transition, all the instances of the old functions in the
DRM core are already replaced in this commit.
A semantic patch is provided that can be used to convert all drivers to
the new helpers.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_atomic.c | 14 +++++------
drivers/gpu/drm/drm_mode_object.c | 26 ++++++++++----------
drivers/gpu/drm/drm_property.c | 6 ++---
include/drm/drm_mode_object.h | 36 ++++++++++++++++++++++-----
scripts/coccinelle/api/drm-get-put.cocci | 42 ++++++++++++++++++++++++++++++++
5 files changed, 95 insertions(+), 29 deletions(-)
create mode 100644 scripts/coccinelle/api/drm-get-put.cocci
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index a5673107db26..2bb0a759e8ec 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -2122,13 +2122,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
}
if (!obj->properties) {
- drm_mode_object_unreference(obj);
+ drm_mode_object_put(obj);
ret = -ENOENT;
goto out;
}
if (get_user(count_props, count_props_ptr + copied_objs)) {
- drm_mode_object_unreference(obj);
+ drm_mode_object_put(obj);
ret = -EFAULT;
goto out;
}
@@ -2141,14 +2141,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
struct drm_property *prop;
if (get_user(prop_id, props_ptr + copied_props)) {
- drm_mode_object_unreference(obj);
+ drm_mode_object_put(obj);
ret = -EFAULT;
goto out;
}
prop = drm_mode_obj_find_prop_id(obj, prop_id);
if (!prop) {
- drm_mode_object_unreference(obj);
+ drm_mode_object_put(obj);
ret = -ENOENT;
goto out;
}
@@ -2156,14 +2156,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
if (copy_from_user(&prop_value,
prop_values_ptr + copied_props,
sizeof(prop_value))) {
- drm_mode_object_unreference(obj);
+ drm_mode_object_put(obj);
ret = -EFAULT;
goto out;
}
ret = atomic_set_prop(state, obj, prop, prop_value);
if (ret) {
- drm_mode_object_unreference(obj);
+ drm_mode_object_put(obj);
goto out;
}
@@ -2176,7 +2176,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
plane_mask |= (1 << drm_plane_index(plane));
plane->old_fb = plane->fb;
}
- drm_mode_object_unreference(obj);
+ drm_mode_object_put(obj);
}
ret = prepare_crtc_signaling(dev, state, arg, file_priv, &fence_state,
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 3b405dbf1b8d..da9a9adbcc98 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -133,7 +133,7 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
*
* This function is used to look up a modeset object. It will acquire a
* reference for reference counted objects. This reference must be dropped again
- * by callind drm_mode_object_unreference().
+ * by callind drm_mode_object_put().
*/
struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
uint32_t id, uint32_t type)
@@ -146,38 +146,38 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
EXPORT_SYMBOL(drm_mode_object_find);
/**
- * drm_mode_object_unreference - decr the object refcnt
- * @obj: mode_object
+ * drm_mode_object_put - release a mode object reference
+ * @obj: DRM mode object
*
* This function decrements the object's refcount if it is a refcounted modeset
* object. It is a no-op on any other object. This is used to drop references
- * acquired with drm_mode_object_reference().
+ * acquired with drm_mode_object_get().
*/
-void drm_mode_object_unreference(struct drm_mode_object *obj)
+void drm_mode_object_put(struct drm_mode_object *obj)
{
if (obj->free_cb) {
DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, kref_read(&obj->refcount));
kref_put(&obj->refcount, obj->free_cb);
}
}
-EXPORT_SYMBOL(drm_mode_object_unreference);
+EXPORT_SYMBOL(drm_mode_object_put);
/**
- * drm_mode_object_reference - incr the object refcnt
- * @obj: mode_object
+ * drm_mode_object_get - acquire a mode object reference
+ * @obj: DRM mode object
*
* This function increments the object's refcount if it is a refcounted modeset
* object. It is a no-op on any other object. References should be dropped again
- * by calling drm_mode_object_unreference().
+ * by calling drm_mode_object_put().
*/
-void drm_mode_object_reference(struct drm_mode_object *obj)
+void drm_mode_object_get(struct drm_mode_object *obj)
{
if (obj->free_cb) {
DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, kref_read(&obj->refcount));
kref_get(&obj->refcount);
}
}
-EXPORT_SYMBOL(drm_mode_object_reference);
+EXPORT_SYMBOL(drm_mode_object_get);
/**
* drm_object_attach_property - attach a property to a modeset object
@@ -363,7 +363,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
&arg->count_props);
out_unref:
- drm_mode_object_unreference(obj);
+ drm_mode_object_put(obj);
out:
drm_modeset_unlock_all(dev);
return ret;
@@ -428,7 +428,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
drm_property_change_valid_put(property, ref);
out_unref:
- drm_mode_object_unreference(arg_obj);
+ drm_mode_object_put(arg_obj);
out:
drm_modeset_unlock_all(dev);
return ret;
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 411e470369c0..15af0d42e8be 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -597,7 +597,7 @@ void drm_property_unreference_blob(struct drm_property_blob *blob)
if (!blob)
return;
- drm_mode_object_unreference(&blob->base);
+ drm_mode_object_put(&blob->base);
}
EXPORT_SYMBOL(drm_property_unreference_blob);
@@ -625,7 +625,7 @@ void drm_property_destroy_user_blobs(struct drm_device *dev,
*/
struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob)
{
- drm_mode_object_reference(&blob->base);
+ drm_mode_object_get(&blob->base);
return blob;
}
EXPORT_SYMBOL(drm_property_reference_blob);
@@ -906,7 +906,7 @@ void drm_property_change_valid_put(struct drm_property *property,
return;
if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
- drm_mode_object_unreference(ref);
+ drm_mode_object_put(ref);
} else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
drm_property_unreference_blob(obj_to_blob(ref));
}
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index 2c017adf6d74..a767b4a30a6d 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -45,10 +45,10 @@ struct drm_device;
* drm_object_attach_property() before the object is visible to userspace.
*
* - For objects with dynamic lifetimes (as indicated by a non-NULL @free_cb) it
- * provides reference counting through drm_mode_object_reference() and
- * drm_mode_object_unreference(). This is used by &drm_framebuffer,
- * &drm_connector and &drm_property_blob. These objects provide specialized
- * reference counting wrappers.
+ * provides reference counting through drm_mode_object_get() and
+ * drm_mode_object_put(). This is used by &drm_framebuffer, &drm_connector
+ * and &drm_property_blob. These objects provide specialized reference
+ * counting wrappers.
*/
struct drm_mode_object {
uint32_t id;
@@ -114,8 +114,32 @@ struct drm_object_properties {
struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
uint32_t id, uint32_t type);
-void drm_mode_object_reference(struct drm_mode_object *obj);
-void drm_mode_object_unreference(struct drm_mode_object *obj);
+void drm_mode_object_get(struct drm_mode_object *obj);
+void drm_mode_object_put(struct drm_mode_object *obj);
+
+/**
+ * drm_mode_object_reference - acquire a mode object reference
+ * @obj: DRM mode object
+ *
+ * This is a compatibility alias for drm_mode_object_get() and should not be
+ * used by new code.
+ */
+static inline void drm_mode_object_reference(struct drm_mode_object *obj)
+{
+ drm_mode_object_get(obj);
+}
+
+/**
+ * drm_mode_object_unreference - release a mode object reference
+ * @obj: DRM mode object
+ *
+ * This is a compatibility alias for drm_mode_object_put() and should not be
+ * used by new code.
+ */
+static inline void drm_mode_object_unreference(struct drm_mode_object *obj)
+{
+ drm_mode_object_put(obj);
+}
int drm_object_property_set_value(struct drm_mode_object *obj,
struct drm_property *property,
diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
new file mode 100644
index 000000000000..a3742447c981
--- /dev/null
+++ b/scripts/coccinelle/api/drm-get-put.cocci
@@ -0,0 +1,42 @@
+///
+/// Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference() and
+/// drm_*_unreference() helpers.
+///
+// Confidence: High
+// Copyright: (C) 2017 NVIDIA Corporation
+// Options: --no-includes --include-headers
+//
+
+virtual patch
+virtual report
+
+@depends on patch@
+expression object;
+@@
+
+(
+- drm_mode_object_reference(object)
++ drm_mode_object_get(object)
+|
+- drm_mode_object_unreference(object)
++ drm_mode_object_put(object)
+)
+
+@r depends on report@
+expression object;
+position p;
+@@
+
+(
+drm_mode_object_unreference@p(object)
+|
+drm_mode_object_reference@p(object)
+)
+
+@script:python depends on report@
+object << r.object;
+p << r.p;
+@@
+
+msg="WARNING: use get/put helpers to reference and dereference %s" % (object)
+coccilib.report.print_report(p[0], msg)
--
2.11.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/6] drm: Introduce drm_connector_{get,put}()
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
2017-02-08 18:24 ` [PATCH 1/6] drm: Rename drm_mode_object_get() Thierry Reding
2017-02-08 18:24 ` [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}() Thierry Reding
@ 2017-02-08 18:24 ` Thierry Reding
2017-02-08 19:33 ` Sean Paul
2017-02-08 18:24 ` [PATCH 4/6] drm: Introduce drm_framebuffer_{get,put}() Thierry Reding
` (4 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thierry Reding @ 2017-02-08 18:24 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
From: Thierry Reding <treding@nvidia.com>
For consistency with other reference counting APIs in the kernel, add
drm_connector_get() and drm_connector_put() functions to reference count
connectors.
Compatibility aliases are added to keep existing code working. To help
speed up the transition, all the instances of the old functions in the
DRM core are already replaced in this commit.
The existing semantic patch for mode object reference count conversion
is extended for these new helpers.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_atomic.c | 8 +++----
drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
drivers/gpu/drm/drm_connector.c | 10 ++++----
drivers/gpu/drm/drm_crtc.c | 2 +-
drivers/gpu/drm/drm_crtc_helper.c | 6 ++---
drivers/gpu/drm/drm_fb_helper.c | 12 +++++-----
drivers/gpu/drm/drm_mode_config.c | 2 +-
include/drm/drm_connector.h | 41 +++++++++++++++++++++++++-------
scripts/coccinelle/api/drm-get-put.cocci | 10 ++++++++
9 files changed, 65 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 2bb0a759e8ec..82bad40b2f3e 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -150,7 +150,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
state->connectors[i].state);
state->connectors[i].ptr = NULL;
state->connectors[i].state = NULL;
- drm_connector_unreference(connector);
+ drm_connector_put(connector);
}
for (i = 0; i < config->num_crtc; i++) {
@@ -1026,7 +1026,7 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
if (!connector_state)
return ERR_PTR(-ENOMEM);
- drm_connector_reference(connector);
+ drm_connector_get(connector);
state->connectors[index].state = connector_state;
state->connectors[index].ptr = connector;
connector_state->state = state;
@@ -1357,7 +1357,7 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
crtc_state->connector_mask &=
~(1 << drm_connector_index(conn_state->connector));
- drm_connector_unreference(conn_state->connector);
+ drm_connector_put(conn_state->connector);
conn_state->crtc = NULL;
}
@@ -1369,7 +1369,7 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
crtc_state->connector_mask |=
1 << drm_connector_index(conn_state->connector);
- drm_connector_reference(conn_state->connector);
+ drm_connector_get(conn_state->connector);
conn_state->crtc = crtc;
DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d:%s]\n",
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9a08445a7a7a..9f2c28ddf948 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3272,7 +3272,7 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
{
memcpy(state, connector->state, sizeof(*state));
if (state->crtc)
- drm_connector_reference(connector);
+ drm_connector_get(connector);
}
EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
@@ -3398,7 +3398,7 @@ void
__drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state)
{
if (state->crtc)
- drm_connector_unreference(state->connector);
+ drm_connector_put(state->connector);
}
EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 0616062b055a..0dc0e5b33f8a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -35,8 +35,8 @@
* als fixed panels or anything else that can display pixels in some form. As
* opposed to all other KMS objects representing hardware (like CRTC, encoder or
* plane abstractions) connectors can be hotplugged and unplugged at runtime.
- * Hence they are reference-counted using drm_connector_reference() and
- * drm_connector_unreference().
+ * Hence they are reference-counted using drm_connector_get() and
+ * drm_connector_put().
*
* KMS driver must create, initialize, register and attach at a &struct
* drm_connector for each such sink. The instance is created as other KMS
@@ -545,7 +545,7 @@ drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
spin_unlock_irqrestore(&config->connector_list_lock, flags);
if (old_conn)
- drm_connector_unreference(old_conn);
+ drm_connector_put(old_conn);
return iter->conn;
}
@@ -564,7 +564,7 @@ void drm_connector_list_iter_put(struct drm_connector_list_iter *iter)
{
iter->dev = NULL;
if (iter->conn)
- drm_connector_unreference(iter->conn);
+ drm_connector_put(iter->conn);
lock_release(&connector_list_iter_dep_map, 0, _RET_IP_);
}
EXPORT_SYMBOL(drm_connector_list_iter_put);
@@ -1249,7 +1249,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
out:
mutex_unlock(&dev->mode_config.mutex);
out_unref:
- drm_connector_unreference(connector);
+ drm_connector_put(connector);
return ret;
}
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e2284539f82c..9594c623799b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -685,7 +685,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
if (connector_set) {
for (i = 0; i < crtc_req->count_connectors; i++) {
if (connector_set[i])
- drm_connector_unreference(connector_set[i]);
+ drm_connector_put(connector_set[i]);
}
}
kfree(connector_set);
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 44ba0e990d6c..536051c627d8 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -465,7 +465,7 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
connector->dpms = DRM_MODE_DPMS_OFF;
/* we keep a reference while the encoder is bound */
- drm_connector_unreference(connector);
+ drm_connector_put(connector);
}
drm_connector_list_iter_put(&conn_iter);
}
@@ -623,7 +623,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
for (ro = 0; ro < set->num_connectors; ro++) {
if (set->connectors[ro]->encoder)
continue;
- drm_connector_reference(set->connectors[ro]);
+ drm_connector_get(set->connectors[ro]);
}
/* a) traverse passed in connector list and get encoders for them */
@@ -772,7 +772,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
for (ro = 0; ro < set->num_connectors; ro++) {
if (set->connectors[ro]->encoder)
continue;
- drm_connector_unreference(set->connectors[ro]);
+ drm_connector_put(set->connectors[ro]);
}
/* Try to restore the config */
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d4d9700734..ee1361a24b3a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -141,7 +141,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
struct drm_fb_helper_connector *fb_helper_connector =
fb_helper->connector_info[i];
- drm_connector_unreference(fb_helper_connector->connector);
+ drm_connector_put(fb_helper_connector->connector);
kfree(fb_helper_connector);
fb_helper->connector_info[i] = NULL;
@@ -178,7 +178,7 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_
if (!fb_helper_connector)
return -ENOMEM;
- drm_connector_reference(connector);
+ drm_connector_get(connector);
fb_helper_connector->connector = connector;
fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
return 0;
@@ -204,7 +204,7 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
if (i == fb_helper->connector_count)
return -EINVAL;
fb_helper_connector = fb_helper->connector_info[i];
- drm_connector_unreference(fb_helper_connector->connector);
+ drm_connector_put(fb_helper_connector->connector);
for (j = i + 1; j < fb_helper->connector_count; j++) {
fb_helper->connector_info[j - 1] = fb_helper->connector_info[j];
@@ -626,7 +626,7 @@ static void drm_fb_helper_modeset_release(struct drm_fb_helper *helper,
int i;
for (i = 0; i < modeset->num_connectors; i++) {
- drm_connector_unreference(modeset->connectors[i]);
+ drm_connector_put(modeset->connectors[i]);
modeset->connectors[i] = NULL;
}
modeset->num_connectors = 0;
@@ -643,7 +643,7 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper)
int i;
for (i = 0; i < helper->connector_count; i++) {
- drm_connector_unreference(helper->connector_info[i]->connector);
+ drm_connector_put(helper->connector_info[i]->connector);
kfree(helper->connector_info[i]);
}
kfree(helper->connector_info);
@@ -2184,7 +2184,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
fb_crtc->y = offset->y;
modeset->mode = drm_mode_duplicate(dev,
fb_crtc->desired_mode);
- drm_connector_reference(connector);
+ drm_connector_get(connector);
modeset->connectors[modeset->num_connectors++] = connector;
modeset->fb = fb_helper->fb;
modeset->x = offset->x;
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 884cc4d26fb5..20aec165abd7 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -418,7 +418,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
* current connector itself, which means it is inherently safe
* against unreferencing the current connector - but not against
* deleting it right away. */
- drm_connector_unreference(connector);
+ drm_connector_put(connector);
}
drm_connector_list_iter_put(&conn_iter);
if (WARN_ON(!list_empty(&dev->mode_config.connector_list))) {
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index e5e1eddd19fb..d649bec96937 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -795,25 +795,50 @@ static inline struct drm_connector *drm_connector_lookup(struct drm_device *dev,
}
/**
- * drm_connector_reference - incr the connector refcnt
- * @connector: connector
+ * drm_connector_get - acquire a connector reference
+ * @connector: DRM connector
*
* This function increments the connector's refcount.
*/
+static inline void drm_connector_get(struct drm_connector *connector)
+{
+ drm_mode_object_get(&connector->base);
+}
+
+/**
+ * drm_connector_put - release a connector reference
+ * @connector: DRM connector
+ *
+ * This function decrements the connector's reference count and frees the
+ * object if the reference count drops to zero.
+ */
+static inline void drm_connector_put(struct drm_connector *connector)
+{
+ drm_mode_object_put(&connector->base);
+}
+
+/**
+ * drm_connector_reference - acquire a connector reference
+ * @connector: DRM connector
+ *
+ * This is a compatibility alias for drm_connector_get() and should not be
+ * used by new code.
+ */
static inline void drm_connector_reference(struct drm_connector *connector)
{
- drm_mode_object_reference(&connector->base);
+ drm_connector_get(connector);
}
/**
- * drm_connector_unreference - unref a connector
- * @connector: connector to unref
+ * drm_connector_unreference - release a connector reference
+ * @connector: DRM connector
*
- * This function decrements the connector's refcount and frees it if it drops to zero.
+ * This is a compatibility alias for drm_connector_put() and should not be
+ * used by new code.
*/
static inline void drm_connector_unreference(struct drm_connector *connector)
{
- drm_mode_object_unreference(&connector->base);
+ drm_connector_put(connector);
}
const char *drm_get_connector_status_name(enum drm_connector_status status);
@@ -905,7 +930,7 @@ void drm_connector_list_iter_put(struct drm_connector_list_iter *iter);
*
* Note that @connector is only valid within the list body, if you want to use
* @connector after calling drm_connector_list_iter_put() then you need to grab
- * your own reference first using drm_connector_reference().
+ * your own reference first using drm_connector_get().
*/
#define drm_for_each_connector_iter(connector, iter) \
while ((connector = drm_connector_list_iter_next(iter)))
diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
index a3742447c981..8a4c2cb7889e 100644
--- a/scripts/coccinelle/api/drm-get-put.cocci
+++ b/scripts/coccinelle/api/drm-get-put.cocci
@@ -20,6 +20,12 @@ expression object;
|
- drm_mode_object_unreference(object)
+ drm_mode_object_put(object)
+|
+- drm_connector_reference(object)
++ drm_connector_get(object)
+|
+- drm_connector_unreference(object)
++ drm_connector_put(object)
)
@r depends on report@
@@ -31,6 +37,10 @@ position p;
drm_mode_object_unreference@p(object)
|
drm_mode_object_reference@p(object)
+|
+drm_connector_unreference@p(object)
+|
+drm_connector_reference@p(object)
)
@script:python depends on report@
--
2.11.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/6] drm: Introduce drm_framebuffer_{get,put}()
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
` (2 preceding siblings ...)
2017-02-08 18:24 ` [PATCH 3/6] drm: Introduce drm_connector_{get,put}() Thierry Reding
@ 2017-02-08 18:24 ` Thierry Reding
2017-02-08 19:36 ` Sean Paul
2017-02-08 18:24 ` [PATCH 5/6] drm: Introduce drm_gem_object_{get,put}() Thierry Reding
` (3 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thierry Reding @ 2017-02-08 18:24 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
From: Thierry Reding <treding@nvidia.com>
For consistency with other reference counting APIs in the kernel, add
drm_framebuffer_get() and drm_framebuffer_put() to reference count DRM
framebuffers.
Compatibility aliases are added to keep existing code working. To help
speed up the transition, all the instances of the old functions in the
DRM core are already replaced in this commit.
The existing semantic patch for the DRM subsystem-wide conversion is
extended to account for these new helpers.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_atomic.c | 6 ++--
drivers/gpu/drm/drm_atomic_helper.c | 4 +--
drivers/gpu/drm/drm_crtc.c | 8 +++---
drivers/gpu/drm/drm_framebuffer.c | 34 +++++++++++-----------
drivers/gpu/drm/drm_plane.c | 12 ++++----
include/drm/drm_framebuffer.h | 49 ++++++++++++++++++++++++--------
scripts/coccinelle/api/drm-get-put.cocci | 10 +++++++
7 files changed, 79 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 82bad40b2f3e..740650b450c5 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -733,7 +733,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, val);
drm_atomic_set_fb_for_plane(state, fb);
if (fb)
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
} else if (property == config->prop_in_fence_fd) {
if (state->fence)
return -EINVAL;
@@ -1837,12 +1837,12 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
if (ret == 0) {
struct drm_framebuffer *new_fb = plane->state->fb;
if (new_fb)
- drm_framebuffer_reference(new_fb);
+ drm_framebuffer_get(new_fb);
plane->fb = new_fb;
plane->crtc = plane->state->crtc;
if (plane->old_fb)
- drm_framebuffer_unreference(plane->old_fb);
+ drm_framebuffer_put(plane->old_fb);
}
plane->old_fb = NULL;
}
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9f2c28ddf948..4dcd64ca34c8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3151,7 +3151,7 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
memcpy(state, plane->state, sizeof(*state));
if (state->fb)
- drm_framebuffer_reference(state->fb);
+ drm_framebuffer_get(state->fb);
state->fence = NULL;
}
@@ -3191,7 +3191,7 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_duplicate_state);
void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
{
if (state->fb)
- drm_framebuffer_unreference(state->fb);
+ drm_framebuffer_put(state->fb);
if (state->fence)
dma_fence_put(state->fence);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9594c623799b..e2974d3c92e7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -471,9 +471,9 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
drm_for_each_crtc(tmp, crtc->dev) {
if (tmp->primary->fb)
- drm_framebuffer_reference(tmp->primary->fb);
+ drm_framebuffer_get(tmp->primary->fb);
if (tmp->primary->old_fb)
- drm_framebuffer_unreference(tmp->primary->old_fb);
+ drm_framebuffer_put(tmp->primary->old_fb);
tmp->primary->old_fb = NULL;
}
@@ -567,7 +567,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
}
fb = crtc->primary->fb;
/* Make refcounting symmetric with the lookup path. */
- drm_framebuffer_reference(fb);
+ drm_framebuffer_get(fb);
} else {
fb = drm_framebuffer_lookup(dev, crtc_req->fb_id);
if (!fb) {
@@ -680,7 +680,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
out:
if (fb)
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
if (connector_set) {
for (i = 0; i < crtc_req->count_connectors; i++) {
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 11daa24692aa..5e8e1d06866a 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -52,13 +52,13 @@
* metadata fields.
*
* The lifetime of a drm framebuffer is controlled with a reference count,
- * drivers can grab additional references with drm_framebuffer_reference() and
- * drop them again with drm_framebuffer_unreference(). For driver-private
- * framebuffers for which the last reference is never dropped (e.g. for the
- * fbdev framebuffer when the struct &struct drm_framebuffer is embedded into
- * the fbdev helper struct) drivers can manually clean up a framebuffer at
- * module unload time with drm_framebuffer_unregister_private(). But doing this
- * is not recommended, and it's better to have a normal free-standing &struct
+ * drivers can grab additional references with drm_framebuffer_get() and drop
+ * them again with drm_framebuffer_put(). For driver-private framebuffers for
+ * which the last reference is never dropped (e.g. for the fbdev framebuffer
+ * when the struct &struct drm_framebuffer is embedded into the fbdev helper
+ * struct) drivers can manually clean up a framebuffer at module unload time
+ * with drm_framebuffer_unregister_private(). But doing this is not
+ * recommended, and it's better to have a normal free-standing &struct
* drm_framebuffer.
*/
@@ -374,7 +374,7 @@ int drm_mode_rmfb(struct drm_device *dev,
mutex_unlock(&file_priv->fbs_lock);
/* drop the reference we picked up in framebuffer lookup */
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
/*
* we now own the reference that was stored in the fbs list
@@ -394,12 +394,12 @@ int drm_mode_rmfb(struct drm_device *dev,
flush_work(&arg.work);
destroy_work_on_stack(&arg.work);
} else
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
return 0;
fail_unref:
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
return -ENOENT;
}
@@ -453,7 +453,7 @@ int drm_mode_getfb(struct drm_device *dev,
ret = -ENODEV;
}
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
return ret;
}
@@ -540,7 +540,7 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
out_err2:
kfree(clips);
out_err1:
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
return ret;
}
@@ -580,7 +580,7 @@ void drm_fb_release(struct drm_file *priv)
list_del_init(&fb->filp_head);
/* This drops the fpriv->fbs reference. */
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
}
}
@@ -661,7 +661,7 @@ EXPORT_SYMBOL(drm_framebuffer_init);
*
* If successful, this grabs an additional reference to the framebuffer -
* callers need to make sure to eventually unreference the returned framebuffer
- * again, using @drm_framebuffer_unreference.
+ * again, using drm_framebuffer_put().
*/
struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
uint32_t id)
@@ -687,8 +687,8 @@ EXPORT_SYMBOL(drm_framebuffer_lookup);
*
* NOTE: This function is deprecated. For driver-private framebuffers it is not
* recommended to embed a framebuffer struct info fbdev struct, instead, a
- * framebuffer pointer is preferred and drm_framebuffer_unreference() should be
- * called when the framebuffer is to be cleaned up.
+ * framebuffer pointer is preferred and drm_framebuffer_put() should be called
+ * when the framebuffer is to be cleaned up.
*/
void drm_framebuffer_unregister_private(struct drm_framebuffer *fb)
{
@@ -790,7 +790,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
drm_modeset_unlock_all(dev);
}
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
}
EXPORT_SYMBOL(drm_framebuffer_remove);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index f42590049a3a..a22e76837065 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -293,7 +293,7 @@ void drm_plane_force_disable(struct drm_plane *plane)
return;
}
/* disconnect the plane from the fb and crtc: */
- drm_framebuffer_unreference(plane->old_fb);
+ drm_framebuffer_put(plane->old_fb);
plane->old_fb = NULL;
plane->fb = NULL;
plane->crtc = NULL;
@@ -520,9 +520,9 @@ static int __setplane_internal(struct drm_plane *plane,
out:
if (fb)
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
if (plane->old_fb)
- drm_framebuffer_unreference(plane->old_fb);
+ drm_framebuffer_put(plane->old_fb);
plane->old_fb = NULL;
return ret;
@@ -638,7 +638,7 @@ static int drm_mode_cursor_universal(struct drm_crtc *crtc,
} else {
fb = crtc->cursor->fb;
if (fb)
- drm_framebuffer_reference(fb);
+ drm_framebuffer_get(fb);
}
if (req->flags & DRM_MODE_CURSOR_MOVE) {
@@ -902,9 +902,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
if (ret && crtc->funcs->page_flip_target)
drm_crtc_vblank_put(crtc);
if (fb)
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_put(fb);
if (crtc->primary->old_fb)
- drm_framebuffer_unreference(crtc->primary->old_fb);
+ drm_framebuffer_put(crtc->primary->old_fb);
crtc->primary->old_fb = NULL;
drm_modeset_unlock_crtc(crtc);
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index dd1e3e99dcff..5244f059d23a 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -101,8 +101,8 @@ struct drm_framebuffer_funcs {
* cleanup (like releasing the reference(s) on the backing GEM bo(s))
* should be deferred. In cases like this, the driver would like to
* hold a ref to the fb even though it has already been removed from
- * userspace perspective. See drm_framebuffer_reference() and
- * drm_framebuffer_unreference().
+ * userspace perspective. See drm_framebuffer_get() and
+ * drm_framebuffer_put().
*
* The refcount is stored inside the mode object @base.
*/
@@ -204,25 +204,50 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb);
void drm_framebuffer_unregister_private(struct drm_framebuffer *fb);
/**
- * drm_framebuffer_reference - incr the fb refcnt
- * @fb: framebuffer
+ * drm_framebuffer_get - acquire a framebuffer reference
+ * @fb: DRM framebuffer
+ *
+ * This function increments the framebuffer's reference count.
+ */
+static inline void drm_framebuffer_get(struct drm_framebuffer *fb)
+{
+ drm_mode_object_get(&fb->base);
+}
+
+/**
+ * drm_framebuffer_put - release a framebuffer reference
+ * @fb: DRM framebuffer
+ *
+ * This function decrements the framebuffer's reference count and frees the
+ * framebuffer if the reference count drops to zero.
+ */
+static inline void drm_framebuffer_put(struct drm_framebuffer *fb)
+{
+ drm_mode_object_put(&fb->base);
+}
+
+/**
+ * drm_framebuffer_reference - acquire a framebuffer reference
+ * @fb: DRM framebuffer
*
- * This functions increments the fb's refcount.
+ * This is a compatibility alias for drm_framebuffer_get() and should not be
+ * used by new code.
*/
static inline void drm_framebuffer_reference(struct drm_framebuffer *fb)
{
- drm_mode_object_reference(&fb->base);
+ drm_framebuffer_get(fb);
}
/**
- * drm_framebuffer_unreference - unref a framebuffer
- * @fb: framebuffer to unref
+ * drm_framebuffer_unreference - release a framebuffer reference
+ * @fb: DRM framebuffer
*
- * This functions decrements the fb's refcount and frees it if it drops to zero.
+ * This is a compatibility alias for drm_framebuffer_put() and should not be
+ * used by new code.
*/
static inline void drm_framebuffer_unreference(struct drm_framebuffer *fb)
{
- drm_mode_object_unreference(&fb->base);
+ drm_framebuffer_put(fb);
}
/**
@@ -248,9 +273,9 @@ static inline void drm_framebuffer_assign(struct drm_framebuffer **p,
struct drm_framebuffer *fb)
{
if (fb)
- drm_framebuffer_reference(fb);
+ drm_framebuffer_get(fb);
if (*p)
- drm_framebuffer_unreference(*p);
+ drm_framebuffer_put(*p);
*p = fb;
}
diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
index 8a4c2cb7889e..fd298c24a465 100644
--- a/scripts/coccinelle/api/drm-get-put.cocci
+++ b/scripts/coccinelle/api/drm-get-put.cocci
@@ -26,6 +26,12 @@ expression object;
|
- drm_connector_unreference(object)
+ drm_connector_put(object)
+|
+- drm_framebuffer_reference(object)
++ drm_framebuffer_get(object)
+|
+- drm_framebuffer_unreference(object)
++ drm_framebuffer_put(object)
)
@r depends on report@
@@ -41,6 +47,10 @@ drm_mode_object_reference@p(object)
drm_connector_unreference@p(object)
|
drm_connector_reference@p(object)
+|
+drm_framebuffer_unreference@p(object)
+|
+drm_framebuffer_reference@p(object)
)
@script:python depends on report@
--
2.11.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/6] drm: Introduce drm_gem_object_{get,put}()
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
` (3 preceding siblings ...)
2017-02-08 18:24 ` [PATCH 4/6] drm: Introduce drm_framebuffer_{get,put}() Thierry Reding
@ 2017-02-08 18:24 ` Thierry Reding
2017-02-08 19:41 ` Sean Paul
2017-02-08 18:24 ` [PATCH 6/6] drm: Introduce drm_property_blob_{get,put}() Thierry Reding
` (2 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Thierry Reding @ 2017-02-08 18:24 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
From: Thierry Reding <treding@nvidia.com>
For consistency with other reference counting APIs in the kernel, add
drm_gem_object_get() and drm_gem_object_put(), as well as an unlocked
variant of the latter, to reference count GEM buffer objects.
Compatibility aliases are added to keep existing code working. To help
speed up the transition, all the instances of the old functions in the
DRM core are already replaced in this commit.
The existing semantic patch for the DRM subsystem-wide conversion is
extended to account for these new helpers.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Documentation/gpu/drm-mm.rst | 14 +++---
drivers/gpu/drm/drm_fb_cma_helper.c | 16 +++----
drivers/gpu/drm/drm_gem.c | 44 +++++++++---------
drivers/gpu/drm/drm_gem_cma_helper.c | 10 ++--
drivers/gpu/drm/drm_prime.c | 10 ++--
include/drm/drm_gem.h | 80 +++++++++++++++++++++++++-------
scripts/coccinelle/api/drm-get-put.cocci | 20 ++++++++
7 files changed, 130 insertions(+), 64 deletions(-)
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index f5760b140f13..fd35998acefc 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -183,14 +183,12 @@ GEM Objects Lifetime
--------------------
All GEM objects are reference-counted by the GEM core. References can be
-acquired and release by :c:func:`calling
-drm_gem_object_reference()` and
-:c:func:`drm_gem_object_unreference()` respectively. The caller
-must hold the :c:type:`struct drm_device <drm_device>`
-struct_mutex lock when calling
-:c:func:`drm_gem_object_reference()`. As a convenience, GEM
-provides :c:func:`drm_gem_object_unreference_unlocked()`
-functions that can be called without holding the lock.
+acquired and release by :c:func:`calling drm_gem_object_get()` and
+:c:func:`drm_gem_object_put()` respectively. The caller must hold the
+:c:type:`struct drm_device <drm_device>` struct_mutex lock when calling
+:c:func:`drm_gem_object_get()`. As a convenience, GEM provides
+:c:func:`drm_gem_object_put_unlocked()` functions that can be called without
+holding the lock.
When the last reference to a GEM object is released the GEM core calls
the :c:type:`struct drm_driver <drm_driver>` gem_free_object
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 596fabf18c3e..eccc07d20925 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -102,7 +102,7 @@ void drm_fb_cma_destroy(struct drm_framebuffer *fb)
for (i = 0; i < 4; i++) {
if (fb_cma->obj[i])
- drm_gem_object_unreference_unlocked(&fb_cma->obj[i]->base);
+ drm_gem_object_put_unlocked(&fb_cma->obj[i]->base);
}
drm_framebuffer_cleanup(fb);
@@ -190,7 +190,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
if (!obj) {
dev_err(dev->dev, "Failed to lookup GEM object\n");
ret = -ENXIO;
- goto err_gem_object_unreference;
+ goto err_gem_object_put;
}
min_size = (height - 1) * mode_cmd->pitches[i]
@@ -198,9 +198,9 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
+ mode_cmd->offsets[i];
if (obj->size < min_size) {
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
ret = -EINVAL;
- goto err_gem_object_unreference;
+ goto err_gem_object_put;
}
objs[i] = to_drm_gem_cma_obj(obj);
}
@@ -208,14 +208,14 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);
if (IS_ERR(fb_cma)) {
ret = PTR_ERR(fb_cma);
- goto err_gem_object_unreference;
+ goto err_gem_object_put;
}
return &fb_cma->fb;
-err_gem_object_unreference:
+err_gem_object_put:
for (i--; i >= 0; i--)
- drm_gem_object_unreference_unlocked(&objs[i]->base);
+ drm_gem_object_put_unlocked(&objs[i]->base);
return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
@@ -477,7 +477,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
err_fb_info_destroy:
drm_fb_helper_release_fbi(helper);
err_gem_free_object:
- drm_gem_object_unreference_unlocked(&obj->base);
+ drm_gem_object_put_unlocked(&obj->base);
return ret;
}
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index bc93de308673..b1e28c944637 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -218,7 +218,7 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
}
static void
-drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
+drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
{
struct drm_device *dev = obj->dev;
bool final = false;
@@ -241,7 +241,7 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
mutex_unlock(&dev->object_name_lock);
if (final)
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
}
/*
@@ -262,7 +262,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
if (dev->driver->gem_close_object)
dev->driver->gem_close_object(obj, file_priv);
- drm_gem_object_handle_unreference_unlocked(obj);
+ drm_gem_object_handle_put_unlocked(obj);
return 0;
}
@@ -352,7 +352,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
WARN_ON(!mutex_is_locked(&dev->object_name_lock));
if (obj->handle_count++ == 0)
- drm_gem_object_reference(obj);
+ drm_gem_object_get(obj);
/*
* Get the user-visible handle using idr. Preload and perform
@@ -392,7 +392,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
idr_remove(&file_priv->object_idr, handle);
spin_unlock(&file_priv->table_lock);
err_unref:
- drm_gem_object_handle_unreference_unlocked(obj);
+ drm_gem_object_handle_put_unlocked(obj);
return ret;
}
@@ -606,7 +606,7 @@ drm_gem_object_lookup(struct drm_file *filp, u32 handle)
/* Check if we currently have a reference on the object */
obj = idr_find(&filp->object_idr, handle);
if (obj)
- drm_gem_object_reference(obj);
+ drm_gem_object_get(obj);
spin_unlock(&filp->table_lock);
@@ -683,7 +683,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
err:
mutex_unlock(&dev->object_name_lock);
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
return ret;
}
@@ -713,7 +713,7 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data,
mutex_lock(&dev->object_name_lock);
obj = idr_find(&dev->object_name_idr, (int) args->name);
if (obj) {
- drm_gem_object_reference(obj);
+ drm_gem_object_get(obj);
} else {
mutex_unlock(&dev->object_name_lock);
return -ENOENT;
@@ -721,7 +721,7 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data,
/* drm_gem_handle_create_tail unlocks dev->object_name_lock. */
ret = drm_gem_handle_create_tail(file_priv, obj, &handle);
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
if (ret)
return ret;
@@ -809,16 +809,16 @@ drm_gem_object_free(struct kref *kref)
EXPORT_SYMBOL(drm_gem_object_free);
/**
- * drm_gem_object_unreference_unlocked - release a GEM BO reference
+ * drm_gem_object_put_unlocked - drop a GEM buffer object reference
* @obj: GEM buffer object
*
* This releases a reference to @obj. Callers must not hold the
* &drm_device.struct_mutex lock when calling this function.
*
- * See also __drm_gem_object_unreference().
+ * See also __drm_gem_object_put().
*/
void
-drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
+drm_gem_object_put_unlocked(struct drm_gem_object *obj)
{
struct drm_device *dev;
@@ -834,10 +834,10 @@ drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
&dev->struct_mutex))
mutex_unlock(&dev->struct_mutex);
}
-EXPORT_SYMBOL(drm_gem_object_unreference_unlocked);
+EXPORT_SYMBOL(drm_gem_object_put_unlocked);
/**
- * drm_gem_object_unreference - release a GEM BO reference
+ * drm_gem_object_put - release a GEM buffer object reference
* @obj: GEM buffer object
*
* This releases a reference to @obj. Callers must hold the
@@ -845,10 +845,10 @@ EXPORT_SYMBOL(drm_gem_object_unreference_unlocked);
* driver doesn't use &drm_device.struct_mutex for anything.
*
* For drivers not encumbered with legacy locking use
- * drm_gem_object_unreference_unlocked() instead.
+ * drm_gem_object_put_unlocked() instead.
*/
void
-drm_gem_object_unreference(struct drm_gem_object *obj)
+drm_gem_object_put(struct drm_gem_object *obj)
{
if (obj) {
WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
@@ -856,7 +856,7 @@ drm_gem_object_unreference(struct drm_gem_object *obj)
kref_put(&obj->refcount, drm_gem_object_free);
}
}
-EXPORT_SYMBOL(drm_gem_object_unreference);
+EXPORT_SYMBOL(drm_gem_object_put);
/**
* drm_gem_vm_open - vma->ops->open implementation for GEM
@@ -869,7 +869,7 @@ void drm_gem_vm_open(struct vm_area_struct *vma)
{
struct drm_gem_object *obj = vma->vm_private_data;
- drm_gem_object_reference(obj);
+ drm_gem_object_get(obj);
}
EXPORT_SYMBOL(drm_gem_vm_open);
@@ -884,7 +884,7 @@ void drm_gem_vm_close(struct vm_area_struct *vma)
{
struct drm_gem_object *obj = vma->vm_private_data;
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
}
EXPORT_SYMBOL(drm_gem_vm_close);
@@ -935,7 +935,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
* (which should happen whether the vma was created by this call, or
* by a vm_open due to mremap or partial unmap or whatever).
*/
- drm_gem_object_reference(obj);
+ drm_gem_object_get(obj);
return 0;
}
@@ -992,14 +992,14 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
return -EINVAL;
if (!drm_vma_node_is_allowed(node, priv)) {
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
return -EACCES;
}
ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
vma);
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
return ret;
}
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index f7ba32bfe39b..bb968e76779b 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -121,7 +121,7 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
return cma_obj;
error:
- drm_gem_object_unreference_unlocked(&cma_obj->base);
+ drm_gem_object_put_unlocked(&cma_obj->base);
return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(drm_gem_cma_create);
@@ -163,7 +163,7 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv,
*/
ret = drm_gem_handle_create(file_priv, gem_obj, handle);
/* drop reference from allocate - handle holds it now. */
- drm_gem_object_unreference_unlocked(gem_obj);
+ drm_gem_object_put_unlocked(gem_obj);
if (ret)
return ERR_PTR(ret);
@@ -293,7 +293,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
*offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
- drm_gem_object_unreference_unlocked(gem_obj);
+ drm_gem_object_put_unlocked(gem_obj);
return 0;
}
@@ -416,13 +416,13 @@ unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
return -EINVAL;
if (!drm_vma_node_is_allowed(node, priv)) {
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
return -EACCES;
}
cma_obj = to_drm_gem_cma_obj(obj);
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
return cma_obj->vaddr ? (unsigned long)cma_obj->vaddr : -EINVAL;
}
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 25aa4558f1b5..866b294e7c61 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -318,7 +318,7 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
return dma_buf;
drm_dev_ref(dev);
- drm_gem_object_reference(exp_info->priv);
+ drm_gem_object_get(exp_info->priv);
return dma_buf;
}
@@ -339,7 +339,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
struct drm_device *dev = obj->dev;
/* drop the reference on the export fd holds */
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
drm_dev_unref(dev);
}
@@ -585,7 +585,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
fail_put_dmabuf:
dma_buf_put(dmabuf);
out:
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
out_unlock:
mutex_unlock(&file_priv->prime.lock);
@@ -616,7 +616,7 @@ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
* Importing dmabuf exported from out own gem increases
* refcount on gem itself instead of f_count of dmabuf.
*/
- drm_gem_object_reference(obj);
+ drm_gem_object_get(obj);
return obj;
}
}
@@ -704,7 +704,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
/* _handle_create_tail unconditionally unlocks dev->object_name_lock. */
ret = drm_gem_handle_create_tail(file_priv, obj, handle);
- drm_gem_object_unreference_unlocked(obj);
+ drm_gem_object_put_unlocked(obj);
if (ret)
goto out_put;
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 449a41b56ffc..3b2a28f7f49f 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -48,9 +48,9 @@ struct drm_gem_object {
*
* Reference count of this object
*
- * Please use drm_gem_object_reference() to acquire and
- * drm_gem_object_unreference() or drm_gem_object_unreference_unlocked()
- * to release a reference to a GEM buffer object.
+ * Please use drm_gem_object_get() to acquire and drm_gem_object_put()
+ * or drm_gem_object_put_unlocked() to release a reference to a GEM
+ * buffer object.
*/
struct kref refcount;
@@ -187,42 +187,90 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
/**
- * drm_gem_object_reference - acquire a GEM BO reference
+ * drm_gem_object_get - acquire a GEM buffer object reference
* @obj: GEM buffer object
*
- * This acquires additional reference to @obj. It is illegal to call this
- * without already holding a reference. No locks required.
+ * This function acquires an additional reference to @obj. It is illegal to
+ * call this without already holding a reference. No locks required.
*/
-static inline void
-drm_gem_object_reference(struct drm_gem_object *obj)
+static inline void drm_gem_object_get(struct drm_gem_object *obj)
{
kref_get(&obj->refcount);
}
/**
- * __drm_gem_object_unreference - raw function to release a GEM BO reference
+ * __drm_gem_object_put - raw function to release a GEM buffer object reference
* @obj: GEM buffer object
*
* This function is meant to be used by drivers which are not encumbered with
* &drm_device.struct_mutex legacy locking and which are using the
* gem_free_object_unlocked callback. It avoids all the locking checks and
- * locking overhead of drm_gem_object_unreference() and
- * drm_gem_object_unreference_unlocked().
+ * locking overhead of drm_gem_object_put() and drm_gem_object_put_unlocked().
*
* Drivers should never call this directly in their code. Instead they should
- * wrap it up into a ``driver_gem_object_unreference(struct driver_gem_object
- * *obj)`` wrapper function, and use that. Shared code should never call this, to
+ * wrap it up into a ``driver_gem_object_put(struct driver_gem_object *obj)``
+ * wrapper function, and use that. Shared code should never call this, to
* avoid breaking drivers by accident which still depend upon
* &drm_device.struct_mutex locking.
*/
static inline void
-__drm_gem_object_unreference(struct drm_gem_object *obj)
+__drm_gem_object_put(struct drm_gem_object *obj)
{
kref_put(&obj->refcount, drm_gem_object_free);
}
-void drm_gem_object_unreference_unlocked(struct drm_gem_object *obj);
-void drm_gem_object_unreference(struct drm_gem_object *obj);
+void drm_gem_object_put_unlocked(struct drm_gem_object *obj);
+void drm_gem_object_put(struct drm_gem_object *obj);
+
+/**
+ * drm_gem_object_reference - acquire a GEM buffer object reference
+ * @obj: GEM buffer object
+ *
+ * This is a compatibility alias for drm_gem_object_get() and should not be
+ * used by new code.
+ */
+static inline void drm_gem_object_reference(struct drm_gem_object *obj)
+{
+ drm_gem_object_get(obj);
+}
+
+/**
+ * __drm_gem_object_unreference - raw function to release a GEM buffer object
+ * reference
+ * @obj: GEM buffer object
+ *
+ * This is a compatibility alias for __drm_gem_object_put() and should not be
+ * used by new code.
+ */
+static inline void __drm_gem_object_unreference(struct drm_gem_object *obj)
+{
+ __drm_gem_object_put(obj);
+}
+
+/**
+ * drm_gem_object_unreference_unlocked - release a GEM buffer object reference
+ * @obj: GEM buffer object
+ *
+ * This is a compatibility alias for drm_gem_object_put_unlocked() and should
+ * not be used by new code.
+ */
+static inline void
+drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
+{
+ drm_gem_object_put_unlocked(obj);
+}
+
+/**
+ * drm_gem_object_unreference - release a GEM buffer object reference
+ * @obj: GEM buffer object
+ *
+ * This is a compatibility alias for drm_gem_object_put() and should not be
+ * used by new code.
+ */
+static inline void drm_gem_object_unreference(struct drm_gem_object *obj)
+{
+ drm_gem_object_put(obj);
+}
int drm_gem_handle_create(struct drm_file *file_priv,
struct drm_gem_object *obj,
diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
index fd298c24a465..24882547b4d1 100644
--- a/scripts/coccinelle/api/drm-get-put.cocci
+++ b/scripts/coccinelle/api/drm-get-put.cocci
@@ -32,6 +32,18 @@ expression object;
|
- drm_framebuffer_unreference(object)
+ drm_framebuffer_put(object)
+|
+- drm_gem_object_reference(object)
++ drm_gem_object_get(object)
+|
+- drm_gem_object_unreference(object)
++ drm_gem_object_put(object)
+|
+- __drm_gem_object_unreference(object)
++ __drm_gem_object_put(object)
+|
+- drm_gem_object_unreference_unlocked(object)
++ drm_gem_object_put_unlocked(object)
)
@r depends on report@
@@ -51,6 +63,14 @@ drm_connector_reference@p(object)
drm_framebuffer_unreference@p(object)
|
drm_framebuffer_reference@p(object)
+|
+drm_gem_object_unreference@p(object)
+|
+drm_gem_object_reference@p(object)
+|
+__drm_gem_object_unreference(object)
+|
+drm_gem_object_unreference_unlocked(object)
)
@script:python depends on report@
--
2.11.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 6/6] drm: Introduce drm_property_blob_{get,put}()
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
` (4 preceding siblings ...)
2017-02-08 18:24 ` [PATCH 5/6] drm: Introduce drm_gem_object_{get,put}() Thierry Reding
@ 2017-02-08 18:24 ` Thierry Reding
2017-02-08 19:45 ` Sean Paul
2017-02-09 14:10 ` [PATCH 0/6] drm: Introduce consistent reference counting APIs Jani Nikula
2017-02-10 14:47 ` Christian König
7 siblings, 1 reply; 25+ messages in thread
From: Thierry Reding @ 2017-02-08 18:24 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
From: Thierry Reding <treding@nvidia.com>
For consistency with other reference counting APIs in the kernel, add
drm_property_blob_get() and drm_property_blob_put() to reference count
DRM blob properties.
Compatibility aliases are added to keep existing code working. To help
speed up the transition, all the instances of the old functions in the
DRM core are already replaced in this commit.
A semantic patch is provided that can be used to convert all drivers to
the new helpers.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/drm_atomic.c | 16 ++++++-------
drivers/gpu/drm/drm_atomic_helper.c | 18 +++++++-------
drivers/gpu/drm/drm_mode_config.c | 2 +-
drivers/gpu/drm/drm_property.c | 40 ++++++++++++++++----------------
include/drm/drm_property.h | 35 ++++++++++++++++++++++++----
scripts/coccinelle/api/drm-get-put.cocci | 10 ++++++++
6 files changed, 78 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 740650b450c5..ff5f1756a7ff 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -322,7 +322,7 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
if (mode && memcmp(&state->mode, mode, sizeof(*mode)) == 0)
return 0;
- drm_property_unreference_blob(state->mode_blob);
+ drm_property_blob_put(state->mode_blob);
state->mode_blob = NULL;
if (mode) {
@@ -368,7 +368,7 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
if (blob == state->mode_blob)
return 0;
- drm_property_unreference_blob(state->mode_blob);
+ drm_property_blob_put(state->mode_blob);
state->mode_blob = NULL;
memset(&state->mode, 0, sizeof(state->mode));
@@ -380,7 +380,7 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
blob->data))
return -EINVAL;
- state->mode_blob = drm_property_reference_blob(blob);
+ state->mode_blob = drm_property_blob_get(blob);
state->enable = true;
DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
state->mode.name, state);
@@ -413,9 +413,9 @@ drm_atomic_replace_property_blob(struct drm_property_blob **blob,
if (old_blob == new_blob)
return;
- drm_property_unreference_blob(old_blob);
+ drm_property_blob_put(old_blob);
if (new_blob)
- drm_property_reference_blob(new_blob);
+ drm_property_blob_get(new_blob);
*blob = new_blob;
*replaced = true;
@@ -437,13 +437,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
return -EINVAL;
if (expected_size > 0 && expected_size != new_blob->length) {
- drm_property_unreference_blob(new_blob);
+ drm_property_blob_put(new_blob);
return -EINVAL;
}
}
drm_atomic_replace_property_blob(blob, new_blob, replaced);
- drm_property_unreference_blob(new_blob);
+ drm_property_blob_put(new_blob);
return 0;
}
@@ -478,7 +478,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
struct drm_property_blob *mode =
drm_property_lookup_blob(dev, val);
ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
- drm_property_unreference_blob(mode);
+ drm_property_blob_put(mode);
return ret;
} else if (property == config->degamma_lut_property) {
ret = drm_atomic_replace_property_blob_from_id(crtc,
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4dcd64ca34c8..863f302b5bf9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3042,13 +3042,13 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
memcpy(state, crtc->state, sizeof(*state));
if (state->mode_blob)
- drm_property_reference_blob(state->mode_blob);
+ drm_property_blob_get(state->mode_blob);
if (state->degamma_lut)
- drm_property_reference_blob(state->degamma_lut);
+ drm_property_blob_get(state->degamma_lut);
if (state->ctm)
- drm_property_reference_blob(state->ctm);
+ drm_property_blob_get(state->ctm);
if (state->gamma_lut)
- drm_property_reference_blob(state->gamma_lut);
+ drm_property_blob_get(state->gamma_lut);
state->mode_changed = false;
state->active_changed = false;
state->planes_changed = false;
@@ -3092,10 +3092,10 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
*/
void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
{
- drm_property_unreference_blob(state->mode_blob);
- drm_property_unreference_blob(state->degamma_lut);
- drm_property_unreference_blob(state->ctm);
- drm_property_unreference_blob(state->gamma_lut);
+ drm_property_blob_put(state->mode_blob);
+ drm_property_blob_put(state->degamma_lut);
+ drm_property_blob_put(state->ctm);
+ drm_property_blob_put(state->gamma_lut);
}
EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
@@ -3493,7 +3493,7 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
goto backoff;
drm_atomic_state_put(state);
- drm_property_unreference_blob(blob);
+ drm_property_blob_put(blob);
return ret;
backoff:
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 20aec165abd7..37779b9f0c1e 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -444,7 +444,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
list_for_each_entry_safe(blob, bt, &dev->mode_config.property_blob_list,
head_global) {
- drm_property_unreference_blob(blob);
+ drm_property_blob_put(blob);
}
/*
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 15af0d42e8be..b17959c3e099 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -587,19 +587,19 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
EXPORT_SYMBOL(drm_property_create_blob);
/**
- * drm_property_unreference_blob - Unreference a blob property
- * @blob: Pointer to blob property
+ * drm_property_blob_put - release a blob property reference
+ * @blob: DRM blob property
*
- * Drop a reference on a blob property. May free the object.
+ * Releases a reference to a blob property. May free the object.
*/
-void drm_property_unreference_blob(struct drm_property_blob *blob)
+void drm_property_blob_put(struct drm_property_blob *blob)
{
if (!blob)
return;
drm_mode_object_put(&blob->base);
}
-EXPORT_SYMBOL(drm_property_unreference_blob);
+EXPORT_SYMBOL(drm_property_blob_put);
void drm_property_destroy_user_blobs(struct drm_device *dev,
struct drm_file *file_priv)
@@ -612,23 +612,23 @@ void drm_property_destroy_user_blobs(struct drm_device *dev,
*/
list_for_each_entry_safe(blob, bt, &file_priv->blobs, head_file) {
list_del_init(&blob->head_file);
- drm_property_unreference_blob(blob);
+ drm_property_blob_put(blob);
}
}
/**
- * drm_property_reference_blob - Take a reference on an existing property
- * @blob: Pointer to blob property
+ * drm_property_blob_get - acquire blob property reference
+ * @blob: DRM blob property
*
- * Take a new reference on an existing blob property. Returns @blob, which
+ * Acquires a reference to an existing blob property. Returns @blob, which
* allows this to be used as a shorthand in assignments.
*/
-struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob)
+struct drm_property_blob *drm_property_blob_get(struct drm_property_blob *blob)
{
drm_mode_object_get(&blob->base);
return blob;
}
-EXPORT_SYMBOL(drm_property_reference_blob);
+EXPORT_SYMBOL(drm_property_blob_get);
/**
* drm_property_lookup_blob - look up a blob property and take a reference
@@ -637,7 +637,7 @@ EXPORT_SYMBOL(drm_property_reference_blob);
*
* If successful, this takes an additional reference to the blob property.
* callers need to make sure to eventually unreference the returned property
- * again, using @drm_property_unreference_blob.
+ * again, using drm_property_blob_put().
*
* Return:
* NULL on failure, pointer to the blob on success.
@@ -712,13 +712,13 @@ int drm_property_replace_global_blob(struct drm_device *dev,
goto err_created;
}
- drm_property_unreference_blob(old_blob);
+ drm_property_blob_put(old_blob);
*replace = new_blob;
return 0;
err_created:
- drm_property_unreference_blob(new_blob);
+ drm_property_blob_put(new_blob);
return ret;
}
EXPORT_SYMBOL(drm_property_replace_global_blob);
@@ -747,7 +747,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
}
out_resp->length = blob->length;
unref:
- drm_property_unreference_blob(blob);
+ drm_property_blob_put(blob);
return ret;
}
@@ -784,7 +784,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev,
return 0;
out_blob:
- drm_property_unreference_blob(blob);
+ drm_property_blob_put(blob);
return ret;
}
@@ -823,14 +823,14 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev,
mutex_unlock(&dev->mode_config.blob_lock);
/* One reference from lookup, and one from the filp. */
- drm_property_unreference_blob(blob);
- drm_property_unreference_blob(blob);
+ drm_property_blob_put(blob);
+ drm_property_blob_put(blob);
return 0;
err:
mutex_unlock(&dev->mode_config.blob_lock);
- drm_property_unreference_blob(blob);
+ drm_property_blob_put(blob);
return ret;
}
@@ -908,5 +908,5 @@ void drm_property_change_valid_put(struct drm_property *property,
if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
drm_mode_object_put(ref);
} else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
- drm_property_unreference_blob(obj_to_blob(ref));
+ drm_property_blob_put(obj_to_blob(ref));
}
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index f66fdb47551c..13e8c17d1c79 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -200,9 +200,8 @@ struct drm_property {
* Blobs are used to store bigger values than what fits directly into the 64
* bits available for a &drm_property.
*
- * Blobs are reference counted using drm_property_reference_blob() and
- * drm_property_unreference_blob(). They are created using
- * drm_property_create_blob().
+ * Blobs are reference counted using drm_property_blob_get() and
+ * drm_property_blob_put(). They are created using drm_property_create_blob().
*/
struct drm_property_blob {
struct drm_mode_object base;
@@ -274,8 +273,34 @@ int drm_property_replace_global_blob(struct drm_device *dev,
const void *data,
struct drm_mode_object *obj_holds_id,
struct drm_property *prop_holds_id);
-struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob);
-void drm_property_unreference_blob(struct drm_property_blob *blob);
+struct drm_property_blob *drm_property_blob_get(struct drm_property_blob *blob);
+void drm_property_blob_put(struct drm_property_blob *blob);
+
+/**
+ * drm_property_reference_blob - acquire a blob property reference
+ * @blob: DRM blob property
+ *
+ * This is a compatibility alias for drm_property_blob_get() and should not be
+ * used by new code.
+ */
+static inline struct drm_property_blob *
+drm_property_reference_blob(struct drm_property_blob *blob)
+{
+ return drm_property_blob_get(blob);
+}
+
+/**
+ * drm_property_unreference_blob - release a blob property reference
+ * @blob: DRM blob property
+ *
+ * This is a compatibility alias for drm_property_blob_put() and should not be
+ * used by new code.
+ */
+static inline void
+drm_property_unreference_blob(struct drm_property_blob *blob)
+{
+ drm_property_blob_put(blob);
+}
/**
* drm_connector_find - find property object
diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
index 24882547b4d1..0c7a9265c07e 100644
--- a/scripts/coccinelle/api/drm-get-put.cocci
+++ b/scripts/coccinelle/api/drm-get-put.cocci
@@ -44,6 +44,12 @@ expression object;
|
- drm_gem_object_unreference_unlocked(object)
+ drm_gem_object_put_unlocked(object)
+|
+- drm_property_reference_blob(object)
++ drm_property_blob_get(object)
+|
+- drm_property_unreference_blob(object)
++ drm_property_blob_put(object)
)
@r depends on report@
@@ -71,6 +77,10 @@ drm_gem_object_reference@p(object)
__drm_gem_object_unreference(object)
|
drm_gem_object_unreference_unlocked(object)
+|
+drm_property_unreference_blob@p(object)
+|
+drm_property_reference_blob@p(object)
)
@script:python depends on report@
--
2.11.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/6] drm: Rename drm_mode_object_get()
2017-02-08 18:24 ` [PATCH 1/6] drm: Rename drm_mode_object_get() Thierry Reding
@ 2017-02-08 19:23 ` Sean Paul
0 siblings, 0 replies; 25+ messages in thread
From: Sean Paul @ 2017-02-08 19:23 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, dri-devel
On Wed, Feb 08, 2017 at 07:24:03PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Subsequent patches will introduce reference counting APIs that are more
> consistent with similar APIs throughout the Linux kernel. These APIs use
> the _get() and _put() suffixes and will collide with this existing
> function.
>
> Rename the function to drm_mode_object_add() which is a slightly more
> accurate description of what it does. Also the kerneldoc for this
> function gives an indication that it's badly named because it doesn't
> actually acquire a reference to anything.
>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/drm_connector.c | 6 +++---
> drivers/gpu/drm/drm_crtc.c | 2 +-
> drivers/gpu/drm/drm_crtc_internal.h | 12 +++++-------
> drivers/gpu/drm/drm_encoder.c | 2 +-
> drivers/gpu/drm/drm_framebuffer.c | 4 ++--
> drivers/gpu/drm/drm_mode_object.c | 18 +++++++-----------
> drivers/gpu/drm/drm_modes.c | 2 +-
> drivers/gpu/drm/drm_plane.c | 2 +-
> drivers/gpu/drm/drm_property.c | 6 +++---
> 9 files changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 45464c8b797d..0616062b055a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -189,9 +189,9 @@ int drm_connector_init(struct drm_device *dev,
> struct ida *connector_ida =
> &drm_connector_enum_list[connector_type].ida;
>
> - ret = drm_mode_object_get_reg(dev, &connector->base,
> - DRM_MODE_OBJECT_CONNECTOR,
> - false, drm_connector_free);
> + ret = __drm_mode_object_add(dev, &connector->base,
> + DRM_MODE_OBJECT_CONNECTOR,
> + false, drm_connector_free);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 6915f897bd8e..e2284539f82c 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -282,7 +282,7 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> spin_lock_init(&crtc->commit_lock);
>
> drm_modeset_lock_init(&crtc->mutex);
> - ret = drm_mode_object_get(dev, &crtc->base, DRM_MODE_OBJECT_CRTC);
> + ret = drm_mode_object_add(dev, &crtc->base, DRM_MODE_OBJECT_CRTC);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index 955c5690bf64..43bbf48ee129 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -98,15 +98,13 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv);
>
> /* drm_mode_object.c */
> -int drm_mode_object_get_reg(struct drm_device *dev,
> - struct drm_mode_object *obj,
> - uint32_t obj_type,
> - bool register_obj,
> - void (*obj_free_cb)(struct kref *kref));
> +int __drm_mode_object_add(struct drm_device *dev, struct drm_mode_object *obj,
> + uint32_t obj_type, bool register_obj,
> + void (*obj_free_cb)(struct kref *kref));
> +int drm_mode_object_add(struct drm_device *dev, struct drm_mode_object *obj,
> + uint32_t obj_type);
> void drm_mode_object_register(struct drm_device *dev,
> struct drm_mode_object *obj);
> -int drm_mode_object_get(struct drm_device *dev,
> - struct drm_mode_object *obj, uint32_t obj_type);
> struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
> uint32_t id, uint32_t type);
> void drm_mode_object_unregister(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 129450713bb7..634ae0244ea9 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -110,7 +110,7 @@ int drm_encoder_init(struct drm_device *dev,
> {
> int ret;
>
> - ret = drm_mode_object_get(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
> + ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 28a0108a1ab8..11daa24692aa 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -638,8 +638,8 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
>
> fb->funcs = funcs;
>
> - ret = drm_mode_object_get_reg(dev, &fb->base, DRM_MODE_OBJECT_FB,
> - false, drm_framebuffer_free);
> + ret = __drm_mode_object_add(dev, &fb->base, DRM_MODE_OBJECT_FB,
> + false, drm_framebuffer_free);
> if (ret)
> goto out;
>
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 220a6c1f4ab9..3b405dbf1b8d 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -31,11 +31,9 @@
> * Internal function to assign a slot in the object idr and optionally
> * register the object into the idr.
> */
> -int drm_mode_object_get_reg(struct drm_device *dev,
> - struct drm_mode_object *obj,
> - uint32_t obj_type,
> - bool register_obj,
> - void (*obj_free_cb)(struct kref *kref))
> +int __drm_mode_object_add(struct drm_device *dev, struct drm_mode_object *obj,
> + uint32_t obj_type, bool register_obj,
> + void (*obj_free_cb)(struct kref *kref))
> {
> int ret;
>
> @@ -59,23 +57,21 @@ int drm_mode_object_get_reg(struct drm_device *dev,
> }
>
> /**
> - * drm_mode_object_get - allocate a new modeset identifier
> + * drm_mode_object_add - allocate a new modeset identifier
> * @dev: DRM device
> * @obj: object pointer, used to generate unique ID
> * @obj_type: object type
> *
> * Create a unique identifier based on @ptr in @dev's identifier space. Used
> - * for tracking modes, CRTCs and connectors. Note that despite the _get postfix
> - * modeset identifiers are _not_ reference counted. Hence don't use this for
> - * reference counted modeset objects like framebuffers.
> + * for tracking modes, CRTCs and connectors.
> *
> * Returns:
> * Zero on success, error code on failure.
> */
> -int drm_mode_object_get(struct drm_device *dev,
> +int drm_mode_object_add(struct drm_device *dev,
> struct drm_mode_object *obj, uint32_t obj_type)
> {
> - return drm_mode_object_get_reg(dev, obj, obj_type, true, NULL);
> + return __drm_mode_object_add(dev, obj, obj_type, true, NULL);
> }
>
> void drm_mode_object_register(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index fd22c1c891bf..f2493b9b82e6 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -71,7 +71,7 @@ struct drm_display_mode *drm_mode_create(struct drm_device *dev)
> if (!nmode)
> return NULL;
>
> - if (drm_mode_object_get(dev, &nmode->base, DRM_MODE_OBJECT_MODE)) {
> + if (drm_mode_object_add(dev, &nmode->base, DRM_MODE_OBJECT_MODE)) {
> kfree(nmode);
> return NULL;
> }
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index c464fc4a874d..f42590049a3a 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -88,7 +88,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> struct drm_mode_config *config = &dev->mode_config;
> int ret;
>
> - ret = drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> + ret = drm_mode_object_add(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index 7fc070f3e49e..411e470369c0 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -91,7 +91,7 @@ struct drm_property *drm_property_create(struct drm_device *dev, int flags,
> goto fail;
> }
>
> - ret = drm_mode_object_get(dev, &property->base, DRM_MODE_OBJECT_PROPERTY);
> + ret = drm_mode_object_add(dev, &property->base, DRM_MODE_OBJECT_PROPERTY);
> if (ret)
> goto fail;
>
> @@ -570,8 +570,8 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
> if (data)
> memcpy(blob->data, data, length);
>
> - ret = drm_mode_object_get_reg(dev, &blob->base, DRM_MODE_OBJECT_BLOB,
> - true, drm_property_free_blob);
> + ret = __drm_mode_object_add(dev, &blob->base, DRM_MODE_OBJECT_BLOB,
> + true, drm_property_free_blob);
> if (ret) {
> kfree(blob);
> return ERR_PTR(-EINVAL);
> --
> 2.11.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()
2017-02-08 18:24 ` [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}() Thierry Reding
@ 2017-02-08 19:28 ` Sean Paul
2017-02-09 17:08 ` Daniel Vetter
0 siblings, 1 reply; 25+ messages in thread
From: Sean Paul @ 2017-02-08 19:28 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, dri-devel
On Wed, Feb 08, 2017 at 07:24:04PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> For consistency with other reference counting APIs in the kernel, add
> drm_mode_object_get() and drm_mode_object_put() to reference count DRM
> mode objects.
>
> Compatibility aliases are added to keep existing code working. To help
> speed up the transition, all the instances of the old functions in the
> DRM core are already replaced in this commit.
>
drm code looks good and is
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> A semantic patch is provided that can be used to convert all drivers to
> the new helpers.
I'm not convinced we need to commit the cocci patch. I think including it in
your cover letter and then following up with a follow on series to actually make
the change is sufficient (See: ickle's s/fence/dma_fence/ series).
Sean
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 14 +++++------
> drivers/gpu/drm/drm_mode_object.c | 26 ++++++++++----------
> drivers/gpu/drm/drm_property.c | 6 ++---
> include/drm/drm_mode_object.h | 36 ++++++++++++++++++++++-----
> scripts/coccinelle/api/drm-get-put.cocci | 42 ++++++++++++++++++++++++++++++++
> 5 files changed, 95 insertions(+), 29 deletions(-)
> create mode 100644 scripts/coccinelle/api/drm-get-put.cocci
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index a5673107db26..2bb0a759e8ec 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -2122,13 +2122,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> }
>
> if (!obj->properties) {
> - drm_mode_object_unreference(obj);
> + drm_mode_object_put(obj);
> ret = -ENOENT;
> goto out;
> }
>
> if (get_user(count_props, count_props_ptr + copied_objs)) {
> - drm_mode_object_unreference(obj);
> + drm_mode_object_put(obj);
> ret = -EFAULT;
> goto out;
> }
> @@ -2141,14 +2141,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> struct drm_property *prop;
>
> if (get_user(prop_id, props_ptr + copied_props)) {
> - drm_mode_object_unreference(obj);
> + drm_mode_object_put(obj);
> ret = -EFAULT;
> goto out;
> }
>
> prop = drm_mode_obj_find_prop_id(obj, prop_id);
> if (!prop) {
> - drm_mode_object_unreference(obj);
> + drm_mode_object_put(obj);
> ret = -ENOENT;
> goto out;
> }
> @@ -2156,14 +2156,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> if (copy_from_user(&prop_value,
> prop_values_ptr + copied_props,
> sizeof(prop_value))) {
> - drm_mode_object_unreference(obj);
> + drm_mode_object_put(obj);
> ret = -EFAULT;
> goto out;
> }
>
> ret = atomic_set_prop(state, obj, prop, prop_value);
> if (ret) {
> - drm_mode_object_unreference(obj);
> + drm_mode_object_put(obj);
> goto out;
> }
>
> @@ -2176,7 +2176,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> plane_mask |= (1 << drm_plane_index(plane));
> plane->old_fb = plane->fb;
> }
> - drm_mode_object_unreference(obj);
> + drm_mode_object_put(obj);
> }
>
> ret = prepare_crtc_signaling(dev, state, arg, file_priv, &fence_state,
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 3b405dbf1b8d..da9a9adbcc98 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -133,7 +133,7 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
> *
> * This function is used to look up a modeset object. It will acquire a
> * reference for reference counted objects. This reference must be dropped again
> - * by callind drm_mode_object_unreference().
> + * by callind drm_mode_object_put().
> */
> struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
> uint32_t id, uint32_t type)
> @@ -146,38 +146,38 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
> EXPORT_SYMBOL(drm_mode_object_find);
>
> /**
> - * drm_mode_object_unreference - decr the object refcnt
> - * @obj: mode_object
> + * drm_mode_object_put - release a mode object reference
> + * @obj: DRM mode object
> *
> * This function decrements the object's refcount if it is a refcounted modeset
> * object. It is a no-op on any other object. This is used to drop references
> - * acquired with drm_mode_object_reference().
> + * acquired with drm_mode_object_get().
> */
> -void drm_mode_object_unreference(struct drm_mode_object *obj)
> +void drm_mode_object_put(struct drm_mode_object *obj)
> {
> if (obj->free_cb) {
> DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, kref_read(&obj->refcount));
> kref_put(&obj->refcount, obj->free_cb);
> }
> }
> -EXPORT_SYMBOL(drm_mode_object_unreference);
> +EXPORT_SYMBOL(drm_mode_object_put);
>
> /**
> - * drm_mode_object_reference - incr the object refcnt
> - * @obj: mode_object
> + * drm_mode_object_get - acquire a mode object reference
> + * @obj: DRM mode object
> *
> * This function increments the object's refcount if it is a refcounted modeset
> * object. It is a no-op on any other object. References should be dropped again
> - * by calling drm_mode_object_unreference().
> + * by calling drm_mode_object_put().
> */
> -void drm_mode_object_reference(struct drm_mode_object *obj)
> +void drm_mode_object_get(struct drm_mode_object *obj)
> {
> if (obj->free_cb) {
> DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, kref_read(&obj->refcount));
> kref_get(&obj->refcount);
> }
> }
> -EXPORT_SYMBOL(drm_mode_object_reference);
> +EXPORT_SYMBOL(drm_mode_object_get);
>
> /**
> * drm_object_attach_property - attach a property to a modeset object
> @@ -363,7 +363,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
> &arg->count_props);
>
> out_unref:
> - drm_mode_object_unreference(obj);
> + drm_mode_object_put(obj);
> out:
> drm_modeset_unlock_all(dev);
> return ret;
> @@ -428,7 +428,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
> drm_property_change_valid_put(property, ref);
>
> out_unref:
> - drm_mode_object_unreference(arg_obj);
> + drm_mode_object_put(arg_obj);
> out:
> drm_modeset_unlock_all(dev);
> return ret;
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index 411e470369c0..15af0d42e8be 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -597,7 +597,7 @@ void drm_property_unreference_blob(struct drm_property_blob *blob)
> if (!blob)
> return;
>
> - drm_mode_object_unreference(&blob->base);
> + drm_mode_object_put(&blob->base);
> }
> EXPORT_SYMBOL(drm_property_unreference_blob);
>
> @@ -625,7 +625,7 @@ void drm_property_destroy_user_blobs(struct drm_device *dev,
> */
> struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob)
> {
> - drm_mode_object_reference(&blob->base);
> + drm_mode_object_get(&blob->base);
> return blob;
> }
> EXPORT_SYMBOL(drm_property_reference_blob);
> @@ -906,7 +906,7 @@ void drm_property_change_valid_put(struct drm_property *property,
> return;
>
> if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
> - drm_mode_object_unreference(ref);
> + drm_mode_object_put(ref);
> } else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
> drm_property_unreference_blob(obj_to_blob(ref));
> }
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index 2c017adf6d74..a767b4a30a6d 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -45,10 +45,10 @@ struct drm_device;
> * drm_object_attach_property() before the object is visible to userspace.
> *
> * - For objects with dynamic lifetimes (as indicated by a non-NULL @free_cb) it
> - * provides reference counting through drm_mode_object_reference() and
> - * drm_mode_object_unreference(). This is used by &drm_framebuffer,
> - * &drm_connector and &drm_property_blob. These objects provide specialized
> - * reference counting wrappers.
> + * provides reference counting through drm_mode_object_get() and
> + * drm_mode_object_put(). This is used by &drm_framebuffer, &drm_connector
> + * and &drm_property_blob. These objects provide specialized reference
> + * counting wrappers.
> */
> struct drm_mode_object {
> uint32_t id;
> @@ -114,8 +114,32 @@ struct drm_object_properties {
>
> struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
> uint32_t id, uint32_t type);
> -void drm_mode_object_reference(struct drm_mode_object *obj);
> -void drm_mode_object_unreference(struct drm_mode_object *obj);
> +void drm_mode_object_get(struct drm_mode_object *obj);
> +void drm_mode_object_put(struct drm_mode_object *obj);
> +
> +/**
> + * drm_mode_object_reference - acquire a mode object reference
> + * @obj: DRM mode object
> + *
> + * This is a compatibility alias for drm_mode_object_get() and should not be
> + * used by new code.
> + */
> +static inline void drm_mode_object_reference(struct drm_mode_object *obj)
> +{
> + drm_mode_object_get(obj);
> +}
> +
> +/**
> + * drm_mode_object_unreference - release a mode object reference
> + * @obj: DRM mode object
> + *
> + * This is a compatibility alias for drm_mode_object_put() and should not be
> + * used by new code.
> + */
> +static inline void drm_mode_object_unreference(struct drm_mode_object *obj)
> +{
> + drm_mode_object_put(obj);
> +}
>
> int drm_object_property_set_value(struct drm_mode_object *obj,
> struct drm_property *property,
> diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
> new file mode 100644
> index 000000000000..a3742447c981
> --- /dev/null
> +++ b/scripts/coccinelle/api/drm-get-put.cocci
> @@ -0,0 +1,42 @@
> +///
> +/// Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference() and
> +/// drm_*_unreference() helpers.
> +///
> +// Confidence: High
> +// Copyright: (C) 2017 NVIDIA Corporation
> +// Options: --no-includes --include-headers
> +//
> +
> +virtual patch
> +virtual report
> +
> +@depends on patch@
> +expression object;
> +@@
> +
> +(
> +- drm_mode_object_reference(object)
> ++ drm_mode_object_get(object)
> +|
> +- drm_mode_object_unreference(object)
> ++ drm_mode_object_put(object)
> +)
> +
> +@r depends on report@
> +expression object;
> +position p;
> +@@
> +
> +(
> +drm_mode_object_unreference@p(object)
> +|
> +drm_mode_object_reference@p(object)
> +)
> +
> +@script:python depends on report@
> +object << r.object;
> +p << r.p;
> +@@
> +
> +msg="WARNING: use get/put helpers to reference and dereference %s" % (object)
> +coccilib.report.print_report(p[0], msg)
> --
> 2.11.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/6] drm: Introduce drm_connector_{get,put}()
2017-02-08 18:24 ` [PATCH 3/6] drm: Introduce drm_connector_{get,put}() Thierry Reding
@ 2017-02-08 19:33 ` Sean Paul
0 siblings, 0 replies; 25+ messages in thread
From: Sean Paul @ 2017-02-08 19:33 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, dri-devel
On Wed, Feb 08, 2017 at 07:24:05PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> For consistency with other reference counting APIs in the kernel, add
> drm_connector_get() and drm_connector_put() functions to reference count
> connectors.
>
> Compatibility aliases are added to keep existing code working. To help
> speed up the transition, all the instances of the old functions in the
> DRM core are already replaced in this commit.
>
> The existing semantic patch for mode object reference count conversion
> is extended for these new helpers.
>
drivers/gpu/drm/*
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 8 +++----
> drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
> drivers/gpu/drm/drm_connector.c | 10 ++++----
> drivers/gpu/drm/drm_crtc.c | 2 +-
> drivers/gpu/drm/drm_crtc_helper.c | 6 ++---
> drivers/gpu/drm/drm_fb_helper.c | 12 +++++-----
> drivers/gpu/drm/drm_mode_config.c | 2 +-
> include/drm/drm_connector.h | 41 +++++++++++++++++++++++++-------
> scripts/coccinelle/api/drm-get-put.cocci | 10 ++++++++
> 9 files changed, 65 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 2bb0a759e8ec..82bad40b2f3e 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -150,7 +150,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
> state->connectors[i].state);
> state->connectors[i].ptr = NULL;
> state->connectors[i].state = NULL;
> - drm_connector_unreference(connector);
> + drm_connector_put(connector);
> }
>
> for (i = 0; i < config->num_crtc; i++) {
> @@ -1026,7 +1026,7 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
> if (!connector_state)
> return ERR_PTR(-ENOMEM);
>
> - drm_connector_reference(connector);
> + drm_connector_get(connector);
> state->connectors[index].state = connector_state;
> state->connectors[index].ptr = connector;
> connector_state->state = state;
> @@ -1357,7 +1357,7 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> crtc_state->connector_mask &=
> ~(1 << drm_connector_index(conn_state->connector));
>
> - drm_connector_unreference(conn_state->connector);
> + drm_connector_put(conn_state->connector);
> conn_state->crtc = NULL;
> }
>
> @@ -1369,7 +1369,7 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> crtc_state->connector_mask |=
> 1 << drm_connector_index(conn_state->connector);
>
> - drm_connector_reference(conn_state->connector);
> + drm_connector_get(conn_state->connector);
> conn_state->crtc = crtc;
>
> DRM_DEBUG_ATOMIC("Link connector state %p to [CRTC:%d:%s]\n",
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 9a08445a7a7a..9f2c28ddf948 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3272,7 +3272,7 @@ __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector,
> {
> memcpy(state, connector->state, sizeof(*state));
> if (state->crtc)
> - drm_connector_reference(connector);
> + drm_connector_get(connector);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_duplicate_state);
>
> @@ -3398,7 +3398,7 @@ void
> __drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state)
> {
> if (state->crtc)
> - drm_connector_unreference(state->connector);
> + drm_connector_put(state->connector);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 0616062b055a..0dc0e5b33f8a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -35,8 +35,8 @@
> * als fixed panels or anything else that can display pixels in some form. As
> * opposed to all other KMS objects representing hardware (like CRTC, encoder or
> * plane abstractions) connectors can be hotplugged and unplugged at runtime.
> - * Hence they are reference-counted using drm_connector_reference() and
> - * drm_connector_unreference().
> + * Hence they are reference-counted using drm_connector_get() and
> + * drm_connector_put().
> *
> * KMS driver must create, initialize, register and attach at a &struct
> * drm_connector for each such sink. The instance is created as other KMS
> @@ -545,7 +545,7 @@ drm_connector_list_iter_next(struct drm_connector_list_iter *iter)
> spin_unlock_irqrestore(&config->connector_list_lock, flags);
>
> if (old_conn)
> - drm_connector_unreference(old_conn);
> + drm_connector_put(old_conn);
>
> return iter->conn;
> }
> @@ -564,7 +564,7 @@ void drm_connector_list_iter_put(struct drm_connector_list_iter *iter)
> {
> iter->dev = NULL;
> if (iter->conn)
> - drm_connector_unreference(iter->conn);
> + drm_connector_put(iter->conn);
> lock_release(&connector_list_iter_dep_map, 0, _RET_IP_);
> }
> EXPORT_SYMBOL(drm_connector_list_iter_put);
> @@ -1249,7 +1249,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> out:
> mutex_unlock(&dev->mode_config.mutex);
> out_unref:
> - drm_connector_unreference(connector);
> + drm_connector_put(connector);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e2284539f82c..9594c623799b 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -685,7 +685,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
> if (connector_set) {
> for (i = 0; i < crtc_req->count_connectors; i++) {
> if (connector_set[i])
> - drm_connector_unreference(connector_set[i]);
> + drm_connector_put(connector_set[i]);
> }
> }
> kfree(connector_set);
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 44ba0e990d6c..536051c627d8 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -465,7 +465,7 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
> connector->dpms = DRM_MODE_DPMS_OFF;
>
> /* we keep a reference while the encoder is bound */
> - drm_connector_unreference(connector);
> + drm_connector_put(connector);
> }
> drm_connector_list_iter_put(&conn_iter);
> }
> @@ -623,7 +623,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> for (ro = 0; ro < set->num_connectors; ro++) {
> if (set->connectors[ro]->encoder)
> continue;
> - drm_connector_reference(set->connectors[ro]);
> + drm_connector_get(set->connectors[ro]);
> }
>
> /* a) traverse passed in connector list and get encoders for them */
> @@ -772,7 +772,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
> for (ro = 0; ro < set->num_connectors; ro++) {
> if (set->connectors[ro]->encoder)
> continue;
> - drm_connector_unreference(set->connectors[ro]);
> + drm_connector_put(set->connectors[ro]);
> }
>
> /* Try to restore the config */
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index f6d4d9700734..ee1361a24b3a 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -141,7 +141,7 @@ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
> struct drm_fb_helper_connector *fb_helper_connector =
> fb_helper->connector_info[i];
>
> - drm_connector_unreference(fb_helper_connector->connector);
> + drm_connector_put(fb_helper_connector->connector);
>
> kfree(fb_helper_connector);
> fb_helper->connector_info[i] = NULL;
> @@ -178,7 +178,7 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_
> if (!fb_helper_connector)
> return -ENOMEM;
>
> - drm_connector_reference(connector);
> + drm_connector_get(connector);
> fb_helper_connector->connector = connector;
> fb_helper->connector_info[fb_helper->connector_count++] = fb_helper_connector;
> return 0;
> @@ -204,7 +204,7 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> if (i == fb_helper->connector_count)
> return -EINVAL;
> fb_helper_connector = fb_helper->connector_info[i];
> - drm_connector_unreference(fb_helper_connector->connector);
> + drm_connector_put(fb_helper_connector->connector);
>
> for (j = i + 1; j < fb_helper->connector_count; j++) {
> fb_helper->connector_info[j - 1] = fb_helper->connector_info[j];
> @@ -626,7 +626,7 @@ static void drm_fb_helper_modeset_release(struct drm_fb_helper *helper,
> int i;
>
> for (i = 0; i < modeset->num_connectors; i++) {
> - drm_connector_unreference(modeset->connectors[i]);
> + drm_connector_put(modeset->connectors[i]);
> modeset->connectors[i] = NULL;
> }
> modeset->num_connectors = 0;
> @@ -643,7 +643,7 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper)
> int i;
>
> for (i = 0; i < helper->connector_count; i++) {
> - drm_connector_unreference(helper->connector_info[i]->connector);
> + drm_connector_put(helper->connector_info[i]->connector);
> kfree(helper->connector_info[i]);
> }
> kfree(helper->connector_info);
> @@ -2184,7 +2184,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> fb_crtc->y = offset->y;
> modeset->mode = drm_mode_duplicate(dev,
> fb_crtc->desired_mode);
> - drm_connector_reference(connector);
> + drm_connector_get(connector);
> modeset->connectors[modeset->num_connectors++] = connector;
> modeset->fb = fb_helper->fb;
> modeset->x = offset->x;
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 884cc4d26fb5..20aec165abd7 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -418,7 +418,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> * current connector itself, which means it is inherently safe
> * against unreferencing the current connector - but not against
> * deleting it right away. */
> - drm_connector_unreference(connector);
> + drm_connector_put(connector);
> }
> drm_connector_list_iter_put(&conn_iter);
> if (WARN_ON(!list_empty(&dev->mode_config.connector_list))) {
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index e5e1eddd19fb..d649bec96937 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -795,25 +795,50 @@ static inline struct drm_connector *drm_connector_lookup(struct drm_device *dev,
> }
>
> /**
> - * drm_connector_reference - incr the connector refcnt
> - * @connector: connector
> + * drm_connector_get - acquire a connector reference
> + * @connector: DRM connector
> *
> * This function increments the connector's refcount.
> */
> +static inline void drm_connector_get(struct drm_connector *connector)
> +{
> + drm_mode_object_get(&connector->base);
> +}
> +
> +/**
> + * drm_connector_put - release a connector reference
> + * @connector: DRM connector
> + *
> + * This function decrements the connector's reference count and frees the
> + * object if the reference count drops to zero.
> + */
> +static inline void drm_connector_put(struct drm_connector *connector)
> +{
> + drm_mode_object_put(&connector->base);
> +}
> +
> +/**
> + * drm_connector_reference - acquire a connector reference
> + * @connector: DRM connector
> + *
> + * This is a compatibility alias for drm_connector_get() and should not be
> + * used by new code.
> + */
> static inline void drm_connector_reference(struct drm_connector *connector)
> {
> - drm_mode_object_reference(&connector->base);
> + drm_connector_get(connector);
> }
>
> /**
> - * drm_connector_unreference - unref a connector
> - * @connector: connector to unref
> + * drm_connector_unreference - release a connector reference
> + * @connector: DRM connector
> *
> - * This function decrements the connector's refcount and frees it if it drops to zero.
> + * This is a compatibility alias for drm_connector_put() and should not be
> + * used by new code.
> */
> static inline void drm_connector_unreference(struct drm_connector *connector)
> {
> - drm_mode_object_unreference(&connector->base);
> + drm_connector_put(connector);
> }
>
> const char *drm_get_connector_status_name(enum drm_connector_status status);
> @@ -905,7 +930,7 @@ void drm_connector_list_iter_put(struct drm_connector_list_iter *iter);
> *
> * Note that @connector is only valid within the list body, if you want to use
> * @connector after calling drm_connector_list_iter_put() then you need to grab
> - * your own reference first using drm_connector_reference().
> + * your own reference first using drm_connector_get().
> */
> #define drm_for_each_connector_iter(connector, iter) \
> while ((connector = drm_connector_list_iter_next(iter)))
> diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
> index a3742447c981..8a4c2cb7889e 100644
> --- a/scripts/coccinelle/api/drm-get-put.cocci
> +++ b/scripts/coccinelle/api/drm-get-put.cocci
> @@ -20,6 +20,12 @@ expression object;
> |
> - drm_mode_object_unreference(object)
> + drm_mode_object_put(object)
> +|
> +- drm_connector_reference(object)
> ++ drm_connector_get(object)
> +|
> +- drm_connector_unreference(object)
> ++ drm_connector_put(object)
> )
>
> @r depends on report@
> @@ -31,6 +37,10 @@ position p;
> drm_mode_object_unreference@p(object)
> |
> drm_mode_object_reference@p(object)
> +|
> +drm_connector_unreference@p(object)
> +|
> +drm_connector_reference@p(object)
> )
>
> @script:python depends on report@
> --
> 2.11.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/6] drm: Introduce drm_framebuffer_{get,put}()
2017-02-08 18:24 ` [PATCH 4/6] drm: Introduce drm_framebuffer_{get,put}() Thierry Reding
@ 2017-02-08 19:36 ` Sean Paul
0 siblings, 0 replies; 25+ messages in thread
From: Sean Paul @ 2017-02-08 19:36 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, dri-devel
On Wed, Feb 08, 2017 at 07:24:06PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> For consistency with other reference counting APIs in the kernel, add
> drm_framebuffer_get() and drm_framebuffer_put() to reference count DRM
> framebuffers.
>
> Compatibility aliases are added to keep existing code working. To help
> speed up the transition, all the instances of the old functions in the
> DRM core are already replaced in this commit.
>
> The existing semantic patch for the DRM subsystem-wide conversion is
> extended to account for these new helpers.
>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 6 ++--
> drivers/gpu/drm/drm_atomic_helper.c | 4 +--
> drivers/gpu/drm/drm_crtc.c | 8 +++---
> drivers/gpu/drm/drm_framebuffer.c | 34 +++++++++++-----------
> drivers/gpu/drm/drm_plane.c | 12 ++++----
> include/drm/drm_framebuffer.h | 49 ++++++++++++++++++++++++--------
> scripts/coccinelle/api/drm-get-put.cocci | 10 +++++++
> 7 files changed, 79 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 82bad40b2f3e..740650b450c5 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -733,7 +733,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
> struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, val);
> drm_atomic_set_fb_for_plane(state, fb);
> if (fb)
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
> } else if (property == config->prop_in_fence_fd) {
> if (state->fence)
> return -EINVAL;
> @@ -1837,12 +1837,12 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> if (ret == 0) {
> struct drm_framebuffer *new_fb = plane->state->fb;
> if (new_fb)
> - drm_framebuffer_reference(new_fb);
> + drm_framebuffer_get(new_fb);
> plane->fb = new_fb;
> plane->crtc = plane->state->crtc;
>
> if (plane->old_fb)
> - drm_framebuffer_unreference(plane->old_fb);
> + drm_framebuffer_put(plane->old_fb);
> }
> plane->old_fb = NULL;
> }
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 9f2c28ddf948..4dcd64ca34c8 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3151,7 +3151,7 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
> memcpy(state, plane->state, sizeof(*state));
>
> if (state->fb)
> - drm_framebuffer_reference(state->fb);
> + drm_framebuffer_get(state->fb);
>
> state->fence = NULL;
> }
> @@ -3191,7 +3191,7 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_duplicate_state);
> void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
> {
> if (state->fb)
> - drm_framebuffer_unreference(state->fb);
> + drm_framebuffer_put(state->fb);
>
> if (state->fence)
> dma_fence_put(state->fence);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 9594c623799b..e2974d3c92e7 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -471,9 +471,9 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
>
> drm_for_each_crtc(tmp, crtc->dev) {
> if (tmp->primary->fb)
> - drm_framebuffer_reference(tmp->primary->fb);
> + drm_framebuffer_get(tmp->primary->fb);
> if (tmp->primary->old_fb)
> - drm_framebuffer_unreference(tmp->primary->old_fb);
> + drm_framebuffer_put(tmp->primary->old_fb);
> tmp->primary->old_fb = NULL;
> }
>
> @@ -567,7 +567,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
> }
> fb = crtc->primary->fb;
> /* Make refcounting symmetric with the lookup path. */
> - drm_framebuffer_reference(fb);
> + drm_framebuffer_get(fb);
> } else {
> fb = drm_framebuffer_lookup(dev, crtc_req->fb_id);
> if (!fb) {
> @@ -680,7 +680,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>
> out:
> if (fb)
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
>
> if (connector_set) {
> for (i = 0; i < crtc_req->count_connectors; i++) {
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 11daa24692aa..5e8e1d06866a 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -52,13 +52,13 @@
> * metadata fields.
> *
> * The lifetime of a drm framebuffer is controlled with a reference count,
> - * drivers can grab additional references with drm_framebuffer_reference() and
> - * drop them again with drm_framebuffer_unreference(). For driver-private
> - * framebuffers for which the last reference is never dropped (e.g. for the
> - * fbdev framebuffer when the struct &struct drm_framebuffer is embedded into
> - * the fbdev helper struct) drivers can manually clean up a framebuffer at
> - * module unload time with drm_framebuffer_unregister_private(). But doing this
> - * is not recommended, and it's better to have a normal free-standing &struct
> + * drivers can grab additional references with drm_framebuffer_get() and drop
> + * them again with drm_framebuffer_put(). For driver-private framebuffers for
> + * which the last reference is never dropped (e.g. for the fbdev framebuffer
> + * when the struct &struct drm_framebuffer is embedded into the fbdev helper
> + * struct) drivers can manually clean up a framebuffer at module unload time
> + * with drm_framebuffer_unregister_private(). But doing this is not
> + * recommended, and it's better to have a normal free-standing &struct
> * drm_framebuffer.
> */
>
> @@ -374,7 +374,7 @@ int drm_mode_rmfb(struct drm_device *dev,
> mutex_unlock(&file_priv->fbs_lock);
>
> /* drop the reference we picked up in framebuffer lookup */
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
>
> /*
> * we now own the reference that was stored in the fbs list
> @@ -394,12 +394,12 @@ int drm_mode_rmfb(struct drm_device *dev,
> flush_work(&arg.work);
> destroy_work_on_stack(&arg.work);
> } else
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
>
> return 0;
>
> fail_unref:
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
> return -ENOENT;
> }
>
> @@ -453,7 +453,7 @@ int drm_mode_getfb(struct drm_device *dev,
> ret = -ENODEV;
> }
>
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
>
> return ret;
> }
> @@ -540,7 +540,7 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
> out_err2:
> kfree(clips);
> out_err1:
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
>
> return ret;
> }
> @@ -580,7 +580,7 @@ void drm_fb_release(struct drm_file *priv)
> list_del_init(&fb->filp_head);
>
> /* This drops the fpriv->fbs reference. */
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
> }
> }
>
> @@ -661,7 +661,7 @@ EXPORT_SYMBOL(drm_framebuffer_init);
> *
> * If successful, this grabs an additional reference to the framebuffer -
> * callers need to make sure to eventually unreference the returned framebuffer
> - * again, using @drm_framebuffer_unreference.
> + * again, using drm_framebuffer_put().
> */
> struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
> uint32_t id)
> @@ -687,8 +687,8 @@ EXPORT_SYMBOL(drm_framebuffer_lookup);
> *
> * NOTE: This function is deprecated. For driver-private framebuffers it is not
> * recommended to embed a framebuffer struct info fbdev struct, instead, a
> - * framebuffer pointer is preferred and drm_framebuffer_unreference() should be
> - * called when the framebuffer is to be cleaned up.
> + * framebuffer pointer is preferred and drm_framebuffer_put() should be called
> + * when the framebuffer is to be cleaned up.
> */
> void drm_framebuffer_unregister_private(struct drm_framebuffer *fb)
> {
> @@ -790,7 +790,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
> drm_modeset_unlock_all(dev);
> }
>
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
> }
> EXPORT_SYMBOL(drm_framebuffer_remove);
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index f42590049a3a..a22e76837065 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -293,7 +293,7 @@ void drm_plane_force_disable(struct drm_plane *plane)
> return;
> }
> /* disconnect the plane from the fb and crtc: */
> - drm_framebuffer_unreference(plane->old_fb);
> + drm_framebuffer_put(plane->old_fb);
> plane->old_fb = NULL;
> plane->fb = NULL;
> plane->crtc = NULL;
> @@ -520,9 +520,9 @@ static int __setplane_internal(struct drm_plane *plane,
>
> out:
> if (fb)
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
> if (plane->old_fb)
> - drm_framebuffer_unreference(plane->old_fb);
> + drm_framebuffer_put(plane->old_fb);
> plane->old_fb = NULL;
>
> return ret;
> @@ -638,7 +638,7 @@ static int drm_mode_cursor_universal(struct drm_crtc *crtc,
> } else {
> fb = crtc->cursor->fb;
> if (fb)
> - drm_framebuffer_reference(fb);
> + drm_framebuffer_get(fb);
> }
>
> if (req->flags & DRM_MODE_CURSOR_MOVE) {
> @@ -902,9 +902,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> if (ret && crtc->funcs->page_flip_target)
> drm_crtc_vblank_put(crtc);
> if (fb)
> - drm_framebuffer_unreference(fb);
> + drm_framebuffer_put(fb);
> if (crtc->primary->old_fb)
> - drm_framebuffer_unreference(crtc->primary->old_fb);
> + drm_framebuffer_put(crtc->primary->old_fb);
> crtc->primary->old_fb = NULL;
> drm_modeset_unlock_crtc(crtc);
>
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index dd1e3e99dcff..5244f059d23a 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -101,8 +101,8 @@ struct drm_framebuffer_funcs {
> * cleanup (like releasing the reference(s) on the backing GEM bo(s))
> * should be deferred. In cases like this, the driver would like to
> * hold a ref to the fb even though it has already been removed from
> - * userspace perspective. See drm_framebuffer_reference() and
> - * drm_framebuffer_unreference().
> + * userspace perspective. See drm_framebuffer_get() and
> + * drm_framebuffer_put().
> *
> * The refcount is stored inside the mode object @base.
> */
> @@ -204,25 +204,50 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb);
> void drm_framebuffer_unregister_private(struct drm_framebuffer *fb);
>
> /**
> - * drm_framebuffer_reference - incr the fb refcnt
> - * @fb: framebuffer
> + * drm_framebuffer_get - acquire a framebuffer reference
> + * @fb: DRM framebuffer
> + *
> + * This function increments the framebuffer's reference count.
> + */
> +static inline void drm_framebuffer_get(struct drm_framebuffer *fb)
> +{
> + drm_mode_object_get(&fb->base);
> +}
> +
> +/**
> + * drm_framebuffer_put - release a framebuffer reference
> + * @fb: DRM framebuffer
> + *
> + * This function decrements the framebuffer's reference count and frees the
> + * framebuffer if the reference count drops to zero.
> + */
> +static inline void drm_framebuffer_put(struct drm_framebuffer *fb)
> +{
> + drm_mode_object_put(&fb->base);
> +}
> +
> +/**
> + * drm_framebuffer_reference - acquire a framebuffer reference
> + * @fb: DRM framebuffer
> *
> - * This functions increments the fb's refcount.
> + * This is a compatibility alias for drm_framebuffer_get() and should not be
> + * used by new code.
> */
> static inline void drm_framebuffer_reference(struct drm_framebuffer *fb)
> {
> - drm_mode_object_reference(&fb->base);
> + drm_framebuffer_get(fb);
> }
>
> /**
> - * drm_framebuffer_unreference - unref a framebuffer
> - * @fb: framebuffer to unref
> + * drm_framebuffer_unreference - release a framebuffer reference
> + * @fb: DRM framebuffer
> *
> - * This functions decrements the fb's refcount and frees it if it drops to zero.
> + * This is a compatibility alias for drm_framebuffer_put() and should not be
> + * used by new code.
> */
> static inline void drm_framebuffer_unreference(struct drm_framebuffer *fb)
> {
> - drm_mode_object_unreference(&fb->base);
> + drm_framebuffer_put(fb);
> }
>
> /**
> @@ -248,9 +273,9 @@ static inline void drm_framebuffer_assign(struct drm_framebuffer **p,
> struct drm_framebuffer *fb)
> {
> if (fb)
> - drm_framebuffer_reference(fb);
> + drm_framebuffer_get(fb);
> if (*p)
> - drm_framebuffer_unreference(*p);
> + drm_framebuffer_put(*p);
> *p = fb;
> }
>
> diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
> index 8a4c2cb7889e..fd298c24a465 100644
> --- a/scripts/coccinelle/api/drm-get-put.cocci
> +++ b/scripts/coccinelle/api/drm-get-put.cocci
> @@ -26,6 +26,12 @@ expression object;
> |
> - drm_connector_unreference(object)
> + drm_connector_put(object)
> +|
> +- drm_framebuffer_reference(object)
> ++ drm_framebuffer_get(object)
> +|
> +- drm_framebuffer_unreference(object)
> ++ drm_framebuffer_put(object)
> )
>
> @r depends on report@
> @@ -41,6 +47,10 @@ drm_mode_object_reference@p(object)
> drm_connector_unreference@p(object)
> |
> drm_connector_reference@p(object)
> +|
> +drm_framebuffer_unreference@p(object)
> +|
> +drm_framebuffer_reference@p(object)
> )
>
> @script:python depends on report@
> --
> 2.11.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/6] drm: Introduce drm_gem_object_{get,put}()
2017-02-08 18:24 ` [PATCH 5/6] drm: Introduce drm_gem_object_{get,put}() Thierry Reding
@ 2017-02-08 19:41 ` Sean Paul
0 siblings, 0 replies; 25+ messages in thread
From: Sean Paul @ 2017-02-08 19:41 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, dri-devel
On Wed, Feb 08, 2017 at 07:24:07PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> For consistency with other reference counting APIs in the kernel, add
> drm_gem_object_get() and drm_gem_object_put(), as well as an unlocked
> variant of the latter, to reference count GEM buffer objects.
>
> Compatibility aliases are added to keep existing code working. To help
> speed up the transition, all the instances of the old functions in the
> DRM core are already replaced in this commit.
>
> The existing semantic patch for the DRM subsystem-wide conversion is
> extended to account for these new helpers.
>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Documentation/gpu/drm-mm.rst | 14 +++---
> drivers/gpu/drm/drm_fb_cma_helper.c | 16 +++----
> drivers/gpu/drm/drm_gem.c | 44 +++++++++---------
> drivers/gpu/drm/drm_gem_cma_helper.c | 10 ++--
> drivers/gpu/drm/drm_prime.c | 10 ++--
> include/drm/drm_gem.h | 80 +++++++++++++++++++++++++-------
> scripts/coccinelle/api/drm-get-put.cocci | 20 ++++++++
> 7 files changed, 130 insertions(+), 64 deletions(-)
>
> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> index f5760b140f13..fd35998acefc 100644
> --- a/Documentation/gpu/drm-mm.rst
> +++ b/Documentation/gpu/drm-mm.rst
> @@ -183,14 +183,12 @@ GEM Objects Lifetime
> --------------------
>
> All GEM objects are reference-counted by the GEM core. References can be
> -acquired and release by :c:func:`calling
> -drm_gem_object_reference()` and
> -:c:func:`drm_gem_object_unreference()` respectively. The caller
> -must hold the :c:type:`struct drm_device <drm_device>`
> -struct_mutex lock when calling
> -:c:func:`drm_gem_object_reference()`. As a convenience, GEM
> -provides :c:func:`drm_gem_object_unreference_unlocked()`
> -functions that can be called without holding the lock.
> +acquired and release by :c:func:`calling drm_gem_object_get()` and
> +:c:func:`drm_gem_object_put()` respectively. The caller must hold the
> +:c:type:`struct drm_device <drm_device>` struct_mutex lock when calling
> +:c:func:`drm_gem_object_get()`. As a convenience, GEM provides
> +:c:func:`drm_gem_object_put_unlocked()` functions that can be called without
> +holding the lock.
>
> When the last reference to a GEM object is released the GEM core calls
> the :c:type:`struct drm_driver <drm_driver>` gem_free_object
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 596fabf18c3e..eccc07d20925 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -102,7 +102,7 @@ void drm_fb_cma_destroy(struct drm_framebuffer *fb)
>
> for (i = 0; i < 4; i++) {
> if (fb_cma->obj[i])
> - drm_gem_object_unreference_unlocked(&fb_cma->obj[i]->base);
> + drm_gem_object_put_unlocked(&fb_cma->obj[i]->base);
> }
>
> drm_framebuffer_cleanup(fb);
> @@ -190,7 +190,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
> if (!obj) {
> dev_err(dev->dev, "Failed to lookup GEM object\n");
> ret = -ENXIO;
> - goto err_gem_object_unreference;
> + goto err_gem_object_put;
> }
>
> min_size = (height - 1) * mode_cmd->pitches[i]
> @@ -198,9 +198,9 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
> + mode_cmd->offsets[i];
>
> if (obj->size < min_size) {
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> ret = -EINVAL;
> - goto err_gem_object_unreference;
> + goto err_gem_object_put;
> }
> objs[i] = to_drm_gem_cma_obj(obj);
> }
> @@ -208,14 +208,14 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
> fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);
> if (IS_ERR(fb_cma)) {
> ret = PTR_ERR(fb_cma);
> - goto err_gem_object_unreference;
> + goto err_gem_object_put;
> }
>
> return &fb_cma->fb;
>
> -err_gem_object_unreference:
> +err_gem_object_put:
> for (i--; i >= 0; i--)
> - drm_gem_object_unreference_unlocked(&objs[i]->base);
> + drm_gem_object_put_unlocked(&objs[i]->base);
> return ERR_PTR(ret);
> }
> EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
> @@ -477,7 +477,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
> err_fb_info_destroy:
> drm_fb_helper_release_fbi(helper);
> err_gem_free_object:
> - drm_gem_object_unreference_unlocked(&obj->base);
> + drm_gem_object_put_unlocked(&obj->base);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index bc93de308673..b1e28c944637 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -218,7 +218,7 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
> }
>
> static void
> -drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
> +drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
> {
> struct drm_device *dev = obj->dev;
> bool final = false;
> @@ -241,7 +241,7 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
> mutex_unlock(&dev->object_name_lock);
>
> if (final)
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> }
>
> /*
> @@ -262,7 +262,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
> if (dev->driver->gem_close_object)
> dev->driver->gem_close_object(obj, file_priv);
>
> - drm_gem_object_handle_unreference_unlocked(obj);
> + drm_gem_object_handle_put_unlocked(obj);
>
> return 0;
> }
> @@ -352,7 +352,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>
> WARN_ON(!mutex_is_locked(&dev->object_name_lock));
> if (obj->handle_count++ == 0)
> - drm_gem_object_reference(obj);
> + drm_gem_object_get(obj);
>
> /*
> * Get the user-visible handle using idr. Preload and perform
> @@ -392,7 +392,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> idr_remove(&file_priv->object_idr, handle);
> spin_unlock(&file_priv->table_lock);
> err_unref:
> - drm_gem_object_handle_unreference_unlocked(obj);
> + drm_gem_object_handle_put_unlocked(obj);
> return ret;
> }
>
> @@ -606,7 +606,7 @@ drm_gem_object_lookup(struct drm_file *filp, u32 handle)
> /* Check if we currently have a reference on the object */
> obj = idr_find(&filp->object_idr, handle);
> if (obj)
> - drm_gem_object_reference(obj);
> + drm_gem_object_get(obj);
>
> spin_unlock(&filp->table_lock);
>
> @@ -683,7 +683,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
>
> err:
> mutex_unlock(&dev->object_name_lock);
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> return ret;
> }
>
> @@ -713,7 +713,7 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data,
> mutex_lock(&dev->object_name_lock);
> obj = idr_find(&dev->object_name_idr, (int) args->name);
> if (obj) {
> - drm_gem_object_reference(obj);
> + drm_gem_object_get(obj);
> } else {
> mutex_unlock(&dev->object_name_lock);
> return -ENOENT;
> @@ -721,7 +721,7 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data,
>
> /* drm_gem_handle_create_tail unlocks dev->object_name_lock. */
> ret = drm_gem_handle_create_tail(file_priv, obj, &handle);
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> if (ret)
> return ret;
>
> @@ -809,16 +809,16 @@ drm_gem_object_free(struct kref *kref)
> EXPORT_SYMBOL(drm_gem_object_free);
>
> /**
> - * drm_gem_object_unreference_unlocked - release a GEM BO reference
> + * drm_gem_object_put_unlocked - drop a GEM buffer object reference
> * @obj: GEM buffer object
> *
> * This releases a reference to @obj. Callers must not hold the
> * &drm_device.struct_mutex lock when calling this function.
> *
> - * See also __drm_gem_object_unreference().
> + * See also __drm_gem_object_put().
> */
> void
> -drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
> +drm_gem_object_put_unlocked(struct drm_gem_object *obj)
> {
> struct drm_device *dev;
>
> @@ -834,10 +834,10 @@ drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
> &dev->struct_mutex))
> mutex_unlock(&dev->struct_mutex);
> }
> -EXPORT_SYMBOL(drm_gem_object_unreference_unlocked);
> +EXPORT_SYMBOL(drm_gem_object_put_unlocked);
>
> /**
> - * drm_gem_object_unreference - release a GEM BO reference
> + * drm_gem_object_put - release a GEM buffer object reference
> * @obj: GEM buffer object
> *
> * This releases a reference to @obj. Callers must hold the
> @@ -845,10 +845,10 @@ EXPORT_SYMBOL(drm_gem_object_unreference_unlocked);
> * driver doesn't use &drm_device.struct_mutex for anything.
> *
> * For drivers not encumbered with legacy locking use
> - * drm_gem_object_unreference_unlocked() instead.
> + * drm_gem_object_put_unlocked() instead.
> */
> void
> -drm_gem_object_unreference(struct drm_gem_object *obj)
> +drm_gem_object_put(struct drm_gem_object *obj)
> {
> if (obj) {
> WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
> @@ -856,7 +856,7 @@ drm_gem_object_unreference(struct drm_gem_object *obj)
> kref_put(&obj->refcount, drm_gem_object_free);
> }
> }
> -EXPORT_SYMBOL(drm_gem_object_unreference);
> +EXPORT_SYMBOL(drm_gem_object_put);
>
> /**
> * drm_gem_vm_open - vma->ops->open implementation for GEM
> @@ -869,7 +869,7 @@ void drm_gem_vm_open(struct vm_area_struct *vma)
> {
> struct drm_gem_object *obj = vma->vm_private_data;
>
> - drm_gem_object_reference(obj);
> + drm_gem_object_get(obj);
> }
> EXPORT_SYMBOL(drm_gem_vm_open);
>
> @@ -884,7 +884,7 @@ void drm_gem_vm_close(struct vm_area_struct *vma)
> {
> struct drm_gem_object *obj = vma->vm_private_data;
>
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> }
> EXPORT_SYMBOL(drm_gem_vm_close);
>
> @@ -935,7 +935,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> * (which should happen whether the vma was created by this call, or
> * by a vm_open due to mremap or partial unmap or whatever).
> */
> - drm_gem_object_reference(obj);
> + drm_gem_object_get(obj);
>
> return 0;
> }
> @@ -992,14 +992,14 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> return -EINVAL;
>
> if (!drm_vma_node_is_allowed(node, priv)) {
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> return -EACCES;
> }
>
> ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
> vma);
>
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index f7ba32bfe39b..bb968e76779b 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -121,7 +121,7 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
> return cma_obj;
>
> error:
> - drm_gem_object_unreference_unlocked(&cma_obj->base);
> + drm_gem_object_put_unlocked(&cma_obj->base);
> return ERR_PTR(ret);
> }
> EXPORT_SYMBOL_GPL(drm_gem_cma_create);
> @@ -163,7 +163,7 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv,
> */
> ret = drm_gem_handle_create(file_priv, gem_obj, handle);
> /* drop reference from allocate - handle holds it now. */
> - drm_gem_object_unreference_unlocked(gem_obj);
> + drm_gem_object_put_unlocked(gem_obj);
> if (ret)
> return ERR_PTR(ret);
>
> @@ -293,7 +293,7 @@ int drm_gem_cma_dumb_map_offset(struct drm_file *file_priv,
>
> *offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
>
> - drm_gem_object_unreference_unlocked(gem_obj);
> + drm_gem_object_put_unlocked(gem_obj);
>
> return 0;
> }
> @@ -416,13 +416,13 @@ unsigned long drm_gem_cma_get_unmapped_area(struct file *filp,
> return -EINVAL;
>
> if (!drm_vma_node_is_allowed(node, priv)) {
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> return -EACCES;
> }
>
> cma_obj = to_drm_gem_cma_obj(obj);
>
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
>
> return cma_obj->vaddr ? (unsigned long)cma_obj->vaddr : -EINVAL;
> }
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 25aa4558f1b5..866b294e7c61 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -318,7 +318,7 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
> return dma_buf;
>
> drm_dev_ref(dev);
> - drm_gem_object_reference(exp_info->priv);
> + drm_gem_object_get(exp_info->priv);
>
> return dma_buf;
> }
> @@ -339,7 +339,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
> struct drm_device *dev = obj->dev;
>
> /* drop the reference on the export fd holds */
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
>
> drm_dev_unref(dev);
> }
> @@ -585,7 +585,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> fail_put_dmabuf:
> dma_buf_put(dmabuf);
> out:
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> out_unlock:
> mutex_unlock(&file_priv->prime.lock);
>
> @@ -616,7 +616,7 @@ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
> * Importing dmabuf exported from out own gem increases
> * refcount on gem itself instead of f_count of dmabuf.
> */
> - drm_gem_object_reference(obj);
> + drm_gem_object_get(obj);
> return obj;
> }
> }
> @@ -704,7 +704,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>
> /* _handle_create_tail unconditionally unlocks dev->object_name_lock. */
> ret = drm_gem_handle_create_tail(file_priv, obj, handle);
> - drm_gem_object_unreference_unlocked(obj);
> + drm_gem_object_put_unlocked(obj);
> if (ret)
> goto out_put;
>
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 449a41b56ffc..3b2a28f7f49f 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -48,9 +48,9 @@ struct drm_gem_object {
> *
> * Reference count of this object
> *
> - * Please use drm_gem_object_reference() to acquire and
> - * drm_gem_object_unreference() or drm_gem_object_unreference_unlocked()
> - * to release a reference to a GEM buffer object.
> + * Please use drm_gem_object_get() to acquire and drm_gem_object_put()
> + * or drm_gem_object_put_unlocked() to release a reference to a GEM
> + * buffer object.
> */
> struct kref refcount;
>
> @@ -187,42 +187,90 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>
> /**
> - * drm_gem_object_reference - acquire a GEM BO reference
> + * drm_gem_object_get - acquire a GEM buffer object reference
> * @obj: GEM buffer object
> *
> - * This acquires additional reference to @obj. It is illegal to call this
> - * without already holding a reference. No locks required.
> + * This function acquires an additional reference to @obj. It is illegal to
> + * call this without already holding a reference. No locks required.
> */
> -static inline void
> -drm_gem_object_reference(struct drm_gem_object *obj)
> +static inline void drm_gem_object_get(struct drm_gem_object *obj)
> {
> kref_get(&obj->refcount);
> }
>
> /**
> - * __drm_gem_object_unreference - raw function to release a GEM BO reference
> + * __drm_gem_object_put - raw function to release a GEM buffer object reference
> * @obj: GEM buffer object
> *
> * This function is meant to be used by drivers which are not encumbered with
> * &drm_device.struct_mutex legacy locking and which are using the
> * gem_free_object_unlocked callback. It avoids all the locking checks and
> - * locking overhead of drm_gem_object_unreference() and
> - * drm_gem_object_unreference_unlocked().
> + * locking overhead of drm_gem_object_put() and drm_gem_object_put_unlocked().
> *
> * Drivers should never call this directly in their code. Instead they should
> - * wrap it up into a ``driver_gem_object_unreference(struct driver_gem_object
> - * *obj)`` wrapper function, and use that. Shared code should never call this, to
> + * wrap it up into a ``driver_gem_object_put(struct driver_gem_object *obj)``
> + * wrapper function, and use that. Shared code should never call this, to
> * avoid breaking drivers by accident which still depend upon
> * &drm_device.struct_mutex locking.
> */
> static inline void
> -__drm_gem_object_unreference(struct drm_gem_object *obj)
> +__drm_gem_object_put(struct drm_gem_object *obj)
> {
> kref_put(&obj->refcount, drm_gem_object_free);
> }
>
> -void drm_gem_object_unreference_unlocked(struct drm_gem_object *obj);
> -void drm_gem_object_unreference(struct drm_gem_object *obj);
> +void drm_gem_object_put_unlocked(struct drm_gem_object *obj);
> +void drm_gem_object_put(struct drm_gem_object *obj);
> +
> +/**
> + * drm_gem_object_reference - acquire a GEM buffer object reference
> + * @obj: GEM buffer object
> + *
> + * This is a compatibility alias for drm_gem_object_get() and should not be
> + * used by new code.
> + */
> +static inline void drm_gem_object_reference(struct drm_gem_object *obj)
> +{
> + drm_gem_object_get(obj);
> +}
> +
> +/**
> + * __drm_gem_object_unreference - raw function to release a GEM buffer object
> + * reference
> + * @obj: GEM buffer object
> + *
> + * This is a compatibility alias for __drm_gem_object_put() and should not be
> + * used by new code.
> + */
> +static inline void __drm_gem_object_unreference(struct drm_gem_object *obj)
> +{
> + __drm_gem_object_put(obj);
> +}
> +
> +/**
> + * drm_gem_object_unreference_unlocked - release a GEM buffer object reference
> + * @obj: GEM buffer object
> + *
> + * This is a compatibility alias for drm_gem_object_put_unlocked() and should
> + * not be used by new code.
> + */
> +static inline void
> +drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)
> +{
> + drm_gem_object_put_unlocked(obj);
> +}
> +
> +/**
> + * drm_gem_object_unreference - release a GEM buffer object reference
> + * @obj: GEM buffer object
> + *
> + * This is a compatibility alias for drm_gem_object_put() and should not be
> + * used by new code.
> + */
> +static inline void drm_gem_object_unreference(struct drm_gem_object *obj)
> +{
> + drm_gem_object_put(obj);
> +}
>
> int drm_gem_handle_create(struct drm_file *file_priv,
> struct drm_gem_object *obj,
> diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
> index fd298c24a465..24882547b4d1 100644
> --- a/scripts/coccinelle/api/drm-get-put.cocci
> +++ b/scripts/coccinelle/api/drm-get-put.cocci
> @@ -32,6 +32,18 @@ expression object;
> |
> - drm_framebuffer_unreference(object)
> + drm_framebuffer_put(object)
> +|
> +- drm_gem_object_reference(object)
> ++ drm_gem_object_get(object)
> +|
> +- drm_gem_object_unreference(object)
> ++ drm_gem_object_put(object)
> +|
> +- __drm_gem_object_unreference(object)
> ++ __drm_gem_object_put(object)
> +|
> +- drm_gem_object_unreference_unlocked(object)
> ++ drm_gem_object_put_unlocked(object)
> )
>
> @r depends on report@
> @@ -51,6 +63,14 @@ drm_connector_reference@p(object)
> drm_framebuffer_unreference@p(object)
> |
> drm_framebuffer_reference@p(object)
> +|
> +drm_gem_object_unreference@p(object)
> +|
> +drm_gem_object_reference@p(object)
> +|
> +__drm_gem_object_unreference(object)
> +|
> +drm_gem_object_unreference_unlocked(object)
> )
>
> @script:python depends on report@
> --
> 2.11.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/6] drm: Introduce drm_property_blob_{get,put}()
2017-02-08 18:24 ` [PATCH 6/6] drm: Introduce drm_property_blob_{get,put}() Thierry Reding
@ 2017-02-08 19:45 ` Sean Paul
0 siblings, 0 replies; 25+ messages in thread
From: Sean Paul @ 2017-02-08 19:45 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, dri-devel
On Wed, Feb 08, 2017 at 07:24:08PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> For consistency with other reference counting APIs in the kernel, add
> drm_property_blob_get() and drm_property_blob_put() to reference count
> DRM blob properties.
>
> Compatibility aliases are added to keep existing code working. To help
> speed up the transition, all the instances of the old functions in the
> DRM core are already replaced in this commit.
>
> A semantic patch is provided that can be used to convert all drivers to
> the new helpers.
>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> drivers/gpu/drm/drm_atomic.c | 16 ++++++-------
> drivers/gpu/drm/drm_atomic_helper.c | 18 +++++++-------
> drivers/gpu/drm/drm_mode_config.c | 2 +-
> drivers/gpu/drm/drm_property.c | 40 ++++++++++++++++----------------
> include/drm/drm_property.h | 35 ++++++++++++++++++++++++----
> scripts/coccinelle/api/drm-get-put.cocci | 10 ++++++++
> 6 files changed, 78 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 740650b450c5..ff5f1756a7ff 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -322,7 +322,7 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
> if (mode && memcmp(&state->mode, mode, sizeof(*mode)) == 0)
> return 0;
>
> - drm_property_unreference_blob(state->mode_blob);
> + drm_property_blob_put(state->mode_blob);
> state->mode_blob = NULL;
>
> if (mode) {
> @@ -368,7 +368,7 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
> if (blob == state->mode_blob)
> return 0;
>
> - drm_property_unreference_blob(state->mode_blob);
> + drm_property_blob_put(state->mode_blob);
> state->mode_blob = NULL;
>
> memset(&state->mode, 0, sizeof(state->mode));
> @@ -380,7 +380,7 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
> blob->data))
> return -EINVAL;
>
> - state->mode_blob = drm_property_reference_blob(blob);
> + state->mode_blob = drm_property_blob_get(blob);
> state->enable = true;
> DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
> state->mode.name, state);
> @@ -413,9 +413,9 @@ drm_atomic_replace_property_blob(struct drm_property_blob **blob,
> if (old_blob == new_blob)
> return;
>
> - drm_property_unreference_blob(old_blob);
> + drm_property_blob_put(old_blob);
> if (new_blob)
> - drm_property_reference_blob(new_blob);
> + drm_property_blob_get(new_blob);
> *blob = new_blob;
> *replaced = true;
>
> @@ -437,13 +437,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
> return -EINVAL;
>
> if (expected_size > 0 && expected_size != new_blob->length) {
> - drm_property_unreference_blob(new_blob);
> + drm_property_blob_put(new_blob);
> return -EINVAL;
> }
> }
>
> drm_atomic_replace_property_blob(blob, new_blob, replaced);
> - drm_property_unreference_blob(new_blob);
> + drm_property_blob_put(new_blob);
>
> return 0;
> }
> @@ -478,7 +478,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> struct drm_property_blob *mode =
> drm_property_lookup_blob(dev, val);
> ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
> - drm_property_unreference_blob(mode);
> + drm_property_blob_put(mode);
> return ret;
> } else if (property == config->degamma_lut_property) {
> ret = drm_atomic_replace_property_blob_from_id(crtc,
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4dcd64ca34c8..863f302b5bf9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3042,13 +3042,13 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
> memcpy(state, crtc->state, sizeof(*state));
>
> if (state->mode_blob)
> - drm_property_reference_blob(state->mode_blob);
> + drm_property_blob_get(state->mode_blob);
> if (state->degamma_lut)
> - drm_property_reference_blob(state->degamma_lut);
> + drm_property_blob_get(state->degamma_lut);
> if (state->ctm)
> - drm_property_reference_blob(state->ctm);
> + drm_property_blob_get(state->ctm);
> if (state->gamma_lut)
> - drm_property_reference_blob(state->gamma_lut);
> + drm_property_blob_get(state->gamma_lut);
> state->mode_changed = false;
> state->active_changed = false;
> state->planes_changed = false;
> @@ -3092,10 +3092,10 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
> */
> void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
> {
> - drm_property_unreference_blob(state->mode_blob);
> - drm_property_unreference_blob(state->degamma_lut);
> - drm_property_unreference_blob(state->ctm);
> - drm_property_unreference_blob(state->gamma_lut);
> + drm_property_blob_put(state->mode_blob);
> + drm_property_blob_put(state->degamma_lut);
> + drm_property_blob_put(state->ctm);
> + drm_property_blob_put(state->gamma_lut);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
>
> @@ -3493,7 +3493,7 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> goto backoff;
>
> drm_atomic_state_put(state);
> - drm_property_unreference_blob(blob);
> + drm_property_blob_put(blob);
> return ret;
>
> backoff:
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 20aec165abd7..37779b9f0c1e 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -444,7 +444,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>
> list_for_each_entry_safe(blob, bt, &dev->mode_config.property_blob_list,
> head_global) {
> - drm_property_unreference_blob(blob);
> + drm_property_blob_put(blob);
> }
>
> /*
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index 15af0d42e8be..b17959c3e099 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -587,19 +587,19 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
> EXPORT_SYMBOL(drm_property_create_blob);
>
> /**
> - * drm_property_unreference_blob - Unreference a blob property
> - * @blob: Pointer to blob property
> + * drm_property_blob_put - release a blob property reference
> + * @blob: DRM blob property
> *
> - * Drop a reference on a blob property. May free the object.
> + * Releases a reference to a blob property. May free the object.
> */
> -void drm_property_unreference_blob(struct drm_property_blob *blob)
> +void drm_property_blob_put(struct drm_property_blob *blob)
> {
> if (!blob)
> return;
>
> drm_mode_object_put(&blob->base);
> }
> -EXPORT_SYMBOL(drm_property_unreference_blob);
> +EXPORT_SYMBOL(drm_property_blob_put);
>
> void drm_property_destroy_user_blobs(struct drm_device *dev,
> struct drm_file *file_priv)
> @@ -612,23 +612,23 @@ void drm_property_destroy_user_blobs(struct drm_device *dev,
> */
> list_for_each_entry_safe(blob, bt, &file_priv->blobs, head_file) {
> list_del_init(&blob->head_file);
> - drm_property_unreference_blob(blob);
> + drm_property_blob_put(blob);
> }
> }
>
> /**
> - * drm_property_reference_blob - Take a reference on an existing property
> - * @blob: Pointer to blob property
> + * drm_property_blob_get - acquire blob property reference
> + * @blob: DRM blob property
> *
> - * Take a new reference on an existing blob property. Returns @blob, which
> + * Acquires a reference to an existing blob property. Returns @blob, which
> * allows this to be used as a shorthand in assignments.
> */
> -struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob)
> +struct drm_property_blob *drm_property_blob_get(struct drm_property_blob *blob)
> {
> drm_mode_object_get(&blob->base);
> return blob;
> }
> -EXPORT_SYMBOL(drm_property_reference_blob);
> +EXPORT_SYMBOL(drm_property_blob_get);
>
> /**
> * drm_property_lookup_blob - look up a blob property and take a reference
> @@ -637,7 +637,7 @@ EXPORT_SYMBOL(drm_property_reference_blob);
> *
> * If successful, this takes an additional reference to the blob property.
> * callers need to make sure to eventually unreference the returned property
> - * again, using @drm_property_unreference_blob.
> + * again, using drm_property_blob_put().
> *
> * Return:
> * NULL on failure, pointer to the blob on success.
> @@ -712,13 +712,13 @@ int drm_property_replace_global_blob(struct drm_device *dev,
> goto err_created;
> }
>
> - drm_property_unreference_blob(old_blob);
> + drm_property_blob_put(old_blob);
> *replace = new_blob;
>
> return 0;
>
> err_created:
> - drm_property_unreference_blob(new_blob);
> + drm_property_blob_put(new_blob);
> return ret;
> }
> EXPORT_SYMBOL(drm_property_replace_global_blob);
> @@ -747,7 +747,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
> }
> out_resp->length = blob->length;
> unref:
> - drm_property_unreference_blob(blob);
> + drm_property_blob_put(blob);
>
> return ret;
> }
> @@ -784,7 +784,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev,
> return 0;
>
> out_blob:
> - drm_property_unreference_blob(blob);
> + drm_property_blob_put(blob);
> return ret;
> }
>
> @@ -823,14 +823,14 @@ int drm_mode_destroyblob_ioctl(struct drm_device *dev,
> mutex_unlock(&dev->mode_config.blob_lock);
>
> /* One reference from lookup, and one from the filp. */
> - drm_property_unreference_blob(blob);
> - drm_property_unreference_blob(blob);
> + drm_property_blob_put(blob);
> + drm_property_blob_put(blob);
>
> return 0;
>
> err:
> mutex_unlock(&dev->mode_config.blob_lock);
> - drm_property_unreference_blob(blob);
> + drm_property_blob_put(blob);
>
> return ret;
> }
> @@ -908,5 +908,5 @@ void drm_property_change_valid_put(struct drm_property *property,
> if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
> drm_mode_object_put(ref);
> } else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
> - drm_property_unreference_blob(obj_to_blob(ref));
> + drm_property_blob_put(obj_to_blob(ref));
> }
> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> index f66fdb47551c..13e8c17d1c79 100644
> --- a/include/drm/drm_property.h
> +++ b/include/drm/drm_property.h
> @@ -200,9 +200,8 @@ struct drm_property {
> * Blobs are used to store bigger values than what fits directly into the 64
> * bits available for a &drm_property.
> *
> - * Blobs are reference counted using drm_property_reference_blob() and
> - * drm_property_unreference_blob(). They are created using
> - * drm_property_create_blob().
> + * Blobs are reference counted using drm_property_blob_get() and
> + * drm_property_blob_put(). They are created using drm_property_create_blob().
> */
> struct drm_property_blob {
> struct drm_mode_object base;
> @@ -274,8 +273,34 @@ int drm_property_replace_global_blob(struct drm_device *dev,
> const void *data,
> struct drm_mode_object *obj_holds_id,
> struct drm_property *prop_holds_id);
> -struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob);
> -void drm_property_unreference_blob(struct drm_property_blob *blob);
> +struct drm_property_blob *drm_property_blob_get(struct drm_property_blob *blob);
> +void drm_property_blob_put(struct drm_property_blob *blob);
> +
> +/**
> + * drm_property_reference_blob - acquire a blob property reference
> + * @blob: DRM blob property
> + *
> + * This is a compatibility alias for drm_property_blob_get() and should not be
> + * used by new code.
> + */
> +static inline struct drm_property_blob *
> +drm_property_reference_blob(struct drm_property_blob *blob)
> +{
> + return drm_property_blob_get(blob);
> +}
> +
> +/**
> + * drm_property_unreference_blob - release a blob property reference
> + * @blob: DRM blob property
> + *
> + * This is a compatibility alias for drm_property_blob_put() and should not be
> + * used by new code.
> + */
> +static inline void
> +drm_property_unreference_blob(struct drm_property_blob *blob)
> +{
> + drm_property_blob_put(blob);
> +}
>
> /**
> * drm_connector_find - find property object
> diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
> index 24882547b4d1..0c7a9265c07e 100644
> --- a/scripts/coccinelle/api/drm-get-put.cocci
> +++ b/scripts/coccinelle/api/drm-get-put.cocci
> @@ -44,6 +44,12 @@ expression object;
> |
> - drm_gem_object_unreference_unlocked(object)
> + drm_gem_object_put_unlocked(object)
> +|
> +- drm_property_reference_blob(object)
> ++ drm_property_blob_get(object)
> +|
> +- drm_property_unreference_blob(object)
> ++ drm_property_blob_put(object)
> )
>
> @r depends on report@
> @@ -71,6 +77,10 @@ drm_gem_object_reference@p(object)
> __drm_gem_object_unreference(object)
> |
> drm_gem_object_unreference_unlocked(object)
> +|
> +drm_property_unreference_blob@p(object)
> +|
> +drm_property_reference_blob@p(object)
> )
>
> @script:python depends on report@
> --
> 2.11.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] drm: Introduce consistent reference counting APIs
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
` (5 preceding siblings ...)
2017-02-08 18:24 ` [PATCH 6/6] drm: Introduce drm_property_blob_{get,put}() Thierry Reding
@ 2017-02-09 14:10 ` Jani Nikula
2017-02-09 17:09 ` Daniel Vetter
2017-02-10 14:47 ` Christian König
7 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2017-02-09 14:10 UTC (permalink / raw)
To: Thierry Reding, dri-devel; +Cc: Daniel Vetter
On Wed, 08 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
> This series introduces DRM reference counting APIs that are consistent
> with other reference counting APIs in the kernel. They are also much
> shorter. Compatibility aliases are added to keep existing code working
> and will stay in place until all users of the old APIs are gone.
I like it.
But it makes drm_connector_list_iter_{get,put} stick out like a sore
thumb. Something for the TODO list I guess.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()
2017-02-08 19:28 ` Sean Paul
@ 2017-02-09 17:08 ` Daniel Vetter
2017-02-09 20:41 ` Thierry Reding
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-02-09 17:08 UTC (permalink / raw)
To: Sean Paul; +Cc: Daniel Vetter, dri-devel
On Wed, Feb 08, 2017 at 02:28:14PM -0500, Sean Paul wrote:
> On Wed, Feb 08, 2017 at 07:24:04PM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > For consistency with other reference counting APIs in the kernel, add
> > drm_mode_object_get() and drm_mode_object_put() to reference count DRM
> > mode objects.
> >
> > Compatibility aliases are added to keep existing code working. To help
> > speed up the transition, all the instances of the old functions in the
> > DRM core are already replaced in this commit.
> >
>
> drm code looks good and is
>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>
> > A semantic patch is provided that can be used to convert all drivers to
> > the new helpers.
>
> I'm not convinced we need to commit the cocci patch. I think including it in
> your cover letter and then following up with a follow on series to actually make
> the change is sufficient (See: ickle's s/fence/dma_fence/ series).
Yeah, if you do a large-scale refactor anyway, I think it's best to just
store the cocci in the commit message. I think storing the cocci is ok if
you have thousands of hits among lots of subsystems, and it's clear it's
going to take at least a few release cycles or maybe even years to clean
it all up. drm is luckily not yet that big :-)
I'll drop this while applying if no one minds ...
-Daniel
>
> Sean
>
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 14 +++++------
> > drivers/gpu/drm/drm_mode_object.c | 26 ++++++++++----------
> > drivers/gpu/drm/drm_property.c | 6 ++---
> > include/drm/drm_mode_object.h | 36 ++++++++++++++++++++++-----
> > scripts/coccinelle/api/drm-get-put.cocci | 42 ++++++++++++++++++++++++++++++++
> > 5 files changed, 95 insertions(+), 29 deletions(-)
> > create mode 100644 scripts/coccinelle/api/drm-get-put.cocci
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index a5673107db26..2bb0a759e8ec 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -2122,13 +2122,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > }
> >
> > if (!obj->properties) {
> > - drm_mode_object_unreference(obj);
> > + drm_mode_object_put(obj);
> > ret = -ENOENT;
> > goto out;
> > }
> >
> > if (get_user(count_props, count_props_ptr + copied_objs)) {
> > - drm_mode_object_unreference(obj);
> > + drm_mode_object_put(obj);
> > ret = -EFAULT;
> > goto out;
> > }
> > @@ -2141,14 +2141,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > struct drm_property *prop;
> >
> > if (get_user(prop_id, props_ptr + copied_props)) {
> > - drm_mode_object_unreference(obj);
> > + drm_mode_object_put(obj);
> > ret = -EFAULT;
> > goto out;
> > }
> >
> > prop = drm_mode_obj_find_prop_id(obj, prop_id);
> > if (!prop) {
> > - drm_mode_object_unreference(obj);
> > + drm_mode_object_put(obj);
> > ret = -ENOENT;
> > goto out;
> > }
> > @@ -2156,14 +2156,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > if (copy_from_user(&prop_value,
> > prop_values_ptr + copied_props,
> > sizeof(prop_value))) {
> > - drm_mode_object_unreference(obj);
> > + drm_mode_object_put(obj);
> > ret = -EFAULT;
> > goto out;
> > }
> >
> > ret = atomic_set_prop(state, obj, prop, prop_value);
> > if (ret) {
> > - drm_mode_object_unreference(obj);
> > + drm_mode_object_put(obj);
> > goto out;
> > }
> >
> > @@ -2176,7 +2176,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > plane_mask |= (1 << drm_plane_index(plane));
> > plane->old_fb = plane->fb;
> > }
> > - drm_mode_object_unreference(obj);
> > + drm_mode_object_put(obj);
> > }
> >
> > ret = prepare_crtc_signaling(dev, state, arg, file_priv, &fence_state,
> > diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> > index 3b405dbf1b8d..da9a9adbcc98 100644
> > --- a/drivers/gpu/drm/drm_mode_object.c
> > +++ b/drivers/gpu/drm/drm_mode_object.c
> > @@ -133,7 +133,7 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
> > *
> > * This function is used to look up a modeset object. It will acquire a
> > * reference for reference counted objects. This reference must be dropped again
> > - * by callind drm_mode_object_unreference().
> > + * by callind drm_mode_object_put().
> > */
> > struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
> > uint32_t id, uint32_t type)
> > @@ -146,38 +146,38 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
> > EXPORT_SYMBOL(drm_mode_object_find);
> >
> > /**
> > - * drm_mode_object_unreference - decr the object refcnt
> > - * @obj: mode_object
> > + * drm_mode_object_put - release a mode object reference
> > + * @obj: DRM mode object
> > *
> > * This function decrements the object's refcount if it is a refcounted modeset
> > * object. It is a no-op on any other object. This is used to drop references
> > - * acquired with drm_mode_object_reference().
> > + * acquired with drm_mode_object_get().
> > */
> > -void drm_mode_object_unreference(struct drm_mode_object *obj)
> > +void drm_mode_object_put(struct drm_mode_object *obj)
> > {
> > if (obj->free_cb) {
> > DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, kref_read(&obj->refcount));
> > kref_put(&obj->refcount, obj->free_cb);
> > }
> > }
> > -EXPORT_SYMBOL(drm_mode_object_unreference);
> > +EXPORT_SYMBOL(drm_mode_object_put);
> >
> > /**
> > - * drm_mode_object_reference - incr the object refcnt
> > - * @obj: mode_object
> > + * drm_mode_object_get - acquire a mode object reference
> > + * @obj: DRM mode object
> > *
> > * This function increments the object's refcount if it is a refcounted modeset
> > * object. It is a no-op on any other object. References should be dropped again
> > - * by calling drm_mode_object_unreference().
> > + * by calling drm_mode_object_put().
> > */
> > -void drm_mode_object_reference(struct drm_mode_object *obj)
> > +void drm_mode_object_get(struct drm_mode_object *obj)
> > {
> > if (obj->free_cb) {
> > DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, kref_read(&obj->refcount));
> > kref_get(&obj->refcount);
> > }
> > }
> > -EXPORT_SYMBOL(drm_mode_object_reference);
> > +EXPORT_SYMBOL(drm_mode_object_get);
> >
> > /**
> > * drm_object_attach_property - attach a property to a modeset object
> > @@ -363,7 +363,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
> > &arg->count_props);
> >
> > out_unref:
> > - drm_mode_object_unreference(obj);
> > + drm_mode_object_put(obj);
> > out:
> > drm_modeset_unlock_all(dev);
> > return ret;
> > @@ -428,7 +428,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
> > drm_property_change_valid_put(property, ref);
> >
> > out_unref:
> > - drm_mode_object_unreference(arg_obj);
> > + drm_mode_object_put(arg_obj);
> > out:
> > drm_modeset_unlock_all(dev);
> > return ret;
> > diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> > index 411e470369c0..15af0d42e8be 100644
> > --- a/drivers/gpu/drm/drm_property.c
> > +++ b/drivers/gpu/drm/drm_property.c
> > @@ -597,7 +597,7 @@ void drm_property_unreference_blob(struct drm_property_blob *blob)
> > if (!blob)
> > return;
> >
> > - drm_mode_object_unreference(&blob->base);
> > + drm_mode_object_put(&blob->base);
> > }
> > EXPORT_SYMBOL(drm_property_unreference_blob);
> >
> > @@ -625,7 +625,7 @@ void drm_property_destroy_user_blobs(struct drm_device *dev,
> > */
> > struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob)
> > {
> > - drm_mode_object_reference(&blob->base);
> > + drm_mode_object_get(&blob->base);
> > return blob;
> > }
> > EXPORT_SYMBOL(drm_property_reference_blob);
> > @@ -906,7 +906,7 @@ void drm_property_change_valid_put(struct drm_property *property,
> > return;
> >
> > if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
> > - drm_mode_object_unreference(ref);
> > + drm_mode_object_put(ref);
> > } else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
> > drm_property_unreference_blob(obj_to_blob(ref));
> > }
> > diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> > index 2c017adf6d74..a767b4a30a6d 100644
> > --- a/include/drm/drm_mode_object.h
> > +++ b/include/drm/drm_mode_object.h
> > @@ -45,10 +45,10 @@ struct drm_device;
> > * drm_object_attach_property() before the object is visible to userspace.
> > *
> > * - For objects with dynamic lifetimes (as indicated by a non-NULL @free_cb) it
> > - * provides reference counting through drm_mode_object_reference() and
> > - * drm_mode_object_unreference(). This is used by &drm_framebuffer,
> > - * &drm_connector and &drm_property_blob. These objects provide specialized
> > - * reference counting wrappers.
> > + * provides reference counting through drm_mode_object_get() and
> > + * drm_mode_object_put(). This is used by &drm_framebuffer, &drm_connector
> > + * and &drm_property_blob. These objects provide specialized reference
> > + * counting wrappers.
> > */
> > struct drm_mode_object {
> > uint32_t id;
> > @@ -114,8 +114,32 @@ struct drm_object_properties {
> >
> > struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
> > uint32_t id, uint32_t type);
> > -void drm_mode_object_reference(struct drm_mode_object *obj);
> > -void drm_mode_object_unreference(struct drm_mode_object *obj);
> > +void drm_mode_object_get(struct drm_mode_object *obj);
> > +void drm_mode_object_put(struct drm_mode_object *obj);
> > +
> > +/**
> > + * drm_mode_object_reference - acquire a mode object reference
> > + * @obj: DRM mode object
> > + *
> > + * This is a compatibility alias for drm_mode_object_get() and should not be
> > + * used by new code.
> > + */
> > +static inline void drm_mode_object_reference(struct drm_mode_object *obj)
> > +{
> > + drm_mode_object_get(obj);
> > +}
> > +
> > +/**
> > + * drm_mode_object_unreference - release a mode object reference
> > + * @obj: DRM mode object
> > + *
> > + * This is a compatibility alias for drm_mode_object_put() and should not be
> > + * used by new code.
> > + */
> > +static inline void drm_mode_object_unreference(struct drm_mode_object *obj)
> > +{
> > + drm_mode_object_put(obj);
> > +}
> >
> > int drm_object_property_set_value(struct drm_mode_object *obj,
> > struct drm_property *property,
> > diff --git a/scripts/coccinelle/api/drm-get-put.cocci b/scripts/coccinelle/api/drm-get-put.cocci
> > new file mode 100644
> > index 000000000000..a3742447c981
> > --- /dev/null
> > +++ b/scripts/coccinelle/api/drm-get-put.cocci
> > @@ -0,0 +1,42 @@
> > +///
> > +/// Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference() and
> > +/// drm_*_unreference() helpers.
> > +///
> > +// Confidence: High
> > +// Copyright: (C) 2017 NVIDIA Corporation
> > +// Options: --no-includes --include-headers
> > +//
> > +
> > +virtual patch
> > +virtual report
> > +
> > +@depends on patch@
> > +expression object;
> > +@@
> > +
> > +(
> > +- drm_mode_object_reference(object)
> > ++ drm_mode_object_get(object)
> > +|
> > +- drm_mode_object_unreference(object)
> > ++ drm_mode_object_put(object)
> > +)
> > +
> > +@r depends on report@
> > +expression object;
> > +position p;
> > +@@
> > +
> > +(
> > +drm_mode_object_unreference@p(object)
> > +|
> > +drm_mode_object_reference@p(object)
> > +)
> > +
> > +@script:python depends on report@
> > +object << r.object;
> > +p << r.p;
> > +@@
> > +
> > +msg="WARNING: use get/put helpers to reference and dereference %s" % (object)
> > +coccilib.report.print_report(p[0], msg)
> > --
> > 2.11.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] drm: Introduce consistent reference counting APIs
2017-02-09 14:10 ` [PATCH 0/6] drm: Introduce consistent reference counting APIs Jani Nikula
@ 2017-02-09 17:09 ` Daniel Vetter
2017-02-09 17:39 ` Jani Nikula
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-02-09 17:09 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, dri-devel
On Thu, Feb 09, 2017 at 04:10:12PM +0200, Jani Nikula wrote:
> On Wed, 08 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
> > This series introduces DRM reference counting APIs that are consistent
> > with other reference counting APIs in the kernel. They are also much
> > shorter. Compatibility aliases are added to keep existing code working
> > and will stay in place until all users of the old APIs are gone.
>
> I like it.
>
> But it makes drm_connector_list_iter_{get,put} stick out like a sore
> thumb. Something for the TODO list I guess.
Hm, why is that one the sore thumb now? Just because it's really long? It
does acquire/drop references behind the scenes, that's why I went with the
_get/put postfixes ...
-Daniel
--
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] drm: Introduce consistent reference counting APIs
2017-02-09 17:09 ` Daniel Vetter
@ 2017-02-09 17:39 ` Jani Nikula
2017-02-09 19:07 ` Daniel Vetter
0 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2017-02-09 17:39 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel
On Thu, 09 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Feb 09, 2017 at 04:10:12PM +0200, Jani Nikula wrote:
>> On Wed, 08 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > This series introduces DRM reference counting APIs that are consistent
>> > with other reference counting APIs in the kernel. They are also much
>> > shorter. Compatibility aliases are added to keep existing code working
>> > and will stay in place until all users of the old APIs are gone.
>>
>> I like it.
>>
>> But it makes drm_connector_list_iter_{get,put} stick out like a sore
>> thumb. Something for the TODO list I guess.
>
> Hm, why is that one the sore thumb now? Just because it's really long? It
> does acquire/drop references behind the scenes, that's why I went with the
> _get/put postfixes ...
I think the assumption is that get/put work on an object, and you can
get/put as many times as you like, as long as you keep them at
balance. AFAICT this doesn't hold for iter.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] drm: Introduce consistent reference counting APIs
2017-02-09 17:39 ` Jani Nikula
@ 2017-02-09 19:07 ` Daniel Vetter
2017-02-09 20:39 ` Thierry Reding
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-02-09 19:07 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, dri-devel
On Thu, Feb 09, 2017 at 07:39:33PM +0200, Jani Nikula wrote:
> On Thu, 09 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Feb 09, 2017 at 04:10:12PM +0200, Jani Nikula wrote:
> >> On Wed, 08 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
> >> > This series introduces DRM reference counting APIs that are consistent
> >> > with other reference counting APIs in the kernel. They are also much
> >> > shorter. Compatibility aliases are added to keep existing code working
> >> > and will stay in place until all users of the old APIs are gone.
> >>
> >> I like it.
> >>
> >> But it makes drm_connector_list_iter_{get,put} stick out like a sore
> >> thumb. Something for the TODO list I guess.
> >
> > Hm, why is that one the sore thumb now? Just because it's really long? It
> > does acquire/drop references behind the scenes, that's why I went with the
> > _get/put postfixes ...
>
> I think the assumption is that get/put work on an object, and you can
> get/put as many times as you like, as long as you keep them at
> balance. AFAICT this doesn't hold for iter.
Hm right. What else should we use instead? start/stop are confusing in the
context of list walking, create/destroy maybe?
I don't have a good name I guess ...
-Daniel
--
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] drm: Introduce consistent reference counting APIs
2017-02-09 19:07 ` Daniel Vetter
@ 2017-02-09 20:39 ` Thierry Reding
2017-02-10 7:29 ` Jani Nikula
0 siblings, 1 reply; 25+ messages in thread
From: Thierry Reding @ 2017-02-09 20:39 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2494 bytes --]
On Thu, Feb 09, 2017 at 08:07:41PM +0100, Daniel Vetter wrote:
> On Thu, Feb 09, 2017 at 07:39:33PM +0200, Jani Nikula wrote:
> > On Thu, 09 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > On Thu, Feb 09, 2017 at 04:10:12PM +0200, Jani Nikula wrote:
> > >> On Wed, 08 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
> > >> > This series introduces DRM reference counting APIs that are consistent
> > >> > with other reference counting APIs in the kernel. They are also much
> > >> > shorter. Compatibility aliases are added to keep existing code working
> > >> > and will stay in place until all users of the old APIs are gone.
> > >>
> > >> I like it.
> > >>
> > >> But it makes drm_connector_list_iter_{get,put} stick out like a sore
> > >> thumb. Something for the TODO list I guess.
> > >
> > > Hm, why is that one the sore thumb now? Just because it's really long? It
> > > does acquire/drop references behind the scenes, that's why I went with the
> > > _get/put postfixes ...
> >
> > I think the assumption is that get/put work on an object, and you can
> > get/put as many times as you like, as long as you keep them at
> > balance. AFAICT this doesn't hold for iter.
>
> Hm right. What else should we use instead? start/stop are confusing in the
> context of list walking, create/destroy maybe?
I rather like the _begin()/_end() suffix that I think iterators have in
the C++ STL. Well, I guess it's really .end() and .begin() methods in
C++.
But the following reads rather nicely:
drm_connector_list_iter_begin(dev, &iter);
drm_for_each_connector_iter(connector, &iter)
...
drm_connector_list_iter_end(&iter);
Alternatively some other iterators simply use an _init() suffix to
initialize the iterator. The kernel has a few of these, such as the
of_phandle_iterator (see include/linux/of.h) or of_pci_range_parser (see
include include/linux/of_address.h).
EFI code uses _begin()/_end() in include/linux/efi.h, cgroups seems to
have _start()/_end() in include/linux/cgroup.h. include/linux/klist.h
uses klist_iter_init()/klist_iter_exit(). include/linux/rhashtable.h has
rhashtable_walk_start()/rhashtable_walk_stop().
A quick grep also shows other variants, one of them being _first(), but
those don't have an explicit end function.
So there's quite a few options to choose from, but I guess that doesn't
make it any easier.
If this was a poll, my vote would go to _begin()/_end().
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()
2017-02-09 17:08 ` Daniel Vetter
@ 2017-02-09 20:41 ` Thierry Reding
2017-02-09 21:55 ` Emil Velikov
0 siblings, 1 reply; 25+ messages in thread
From: Thierry Reding @ 2017-02-09 20:41 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 1780 bytes --]
On Thu, Feb 09, 2017 at 06:08:10PM +0100, Daniel Vetter wrote:
> On Wed, Feb 08, 2017 at 02:28:14PM -0500, Sean Paul wrote:
> > On Wed, Feb 08, 2017 at 07:24:04PM +0100, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > >
> > > For consistency with other reference counting APIs in the kernel, add
> > > drm_mode_object_get() and drm_mode_object_put() to reference count DRM
> > > mode objects.
> > >
> > > Compatibility aliases are added to keep existing code working. To help
> > > speed up the transition, all the instances of the old functions in the
> > > DRM core are already replaced in this commit.
> > >
> >
> > drm code looks good and is
> >
> > Reviewed-by: Sean Paul <seanpaul@chromium.org>
> >
> > > A semantic patch is provided that can be used to convert all drivers to
> > > the new helpers.
> >
> > I'm not convinced we need to commit the cocci patch. I think including it in
> > your cover letter and then following up with a follow on series to actually make
> > the change is sufficient (See: ickle's s/fence/dma_fence/ series).
>
> Yeah, if you do a large-scale refactor anyway, I think it's best to just
> store the cocci in the commit message. I think storing the cocci is ok if
> you have thousands of hits among lots of subsystems, and it's clear it's
> going to take at least a few release cycles or maybe even years to clean
> it all up. drm is luckily not yet that big :-)
>
> I'll drop this while applying if no one minds ...
I thought it was actually quite nice that this was part of the series.
That way it doesn't get lost and it is really easy to rerun. Also it can
trivially be removed once we've converted everyone to the new functions
and removed the old ones.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()
2017-02-09 20:41 ` Thierry Reding
@ 2017-02-09 21:55 ` Emil Velikov
2017-02-14 20:12 ` Daniel Vetter
0 siblings, 1 reply; 25+ messages in thread
From: Emil Velikov @ 2017-02-09 21:55 UTC (permalink / raw)
To: Thierry Reding; +Cc: Daniel Vetter, ML dri-devel
On 9 February 2017 at 20:41, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Thu, Feb 09, 2017 at 06:08:10PM +0100, Daniel Vetter wrote:
>> On Wed, Feb 08, 2017 at 02:28:14PM -0500, Sean Paul wrote:
>> > On Wed, Feb 08, 2017 at 07:24:04PM +0100, Thierry Reding wrote:
>> > > From: Thierry Reding <treding@nvidia.com>
>> > >
>> > > For consistency with other reference counting APIs in the kernel, add
>> > > drm_mode_object_get() and drm_mode_object_put() to reference count DRM
>> > > mode objects.
>> > >
>> > > Compatibility aliases are added to keep existing code working. To help
>> > > speed up the transition, all the instances of the old functions in the
>> > > DRM core are already replaced in this commit.
>> > >
>> >
>> > drm code looks good and is
>> >
>> > Reviewed-by: Sean Paul <seanpaul@chromium.org>
>> >
>> > > A semantic patch is provided that can be used to convert all drivers to
>> > > the new helpers.
>> >
>> > I'm not convinced we need to commit the cocci patch. I think including it in
>> > your cover letter and then following up with a follow on series to actually make
>> > the change is sufficient (See: ickle's s/fence/dma_fence/ series).
>>
>> Yeah, if you do a large-scale refactor anyway, I think it's best to just
>> store the cocci in the commit message. I think storing the cocci is ok if
>> you have thousands of hits among lots of subsystems, and it's clear it's
>> going to take at least a few release cycles or maybe even years to clean
>> it all up. drm is luckily not yet that big :-)
>>
>> I'll drop this while applying if no one minds ...
>
> I thought it was actually quite nice that this was part of the series.
> That way it doesn't get lost and it is really easy to rerun. Also it can
> trivially be removed once we've converted everyone to the new functions
> and removed the old ones.
>
Hidden bonus:
Some of the people who run those on semi-regular basis can update
some/all the drivers ;-)
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] drm: Introduce consistent reference counting APIs
2017-02-09 20:39 ` Thierry Reding
@ 2017-02-10 7:29 ` Jani Nikula
2017-02-10 15:58 ` Daniel Vetter
0 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2017-02-10 7:29 UTC (permalink / raw)
To: Thierry Reding, Daniel Vetter; +Cc: Daniel Vetter, dri-devel
On Thu, 09 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Thu, Feb 09, 2017 at 08:07:41PM +0100, Daniel Vetter wrote:
>> On Thu, Feb 09, 2017 at 07:39:33PM +0200, Jani Nikula wrote:
>> > On Thu, 09 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > > On Thu, Feb 09, 2017 at 04:10:12PM +0200, Jani Nikula wrote:
>> > >> On Wed, 08 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > >> > This series introduces DRM reference counting APIs that are consistent
>> > >> > with other reference counting APIs in the kernel. They are also much
>> > >> > shorter. Compatibility aliases are added to keep existing code working
>> > >> > and will stay in place until all users of the old APIs are gone.
>> > >>
>> > >> I like it.
>> > >>
>> > >> But it makes drm_connector_list_iter_{get,put} stick out like a sore
>> > >> thumb. Something for the TODO list I guess.
>> > >
>> > > Hm, why is that one the sore thumb now? Just because it's really long? It
>> > > does acquire/drop references behind the scenes, that's why I went with the
>> > > _get/put postfixes ...
>> >
>> > I think the assumption is that get/put work on an object, and you can
>> > get/put as many times as you like, as long as you keep them at
>> > balance. AFAICT this doesn't hold for iter.
>>
>> Hm right. What else should we use instead? start/stop are confusing in the
>> context of list walking, create/destroy maybe?
>
> I rather like the _begin()/_end() suffix that I think iterators have in
> the C++ STL. Well, I guess it's really .end() and .begin() methods in
> C++.
>
> But the following reads rather nicely:
>
> drm_connector_list_iter_begin(dev, &iter);
>
> drm_for_each_connector_iter(connector, &iter)
> ...
>
> drm_connector_list_iter_end(&iter);
>
> Alternatively some other iterators simply use an _init() suffix to
> initialize the iterator. The kernel has a few of these, such as the
> of_phandle_iterator (see include/linux/of.h) or of_pci_range_parser (see
> include include/linux/of_address.h).
>
> EFI code uses _begin()/_end() in include/linux/efi.h, cgroups seems to
> have _start()/_end() in include/linux/cgroup.h. include/linux/klist.h
> uses klist_iter_init()/klist_iter_exit(). include/linux/rhashtable.h has
> rhashtable_walk_start()/rhashtable_walk_stop().
>
> A quick grep also shows other variants, one of them being _first(), but
> those don't have an explicit end function.
>
> So there's quite a few options to choose from, but I guess that doesn't
> make it any easier.
>
> If this was a poll, my vote would go to _begin()/_end().
I think begin/end are sensible.
An alternative is to make iter get/put work as get/put do, but I suppose
there's no real need for them to behave that way, i.e. it's
overengineering.
And if it wasn't clear, I didn't mean that this should block the patches
at hand in any way. Someone(tm) can follow up with the iter stuff later
on. It's not broken, it just feels slightly misleading after these
changes.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] drm: Introduce consistent reference counting APIs
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
` (6 preceding siblings ...)
2017-02-09 14:10 ` [PATCH 0/6] drm: Introduce consistent reference counting APIs Jani Nikula
@ 2017-02-10 14:47 ` Christian König
7 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2017-02-10 14:47 UTC (permalink / raw)
To: Thierry Reding, dri-devel; +Cc: Daniel Vetter
Am 08.02.2017 um 19:24 schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
>
> This series introduces DRM reference counting APIs that are consistent
> with other reference counting APIs in the kernel. They are also much
> shorter. Compatibility aliases are added to keep existing code working
> and will stay in place until all users of the old APIs are gone.
>
> All occurrences of the old APIs in the core are already replaced by
> this series, and a semantic patch is provided that allows drivers to
> be converted automatically. I plan on generating patches against the
> drivers once the new functions have been merged and send them out to
> their respective maintainers.
>
> Oh, and this fixes one of the items in our recently added TODO list.
>
> Thanks,
> Thierry
Oh, yes please. Really nice to get the naming in line with the rest of
the kernel.
Whole set is Acked-by: Christian König <christian.koenig@amd.com>.
Leave me a note if you have patches for amdgpu/radeon and need an rb.
Regards,
Christian.
>
> Thierry Reding (6):
> drm: Rename drm_mode_object_get()
> drm: Introduce drm_mode_object_{get,put}()
> drm: Introduce drm_connector_{get,put}()
> drm: Introduce drm_framebuffer_{get,put}()
> drm: Introduce drm_gem_object_{get,put}()
> drm: Introduce drm_property_blob_{get,put}()
>
> Documentation/gpu/drm-mm.rst | 14 +++--
> drivers/gpu/drm/drm_atomic.c | 44 +++++++--------
> drivers/gpu/drm/drm_atomic_helper.c | 26 ++++-----
> drivers/gpu/drm/drm_connector.c | 16 +++---
> drivers/gpu/drm/drm_crtc.c | 12 ++---
> drivers/gpu/drm/drm_crtc_helper.c | 6 +--
> drivers/gpu/drm/drm_crtc_internal.h | 12 ++---
> drivers/gpu/drm/drm_encoder.c | 2 +-
> drivers/gpu/drm/drm_fb_cma_helper.c | 16 +++---
> drivers/gpu/drm/drm_fb_helper.c | 12 ++---
> drivers/gpu/drm/drm_framebuffer.c | 38 ++++++-------
> drivers/gpu/drm/drm_gem.c | 44 +++++++--------
> drivers/gpu/drm/drm_gem_cma_helper.c | 10 ++--
> drivers/gpu/drm/drm_mode_config.c | 4 +-
> drivers/gpu/drm/drm_mode_object.c | 44 +++++++--------
> drivers/gpu/drm/drm_modes.c | 2 +-
> drivers/gpu/drm/drm_plane.c | 14 ++---
> drivers/gpu/drm/drm_prime.c | 10 ++--
> drivers/gpu/drm/drm_property.c | 52 +++++++++---------
> include/drm/drm_connector.h | 41 +++++++++++---
> include/drm/drm_framebuffer.h | 49 ++++++++++++-----
> include/drm/drm_gem.h | 80 +++++++++++++++++++++------
> include/drm/drm_mode_object.h | 36 ++++++++++---
> include/drm/drm_property.h | 35 ++++++++++--
> scripts/coccinelle/api/drm-get-put.cocci | 92 ++++++++++++++++++++++++++++++++
> 25 files changed, 471 insertions(+), 240 deletions(-)
> create mode 100644 scripts/coccinelle/api/drm-get-put.cocci
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/6] drm: Introduce consistent reference counting APIs
2017-02-10 7:29 ` Jani Nikula
@ 2017-02-10 15:58 ` Daniel Vetter
0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-02-10 15:58 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, dri-devel
On Fri, Feb 10, 2017 at 8:29 AM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
> On Thu, 09 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
>> On Thu, Feb 09, 2017 at 08:07:41PM +0100, Daniel Vetter wrote:
>>> On Thu, Feb 09, 2017 at 07:39:33PM +0200, Jani Nikula wrote:
>>> > On Thu, 09 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
>>> > > On Thu, Feb 09, 2017 at 04:10:12PM +0200, Jani Nikula wrote:
>>> > >> On Wed, 08 Feb 2017, Thierry Reding <thierry.reding@gmail.com> wrote:
>>> > >> > This series introduces DRM reference counting APIs that are consistent
>>> > >> > with other reference counting APIs in the kernel. They are also much
>>> > >> > shorter. Compatibility aliases are added to keep existing code working
>>> > >> > and will stay in place until all users of the old APIs are gone.
>>> > >>
>>> > >> I like it.
>>> > >>
>>> > >> But it makes drm_connector_list_iter_{get,put} stick out like a sore
>>> > >> thumb. Something for the TODO list I guess.
>>> > >
>>> > > Hm, why is that one the sore thumb now? Just because it's really long? It
>>> > > does acquire/drop references behind the scenes, that's why I went with the
>>> > > _get/put postfixes ...
>>> >
>>> > I think the assumption is that get/put work on an object, and you can
>>> > get/put as many times as you like, as long as you keep them at
>>> > balance. AFAICT this doesn't hold for iter.
>>>
>>> Hm right. What else should we use instead? start/stop are confusing in the
>>> context of list walking, create/destroy maybe?
>>
>> I rather like the _begin()/_end() suffix that I think iterators have in
>> the C++ STL. Well, I guess it's really .end() and .begin() methods in
>> C++.
>>
>> But the following reads rather nicely:
>>
>> drm_connector_list_iter_begin(dev, &iter);
>>
>> drm_for_each_connector_iter(connector, &iter)
>> ...
>>
>> drm_connector_list_iter_end(&iter);
>>
>> Alternatively some other iterators simply use an _init() suffix to
>> initialize the iterator. The kernel has a few of these, such as the
>> of_phandle_iterator (see include/linux/of.h) or of_pci_range_parser (see
>> include include/linux/of_address.h).
>>
>> EFI code uses _begin()/_end() in include/linux/efi.h, cgroups seems to
>> have _start()/_end() in include/linux/cgroup.h. include/linux/klist.h
>> uses klist_iter_init()/klist_iter_exit(). include/linux/rhashtable.h has
>> rhashtable_walk_start()/rhashtable_walk_stop().
>>
>> A quick grep also shows other variants, one of them being _first(), but
>> those don't have an explicit end function.
>>
>> So there's quite a few options to choose from, but I guess that doesn't
>> make it any easier.
>>
>> If this was a poll, my vote would go to _begin()/_end().
>
> I think begin/end are sensible.
>
> An alternative is to make iter get/put work as get/put do, but I suppose
> there's no real need for them to behave that way, i.e. it's
> overengineering.
>
> And if it wasn't clear, I didn't mean that this should block the patches
> at hand in any way. Someone(tm) can follow up with the iter stuff later
> on. It's not broken, it just feels slightly misleading after these
> changes.
+1 on begin/end for the iters, if someone gets around to it.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}()
2017-02-09 21:55 ` Emil Velikov
@ 2017-02-14 20:12 ` Daniel Vetter
0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2017-02-14 20:12 UTC (permalink / raw)
To: Emil Velikov; +Cc: Daniel Vetter, ML dri-devel
On Thu, Feb 09, 2017 at 09:55:22PM +0000, Emil Velikov wrote:
> On 9 February 2017 at 20:41, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Thu, Feb 09, 2017 at 06:08:10PM +0100, Daniel Vetter wrote:
> >> On Wed, Feb 08, 2017 at 02:28:14PM -0500, Sean Paul wrote:
> >> > On Wed, Feb 08, 2017 at 07:24:04PM +0100, Thierry Reding wrote:
> >> > > From: Thierry Reding <treding@nvidia.com>
> >> > >
> >> > > For consistency with other reference counting APIs in the kernel, add
> >> > > drm_mode_object_get() and drm_mode_object_put() to reference count DRM
> >> > > mode objects.
> >> > >
> >> > > Compatibility aliases are added to keep existing code working. To help
> >> > > speed up the transition, all the instances of the old functions in the
> >> > > DRM core are already replaced in this commit.
> >> > >
> >> >
> >> > drm code looks good and is
> >> >
> >> > Reviewed-by: Sean Paul <seanpaul@chromium.org>
> >> >
> >> > > A semantic patch is provided that can be used to convert all drivers to
> >> > > the new helpers.
> >> >
> >> > I'm not convinced we need to commit the cocci patch. I think including it in
> >> > your cover letter and then following up with a follow on series to actually make
> >> > the change is sufficient (See: ickle's s/fence/dma_fence/ series).
> >>
> >> Yeah, if you do a large-scale refactor anyway, I think it's best to just
> >> store the cocci in the commit message. I think storing the cocci is ok if
> >> you have thousands of hits among lots of subsystems, and it's clear it's
> >> going to take at least a few release cycles or maybe even years to clean
> >> it all up. drm is luckily not yet that big :-)
> >>
> >> I'll drop this while applying if no one minds ...
> >
> > I thought it was actually quite nice that this was part of the series.
> > That way it doesn't get lost and it is really easy to rerun. Also it can
> > trivially be removed once we've converted everyone to the new functions
> > and removed the old ones.
> >
> Hidden bonus:
> Some of the people who run those on semi-regular basis can update
> some/all the drivers ;-)
I agree that this is a nice bonus, but thus far we just mass-converted
everyone. That seems to be the plan here too, which is why I don't see
much value in recording the cocci patch (outside of the commit message).
But drm is growing, and in the future I guess we'll get more refactorings
that can't be done in one go, and then adding the spatch makes perfect
sense.
Anyway, no strong opinions from my side at all, whatever makes sense, just
wanted to explain my reasoning quickly.
-Daniel
--
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
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-02-14 20:13 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-08 18:24 [PATCH 0/6] drm: Introduce consistent reference counting APIs Thierry Reding
2017-02-08 18:24 ` [PATCH 1/6] drm: Rename drm_mode_object_get() Thierry Reding
2017-02-08 19:23 ` Sean Paul
2017-02-08 18:24 ` [PATCH 2/6] drm: Introduce drm_mode_object_{get,put}() Thierry Reding
2017-02-08 19:28 ` Sean Paul
2017-02-09 17:08 ` Daniel Vetter
2017-02-09 20:41 ` Thierry Reding
2017-02-09 21:55 ` Emil Velikov
2017-02-14 20:12 ` Daniel Vetter
2017-02-08 18:24 ` [PATCH 3/6] drm: Introduce drm_connector_{get,put}() Thierry Reding
2017-02-08 19:33 ` Sean Paul
2017-02-08 18:24 ` [PATCH 4/6] drm: Introduce drm_framebuffer_{get,put}() Thierry Reding
2017-02-08 19:36 ` Sean Paul
2017-02-08 18:24 ` [PATCH 5/6] drm: Introduce drm_gem_object_{get,put}() Thierry Reding
2017-02-08 19:41 ` Sean Paul
2017-02-08 18:24 ` [PATCH 6/6] drm: Introduce drm_property_blob_{get,put}() Thierry Reding
2017-02-08 19:45 ` Sean Paul
2017-02-09 14:10 ` [PATCH 0/6] drm: Introduce consistent reference counting APIs Jani Nikula
2017-02-09 17:09 ` Daniel Vetter
2017-02-09 17:39 ` Jani Nikula
2017-02-09 19:07 ` Daniel Vetter
2017-02-09 20:39 ` Thierry Reding
2017-02-10 7:29 ` Jani Nikula
2017-02-10 15:58 ` Daniel Vetter
2017-02-10 14:47 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.