* [PATCH] drm: WARN() when drm_connector_attach_property fails
2012-05-15 21:08 [PATCH 1/8] drm: add drm_property_change_is_valid Paulo Zanoni
@ 2012-05-15 21:09 ` Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: create struct drm_object_properties and use it Paulo Zanoni
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2012-05-15 21:09 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Also return void instead of int. We have more than 100 callers and
no one checks for the return value.
If this function fails the property won't be exposed by the get/set
ioctls, but we should probably survive. If this starts happening,
the solution will be to increase DRM_CONNECTOR_MAX_PROPERTY and
recompile the Kernel.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 10 +++++-----
include/drm/drm_crtc.h | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
>From the previous patch series sent, the only comment this patch had was this,
from Chris:
> Please add that to the error message and use printk_once() since we only
> really want to annoy the user the first time.
For each time we see the message, we need to increase the value by 1, so instead
of using printk_once, I just added this information to the message. Anyway, we
shouldn't be seeing this message too many times, since we don't add new
properties that often.
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 0a22ef8..37bb7ab 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2819,7 +2819,7 @@ void drm_property_destroy(struct drm_device *dev, struct drm_property *property)
}
EXPORT_SYMBOL(drm_property_destroy);
-int drm_connector_attach_property(struct drm_connector *connector,
+void drm_connector_attach_property(struct drm_connector *connector,
struct drm_property *property, uint64_t init_val)
{
int i;
@@ -2828,13 +2828,13 @@ int drm_connector_attach_property(struct drm_connector *connector,
if (connector->property_ids[i] == 0) {
connector->property_ids[i] = property->base.id;
connector->property_values[i] = init_val;
- break;
+ return;
}
}
- if (i == DRM_CONNECTOR_MAX_PROPERTY)
- return -EINVAL;
- return 0;
+ WARN(1, "Failed to attach connector property. Please increase "
+ "DRM_CONNECTOR_MAX_PROPERTY by 1 for each time you see this "
+ "message\n");
}
EXPORT_SYMBOL(drm_connector_attach_property);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f35e7ed..036faec 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -910,8 +910,8 @@ extern int drmfb_remove(struct drm_device *dev, struct drm_framebuffer *fb);
extern void drm_crtc_probe_connector_modes(struct drm_device *dev, int maxX, int maxY);
extern bool drm_crtc_in_use(struct drm_crtc *crtc);
-extern int drm_connector_attach_property(struct drm_connector *connector,
- struct drm_property *property, uint64_t init_val);
+extern void drm_connector_attach_property(struct drm_connector *connector,
+ struct drm_property *property, uint64_t init_val);
extern struct drm_property *drm_property_create(struct drm_device *dev, int flags,
const char *name, int num_values);
extern struct drm_property *drm_property_create_enum(struct drm_device *dev, int flags,
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] drm: create struct drm_object_properties and use it
2012-05-15 21:08 [PATCH 1/8] drm: add drm_property_change_is_valid Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: WARN() when drm_connector_attach_property fails Paulo Zanoni
@ 2012-05-15 21:09 ` Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: add generic ioctls to get/set properties on any object Paulo Zanoni
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2012-05-15 21:09 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
For now, only connectors have it. In the future, all objects that need
properties should use it. Since the structure is referenced inside
struct drm_mode_object, we will be able to deal with object properties
without knowing the real type of the object.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 45 ++++++++++++++++++++++----------------------
include/drm/drm_crtc.h | 15 ++++++++++-----
2 files changed, 33 insertions(+), 27 deletions(-)
Comment by Eugeni on the previous version:
> *structure
Fixed.
Comment by Ville:
> Unrelated change?
Ooops... Left-over from a previous version (unsent) of this patch. Fixed.
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 37bb7ab..19a289f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -483,6 +483,7 @@ int drm_connector_init(struct drm_device *dev,
if (ret)
goto out;
+ connector->base.properties = &connector->properties;
connector->dev = dev;
connector->funcs = funcs;
connector->connector_type = connector_type;
@@ -1424,8 +1425,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
}
connector = obj_to_connector(obj);
- for (i = 0; i < DRM_CONNECTOR_MAX_PROPERTY; i++) {
- if (connector->property_ids[i] != 0) {
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (connector->properties.ids[i] != 0) {
props_count++;
}
}
@@ -1481,15 +1482,15 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
copied = 0;
prop_ptr = (uint32_t __user *)(unsigned long)(out_resp->props_ptr);
prop_values = (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr);
- for (i = 0; i < DRM_CONNECTOR_MAX_PROPERTY; i++) {
- if (connector->property_ids[i] != 0) {
- if (put_user(connector->property_ids[i],
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (connector->properties.ids[i] != 0) {
+ if (put_user(connector->properties.ids[i],
prop_ptr + copied)) {
ret = -EFAULT;
goto out;
}
- if (put_user(connector->property_values[i],
+ if (put_user(connector->properties.values[i],
prop_values + copied)) {
ret = -EFAULT;
goto out;
@@ -2824,16 +2825,16 @@ void drm_connector_attach_property(struct drm_connector *connector,
{
int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_PROPERTY; i++) {
- if (connector->property_ids[i] == 0) {
- connector->property_ids[i] = property->base.id;
- connector->property_values[i] = init_val;
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (connector->properties.ids[i] == 0) {
+ connector->properties.ids[i] = property->base.id;
+ connector->properties.values[i] = init_val;
return;
}
}
WARN(1, "Failed to attach connector property. Please increase "
- "DRM_CONNECTOR_MAX_PROPERTY by 1 for each time you see this "
+ "DRM_OBJECT_MAX_PROPERTY by 1 for each time you see this "
"message\n");
}
EXPORT_SYMBOL(drm_connector_attach_property);
@@ -2843,14 +2844,14 @@ int drm_connector_property_set_value(struct drm_connector *connector,
{
int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_PROPERTY; i++) {
- if (connector->property_ids[i] == property->base.id) {
- connector->property_values[i] = value;
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (connector->properties.ids[i] == property->base.id) {
+ connector->properties.values[i] = value;
break;
}
}
- if (i == DRM_CONNECTOR_MAX_PROPERTY)
+ if (i == DRM_OBJECT_MAX_PROPERTY)
return -EINVAL;
return 0;
}
@@ -2861,14 +2862,14 @@ int drm_connector_property_get_value(struct drm_connector *connector,
{
int i;
- for (i = 0; i < DRM_CONNECTOR_MAX_PROPERTY; i++) {
- if (connector->property_ids[i] == property->base.id) {
- *val = connector->property_values[i];
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (connector->properties.ids[i] == property->base.id) {
+ *val = connector->properties.values[i];
break;
}
}
- if (i == DRM_CONNECTOR_MAX_PROPERTY)
+ if (i == DRM_OBJECT_MAX_PROPERTY)
return -EINVAL;
return 0;
}
@@ -3113,12 +3114,12 @@ int drm_mode_connector_property_set_ioctl(struct drm_device *dev,
}
connector = obj_to_connector(obj);
- for (i = 0; i < DRM_CONNECTOR_MAX_PROPERTY; i++) {
- if (connector->property_ids[i] == out_resp->prop_id)
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (connector->properties.ids[i] == out_resp->prop_id)
break;
}
- if (i == DRM_CONNECTOR_MAX_PROPERTY) {
+ if (i == DRM_OBJECT_MAX_PROPERTY) {
goto out;
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 036faec..7760679 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -36,6 +36,7 @@
struct drm_device;
struct drm_mode_set;
struct drm_framebuffer;
+struct drm_object_properties;
#define DRM_MODE_OBJECT_CRTC 0xcccccccc
@@ -50,6 +51,13 @@ struct drm_framebuffer;
struct drm_mode_object {
uint32_t id;
uint32_t type;
+ struct drm_object_properties *properties;
+};
+
+#define DRM_OBJECT_MAX_PROPERTY 16
+struct drm_object_properties {
+ uint32_t ids[DRM_OBJECT_MAX_PROPERTY];
+ uint64_t values[DRM_OBJECT_MAX_PROPERTY];
};
/*
@@ -451,7 +459,6 @@ struct drm_encoder_funcs {
};
#define DRM_CONNECTOR_MAX_UMODES 16
-#define DRM_CONNECTOR_MAX_PROPERTY 16
#define DRM_CONNECTOR_LEN 32
#define DRM_CONNECTOR_MAX_ENCODER 3
@@ -520,8 +527,7 @@ enum drm_connector_force {
* @funcs: connector control functions
* @user_modes: user added mode list
* @edid_blob_ptr: DRM property containing EDID if present
- * @property_ids: property tracking for this connector
- * @property_values: value pointers or data for properties
+ * @properties: property tracking for this connector
* @polled: a %DRM_CONNECTOR_POLL_<foo> value for core driven polling
* @dpms: current dpms state
* @helper_private: mid-layer private data
@@ -565,8 +571,7 @@ struct drm_connector {
struct list_head user_modes;
struct drm_property_blob *edid_blob_ptr;
- u32 property_ids[DRM_CONNECTOR_MAX_PROPERTY];
- uint64_t property_values[DRM_CONNECTOR_MAX_PROPERTY];
+ struct drm_object_properties properties;
uint8_t polled; /* DRM_CONNECTOR_POLL_* */
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] drm: add generic ioctls to get/set properties on any object
2012-05-15 21:08 [PATCH 1/8] drm: add drm_property_change_is_valid Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: WARN() when drm_connector_attach_property fails Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: create struct drm_object_properties and use it Paulo Zanoni
@ 2012-05-15 21:09 ` Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: make the connector properties code use the object properties code Paulo Zanoni
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2012-05-15 21:09 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Useless for connector properties (since they already have their own
ioctls), but useful when we add properties to CRTCs, planes and other
objects.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 182 ++++++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_drv.c | 4 +-
include/drm/drm.h | 2 +
include/drm/drm_crtc.h | 13 ++++
include/drm/drm_mode.h | 15 ++++
5 files changed, 215 insertions(+), 1 deletion(-)
Comments from previous versions:
>From Eugeni:
> <bikeshedding>
> Perhaps we could add a debug message here for catch the unhandled types..
> </bikeshedding>
This shouldn't happen. The previous checks should eliminate all objects that
don't have properties. The only possibility I can think of is if we add
properties to a new kind of object and forget to include it in the switch...
In this case, we'll still return -EINVAL, which is good.
>From Ville:
> How about adding obj->properties.count to avoid having to count every
> time?
I just blindly followed the connector properties example. But your idea is good,
so I implemented it. Fixed, but in a new patch near the end of the series.
Putting the in the end of the series should make it easier to review IMHO.
> Just add the trailing comma to the last line as well. Otherwise we keep
> on having ugly diffs when new ioctls are added.
Fixed.
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 19a289f..b6783f9 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2875,6 +2875,58 @@ int drm_connector_property_get_value(struct drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_property_get_value);
+void drm_object_attach_property(struct drm_mode_object *obj,
+ struct drm_property *property,
+ uint64_t init_val)
+{
+ int i;
+
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (obj->properties->ids[i] == 0) {
+ obj->properties->ids[i] = property->base.id;
+ obj->properties->values[i] = init_val;
+ return;
+ }
+ }
+
+ 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);
+}
+EXPORT_SYMBOL(drm_object_attach_property);
+
+int drm_object_property_set_value(struct drm_mode_object *obj,
+ struct drm_property *property, uint64_t val)
+{
+ int i;
+
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (obj->properties->ids[i] == property->base.id) {
+ obj->properties->values[i] = val;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL(drm_object_property_set_value);
+
+int drm_object_property_get_value(struct drm_mode_object *obj,
+ struct drm_property *property, uint64_t *val)
+{
+ int i;
+
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ if (obj->properties->ids[i] == property->base.id) {
+ *val = obj->properties->values[i];
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL(drm_object_property_get_value);
+
int drm_mode_getproperty_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
@@ -3148,6 +3200,136 @@ out:
return ret;
}
+static int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
+ struct drm_property *property,
+ uint64_t value)
+{
+ int ret = -EINVAL;
+ struct drm_connector *connector = obj_to_connector(obj);
+
+ /* Do DPMS ourselves */
+ if (property == connector->dev->mode_config.dpms_property) {
+ if (connector->funcs->dpms)
+ (*connector->funcs->dpms)(connector, (int)value);
+ ret = 0;
+ } else if (connector->funcs->set_property)
+ ret = connector->funcs->set_property(connector, property, value);
+
+ /* store the property value if successful */
+ if (!ret)
+ drm_connector_property_set_value(connector, property, value);
+ return ret;
+}
+
+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;
+ int i;
+ int copied = 0;
+ int props_count = 0;
+ uint32_t __user *props_ptr;
+ uint64_t __user *prop_values_ptr;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return -EINVAL;
+
+ mutex_lock(&dev->mode_config.mutex);
+
+ obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type);
+ if (!obj) {
+ ret = -EINVAL;
+ goto out;
+ }
+ if (!obj->properties) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Assume [ prop, 0, prop ] won't happen (if we ever delete properties,
+ * we need to remove the gap inside the array). */
+ for (props_count = 0; props_count < DRM_OBJECT_MAX_PROPERTY &&
+ obj->properties->ids[props_count] != 0; props_count++)
+ ;
+
+ /* This ioctl is called twice, once to determine how much space is
+ * needed, and the 2nd time to fill it. */
+ if ((arg->count_props >= props_count) && props_count) {
+ copied = 0;
+ props_ptr = (uint32_t __user *)(unsigned long)(arg->props_ptr);
+ prop_values_ptr = (uint64_t __user *)(unsigned long)
+ (arg->prop_values_ptr);
+ for (i = 0; i < props_count; i++) {
+ if (put_user(obj->properties->ids[i],
+ props_ptr + copied)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ if (put_user(obj->properties->values[i],
+ prop_values_ptr + copied)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ copied++;
+ }
+ }
+ arg->count_props = props_count;
+out:
+ mutex_unlock(&dev->mode_config.mutex);
+ 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 ret = -EINVAL;
+ int i;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return -EINVAL;
+
+ mutex_lock(&dev->mode_config.mutex);
+
+ arg_obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type);
+ if (!arg_obj)
+ goto out;
+ if (!arg_obj->properties)
+ goto out;
+
+ for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++)
+ if (arg_obj->properties->ids[i] == arg->prop_id)
+ break;
+
+ if (i == DRM_OBJECT_MAX_PROPERTY)
+ goto out;
+
+ prop_obj = drm_mode_object_find(dev, arg->prop_id,
+ DRM_MODE_OBJECT_PROPERTY);
+ if (!prop_obj)
+ goto out;
+ property = obj_to_property(prop_obj);
+
+ if (!drm_property_change_is_valid(property, arg->value))
+ goto out;
+
+ switch (arg_obj->type) {
+ case DRM_MODE_OBJECT_CONNECTOR:
+ ret = drm_mode_connector_set_obj_prop(arg_obj, property,
+ arg->value);
+ break;
+ }
+
+out:
+ mutex_unlock(&dev->mode_config.mutex);
+ return ret;
+}
+
int drm_mode_connector_attach_encoder(struct drm_connector *connector,
struct drm_encoder *encoder)
{
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 6116e3b..8a9d079 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -163,7 +163,9 @@ static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
- DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED)
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
};
#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
diff --git a/include/drm/drm.h b/include/drm/drm.h
index 64ff02d..5b831df 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -730,6 +730,8 @@ struct drm_prime_handle {
#define DRM_IOCTL_MODE_GETPLANE DRM_IOWR(0xB6, struct drm_mode_get_plane)
#define DRM_IOCTL_MODE_SETPLANE DRM_IOWR(0xB7, struct drm_mode_set_plane)
#define DRM_IOCTL_MODE_ADDFB2 DRM_IOWR(0xB8, struct drm_mode_fb_cmd2)
+#define DRM_IOCTL_MODE_OBJ_GETPROPERTIES DRM_IOWR(0xB9, struct drm_mode_obj_get_properties)
+#define DRM_IOCTL_MODE_OBJ_SETPROPERTY DRM_IOWR(0xBA, struct drm_mode_obj_set_property)
/**
* Device specific ioctls should only be in their respective headers
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 7760679..b0c3249 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -903,6 +903,12 @@ extern int drm_connector_property_set_value(struct drm_connector *connector,
extern int drm_connector_property_get_value(struct drm_connector *connector,
struct drm_property *property,
uint64_t *value);
+extern int drm_object_property_set_value(struct drm_mode_object *obj,
+ struct drm_property *property,
+ uint64_t val);
+extern int drm_object_property_get_value(struct drm_mode_object *obj,
+ struct drm_property *property,
+ uint64_t *value);
extern struct drm_display_mode *drm_crtc_mode_create(struct drm_device *dev);
extern void drm_framebuffer_set_object(struct drm_device *dev,
unsigned long handle);
@@ -917,6 +923,9 @@ extern bool drm_crtc_in_use(struct drm_crtc *crtc);
extern void drm_connector_attach_property(struct drm_connector *connector,
struct drm_property *property, uint64_t init_val);
+extern void drm_object_attach_property(struct drm_mode_object *obj,
+ struct drm_property *property,
+ uint64_t init_val);
extern struct drm_property *drm_property_create(struct drm_device *dev, int flags,
const char *name, int num_values);
extern struct drm_property *drm_property_create_enum(struct drm_device *dev, int flags,
@@ -1029,6 +1038,10 @@ extern int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv);
extern int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv);
+extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
+extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
int *bpp);
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 4a0aae3..326f2be 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -254,6 +254,21 @@ struct drm_mode_connector_set_property {
__u32 connector_id;
};
+struct drm_mode_obj_get_properties {
+ __u64 props_ptr;
+ __u64 prop_values_ptr;
+ __u32 count_props;
+ __u32 obj_id;
+ __u32 obj_type;
+};
+
+struct drm_mode_obj_set_property {
+ __u64 value;
+ __u32 prop_id;
+ __u32 obj_id;
+ __u32 obj_type;
+};
+
struct drm_mode_get_blob {
__u32 blob_id;
__u32 length;
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] drm: make the connector properties code use the object properties code
2012-05-15 21:08 [PATCH 1/8] drm: add drm_property_change_is_valid Paulo Zanoni
` (2 preceding siblings ...)
2012-05-15 21:09 ` [PATCH] drm: add generic ioctls to get/set properties on any object Paulo Zanoni
@ 2012-05-15 21:09 ` Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: add 'count' to struct drm_object_properties Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: add CRTC properties Paulo Zanoni
5 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2012-05-15 21:09 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
In the future, we may want to kill the internal functions:
- drm_connector_attach_property
- drm_connector_property_set_value
- drm_connector_property_get_value
It seems the IOCTL drm_mode_connector_property_set_ioctl will have to live, but
we may change libdrm to not use it anymore, if we want.
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 98 ++++++--------------------------------------
1 file changed, 12 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b6783f9..793f51b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2823,55 +2823,21 @@ EXPORT_SYMBOL(drm_property_destroy);
void drm_connector_attach_property(struct drm_connector *connector,
struct drm_property *property, uint64_t init_val)
{
- int i;
-
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
- if (connector->properties.ids[i] == 0) {
- connector->properties.ids[i] = property->base.id;
- connector->properties.values[i] = init_val;
- return;
- }
- }
-
- WARN(1, "Failed to attach connector property. Please increase "
- "DRM_OBJECT_MAX_PROPERTY by 1 for each time you see this "
- "message\n");
+ drm_object_attach_property(&connector->base, property, init_val);
}
EXPORT_SYMBOL(drm_connector_attach_property);
int drm_connector_property_set_value(struct drm_connector *connector,
struct drm_property *property, uint64_t value)
{
- int i;
-
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
- if (connector->properties.ids[i] == property->base.id) {
- connector->properties.values[i] = value;
- break;
- }
- }
-
- if (i == DRM_OBJECT_MAX_PROPERTY)
- return -EINVAL;
- return 0;
+ return drm_object_property_set_value(&connector->base, property, value);
}
EXPORT_SYMBOL(drm_connector_property_set_value);
int drm_connector_property_get_value(struct drm_connector *connector,
struct drm_property *property, uint64_t *val)
{
- int i;
-
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
- if (connector->properties.ids[i] == property->base.id) {
- *val = connector->properties.values[i];
- break;
- }
- }
-
- if (i == DRM_OBJECT_MAX_PROPERTY)
- return -EINVAL;
- return 0;
+ return drm_object_property_get_value(&connector->base, property, val);
}
EXPORT_SYMBOL(drm_connector_property_get_value);
@@ -3148,56 +3114,16 @@ static bool drm_property_change_is_valid(struct drm_property *property,
int drm_mode_connector_property_set_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
- struct drm_mode_connector_set_property *out_resp = data;
- struct drm_mode_object *obj;
- struct drm_property *property;
- struct drm_connector *connector;
- int ret = -EINVAL;
- int i;
-
- if (!drm_core_check_feature(dev, DRIVER_MODESET))
- return -EINVAL;
-
- mutex_lock(&dev->mode_config.mutex);
-
- obj = drm_mode_object_find(dev, out_resp->connector_id, DRM_MODE_OBJECT_CONNECTOR);
- if (!obj) {
- goto out;
- }
- connector = obj_to_connector(obj);
-
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
- if (connector->properties.ids[i] == out_resp->prop_id)
- break;
- }
-
- if (i == DRM_OBJECT_MAX_PROPERTY) {
- goto out;
- }
-
- obj = drm_mode_object_find(dev, out_resp->prop_id, DRM_MODE_OBJECT_PROPERTY);
- if (!obj) {
- goto out;
- }
- property = obj_to_property(obj);
+ struct drm_mode_connector_set_property *conn_set_prop = data;
+ struct drm_mode_obj_set_property obj_set_prop = {
+ .value = conn_set_prop->value,
+ .prop_id = conn_set_prop->prop_id,
+ .obj_id = conn_set_prop->connector_id,
+ .obj_type = DRM_MODE_OBJECT_CONNECTOR
+ };
- if (!drm_property_change_is_valid(property, out_resp->value))
- goto out;
-
- /* Do DPMS ourselves */
- if (property == connector->dev->mode_config.dpms_property) {
- if (connector->funcs->dpms)
- (*connector->funcs->dpms)(connector, (int) out_resp->value);
- ret = 0;
- } else if (connector->funcs->set_property)
- ret = connector->funcs->set_property(connector, property, out_resp->value);
-
- /* store the property value if successful */
- if (!ret)
- drm_connector_property_set_value(connector, property, out_resp->value);
-out:
- mutex_unlock(&dev->mode_config.mutex);
- return ret;
+ /* It does all the locking and checking we need */
+ return drm_mode_obj_set_property_ioctl(dev, &obj_set_prop, file_priv);
}
static int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] drm: add 'count' to struct drm_object_properties
2012-05-15 21:08 [PATCH 1/8] drm: add drm_property_change_is_valid Paulo Zanoni
` (3 preceding siblings ...)
2012-05-15 21:09 ` [PATCH] drm: make the connector properties code use the object properties code Paulo Zanoni
@ 2012-05-15 21:09 ` Paulo Zanoni
2012-05-15 21:09 ` [PATCH] drm: add CRTC properties Paulo Zanoni
5 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2012-05-15 21:09 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
This way, we don't need to count every time, so we're a little bit
faster and code is a little bit smaller.
Change suggested by Ville Syrjälä.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 64 +++++++++++++++++++-------------------------
include/drm/drm_crtc.h | 1 +
2 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 793f51b..368e3e7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1425,11 +1425,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
}
connector = obj_to_connector(obj);
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
- if (connector->properties.ids[i] != 0) {
- props_count++;
- }
- }
+ props_count = connector->properties.count;
for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
if (connector->encoder_ids[i] != 0) {
@@ -1482,21 +1478,19 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
copied = 0;
prop_ptr = (uint32_t __user *)(unsigned long)(out_resp->props_ptr);
prop_values = (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr);
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
- if (connector->properties.ids[i] != 0) {
- if (put_user(connector->properties.ids[i],
- prop_ptr + copied)) {
- ret = -EFAULT;
- goto out;
- }
+ for (i = 0; i < connector->properties.count; i++) {
+ if (put_user(connector->properties.ids[i],
+ prop_ptr + copied)) {
+ ret = -EFAULT;
+ goto out;
+ }
- if (put_user(connector->properties.values[i],
- prop_values + copied)) {
- ret = -EFAULT;
- goto out;
- }
- copied++;
+ if (put_user(connector->properties.values[i],
+ prop_values + copied)) {
+ ret = -EFAULT;
+ goto out;
}
+ copied++;
}
}
out_resp->count_props = props_count;
@@ -2845,19 +2839,19 @@ void drm_object_attach_property(struct drm_mode_object *obj,
struct drm_property *property,
uint64_t init_val)
{
- int i;
+ int count = obj->properties->count;
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
- if (obj->properties->ids[i] == 0) {
- obj->properties->ids[i] = property->base.id;
- obj->properties->values[i] = init_val;
- return;
- }
+ 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;
}
- 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);
+ obj->properties->ids[count] = property->base.id;
+ obj->properties->values[count] = init_val;
+ obj->properties->count++;
}
EXPORT_SYMBOL(drm_object_attach_property);
@@ -2866,7 +2860,7 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
{
int i;
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ for (i = 0; i < obj->properties->count; i++) {
if (obj->properties->ids[i] == property->base.id) {
obj->properties->values[i] = val;
return 0;
@@ -2882,7 +2876,7 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
{
int i;
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++) {
+ for (i = 0; i < obj->properties->count; i++) {
if (obj->properties->ids[i] == property->base.id) {
*val = obj->properties->values[i];
return 0;
@@ -3174,11 +3168,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
goto out;
}
- /* Assume [ prop, 0, prop ] won't happen (if we ever delete properties,
- * we need to remove the gap inside the array). */
- for (props_count = 0; props_count < DRM_OBJECT_MAX_PROPERTY &&
- obj->properties->ids[props_count] != 0; props_count++)
- ;
+ props_count = obj->properties->count;
/* This ioctl is called twice, once to determine how much space is
* needed, and the 2nd time to fill it. */
@@ -3228,11 +3218,11 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
if (!arg_obj->properties)
goto out;
- for (i = 0; i < DRM_OBJECT_MAX_PROPERTY; i++)
+ for (i = 0; i < arg_obj->properties->count; i++)
if (arg_obj->properties->ids[i] == arg->prop_id)
break;
- if (i == DRM_OBJECT_MAX_PROPERTY)
+ if (i == arg_obj->properties->count)
goto out;
prop_obj = drm_mode_object_find(dev, arg->prop_id,
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b0c3249..6d36552 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -56,6 +56,7 @@ struct drm_mode_object {
#define DRM_OBJECT_MAX_PROPERTY 16
struct drm_object_properties {
+ int count;
uint32_t ids[DRM_OBJECT_MAX_PROPERTY];
uint64_t values[DRM_OBJECT_MAX_PROPERTY];
};
--
1.7.10
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] drm: add CRTC properties
2012-05-15 21:08 [PATCH 1/8] drm: add drm_property_change_is_valid Paulo Zanoni
` (4 preceding siblings ...)
2012-05-15 21:09 ` [PATCH] drm: add 'count' to struct drm_object_properties Paulo Zanoni
@ 2012-05-15 21:09 ` Paulo Zanoni
2012-05-15 21:27 ` Paulo Zanoni
5 siblings, 1 reply; 10+ messages in thread
From: Paulo Zanoni @ 2012-05-15 21:09 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
The i915 driver needs this for the rotation and overscan compensation
properties. Other drivers might need this too.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 20 ++++++++++++++++++++
include/drm/drm_crtc.h | 9 ++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
Hi
As you may have noticed, this is, for now, the last patch of the series (there
are no users of the CRTC props). This is because there were some suggestions to
the i915 properties I implemented, so while I don't implement them in a way that
pleases everybody, I'll just submit the infrastructure work. There are other
people interested in having these "object properties", so they can add their own
properties on top of this patch series while I still don't have the i915 stuff.
Patches 0 to 6 are not completely useless: they change the code that's executed
when someone changes the Connector properties, and they allow the implementation
of other object properties. This patch (0007) is an example of "how to add
properties to other objects".
Although I didn't send more patches implementing the CRTC i915 properties, I
have these implemented locally and they work, as far as I have tested. I will
send these patches to the list as soon as I reimplement them following the
suggestions I received.
My series of libdrm patches implement a "proptest" program that allows us to
read and set object properties, so they are the first thing you should try when
testing this patch series.
Thank you,
Paulo
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 368e3e7..a177d0a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -384,6 +384,8 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
if (ret)
goto out;
+ crtc->base.properties = &crtc->properties;
+
list_add_tail(&crtc->head, &dev->mode_config.crtc_list);
dev->mode_config.num_crtc++;
@@ -3141,6 +3143,21 @@ static int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj,
return ret;
}
+static int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj,
+ struct drm_property *property,
+ uint64_t value)
+{
+ int ret = -EINVAL;
+ struct drm_crtc *crtc = obj_to_crtc(obj);
+
+ if (crtc->funcs->set_property)
+ ret = crtc->funcs->set_property(crtc, property, value);
+ if (!ret)
+ drm_object_property_set_value(obj, property, value);
+
+ return ret;
+}
+
int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
@@ -3239,6 +3256,9 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
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;
}
out:
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 6d36552..d59bb7d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -306,7 +306,8 @@ struct drm_plane;
* @mode_fixup: fixup proposed mode
* @mode_set: set the desired mode on the CRTC
* @gamma_set: specify color ramp for CRTC
- * @destroy: deinit and free object.
+ * @destroy: deinit and free object
+ * @set_property: called when a property is changed
*
* The drm_crtc_funcs structure is the central CRTC management structure
* in the DRM. Each CRTC controls one or more connectors (note that the name
@@ -350,6 +351,9 @@ struct drm_crtc_funcs {
int (*page_flip)(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
struct drm_pending_vblank_event *event);
+
+ int (*set_property)(struct drm_crtc *crtc,
+ struct drm_property *property, uint64_t val);
};
/**
@@ -369,6 +373,7 @@ struct drm_crtc_funcs {
* @framedur_ns: precise line timing
* @pixeldur_ns: precise pixel timing
* @helper_private: mid-layer private data
+ * @properties: property tracking for this CRTC
*
* Each CRTC may have one or more connectors associated with it. This structure
* allows the CRTC to be controlled.
@@ -404,6 +409,8 @@ struct drm_crtc {
/* if you are using the helper */
void *helper_private;
+
+ struct drm_object_properties properties;
};
--
1.7.10
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] drm: add CRTC properties
2012-05-15 21:09 ` [PATCH] drm: add CRTC properties Paulo Zanoni
@ 2012-05-15 21:27 ` Paulo Zanoni
0 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2012-05-15 21:27 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
2012/5/15 Paulo Zanoni <przanoni@gmail.com>:
> Patches 0 to 6 are not completely useless: they change the code that's executed
> when someone changes the Connector properties, and they allow the implementation
> of other object properties. This patch (0007) is an example of "how to add
> properties to other objects".
And I completely messed the order of the patches while editing them to
add the comments about the previous patches :(
The order was supposed to be:
0001: drm: add drm_property_change_is_valid
0002: drm: WARN() when drm_connector_attach_property fails
0003: drm: create struct drm_object_properties and use it
0004: drm: add generic ioctls to get/set properties on any object
0005: drm: make the connector properties code use the object properties code
0006: drm: add 'count' to struct drm_object_properties
0007: drm: add CRTC properties
Should I resend them? Wait for review?
Sorry,
Paulo
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 10+ messages in thread