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
