drm: revamp framebuffer cleanup interfaces
We have two classes of framebuffer
- Created by the driver (atm only for fbdev), and the driver holds
onto the last reference count until destruction.
- Created by userspace and associated with a given fd. These
framebuffers will be reaped when their assoiciated fb is closed.
Now these two cases are set up differently, the framebuffers are on
different lists and hence destruction needs to clean up different
things. Also, for userspace framebuffers we remove them from any
current usage, whereas for internal framebuffers it is assumed that
the driver has done this already.
Long story short, we need two different ways to cleanup such drivers.
Three functions are involved in total:
- drm_framebuffer_remove: Convenience function which removes the fb
from all active usage and then drops the passed-in reference.
- drm_framebuffer_unregister_private: Will remove driver-private
framebuffers from relevant lists and drop the corresponding
references. Should be called for driver-private framebuffers before
dropping the last reference (or like for a lot of the drivers where
the fbdev is embedded someplace else, before doing the cleanup
manually).
- drm_framebuffer_cleanup: Final cleanup for both classes of fbs,
should be called by the driver's ->destroy callback once the last
reference is gone.
This patch just rolls out the new interfaces and updates all drivers
(by adding calls to drm_framebuffer_unregister_private at all the
right places)- no functional changes yet. Follow-on patches will move
drm core code around and update the lifetime management for
framebuffers, so that we are no longer required to keep framebuffers
alive by locking mode_config.mutex.
I've also updated the kerneldoc already.
vmwgfx seems to again be a bit special, at least I haven't figured out
how the fbdev support in that driver works. It smells like it's
external though.
v2: The i915 driver creates another private framebuffer in the
load-detect code. Adjust its cleanup code, too.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index d9ec779..3e6584b 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -290,6 +290,7 @@
drm_fb_helper_fini(&afbdev->helper);
vfree(afbdev->sysram);
+ drm_framebuffer_unregister_private(&afb->base);
drm_framebuffer_cleanup(&afb->base);
}
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 6c6b4c8..3daea0f 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -258,6 +258,7 @@
vfree(gfbdev->sysram);
drm_fb_helper_fini(&gfbdev->helper);
+ drm_framebuffer_unregister_private(&gfb->base);
drm_framebuffer_cleanup(&gfb->base);
return 0;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f2ccda8..3eddfab 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -68,6 +68,7 @@
mutex_unlock(&dev->mode_config.mutex);
}
+
EXPORT_SYMBOL(drm_modeset_unlock_all);
/* Avoid boilerplate. I'm tired of typing. */
@@ -430,11 +431,34 @@
EXPORT_SYMBOL(drm_framebuffer_reference);
/**
+ * drm_framebuffer_unregister_private - unregister a private fb from the lookup idr
+ * @fb: fb to unregister
+ *
+ * Drivers need to call this when cleaning up driver-private framebuffers, e.g.
+ * those used for fbdev. Note that the caller must hold a reference of it's own,
+ * i.e. the object may not be destroyed through this call (since it'll lead to a
+ * locking inversion).
+ */
+void drm_framebuffer_unregister_private(struct drm_framebuffer *fb)
+{
+}
+EXPORT_SYMBOL(drm_framebuffer_unregister_private);
+
+/**
* drm_framebuffer_cleanup - remove a framebuffer object
* @fb: framebuffer to remove
*
- * Scans all the CRTCs in @dev's mode_config. If they're using @fb, removes
- * it, setting it to NULL.
+ * Cleanup references to a user-created framebuffer. This function is intended
+ * to be used from the drivers ->destroy callback.
+ *
+ * Note that this function does not remove the fb from active usuage - if it is
+ * still used anywhere, hilarity can ensue since userspace could call getfb on
+ * the id and get back -EINVAL. Obviously no concern at driver unload time.
+ *
+ * Also, the framebuffer will not be removed from the lookup idr - for
+ * user-created framebuffers this will happen in in the rmfb ioctl. For
+ * driver-private objects (e.g. for fbdev) drivers need to explicitly call
+ * drm_framebuffer_unregister_private.
*/
void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
{
@@ -460,7 +484,8 @@
* @fb: framebuffer to remove
*
* Scans all the CRTCs and planes in @dev's mode_config. If they're
- * using @fb, removes it, setting it to NULL.
+ * using @fb, removes it, setting it to NULL. Then drops the reference to the
+ * passed-in framebuffer.
*/
void drm_framebuffer_remove(struct drm_framebuffer *fb)
{
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index e1e0cb0..3742bc9 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -266,6 +266,7 @@
return 0;
err_drm_fb_cma_destroy:
+ drm_framebuffer_unregister_private(fb);
drm_fb_cma_destroy(fb);
err_framebuffer_release:
framebuffer_release(fbi);
@@ -370,8 +371,10 @@
framebuffer_release(info);
}
- if (fbdev_cma->fb)
+ if (fbdev_cma->fb) {
+ drm_framebuffer_unregister_private(&fbdev_cma->fb->fb);
drm_fb_cma_destroy(&fbdev_cma->fb->fb);
+ }
drm_fb_helper_fini(&fbdev_cma->fb_helper);
kfree(fbdev_cma);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 71f8673..90d335c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -326,8 +326,10 @@
/* release drm framebuffer and real buffer */
if (fb_helper->fb && fb_helper->fb->funcs) {
fb = fb_helper->fb;
- if (fb)
+ if (fb) {
+ drm_framebuffer_unregister_private(fb);
drm_framebuffer_remove(fb);
+ }
}
/* release linux framebuffer */
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 49800d2..c1ef37e 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -590,6 +590,7 @@
framebuffer_release(info);
}
drm_fb_helper_fini(&fbdev->psb_fb_helper);
+ drm_framebuffer_unregister_private(&psbfb->base);
drm_framebuffer_cleanup(&psbfb->base);
if (psbfb->gtt)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 26fa6a7..df51203 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6789,8 +6789,10 @@
intel_encoder->new_crtc = NULL;
intel_set_mode(crtc, NULL, 0, 0, NULL);
- if (old->release_fb)
- old->release_fb->funcs->destroy(old->release_fb);
+ if (old->release_fb) {
+ drm_framebuffer_unregister_private(old->release_fb);
+ drm_framebuffer_unreference(old->release_fb);
+ }
return;
}
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index a9f7280..c7b8316 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -212,6 +212,7 @@
drm_fb_helper_fini(&ifbdev->helper);
+ drm_framebuffer_unregister_private(&ifb->base);
drm_framebuffer_cleanup(&ifb->base);
if (ifb->obj) {
drm_gem_object_unreference_unlocked(&ifb->obj->base);
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 2f48648..5c69b43 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -247,6 +247,7 @@
}
drm_fb_helper_fini(&mfbdev->helper);
vfree(mfbdev->sysram);
+ drm_framebuffer_unregister_private(&mfb->base);
drm_framebuffer_cleanup(&mfb->base);
return 0;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 67a1a06..d4ecb4d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -433,6 +433,7 @@
nouveau_fb->nvbo = NULL;
}
drm_fb_helper_fini(&fbcon->helper);
+ drm_framebuffer_unregister_private(&nouveau_fb->base);
drm_framebuffer_cleanup(&nouveau_fb->base);
return 0;
}
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index cc8489d..515e5ee 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -293,6 +293,7 @@
}
if (fb && ret) {
drm_gem_object_unreference(gobj);
+ drm_framebuffer_unregister_private(fb);
drm_framebuffer_cleanup(fb);
kfree(fb);
}
@@ -339,6 +340,7 @@
rfb->obj = NULL;
}
drm_fb_helper_fini(&rfbdev->helper);
+ drm_framebuffer_unregister_private(&rfb->base);
drm_framebuffer_cleanup(&rfb->base);
return 0;
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index f8904b4..caa84f1 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -555,6 +555,7 @@
framebuffer_release(info);
}
drm_fb_helper_fini(&ufbdev->helper);
+ drm_framebuffer_unregister_private(&ufbdev->ufb.base);
drm_framebuffer_cleanup(&ufbdev->ufb.base);
drm_gem_object_unreference_unlocked(&ufbdev->ufb.obj->base);
}
diff --git a/drivers/staging/omapdrm/omap_fbdev.c b/drivers/staging/omapdrm/omap_fbdev.c
index 8a027bb..2728e37 100644
--- a/drivers/staging/omapdrm/omap_fbdev.c
+++ b/drivers/staging/omapdrm/omap_fbdev.c
@@ -275,8 +275,10 @@
if (ret) {
if (fbi)
framebuffer_release(fbi);
- if (fb)
+ if (fb) {
+ drm_framebuffer_unregister_private(fb);
drm_framebuffer_remove(fb);
+ }
}
return ret;
@@ -400,8 +402,10 @@
fbdev = to_omap_fbdev(priv->fbdev);
/* this will free the backing object */
- if (fbdev->fb)
+ if (fbdev->fb) {
+ drm_framebuffer_unregister_private(fbdev->fb);
drm_framebuffer_remove(fbdev->fb);
+ }
kfree(fbdev);