[RFC PATCH 28/37] drm: mode: Add kref to modes

Daniel Stone daniels at collabora.com
Wed Mar 18 21:33:27 PDT 2015


In order to expose modes to users, we need to be able to reason about
their lifetimes. As we currently treat modes as just a bucket of bits to
be shovelled around, we can't do that: reference counting them enables
us to sensibly deal with their lifetimes, to provide useful guarantees
to userspace.

Signed-off-by: Daniel Stone <daniels at collabora.com>
---
 drivers/gpu/drm/drm_crtc.c        |  2 +-
 drivers/gpu/drm/drm_crtc_helper.c |  3 ++-
 drivers/gpu/drm/drm_modes.c       | 40 ++++++++++++++++++++++++++++++++-------
 drivers/gpu/drm/tegra/dc.c        |  2 +-
 include/drm/drm_modes.h           |  6 ++++++
 5 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e50eda2..90bf355 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -701,7 +701,7 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 
-	kfree(crtc->mode);
+	drm_mode_destroy(crtc->dev, crtc->mode);
 	crtc->mode = NULL;
 
 	kfree(crtc->gamma_store);
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 46497dd..c23f31f 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -990,7 +990,8 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc,
 	if (crtc_funcs->atomic_check) {
 		ret = crtc_funcs->atomic_check(crtc, crtc_state);
 		if (ret) {
-			kfree(crtc_state->mode);
+			/* XXX: crtc_state_destroy?! */
+			drm_mode_destroy(crtc->dev, crtc_state->mode);
 			kfree(crtc_state);
 
 			return ret;
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 0883f64..e87f547 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -82,25 +82,44 @@ struct drm_display_mode *drm_mode_create(struct drm_device *dev)
 		return NULL;
 	}
 
+	kref_init(&nmode->refcount);
+	nmode->dev = dev;
+
 	return nmode;
 }
 EXPORT_SYMBOL(drm_mode_create);
 
 /**
- * drm_mode_destroy - remove a mode
+ * drm_mode_free - free a mode
+ * @kref: Reference count inside mode
+ *
+ * Release the unique ID of the mode referred to by @kref, then free the
+ * structure itself using kfree.
+ */
+static void drm_mode_free(struct kref *kref)
+{
+	struct drm_display_mode *mode =
+		container_of(kref, struct drm_display_mode, refcount);
+
+	drm_mode_object_put(mode->dev, &mode->base);
+
+	kfree(mode);
+}
+
+/**
+ * drm_mode_destroy - drop reference count on mode
  * @dev: DRM device
  * @mode: mode to remove
  *
- * Release @mode's unique ID, then free it @mode structure itself using kfree.
+ * Drop a reference on a drm_mode. Does not actually free the mode, unless
+ * the reference count drops to zero.
  */
 void drm_mode_destroy(struct drm_device *dev, struct drm_display_mode *mode)
 {
 	if (!mode)
 		return;
 
-	drm_mode_object_put(dev, &mode->base);
-
-	kfree(mode);
+	kref_put(&mode->refcount, drm_mode_free);
 }
 EXPORT_SYMBOL(drm_mode_destroy);
 
@@ -851,15 +870,17 @@ EXPORT_SYMBOL(drm_mode_set_crtcinfo);
  * @dst: mode to overwrite
  * @src: mode to copy
  *
- * Copy an existing mode into another mode, preserving the object id and
- * list head of the destination mode.
+ * Copy an existing mode into another mode, preserving the object id,
+ * refcount, and list head of the destination mode.
  */
 void drm_mode_copy(struct drm_display_mode *dst, const struct drm_display_mode *src)
 {
+	struct kref refcount = dst->refcount;
 	int id = dst->base.id;
 	struct list_head head = dst->head;
 
 	*dst = *src;
+	dst->refcount = refcount;
 	dst->base.id = id;
 	dst->head = head;
 }
@@ -887,6 +908,11 @@ struct drm_display_mode *drm_mode_duplicate(struct drm_device *dev,
 
 	drm_mode_copy(nmode, mode);
 
+	/* Often modes are duplicated from static lists created upfront without
+	 * the device member set. Ensure that we have a real usable device to
+	 * work from. */
+	nmode->dev = dev;
+
 	return nmode;
 }
 EXPORT_SYMBOL(drm_mode_duplicate);
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 6717f07..4dd51fe 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1039,7 +1039,7 @@ err:
 static void tegra_crtc_atomic_destroy_state(struct drm_crtc *crtc,
 					    struct drm_crtc_state *state)
 {
-	kfree(state->mode);
+	drm_mode_destroy(crtc->dev, state->mode);
 	kfree(state);
 }
 
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 46ed207..70a6c71 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -99,9 +99,15 @@ enum drm_mode_status {
 struct drm_display_mode {
 	/* Header */
 	struct list_head head;
+	struct drm_device *dev;
 	struct drm_mode_object base;
 	struct drm_mode_modeinfo umode;
 
+	/* As mode objects are now exposed to userspace, rather than just
+	 * having their contents shovelled around, we maintain a refcount
+	 * on them in order to reason about their lifetimes. */
+	struct kref refcount;
+
 	char name[DRM_DISPLAY_MODE_LEN];
 
 	enum drm_mode_status status;
-- 
2.3.2



More information about the dri-devel mailing list