[PATCH] drm: refcnt drm_framebuffer
Rob Clark
rob.clark at linaro.org
Wed Sep 5 11:17:52 PDT 2012
From: Rob Clark <rob at ti.com>
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
Signed-off-by: Rob Clark <rob at ti.com>
Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
drivers/gpu/drm/drm_crtc.c | 78 +++++++++++++++++++++++++----
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 +-
drivers/gpu/drm/i915/intel_display.c | 4 +-
drivers/staging/omapdrm/omap_fbdev.c | 4 +-
include/drm/drm_crtc.h | 14 ++++++
5 files changed, 87 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 901de9a..96e236f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -294,6 +294,8 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
{
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 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
}
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 @@ EXPORT_SYMBOL(drm_framebuffer_init);
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 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
}
}
- 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
@@ -1032,7 +1092,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
}
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(crtc, ct, &dev->mode_config.crtc_list, head) {
@@ -2343,11 +2403,8 @@ int drm_mode_rmfb(struct drm_device *dev,
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);
@@ -2497,8 +2554,7 @@ void drm_fb_release(struct drm_file *priv)
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);
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index d5586cc..f4ac433 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -266,8 +266,8 @@ static void exynos_drm_fbdev_destroy(struct drm_device *dev,
/* release drm framebuffer and real buffer */
if (fb_helper->fb && fb_helper->fb->funcs) {
fb = fb_helper->fb;
- if (fb && fb->funcs->destroy)
- fb->funcs->destroy(fb);
+ if (fb)
+ drm_framebuffer_remove(fb);
}
/* release linux framebuffer */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a69a3d0..2109c9f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5687,7 +5687,7 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder,
if (!drm_crtc_helper_set_mode(crtc, mode, 0, 0, old_fb)) {
DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
if (old->release_fb)
- old->release_fb->funcs->destroy(old->release_fb);
+ drm_framebuffer_remove(old->release_fb);
crtc->fb = old_fb;
return false;
}
@@ -5717,7 +5717,7 @@ void intel_release_load_detect_pipe(struct intel_encoder *intel_encoder,
drm_helper_disable_unused_functions(dev);
if (old->release_fb)
- old->release_fb->funcs->destroy(old->release_fb);
+ drm_framebuffer_remove(old->release_fb);
return;
}
diff --git a/drivers/staging/omapdrm/omap_fbdev.c b/drivers/staging/omapdrm/omap_fbdev.c
index 8c6ed3b..8a027bb 100644
--- a/drivers/staging/omapdrm/omap_fbdev.c
+++ b/drivers/staging/omapdrm/omap_fbdev.c
@@ -276,7 +276,7 @@ fail:
if (fbi)
framebuffer_release(fbi);
if (fb)
- fb->funcs->destroy(fb);
+ drm_framebuffer_remove(fb);
}
return ret;
@@ -401,7 +401,7 @@ void omap_fbdev_free(struct drm_device *dev)
/* this will free the backing object */
if (fbdev->fb)
- fbdev->fb->funcs->destroy(fbdev->fb);
+ drm_framebuffer_remove(fbdev->fb);
kfree(fbdev);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a82e0a2..1422b36 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -219,6 +219,7 @@ struct drm_display_info {
};
struct drm_framebuffer_funcs {
+ /* note: use drm_framebuffer_remove() */
void (*destroy)(struct drm_framebuffer *framebuffer);
int (*create_handle)(struct drm_framebuffer *fb,
struct drm_file *file_priv,
@@ -243,6 +244,16 @@ struct drm_framebuffer_funcs {
struct drm_framebuffer {
struct drm_device *dev;
+ /*
+ * Note that the fb is refcounted for the benefit of driver internals,
+ * for example some hw, disabling a CRTC/plane is asynchronous, and
+ * scanout does not actually complete until the next vblank. So some
+ * 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.
+ */
+ struct kref refcount;
struct list_head head;
struct drm_mode_object base;
const struct drm_framebuffer_funcs *funcs;
@@ -921,6 +932,9 @@ extern int drm_object_property_get_value(struct drm_mode_object *obj,
extern int drm_framebuffer_init(struct drm_device *dev,
struct drm_framebuffer *fb,
const struct drm_framebuffer_funcs *funcs);
+void drm_framebuffer_unreference(struct drm_framebuffer *fb);
+void drm_framebuffer_reference(struct drm_framebuffer *fb);
+void drm_framebuffer_remove(struct drm_framebuffer *fb);
extern void drm_framebuffer_cleanup(struct drm_framebuffer *fb);
extern void drm_connector_attach_property(struct drm_connector *connector,
--
1.7.9.5
More information about the dri-devel
mailing list