drm: refcnt drm_framebuffer (v4.1)
This simplifies drm fb lifetime, and if the crtc/plane needs to hold
a ref to the fb when disabling a pipe until the next vblank, this
avoids the need to make disabling an overlay synchronous. This is a
problem that shows up when userspace is using a drm plane to
implement a hw cursor.. making overlay disable synchronous causes
a performance problem when x11 is rapidly enabling/disabling the
hw cursor. But not making it synchronous opens up a race condition
for crashing if userspace turns around and immediately deletes the
fb. Refcnt'ing the fb makes it possible to solve this problem.
v1: original
v2: add drm_framebuffer_remove() which is called in all paths where
fb->funcs->destroy() was directly called before. This cleans
up the CRTCs/planes that the fb was attached to. You should
only directly use drm_framebuffer_unreference() if you are also
using drm_framebuffer_reference() to keep a ref to the fb.
v3: add comment explaining the fb refcount
v4: remove duplicate 'list_del(&fb->filp_head)'
[airlied: v4.1: fix local rejection]
Signed-off-by: Rob Clark <rob@ti.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index c418c77..39afe13 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -294,6 +294,8 @@
{
int ret;
+ kref_init(&fb->refcount);
+
ret = drm_mode_object_get(dev, &fb->base, DRM_MODE_OBJECT_FB);
if (ret)
return ret;
@@ -307,6 +309,38 @@
}
EXPORT_SYMBOL(drm_framebuffer_init);
+static void drm_framebuffer_free(struct kref *kref)
+{
+ struct drm_framebuffer *fb =
+ container_of(kref, struct drm_framebuffer, refcount);
+ fb->funcs->destroy(fb);
+}
+
+/**
+ * drm_framebuffer_unreference - unref a framebuffer
+ *
+ * LOCKING:
+ * Caller must hold mode config lock.
+ */
+void drm_framebuffer_unreference(struct drm_framebuffer *fb)
+{
+ struct drm_device *dev = fb->dev;
+ DRM_DEBUG("FB ID: %d\n", fb->base.id);
+ WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
+ kref_put(&fb->refcount, drm_framebuffer_free);
+}
+EXPORT_SYMBOL(drm_framebuffer_unreference);
+
+/**
+ * drm_framebuffer_reference - incr the fb refcnt
+ */
+void drm_framebuffer_reference(struct drm_framebuffer *fb)
+{
+ DRM_DEBUG("FB ID: %d\n", fb->base.id);
+ kref_get(&fb->refcount);
+}
+EXPORT_SYMBOL(drm_framebuffer_reference);
+
/**
* drm_framebuffer_cleanup - remove a framebuffer object
* @fb: framebuffer to remove
@@ -320,6 +354,32 @@
void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
{
struct drm_device *dev = fb->dev;
+ /*
+ * This could be moved to drm_framebuffer_remove(), but for
+ * debugging is nice to keep around the list of fb's that are
+ * no longer associated w/ a drm_file but are not unreferenced
+ * yet. (i915 and omapdrm have debugfs files which will show
+ * this.)
+ */
+ drm_mode_object_put(dev, &fb->base);
+ list_del(&fb->head);
+ dev->mode_config.num_fb--;
+}
+EXPORT_SYMBOL(drm_framebuffer_cleanup);
+
+/**
+ * drm_framebuffer_remove - remove and unreference a framebuffer object
+ * @fb: framebuffer to remove
+ *
+ * LOCKING:
+ * Caller must hold mode config lock.
+ *
+ * Scans all the CRTCs and planes in @dev's mode_config. If they're
+ * using @fb, removes it, setting it to NULL.
+ */
+void drm_framebuffer_remove(struct drm_framebuffer *fb)
+{
+ struct drm_device *dev = fb->dev;
struct drm_crtc *crtc;
struct drm_plane *plane;
struct drm_mode_set set;
@@ -350,11 +410,11 @@
}
}
- drm_mode_object_put(dev, &fb->base);
- list_del(&fb->head);
- dev->mode_config.num_fb--;
+ list_del(&fb->filp_head);
+
+ drm_framebuffer_unreference(fb);
}
-EXPORT_SYMBOL(drm_framebuffer_cleanup);
+EXPORT_SYMBOL(drm_framebuffer_remove);
/**
* drm_crtc_init - Initialise a new CRTC object
@@ -1031,7 +1091,7 @@
}
list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
- fb->funcs->destroy(fb);
+ drm_framebuffer_remove(fb);
}
list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
@@ -2337,11 +2397,7 @@
goto out;
}
- /* TODO release all crtc connected to the framebuffer */
- /* TODO unhock the destructor from the buffer object */
-
- list_del(&fb->filp_head);
- fb->funcs->destroy(fb);
+ drm_framebuffer_remove(fb);
out:
mutex_unlock(&dev->mode_config.mutex);
@@ -2491,8 +2547,7 @@
mutex_lock(&dev->mode_config.mutex);
list_for_each_entry_safe(fb, tfb, &priv->fbs, filp_head) {
- list_del(&fb->filp_head);
- fb->funcs->destroy(fb);
+ drm_framebuffer_remove(fb);
}
mutex_unlock(&dev->mode_config.mutex);
}