On Wed, Feb 08, 2017 at 07:24:06PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
