dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Clark <robdclark@gmail.com>
To: dri-devel@lists.freedesktop.org
Subject: [PATCH 01/13] drm: allow property validation for refcnted props
Date: Tue, 16 Dec 2014 18:05:29 -0500	[thread overview]
Message-ID: <1418771141-16954-2-git-send-email-robdclark@gmail.com> (raw)
In-Reply-To: <1418771141-16954-1-git-send-email-robdclark@gmail.com>

We can already have object properties, which technically can be fb's.
Switch the property validation logic over to a get/put style interface
so it can take a ref to refcnt'd objects, and then drop that ref after
the driver has a chance to take it's own ref.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/drm_crtc.c | 58 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5213da4..5ee4b88 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4193,12 +4193,22 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_mode_connector_update_edid_property);
 
-static bool drm_property_change_is_valid(struct drm_property *property,
-					 uint64_t value)
+/* Some properties could refer to dynamic refcnt'd objects, or things that
+ * need special locking to handle lifetime issues (ie. to ensure the prop
+ * value doesn't become invalid part way through the property update due to
+ * race).  The value returned by reference via 'obj' should be passed back
+ * to drm_property_change_valid_put() after the property is set (and the
+ * object to which the property is attached has a chance to take it's own
+ * reference).
+ */
+static bool drm_property_change_valid_get(struct drm_property *property,
+					 uint64_t value, struct drm_mode_object **ref)
 {
 	if (property->flags & DRM_MODE_PROP_IMMUTABLE)
 		return false;
 
+	*ref = NULL;
+
 	if (drm_property_type_is(property, DRM_MODE_PROP_RANGE)) {
 		if (value < property->values[0] || value > property->values[1])
 			return false;
@@ -4219,19 +4229,23 @@ static bool drm_property_change_is_valid(struct drm_property *property,
 		/* Only the driver knows */
 		return true;
 	} else if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
-		struct drm_mode_object *obj;
 		/* a zero value for an object property translates to null: */
 		if (value == 0)
 			return true;
-		/*
-		 * NOTE: use _object_find() directly to bypass restriction on
-		 * looking up refcnt'd objects (ie. fb's).  For a refcnt'd
-		 * object this could race against object finalization, so it
-		 * simply tells us that the object *was* valid.  Which is good
-		 * enough.
-		 */
-		obj = _object_find(property->dev, value, property->values[0]);
-		return obj != NULL;
+
+		/* handle refcnt'd objects specially: */
+		if (property->values[0] == DRM_MODE_OBJECT_FB) {
+			struct drm_framebuffer *fb;
+			fb = drm_framebuffer_lookup(property->dev, value);
+			if (fb) {
+				*ref = &fb->base;
+				return true;
+			} else {
+				return false;
+			}
+		} else {
+			return _object_find(property->dev, value, property->values[0]) != NULL;
+		}
 	} else {
 		int i;
 		for (i = 0; i < property->num_values; i++)
@@ -4241,6 +4255,18 @@ static bool drm_property_change_is_valid(struct drm_property *property,
 	}
 }
 
+static void drm_property_change_valid_put(struct drm_property *property,
+		struct drm_mode_object *ref)
+{
+	if (!ref)
+		return;
+
+	if (drm_property_type_is(property, DRM_MODE_PROP_OBJECT)) {
+		if (property->values[0] == DRM_MODE_OBJECT_FB)
+			drm_framebuffer_unreference(obj_to_fb(ref));
+	}
+}
+
 /**
  * drm_mode_connector_property_set_ioctl - set the current value of a connector property
  * @dev: DRM device
@@ -4429,8 +4455,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
 	struct drm_mode_object *arg_obj;
 	struct drm_mode_object *prop_obj;
 	struct drm_property *property;
-	int ret = -EINVAL;
-	int i;
+	int i, ret = -EINVAL;
+	struct drm_mode_object *ref;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -4460,7 +4486,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
 	}
 	property = obj_to_property(prop_obj);
 
-	if (!drm_property_change_is_valid(property, arg->value))
+	if (!drm_property_change_valid_get(property, arg->value, &ref))
 		goto out;
 
 	switch (arg_obj->type) {
@@ -4477,6 +4503,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
 		break;
 	}
 
+	drm_property_change_valid_put(property, ref);
+
 out:
 	drm_modeset_unlock_all(dev);
 	return ret;
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2014-12-16 23:05 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-16 23:05 [PATCH 00/13] Atomic Properties Rob Clark
2014-12-16 23:05 ` Rob Clark [this message]
2014-12-17 12:31   ` [PATCH 01/13] drm: allow property validation for refcnted props Daniel Vetter
2014-12-16 23:05 ` [PATCH 02/13] drm: store property instead of id in obj attachment Rob Clark
2014-12-17 13:29   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 03/13] drm: get rid of direct property value access Rob Clark
2014-12-17 13:37   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 04/13] drm: add atomic_set_property wrappers Rob Clark
2014-12-17 12:49   ` Sean Paul
2014-12-17 13:43   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 05/13] drm: add atomic_get_property Rob Clark
2014-12-17 12:52   ` Sean Paul
2014-12-17 13:54   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 06/13] drm: add atomic hook to read property plus helper Rob Clark
2014-12-17 14:02   ` Daniel Vetter
2014-12-17 14:06     ` Daniel Vetter
2014-12-17 14:15       ` Rob Clark
2014-12-17 14:29         ` Daniel Vetter
2014-12-17 14:12     ` Rob Clark
2014-12-17 14:32       ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 07/13] drm: small property creation cleanup Rob Clark
2014-12-17 14:13   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 08/13] drm: tweak getconnector locking Rob Clark
2014-12-17 14:14   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 09/13] drm/atomic: atomic_check functions Rob Clark
2014-12-17 14:25   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 10/13] drm/atomic: atomic plane properties Rob Clark
2014-12-17 14:41   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 11/13] drm/atomic: atomic connector properties Rob Clark
2014-12-17 14:48   ` Daniel Vetter
2014-12-16 23:05 ` [PATCH 12/13] drm/msm: atomic property support Rob Clark
2014-12-16 23:05 ` [PATCH 13/13] RFC: drm: Atomic modeset ioctl Rob Clark
2014-12-17  2:48   ` Michel Dänzer
2014-12-17  7:20     ` Pekka Paalanen
2014-12-17  9:31       ` Michel Dänzer
2014-12-17 11:18         ` Daniel Vetter
2014-12-19  3:29           ` Michel Dänzer
2014-12-19  7:55             ` Daniel Vetter
2014-12-17 14:04         ` Rob Clark
2014-12-17 15:04   ` Daniel Vetter
2014-12-17 13:08 ` [PATCH 00/13] Atomic Properties Sean Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1418771141-16954-2-git-send-email-robdclark@gmail.com \
    --to=robdclark@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox