From 949619f32eee37a6385de1e976523501c8256768 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 29 Aug 2016 10:27:51 +0200 Subject: drm: Extract drm_mode_object.[hc] Just for the struct drm_mode_object base class. The header file was already partially extracted to help untangle the include loops. v2: - Also move the generic get/set property ioctls. At first this seemed like a bad idea since it requires making drm_mode_crtc_set_obj_prop non-static. But eventually that will get split away too (like the connector version already is) for both crtc and planes. Hence I reconsidered. - drm_mode_object.[hc] instead of drm_modeset.[hc], which requires renaming the drm_modeset.h header I already started building up. This is more consistent (matches the name of the main structure), and I want to be able to use drm_modeset.[hc] for the basic modeset init/cleanup functionality like drm_mode_config_init. Reviewed-by: Archit Taneja Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20160829082757.17913-3-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_mode_object.c | 435 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 435 insertions(+) create mode 100644 drivers/gpu/drm/drm_mode_object.c (limited to 'drivers/gpu/drm/drm_mode_object.c') diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c new file mode 100644 index 000000000000..cef9104e8285 --- /dev/null +++ b/drivers/gpu/drm/drm_mode_object.c @@ -0,0 +1,435 @@ +/* + * Copyright (c) 2016 Intel Corporation + * + * Permission to use, copy, modify, distribute, and sell this software and its + * documentation for any purpose is hereby granted without fee, provided that + * the above copyright notice appear in all copies and that both that copyright + * notice and this permission notice appear in supporting documentation, and + * that the name of the copyright holders not be used in advertising or + * publicity pertaining to distribution of the software without specific, + * written prior permission. The copyright holders make no representations + * about the suitability of this software for any purpose. It is provided "as + * is" without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE + * OF THIS SOFTWARE. + */ + +#include +#include +#include + +#include "drm_crtc_internal.h" + +/* + * Internal function to assign a slot in the object idr and optionally + * register the object into the idr. + */ +int drm_mode_object_get_reg(struct drm_device *dev, + struct drm_mode_object *obj, + uint32_t obj_type, + bool register_obj, + void (*obj_free_cb)(struct kref *kref)) +{ + int ret; + + mutex_lock(&dev->mode_config.idr_mutex); + ret = idr_alloc(&dev->mode_config.crtc_idr, register_obj ? obj : NULL, 1, 0, GFP_KERNEL); + if (ret >= 0) { + /* + * Set up the object linking under the protection of the idr + * lock so that other users can't see inconsistent state. + */ + obj->id = ret; + obj->type = obj_type; + if (obj_free_cb) { + obj->free_cb = obj_free_cb; + kref_init(&obj->refcount); + } + } + mutex_unlock(&dev->mode_config.idr_mutex); + + return ret < 0 ? ret : 0; +} + +/** + * drm_mode_object_get - allocate a new modeset identifier + * @dev: DRM device + * @obj: object pointer, used to generate unique ID + * @obj_type: object type + * + * Create a unique identifier based on @ptr in @dev's identifier space. Used + * for tracking modes, CRTCs and connectors. Note that despite the _get postfix + * modeset identifiers are _not_ reference counted. Hence don't use this for + * reference counted modeset objects like framebuffers. + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_mode_object_get(struct drm_device *dev, + struct drm_mode_object *obj, uint32_t obj_type) +{ + return drm_mode_object_get_reg(dev, obj, obj_type, true, NULL); +} + +void drm_mode_object_register(struct drm_device *dev, + struct drm_mode_object *obj) +{ + mutex_lock(&dev->mode_config.idr_mutex); + idr_replace(&dev->mode_config.crtc_idr, obj, obj->id); + mutex_unlock(&dev->mode_config.idr_mutex); +} + +/** + * drm_mode_object_unregister - free a modeset identifer + * @dev: DRM device + * @object: object to free + * + * Free @id from @dev's unique identifier pool. + * This function can be called multiple times, and guards against + * multiple removals. + * These modeset identifiers are _not_ reference counted. Hence don't use this + * for reference counted modeset objects like framebuffers. + */ +void drm_mode_object_unregister(struct drm_device *dev, + struct drm_mode_object *object) +{ + mutex_lock(&dev->mode_config.idr_mutex); + if (object->id) { + idr_remove(&dev->mode_config.crtc_idr, object->id); + object->id = 0; + } + mutex_unlock(&dev->mode_config.idr_mutex); +} + +struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev, + uint32_t id, uint32_t type) +{ + struct drm_mode_object *obj = NULL; + + mutex_lock(&dev->mode_config.idr_mutex); + obj = idr_find(&dev->mode_config.crtc_idr, id); + if (obj && type != DRM_MODE_OBJECT_ANY && obj->type != type) + obj = NULL; + if (obj && obj->id != id) + obj = NULL; + + if (obj && obj->free_cb) { + if (!kref_get_unless_zero(&obj->refcount)) + obj = NULL; + } + mutex_unlock(&dev->mode_config.idr_mutex); + + return obj; +} + +/** + * drm_mode_object_find - look up a drm object with static lifetime + * @dev: drm device + * @id: id of the mode object + * @type: type of the mode object + * + * This function is used to look up a modeset object. It will acquire a + * reference for reference counted objects. This reference must be dropped again + * by callind drm_mode_object_unreference(). + */ +struct drm_mode_object *drm_mode_object_find(struct drm_device *dev, + uint32_t id, uint32_t type) +{ + struct drm_mode_object *obj = NULL; + + obj = __drm_mode_object_find(dev, id, type); + return obj; +} +EXPORT_SYMBOL(drm_mode_object_find); + +/** + * drm_mode_object_unreference - decr the object refcnt + * @obj: mode_object + * + * This functions decrements the object's refcount if it is a refcounted modeset + * object. It is a no-op on any other object. This is used to drop references + * acquired with drm_mode_object_reference(). + */ +void drm_mode_object_unreference(struct drm_mode_object *obj) +{ + if (obj->free_cb) { + DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, atomic_read(&obj->refcount.refcount)); + kref_put(&obj->refcount, obj->free_cb); + } +} +EXPORT_SYMBOL(drm_mode_object_unreference); + +/** + * drm_mode_object_reference - incr the object refcnt + * @obj: mode_object + * + * This functions increments the object's refcount if it is a refcounted modeset + * object. It is a no-op on any other object. References should be dropped again + * by calling drm_mode_object_unreference(). + */ +void drm_mode_object_reference(struct drm_mode_object *obj) +{ + if (obj->free_cb) { + DRM_DEBUG("OBJ ID: %d (%d)\n", obj->id, atomic_read(&obj->refcount.refcount)); + kref_get(&obj->refcount); + } +} +EXPORT_SYMBOL(drm_mode_object_reference); + +/** + * drm_object_attach_property - attach a property to a modeset object + * @obj: drm modeset object + * @property: property to attach + * @init_val: initial value of the property + * + * This attaches the given property to the modeset object with the given initial + * value. Currently this function cannot fail since the properties are stored in + * a statically sized array. + */ +void drm_object_attach_property(struct drm_mode_object *obj, + struct drm_property *property, + uint64_t init_val) +{ + int count = obj->properties->count; + + if (count == DRM_OBJECT_MAX_PROPERTY) { + WARN(1, "Failed to attach object property (type: 0x%x). Please " + "increase DRM_OBJECT_MAX_PROPERTY by 1 for each time " + "you see this message on the same object type.\n", + obj->type); + return; + } + + obj->properties->properties[count] = property; + obj->properties->values[count] = init_val; + obj->properties->count++; + if (property->flags & DRM_MODE_PROP_ATOMIC) + obj->properties->atomic_count++; +} +EXPORT_SYMBOL(drm_object_attach_property); + +/** + * drm_object_property_set_value - set the value of a property + * @obj: drm mode object to set property value for + * @property: property to set + * @val: value the property should be set to + * + * This functions sets a given property on a given object. This function only + * changes the software state of the property, it does not call into the + * driver's ->set_property callback. + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_object_property_set_value(struct drm_mode_object *obj, + struct drm_property *property, uint64_t val) +{ + int i; + + for (i = 0; i < obj->properties->count; i++) { + if (obj->properties->properties[i] == property) { + obj->properties->values[i] = val; + return 0; + } + } + + return -EINVAL; +} +EXPORT_SYMBOL(drm_object_property_set_value); + +/** + * drm_object_property_get_value - retrieve the value of a property + * @obj: drm mode object to get property value from + * @property: property to retrieve + * @val: storage for the property value + * + * This function retrieves the softare state of the given property for the given + * property. Since there is no driver callback to retrieve the current property + * value this might be out of sync with the hardware, depending upon the driver + * and property. + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_object_property_get_value(struct drm_mode_object *obj, + struct drm_property *property, uint64_t *val) +{ + int i; + + /* read-only properties bypass atomic mechanism and still store + * their value in obj->properties->values[].. mostly to avoid + * having to deal w/ EDID and similar props in atomic paths: + */ + if (drm_core_check_feature(property->dev, DRIVER_ATOMIC) && + !(property->flags & DRM_MODE_PROP_IMMUTABLE)) + return drm_atomic_get_property(obj, property, val); + + for (i = 0; i < obj->properties->count; i++) { + if (obj->properties->properties[i] == property) { + *val = obj->properties->values[i]; + return 0; + } + + } + + return -EINVAL; +} +EXPORT_SYMBOL(drm_object_property_get_value); + +/* helper for getconnector and getproperties ioctls */ +int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic, + uint32_t __user *prop_ptr, + uint64_t __user *prop_values, + uint32_t *arg_count_props) +{ + int props_count; + int i, ret, copied; + + props_count = obj->properties->count; + if (!atomic) + props_count -= obj->properties->atomic_count; + + if ((*arg_count_props >= props_count) && props_count) { + for (i = 0, copied = 0; copied < props_count; i++) { + struct drm_property *prop = obj->properties->properties[i]; + uint64_t val; + + if ((prop->flags & DRM_MODE_PROP_ATOMIC) && !atomic) + continue; + + ret = drm_object_property_get_value(obj, prop, &val); + if (ret) + return ret; + + if (put_user(prop->base.id, prop_ptr + copied)) + return -EFAULT; + + if (put_user(val, prop_values + copied)) + return -EFAULT; + + copied++; + } + } + *arg_count_props = props_count; + + return 0; +} + +/** + * drm_mode_obj_get_properties_ioctl - get the current value of a object's property + * @dev: DRM device + * @data: ioctl data + * @file_priv: DRM file info + * + * This function retrieves the current value for an object's property. Compared + * to the connector specific ioctl this one is extended to also work on crtc and + * plane objects. + * + * Called by the user via ioctl. + * + * Returns: + * Zero on success, negative errno on failure. + */ +int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_mode_obj_get_properties *arg = data; + struct drm_mode_object *obj; + int ret = 0; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + drm_modeset_lock_all(dev); + + obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type); + if (!obj) { + ret = -ENOENT; + goto out; + } + if (!obj->properties) { + ret = -EINVAL; + goto out_unref; + } + + ret = drm_mode_object_get_properties(obj, file_priv->atomic, + (uint32_t __user *)(unsigned long)(arg->props_ptr), + (uint64_t __user *)(unsigned long)(arg->prop_values_ptr), + &arg->count_props); + +out_unref: + drm_mode_object_unreference(obj); +out: + drm_modeset_unlock_all(dev); + return ret; +} + +int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_mode_obj_set_property *arg = data; + struct drm_mode_object *arg_obj; + struct drm_mode_object *prop_obj; + struct drm_property *property; + int i, ret = -EINVAL; + struct drm_mode_object *ref; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + + drm_modeset_lock_all(dev); + + arg_obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type); + if (!arg_obj) { + ret = -ENOENT; + goto out; + } + if (!arg_obj->properties) + goto out_unref; + + for (i = 0; i < arg_obj->properties->count; i++) + if (arg_obj->properties->properties[i]->base.id == arg->prop_id) + break; + + if (i == arg_obj->properties->count) + goto out_unref; + + prop_obj = drm_mode_object_find(dev, arg->prop_id, + DRM_MODE_OBJECT_PROPERTY); + if (!prop_obj) { + ret = -ENOENT; + goto out_unref; + } + property = obj_to_property(prop_obj); + + if (!drm_property_change_valid_get(property, arg->value, &ref)) + goto out_unref; + + switch (arg_obj->type) { + case DRM_MODE_OBJECT_CONNECTOR: + ret = drm_mode_connector_set_obj_prop(arg_obj, property, + arg->value); + break; + case DRM_MODE_OBJECT_CRTC: + ret = drm_mode_crtc_set_obj_prop(arg_obj, property, arg->value); + break; + case DRM_MODE_OBJECT_PLANE: + ret = drm_mode_plane_set_obj_prop(obj_to_plane(arg_obj), + property, arg->value); + break; + } + + drm_property_change_valid_put(property, ref); + +out_unref: + drm_mode_object_unreference(arg_obj); +out: + drm_modeset_unlock_all(dev); + return ret; +} -- cgit v1.2.3 From f094d881954982b559e98e90aca1bf6f45141420 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 29 Aug 2016 10:27:52 +0200 Subject: drm: Remove drm_mode_object->atomic_count It's only used in drm_mode_object_get_properties, and we can compute it there directly with a bit of code shuffling. Reviewed-by: Archit Taneja Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20160829082757.17913-4-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_mode_object.c | 31 ++++++++++++------------------- include/drm/drm_mode_object.h | 2 +- 2 files changed, 13 insertions(+), 20 deletions(-) (limited to 'drivers/gpu/drm/drm_mode_object.c') diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index cef9104e8285..a92aeed51156 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -209,8 +209,6 @@ void drm_object_attach_property(struct drm_mode_object *obj, obj->properties->properties[count] = property; obj->properties->values[count] = init_val; obj->properties->count++; - if (property->flags & DRM_MODE_PROP_ATOMIC) - obj->properties->atomic_count++; } EXPORT_SYMBOL(drm_object_attach_property); @@ -288,35 +286,30 @@ int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic, uint64_t __user *prop_values, uint32_t *arg_count_props) { - int props_count; - int i, ret, copied; + int i, ret, count; - props_count = obj->properties->count; - if (!atomic) - props_count -= obj->properties->atomic_count; + for (i = 0, count = 0; i < obj->properties->count; i++) { + struct drm_property *prop = obj->properties->properties[i]; + uint64_t val; - if ((*arg_count_props >= props_count) && props_count) { - for (i = 0, copied = 0; copied < props_count; i++) { - struct drm_property *prop = obj->properties->properties[i]; - uint64_t val; - - if ((prop->flags & DRM_MODE_PROP_ATOMIC) && !atomic) - continue; + if ((prop->flags & DRM_MODE_PROP_ATOMIC) && !atomic) + continue; + if (*arg_count_props > count) { ret = drm_object_property_get_value(obj, prop, &val); if (ret) return ret; - if (put_user(prop->base.id, prop_ptr + copied)) + if (put_user(prop->base.id, prop_ptr + count)) return -EFAULT; - if (put_user(val, prop_values + copied)) + if (put_user(val, prop_values + count)) return -EFAULT; - - copied++; } + + count++; } - *arg_count_props = props_count; + *arg_count_props = count; return 0; } diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index c0e4414299f7..b8adb6425f2a 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -37,7 +37,7 @@ struct drm_mode_object { #define DRM_OBJECT_MAX_PROPERTY 24 struct drm_object_properties { - int count, atomic_count; + int count; /* NOTE: if we ever start dynamically destroying properties (ie. * not at drm_mode_config_cleanup() time), then we'd have to do * a better job of detaching property from mode objects to avoid -- cgit v1.2.3 From a2511a557eb868dc5fb28808ef58bd8af2e51f3b Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 29 Aug 2016 10:27:53 +0200 Subject: drm/doc: Polish docs for drm_mode_object I figured an overview section here is overkill, and better to just document the 2 structures themselves well enough. v2: Review from Archit: - Appease checkpatch in moved code. - Spelling fixes in the kerneldoc. Reviewed-by: Archit Taneja Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20160829082757.17913-5-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_mode_object.c | 17 +++++++++---- include/drm/drm_mode_object.h | 50 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/drm_mode_object.c') diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index a92aeed51156..6edda8382a4c 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -97,7 +97,7 @@ void drm_mode_object_register(struct drm_device *dev, * for reference counted modeset objects like framebuffers. */ void drm_mode_object_unregister(struct drm_device *dev, - struct drm_mode_object *object) + struct drm_mode_object *object) { mutex_lock(&dev->mode_config.idr_mutex); if (object->id) { @@ -152,7 +152,7 @@ EXPORT_SYMBOL(drm_mode_object_find); * drm_mode_object_unreference - decr the object refcnt * @obj: mode_object * - * This functions decrements the object's refcount if it is a refcounted modeset + * This function decrements the object's refcount if it is a refcounted modeset * object. It is a no-op on any other object. This is used to drop references * acquired with drm_mode_object_reference(). */ @@ -169,7 +169,7 @@ EXPORT_SYMBOL(drm_mode_object_unreference); * drm_mode_object_reference - incr the object refcnt * @obj: mode_object * - * This functions increments the object's refcount if it is a refcounted modeset + * This function increments the object's refcount if it is a refcounted modeset * object. It is a no-op on any other object. References should be dropped again * by calling drm_mode_object_unreference(). */ @@ -218,10 +218,16 @@ EXPORT_SYMBOL(drm_object_attach_property); * @property: property to set * @val: value the property should be set to * - * This functions sets a given property on a given object. This function only + * This function sets a given property on a given object. This function only * changes the software state of the property, it does not call into the * driver's ->set_property callback. * + * Note that atomic drivers should not have any need to call this, the core will + * ensure consistency of values reported back to userspace through the + * appropriate ->atomic_get_property callback. Only legacy drivers should call + * this function to update the tracked value (after clamping and other + * restrictions have been applied). + * * Returns: * Zero on success, error code on failure. */ @@ -252,6 +258,9 @@ EXPORT_SYMBOL(drm_object_property_set_value); * value this might be out of sync with the hardware, depending upon the driver * and property. * + * Atomic drivers should never call this function directly, the core will read + * out property values through the various ->atomic_get_property callbacks. + * * Returns: * Zero on success, error code on failure. */ diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h index b8adb6425f2a..be3d93839ae2 100644 --- a/include/drm/drm_mode_object.h +++ b/include/drm/drm_mode_object.h @@ -27,6 +27,28 @@ struct drm_object_properties; struct drm_property; +/** + * struct drm_mode_object - base structure for modeset objects + * @id: userspace visible identifier + * @type: type of the object, one of DRM_MODE_OBJECT\_\* + * @properties: properties attached to this object, including values + * @refcount: reference count for objects which with dynamic lifetime + * @free_cb: free function callback, only set for objects with dynamic lifetime + * + * Base structure for modeset objects visible to userspace. Objects can be + * looked up using drm_mode_object_find(). Besides basic uapi interface + * properties like @id and @type it provides two services: + * + * - It tracks attached properties and their values. This is used by &drm_crtc, + * &drm_plane and &drm_connector. Properties are attached by calling + * drm_object_attach_property() before the object is visible to userspace. + * + * - For objects with dynamic lifetimes (as indicated by a non-NULL @free_cb) it + * provides reference counting through drm_mode_object_reference() and + * drm_mode_object_unreference(). This is used by &drm_framebuffer, + * &drm_connector and &drm_property_blob. These objects provide specialized + * reference counting wrappers. + */ struct drm_mode_object { uint32_t id; uint32_t type; @@ -36,16 +58,38 @@ struct drm_mode_object { }; #define DRM_OBJECT_MAX_PROPERTY 24 +/** + * struct drm_object_properties - property tracking for &drm_mode_object + */ struct drm_object_properties { + /** + * @count: number of valid properties, must be less than or equal to + * DRM_OBJECT_MAX_PROPERTY. + */ + int count; - /* NOTE: if we ever start dynamically destroying properties (ie. + /** + * @properties: Array of pointers to &drm_property. + * + * NOTE: if we ever start dynamically destroying properties (ie. * not at drm_mode_config_cleanup() time), then we'd have to do * a better job of detaching property from mode objects to avoid * dangling property pointers: */ struct drm_property *properties[DRM_OBJECT_MAX_PROPERTY]; - /* do not read/write values directly, but use drm_object_property_get_value() - * and drm_object_property_set_value(): + + /** + * @values: Array to store the property values, matching @properties. Do + * not read/write values directly, but use + * drm_object_property_get_value() and drm_object_property_set_value(). + * + * Note that atomic drivers do not store mutable properties in this + * array, but only the decoded values in the corresponding state + * structure. The decoding is done using the ->atomic_get_property and + * ->atomic_set_property hooks of the corresponding object. Hence atomic + * drivers should not use drm_object_property_set_value() and + * drm_object_property_get_value() on mutable objects, i.e. those + * without the DRM_MODE_PROP_IMMUTABLE flag set. */ uint64_t values[DRM_OBJECT_MAX_PROPERTY]; }; -- cgit v1.2.3 From f92f053bb60924297afb8a1bd9166712c0fe5e88 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Thu, 8 Sep 2016 12:30:01 +0200 Subject: drm: Move property validation to a helper, v2. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Property lifetimes are equal to the device lifetime, so the separate drm_property_find is not needed. The pointer can be retrieved from the properties member, which saves us some locking and a extra lookup. The lifetime for properties is until the device is destroyed, which happens late in the device unload path. kms_atomic is also testing for invalid properties which returns -ENOENT, to be consistent return -ENOENT for valid properties that don't appear on the object property list. Changes since v1: - Return -ENOENT for invalid properties to make kms_atomic pass. - Change commit message slightly to take this into account. Testcase: kms_atomic Testcase: kms_properties Fixes: 4e9951d96093 ("drm/atomic: Reject properties not part of the object.") Suggested-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst Signed-off-by: Sean Paul Link: http://patchwork.freedesktop.org/patch/msgid/599c7fa8-b6fd-a42b-c619-a9e4a9c5c244@linux.intel.com --- drivers/gpu/drm/drm_atomic.c | 13 ++----------- drivers/gpu/drm/drm_crtc_internal.h | 2 ++ drivers/gpu/drm/drm_mode_object.c | 31 ++++++++++++++++--------------- 3 files changed, 20 insertions(+), 26 deletions(-) (limited to 'drivers/gpu/drm/drm_mode_object.c') diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index a5126e5c05ee..904d29c012ad 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1609,7 +1609,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, struct drm_crtc_state *crtc_state; unsigned plane_mask; int ret = 0; - unsigned int i, j, k; + unsigned int i, j; /* disallow for drivers not supporting atomic: */ if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) @@ -1691,16 +1691,7 @@ retry: goto out; } - for (k = 0; k < obj->properties->count; k++) - if (obj->properties->properties[k]->base.id == prop_id) - break; - - if (k == obj->properties->count) { - ret = -EINVAL; - goto out; - } - - prop = drm_property_find(dev, prop_id); + prop = drm_mode_obj_find_prop_id(obj, prop_id); if (!prop) { drm_mode_object_unreference(obj); ret = -ENOENT; diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index a3622644bccf..444e609078cc 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -115,6 +115,8 @@ int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic, uint32_t __user *prop_ptr, uint64_t __user *prop_values, uint32_t *arg_count_props); +struct drm_property *drm_mode_obj_find_prop_id(struct drm_mode_object *obj, + uint32_t prop_id); /* IOCTL */ diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index 6edda8382a4c..9f17085b1fdd 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -372,14 +372,25 @@ out: return ret; } +struct drm_property *drm_mode_obj_find_prop_id(struct drm_mode_object *obj, + uint32_t prop_id) +{ + int i; + + for (i = 0; i < obj->properties->count; i++) + if (obj->properties->properties[i]->base.id == prop_id) + return obj->properties->properties[i]; + + return NULL; +} + int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_mode_obj_set_property *arg = data; struct drm_mode_object *arg_obj; - struct drm_mode_object *prop_obj; struct drm_property *property; - int i, ret = -EINVAL; + int ret = -EINVAL; struct drm_mode_object *ref; if (!drm_core_check_feature(dev, DRIVER_MODESET)) @@ -392,23 +403,13 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, ret = -ENOENT; goto out; } - if (!arg_obj->properties) - goto out_unref; - - for (i = 0; i < arg_obj->properties->count; i++) - if (arg_obj->properties->properties[i]->base.id == arg->prop_id) - break; - if (i == arg_obj->properties->count) + if (!arg_obj->properties) goto out_unref; - prop_obj = drm_mode_object_find(dev, arg->prop_id, - DRM_MODE_OBJECT_PROPERTY); - if (!prop_obj) { - ret = -ENOENT; + property = drm_mode_obj_find_prop_id(arg_obj, arg->prop_id); + if (!property) goto out_unref; - } - property = obj_to_property(prop_obj); if (!drm_property_change_valid_get(property, arg->value, &ref)) goto out_unref; -- cgit v1.2.3