Re: [PATCH 05/15] drm/mode: move framebuffer reference into object.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 15, 2016 at 03:10:36PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@xxxxxxxxxx>
> 
> This is the initial code to add references to some mode objects.
> In the future we need to start reference counting connectors so
> firstly I want to reorganise the code so the framebuffer ref counting
> uses the same paths.
> 
> This patch shouldn't change any functionality, just moves the kref.
> 
> Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_crtc.c | 72 ++++++++++++++++++++++++----------------------
>  include/drm/drm_crtc.h     | 20 ++++++++++---
>  2 files changed, 54 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 8616737..75a45e9 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -275,7 +275,8 @@ EXPORT_SYMBOL(drm_get_format_name);
>  static int drm_mode_object_get_reg(struct drm_device *dev,
>  				   struct drm_mode_object *obj,
>  				   uint32_t obj_type,
> -				   bool register_obj)
> +				   bool register_obj,
> +				   void (*obj_free_cb)(struct kref *kref))
>  {
>  	int ret;
>  
> @@ -288,6 +289,10 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
>  		 */
>  		obj->id = ret;
>  		obj->type = obj_type;
> +		if (obj_free_cb) {
> +			obj->free_cb = obj_free_cb;
> +			kref_init(&obj->refcount);
> +		}
>  	}
>  	mutex_unlock(&dev->mode_config.idr_mutex);
>  
> @@ -311,7 +316,7 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
>  int drm_mode_object_get(struct drm_device *dev,
>  			struct drm_mode_object *obj, uint32_t obj_type)
>  {
> -	return drm_mode_object_get_reg(dev, obj, obj_type, true);
> +	return drm_mode_object_get_reg(dev, obj, obj_type, true, NULL);
>  }
>  
>  static void drm_mode_object_register(struct drm_device *dev,
> @@ -389,10 +394,35 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_mode_object_find);
>  

Kerneldoc for this one would be nice.

