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