On Mon, Apr 20, 2015 at 07:22:55PM +0100, Daniel Stone wrote:
> Reference-count drm_property_blob objects, changing the API to
> ref/unref.
>
> Signed-off-by: Daniel Stone <daniels@xxxxxxxxxxxxx>
Merged up to this on (except patch 2) to topic/drm-misc.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_crtc.c | 164 +++++++++++++++++++++++++++++++++++++++++----
> include/drm/drm_crtc.h | 17 ++---
> 2 files changed, 159 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 9947078..03245fb 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -352,7 +352,9 @@ static struct drm_mode_object *_object_find(struct drm_device *dev,
> if (obj && obj->id != id)
> obj = NULL;
> /* don't leak out unref'd fb's */
> - if (obj && (obj->type == DRM_MODE_OBJECT_FB))
> + if (obj &&
> + (obj->type == DRM_MODE_OBJECT_FB ||
> + obj->type == DRM_MODE_OBJECT_BLOB))
> obj = NULL;
> mutex_unlock(&dev->mode_config.idr_mutex);
>
> @@ -377,7 +379,7 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
>
> /* Framebuffers are reference counted and need their own lookup
> * function.*/
> - WARN_ON(type == DRM_MODE_OBJECT_FB);
> + WARN_ON(type == DRM_MODE_OBJECT_FB || type == DRM_MODE_OBJECT_BLOB);
> obj = _object_find(dev, id, type);
> return obj;
> }
> @@ -4200,7 +4202,7 @@ done:
> return ret;
> }
>
> -static struct drm_property_blob *
> +struct drm_property_blob *
> drm_property_create_blob(struct drm_device *dev, size_t length,
> const void *data)
> {
> @@ -4215,6 +4217,7 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
> return NULL;
>
> blob->length = length;
> + blob->dev = dev;
>
> memcpy(blob->data, data, length);
>
> @@ -4227,25 +4230,148 @@ drm_property_create_blob(struct drm_device *dev, size_t length,
> return NULL;
> }
>
> + kref_init(&blob->refcount);
> +
> list_add_tail(&blob->head, &dev->mode_config.property_blob_list);
>
> mutex_unlock(&dev->mode_config.blob_lock);
>
> return blob;
> }
> +EXPORT_SYMBOL(drm_property_create_blob);
>
> -static void drm_property_destroy_blob(struct drm_device *dev,
> - struct drm_property_blob *blob)
> +/**
> + * drm_property_free_blob - Blob property destructor
> + *
> + * Internal free function for blob properties; must not be used directly.
> + *
> + * @param kref Reference
> + */
> +static void drm_property_free_blob(struct kref *kref)
> {
> - mutex_lock(&dev->mode_config.blob_lock);
> - drm_mode_object_put(dev, &blob->base);
> + struct drm_property_blob *blob =
> + container_of(kref, struct drm_property_blob, refcount);
> +
> + WARN_ON(!mutex_is_locked(&blob->dev->mode_config.blob_lock));
> +
> list_del(&blob->head);
> - mutex_unlock(&dev->mode_config.blob_lock);
> + drm_mode_object_put(blob->dev, &blob->base);
>
> kfree(blob);
> }
>
> /**
> + * drm_property_unreference_blob - Unreference a blob property
> + *
> + * Drop a reference on a blob property. May free the object.
> + *
> + * @param dev Device the blob was created on
> + * @param blob Pointer to blob property
> + */
> +void drm_property_unreference_blob(struct drm_property_blob *blob)
> +{
> + struct drm_device *dev;
> +
> + if (!blob)
> + return;
> +
> + dev = blob->dev;
> +
> + DRM_DEBUG("%p: blob ID: %d (%d)\n", blob, blob->base.id, atomic_read(&blob->refcount.refcount));
> +
> + if (kref_put_mutex(&blob->refcount, drm_property_free_blob,
> + &dev->mode_config.blob_lock))
> + mutex_unlock(&blob->dev->mode_config.blob_lock);
> + else
> + might_lock(&dev->mode_config.blob_lock);
> +
> +}
> +EXPORT_SYMBOL(drm_property_unreference_blob);
> +
> +/**
> + * drm_property_unreference_blob_locked - Unreference a blob property with blob_lock held
> + *
> + * Drop a reference on a blob property. May free the object. This must be
> + * called with blob_lock held.
> + *
> + * @param dev Device the blob was created on
> + * @param blob Pointer to blob property
> + */
> +static void drm_property_unreference_blob_locked(struct drm_property_blob *blob)
> +{
> + if (!blob)
> + return;
> +
> + DRM_DEBUG("%p: blob ID: %d (%d)\n", blob, blob->base.id, atomic_read(&blob->refcount.refcount));
> +
> + kref_put(&blob->refcount, drm_property_free_blob);
> +}
> +
> +/**
> + * drm_property_reference_blob - Take a reference on an existing property
> + *
> + * Take a new reference on an existing blob property.
> + *
> + * @param blob Pointer to blob property
> + */
> +struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob)
> +{
> + DRM_DEBUG("%p: blob ID: %d (%d)\n", blob, blob->base.id, atomic_read(&blob->refcount.refcount));
> + kref_get(&blob->refcount);
> + return blob;
> +}
> +EXPORT_SYMBOL(drm_property_reference_blob);
> +
> +/*
> + * Like drm_property_lookup_blob, but does not return an additional reference.
> + * Must be called with blob_lock held.
> + */
> +static struct drm_property_blob *__drm_property_lookup_blob(struct drm_device *dev,
> + uint32_t id)
> +{
> + struct drm_mode_object *obj = NULL;
> + struct drm_property_blob *blob;
> +
> + WARN_ON(!mutex_is_locked(&dev->mode_config.blob_lock));
> +
> + mutex_lock(&dev->mode_config.idr_mutex);
> + obj = idr_find(&dev->mode_config.crtc_idr, id);
> + if (!obj || (obj->type != DRM_MODE_OBJECT_BLOB) || (obj->id != id))
> + blob = NULL;
> + else
> + blob = obj_to_blob(obj);
> + mutex_unlock(&dev->mode_config.idr_mutex);
> +
> + return blob;
> +}
> +
> +/**
> + * drm_property_lookup_blob - look up a blob property and take a reference
> + * @dev: drm device
> + * @id: id of the blob property
> + *
> + * 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.
> + */
> +struct drm_property_blob *drm_property_lookup_blob(struct drm_device *dev,
> + uint32_t id)
> +{
> + struct drm_property_blob *blob;
> +
> + mutex_lock(&dev->mode_config.blob_lock);
> + blob = __drm_property_lookup_blob(dev, id);
> + if (blob) {
> + if (!kref_get_unless_zero(&blob->refcount))
> + blob = NULL;
> + }
> + mutex_unlock(&dev->mode_config.blob_lock);
> +
> + return blob;
> +}
> +EXPORT_SYMBOL(drm_property_lookup_blob);
> +
> +/**
> * drm_property_replace_global_blob - atomically replace existing blob property
> * @dev: drm device
> * @replace: location of blob property pointer to be replaced
> @@ -4311,14 +4437,14 @@ static int drm_property_replace_global_blob(struct drm_device *dev,
> }
>
> if (old_blob)
> - drm_property_destroy_blob(dev, old_blob);
> + drm_property_unreference_blob(old_blob);
>
> *replace = new_blob;
>
> return 0;
>
> err_created:
> - drm_property_destroy_blob(dev, new_blob);
> + drm_property_unreference_blob(new_blob);
> return ret;
> }
>
> @@ -4349,7 +4475,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
>
> drm_modeset_lock_all(dev);
> mutex_lock(&dev->mode_config.blob_lock);
> - blob = drm_property_blob_find(dev, out_resp->blob_id);
> + blob = __drm_property_lookup_blob(dev, out_resp->blob_id);
> if (!blob) {
> ret = -ENOENT;
> goto done;
> @@ -4513,8 +4639,18 @@ bool drm_property_change_valid_get(struct drm_property *property,
> valid_mask |= (1ULL << property->values[i]);
> return !(value & ~valid_mask);
> } else if (drm_property_type_is(property, DRM_MODE_PROP_BLOB)) {
> - /* Only the driver knows */
> - return true;
> + struct drm_property_blob *blob;
> +
> + if (value == 0)
> + return true;
> +
> + blob = drm_property_lookup_blob(property->dev, value);
> + if (blob) {
> + *ref = &blob->base;
> + return true;
> + } else {
> + return false;
> + }
> } else if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
> /* a zero value for an object property translates to null: */
> if (value == 0)
> @@ -5564,7 +5700,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>
> list_for_each_entry_safe(blob, bt, &dev->mode_config.property_blob_list,
> head) {
> - drm_property_destroy_blob(dev, blob);
> + drm_property_unreference_blob(blob);
> }
>
> /*
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 43a3758..07c99f6 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -217,6 +217,8 @@ struct drm_framebuffer {
>
> struct drm_property_blob {
> struct drm_mode_object base;
> + struct drm_device *dev;
> + struct kref refcount;
> struct list_head head;
> size_t length;
> unsigned char data[];
> @@ -1366,6 +1368,13 @@ struct drm_property *drm_property_create_object(struct drm_device *dev,
> int flags, const char *name, uint32_t type);
> struct drm_property *drm_property_create_bool(struct drm_device *dev, int flags,
> const char *name);
> +struct drm_property_blob *drm_property_create_blob(struct drm_device *dev,
> + size_t length,
> + const void *data);
> +struct drm_property_blob *drm_property_lookup_blob(struct drm_device *dev,
> + uint32_t id);
> +struct drm_property_blob *drm_property_reference_blob(struct drm_property_blob *blob);
> +void drm_property_unreference_blob(struct drm_property_blob *blob);
> extern void drm_property_destroy(struct drm_device *dev, struct drm_property *property);
> extern int drm_property_add_enum(struct drm_property *property, int index,
> uint64_t value, const char *name);
> @@ -1529,14 +1538,6 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
> return mo ? obj_to_property(mo) : NULL;
> }
>
> -static inline struct drm_property_blob *
> -drm_property_blob_find(struct drm_device *dev, uint32_t id)
> -{
> - struct drm_mode_object *mo;
> - mo = drm_mode_object_find(dev, id, DRM_MODE_OBJECT_BLOB);
> - return mo ? obj_to_blob(mo) : NULL;
> -}
> -
> /* Plane list iterator for legacy (overlay only) planes. */
> #define drm_for_each_legacy_plane(plane, planelist) \
> list_for_each_entry(plane, planelist, head) \
> --
> 2.3.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