> +void drm_mode_object_unreference(struct drm_mode_object *obj)
> +{
> +	if (obj->free_cb) {
> +		DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, atomic_read(&obj->refcount.refcount));
> +		kref_put(&obj->refcount, obj->free_cb);
> +	}
> +}
> +EXPORT_SYMBOL(drm_mode_object_unreference);
> +
> +/**
> + * drm_mode_object_reference - incr the fb refcnt
> + * @obj: mode_object
> + *
> + * This function operates only on refcounted objects.
> + * This functions increments the object's refcount.
> + */
> +void drm_mode_object_reference(struct drm_mode_object *obj)
> +{
> +	if (obj->free_cb) {
> +		DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, atomic_read(&obj->refcount.refcount));
> +		kref_get(&obj->refcount);
> +	}
> +}
> +EXPORT_SYMBOL(drm_mode_object_reference);
> +
>  static void drm_framebuffer_free(struct kref *kref)
>  {
>  	struct drm_framebuffer *fb =
> -			container_of(kref, struct drm_framebuffer, refcount);
> +			container_of(kref, struct drm_framebuffer, base.refcount);
>  	struct drm_device *dev = fb->dev;
>  
>  	/*
> @@ -430,12 +460,12 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
>  	int ret;
>  
>  	mutex_lock(&dev->mode_config.fb_lock);
> -	kref_init(&fb->refcount);
>  	INIT_LIST_HEAD(&fb->filp_head);
>  	fb->dev = dev;
>  	fb->funcs = funcs;
>  
> -	ret = drm_mode_object_get(dev, &fb->base, DRM_MODE_OBJECT_FB);
> +	ret = drm_mode_object_get_reg(dev, &fb->base, DRM_MODE_OBJECT_FB,
> +				      true, drm_framebuffer_free);
>  	if (ret)
>  		goto out;
>  
> @@ -482,7 +512,7 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
>  	mutex_lock(&dev->mode_config.fb_lock);
>  	fb = __drm_framebuffer_lookup(dev, id);
>  	if (fb) {
> -		if (!kref_get_unless_zero(&fb->refcount))
> +		if (!kref_get_unless_zero(&fb->base.refcount))
>  			fb = NULL;
>  	}
>  	mutex_unlock(&dev->mode_config.fb_lock);
> @@ -492,32 +522,6 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
>  EXPORT_SYMBOL(drm_framebuffer_lookup);
>  
>  /**
> - * drm_framebuffer_unreference - unref a framebuffer
> - * @fb: framebuffer to unref
> - *
> - * This functions decrements the fb's refcount and frees it if it drops to zero.
> - */
> -void drm_framebuffer_unreference(struct drm_framebuffer *fb)
> -{
> -	DRM_DEBUG("%p: FB ID: %d (%d)\n", fb, fb->base.id, atomic_read(&fb->refcount.refcount));
> -	kref_put(&fb->refcount, drm_framebuffer_free);
> -}
> -EXPORT_SYMBOL(drm_framebuffer_unreference);
> -
> -/**
> - * drm_framebuffer_reference - incr the fb refcnt
> - * @fb: framebuffer
> - *
> - * This functions increments the fb's refcount.
> - */
> -void drm_framebuffer_reference(struct drm_framebuffer *fb)
> -{
> -	DRM_DEBUG("%p: FB ID: %d (%d)\n", fb, fb->base.id, atomic_read(&fb->refcount.refcount));
> -	kref_get(&fb->refcount);
> -}
> -EXPORT_SYMBOL(drm_framebuffer_reference);
> -
> -/**
>   * drm_framebuffer_unregister_private - unregister a private fb from the lookup idr
>   * @fb: fb to unregister
>   *
> @@ -902,7 +906,7 @@ int drm_connector_init(struct drm_device *dev,
>  
>  	drm_modeset_lock_all(dev);
>  
> -	ret = drm_mode_object_get_reg(dev, &connector->base, DRM_MODE_OBJECT_CONNECTOR, false);
> +	ret = drm_mode_object_get_reg(dev, &connector->base, DRM_MODE_OBJECT_CONNECTOR, false, NULL);
>  	if (ret)
>  		goto out_unlock;
>  
> @@ -5922,7 +5926,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>  	 */
>  	WARN_ON(!list_empty(&dev->mode_config.fb_list));
>  	list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
> -		drm_framebuffer_free(&fb->refcount);
> +		drm_framebuffer_free(&fb->base.refcount);
>  	}
>  
>  	list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 99a12f0..576faf4 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -59,6 +59,8 @@ struct drm_mode_object {
>  	uint32_t id;
>  	uint32_t type;
>  	struct drm_object_properties *properties;
> +	struct kref refcount;
> +	void (*free_cb)(struct kref *kref);
>  };
>  
>  #define DRM_OBJECT_MAX_PROPERTY 24
> @@ -233,8 +235,8 @@ struct drm_framebuffer {
>  	 * 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.

Bikeshed: Maybe new paragraph here for better formatting with kerneldoc.

> +	 * The refcount is stored inside the mode object.
>  	 */
> -	struct kref refcount;
>  	/*
>  	 * Place on the dev->mode_config.fb_list, access protected by
>  	 * dev->mode_config.fb_lock.
> @@ -2386,8 +2388,6 @@ extern int drm_framebuffer_init(struct drm_device *dev,
>  				const struct drm_framebuffer_funcs *funcs);
>  extern struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
>  						      uint32_t id);
> -extern void drm_framebuffer_unreference(struct drm_framebuffer *fb);
> -extern void drm_framebuffer_reference(struct drm_framebuffer *fb);
>  extern void drm_framebuffer_remove(struct drm_framebuffer *fb);
>  extern void drm_framebuffer_cleanup(struct drm_framebuffer *fb);
>  extern void drm_framebuffer_unregister_private(struct drm_framebuffer *fb);
> @@ -2445,6 +2445,8 @@ extern int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
>  					 int gamma_size);
>  extern 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);
>  
>  /* IOCTLs */
>  extern int drm_mode_getresources(struct drm_device *dev,
> @@ -2608,9 +2610,19 @@ static inline uint32_t drm_color_lut_extract(uint32_t user_input,
>  	return clamp_val(val, 0, max);
>  }
>  
> +static inline void drm_framebuffer_reference(struct drm_framebuffer *fb)
> +{
> +	drm_mode_object_reference(&fb->base);
> +}
> +
> +static inline void drm_framebuffer_unreference(struct drm_framebuffer *fb)
> +{
> +	drm_mode_object_unreference(&fb->base);
> +}

You lost the kerneldoc for the above two.

With the kerneldoc commments above addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>


> +
>  static inline uint32_t drm_framebuffer_read_refcount(struct drm_framebuffer *fb)
>  {
> -	return atomic_read(&fb->refcount.refcount);
> +	return atomic_read(&fb->base.refcount.refcount);
>  }
>  
>  /* Plane list iterator for legacy (overlay only) planes. */
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux