[PATCH 1/8] drm: add drm_property_change_is_valid

[PATCH 1/8] drm: add drm_property_change_is_valid

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Move code from drm_mode_connector_property_set_ioctl to a new
function, so we can reuse this code when we add crtc properties.

v2: use bool instead of int

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/drm_crtc.c |   41 +++++++++++++++++++++--------------------
 1 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d3aaeb6..2e9cfb5 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3033,6 +3033,26 @@ 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,
+					 __u64 value)
+{
+	if (property->flags & DRM_MODE_PROP_IMMUTABLE)
+		return false;
+	if (property->flags & DRM_MODE_PROP_RANGE) {
+		if (value < property->values[0])
+			return false;
+		if (value > property->values[1])
+			return false;
+		return true;
+	} else {
+		int i;
+		for (i = 0; i < property->num_values; i++)
+			if (property->values[i] == value)
+				return true;
+		return false;
+	}
+}
+
 int drm_mode_connector_property_set_ioctl(struct drm_device *dev,
 				       void *data, struct drm_file *file_priv)
 {
@@ -3069,28 +3089,9 @@ int drm_mode_connector_property_set_ioctl(struct drm_device *dev,
 	}
 	property = obj_to_property(obj);
 
-	if (property->flags & DRM_MODE_PROP_IMMUTABLE)
+	if (!drm_property_change_is_valid(property, out_resp->value))
 		goto out;
 
-	if (property->flags & DRM_MODE_PROP_RANGE) {
-		if (out_resp->value < property->values[0])
-			goto out;
-
-		if (out_resp->value > property->values[1])
-			goto out;
-	} else {
-		int found = 0;
-		for (i = 0; i < property->num_values; i++) {
-			if (property->values[i] == out_resp->value) {
-				found = 1;
-				break;
-			}
-		}
-		if (!found) {
-			goto out;
-		}
-	}
-
 	/* Do DPMS ourselves */
 	if (property == connector->dev->mode_config.dpms_property) {
 		if (connector->funcs->dpms)
-- 
1.7.9.1

[PATCH 2/8] drm: WARN() when drm_connector_attach_property fails

[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 |    8 +++-----
 include/drm/drm_crtc.h     |    4 ++--
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 2e9cfb5..e3a5b0e 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2778,7 +2778,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;
@@ -2787,13 +2787,11 @@ 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\n");
 }
 EXPORT_SYMBOL(drm_connector_attach_property);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e250eda..c3d429a 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.9.1

Re: [PATCH 2/8] drm: WARN() when drm_connector_attach_property fails

[Chris Wilson]

On Thu, 29 Mar 2012 18:27:20 -0300, Paulo Zanoni <przanoni@gmail.com> wrote:
> 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.

Please add that to the error message and use printk_once() since we only
really want to annoy the user the first time.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 3/8] drm: create struct drm_object_properties and use it

[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 strucutre is referenced inside
struct drm_mode_object, we will be able to deal with object properties
without knowing the real type of the object.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/drm_crtc.c |   43 ++++++++++++++++++++++---------------------
 include/drm/drm_crtc.h     |   16 +++++++++++-----
 2 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e3a5b0e..8800830 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -481,6 +481,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;
@@ -1422,8 +1423,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++;
 		}
 	}
@@ -1479,15 +1480,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;
@@ -2783,10 +2784,10 @@ 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;
 		}
 	}
@@ -2800,14 +2801,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;
 }
@@ -2818,14 +2819,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;
 }
@@ -3072,12 +3073,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 c3d429a..84880a7 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -36,6 +36,8 @@
 struct drm_device;
 struct drm_mode_set;
 struct drm_framebuffer;
+struct drm_property;
+struct drm_object_properties;
 
 
 #define DRM_MODE_OBJECT_CRTC 0xcccccccc
@@ -50,6 +52,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 +460,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 +528,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 +572,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.9.1

Re: [PATCH 3/8] drm: create struct drm_object_properties and use it

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 568 bytes --]

On Thu, Mar 29, 2012 at 18:27, Paulo Zanoni <przanoni@gmail.com> wrote:

> 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 strucutre is referenced inside
>

*structure


> struct drm_mode_object, we will be able to deal with object properties
> without knowing the real type of the object.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1204 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 3/8] drm: create struct drm_object_properties and use it

[Ville Syrjälä]

On Thu, Mar 29, 2012 at 06:27:21PM -0300, Paulo Zanoni wrote:
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c3d429a..84880a7 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -36,6 +36,8 @@
>  struct drm_device;
>  struct drm_mode_set;
>  struct drm_framebuffer;
> +struct drm_property;

Unrelated change?

> +struct drm_object_properties;
>  
>  
>  #define DRM_MODE_OBJECT_CRTC 0xcccccccc
> @@ -50,6 +52,13 @@ struct drm_framebuffer;

-- 
Ville Syrjälä
Intel OTC

[PATCH 4/8] drm: add generic ioctls to get/set properties on any object

[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.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/drm_crtc.c |  180 ++++++++++++++++++++++++++++++++++++++++++++
 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, 213 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 8800830..12f93e4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2832,6 +2832,56 @@ 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\n");
+}
+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)
 {
@@ -3107,6 +3157,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 0b65fbc..18db86e 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -159,7 +159,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 34a7b89..8c818bc 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -718,6 +718,8 @@ struct drm_get_cap {
 #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 84880a7..ffa6259 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -904,6 +904,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);
@@ -918,6 +924,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 2a2acda..de5de2a 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -252,6 +252,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.9.1

Re: [PATCH 4/8] drm: add generic ioctls to get/set properties on any object

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 565 bytes --]

On Thu, Mar 29, 2012 at 18:27, Paulo Zanoni <przanoni@gmail.com> wrote:

> +       switch (arg_obj->type) {
> +       case DRM_MODE_OBJECT_CONNECTOR:
> +               ret = drm_mode_connector_set_obj_prop(arg_obj, property,
> +                                                     arg->value);
> +               break;
> +       }
>

<bikeshedding>
Perhaps we could add a debug message here for catch the unhandled types..
</bikeshedding>

But apart from that,
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1010 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 4/8] drm: add generic ioctls to get/set properties on any object

[Ville Syrjälä]

On Thu, Mar 29, 2012 at 06:27:22PM -0300, Paulo Zanoni wrote:
> 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.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c |  180 ++++++++++++++++++++++++++++++++++++++++++++
>  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, 213 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 8800830..12f93e4 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2832,6 +2832,56 @@ 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\n");
> +}
> +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)
>  {
> @@ -3107,6 +3157,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++)
> +		;

How about adding obj->properties.count to avoid having to count every
time?

> +
> +	/* 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 0b65fbc..18db86e 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -159,7 +159,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)

Just add the trailing comma to the last line as well. Otherwise we keep
on having ugly diffs when new ioctls are added.

>  };
>  
>  #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )

-- 
Ville Syrjälä
Intel OTC

[PATCH 5/8] drm: make the connector properties code use the object properties code

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/drm_crtc.c |   96 +++++--------------------------------------
 1 files changed, 12 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 12f93e4..b21bfcd 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2782,53 +2782,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\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);
 
@@ -3105,56 +3073,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.9.1

Re: [PATCH 5/8] drm: make the connector properties code use the object properties code

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 287 bytes --]

On Thu, Mar 29, 2012 at 18:27, Paulo Zanoni <przanoni@gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 752 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 6/8] drm: add CRTC properties

[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 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b21bfcd..95c7ab2 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -382,6 +382,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++;
 
@@ -3106,6 +3108,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)
 {
@@ -3208,6 +3225,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 ffa6259..32e9c51 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.9.1

[PATCH RFC 7/8] drm/i915: add 'rotation' CRTC property

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

This property is needed so we can inform the KVMr feature about our
current rotation: whenever we change the rotation, we should change
that property so that the KVMr knows that the screen is rotated.

How to reproduce the problem:
- on an AMT machine, enable KVM
- boot the machine, use xrandr to rotate the display
- use VNC to connect to the KVM
- try to use the mouse

v2: only create the property once
v3: adapt to the new drm code for object properties

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/i915_reg.h      |    5 +++
 drivers/gpu/drm/i915/intel_display.c |   66 ++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 0 deletions(-)

After talking to Rob we concluded that instead of having a value from 0-359 we
should have a bitmask (just like X's RR_Rotate_X stuff). This will allow not
only rotations, but also reflections. OTOH, rotations that are not 0, 90, 180 or
270 will not be allowed. If we ever need these arbitrary rotations, I think
we should create a property that matches X's transformation matrix and keep this
one existing (just like X).

Rob plans to implement "bit mask properties" support, so after his patch I'll
update this one to use a bit mask property.

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c0f19f5..98d7d74 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -779,6 +779,7 @@ typedef struct drm_i915_private {
 
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
+	struct drm_property *rotation_property;
 } drm_i915_private_t;
 
 enum hdmi_force_audio {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f59cd3a..ca99450 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2407,6 +2407,11 @@
 #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
 #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
 #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
+#define   PIPECONF_ROTATION_MASK	(3 << 14)
+#define   PIPECONF_ROTATION_0	(0 << 14)
+#define   PIPECONF_ROTATION_90	(1 << 14)
+#define   PIPECONF_ROTATION_180	(2 << 14)
+#define   PIPECONF_ROTATION_270	(3 << 14)
 #define   PIPECONF_BPP_MASK	(0x000000e0)
 #define   PIPECONF_BPP_8	(0<<5)
 #define   PIPECONF_BPP_10	(1<<5)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 72b292a..d621a54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7626,6 +7626,50 @@ static void intel_crtc_reset(struct drm_crtc *crtc)
 	intel_sanitize_modesetting(dev, intel_crtc->pipe, intel_crtc->plane);
 }
 
+static void intel_crtc_set_rotation(struct drm_crtc *crtc,
+				    uint64_t rotation)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int reg = PIPECONF(intel_crtc->pipe);
+	u32 val = I915_READ(reg);
+
+	val &= ~PIPECONF_ROTATION_MASK;
+
+	switch (rotation) {
+	case 0:
+		val |= PIPECONF_ROTATION_0;
+		break;
+	case 90:
+		val |= PIPECONF_ROTATION_90;
+		break;
+	case 180:
+		val |= PIPECONF_ROTATION_180;
+		break;
+	case 270:
+		val |= PIPECONF_ROTATION_270;
+		break;
+	default:
+		DRM_ERROR("Unsupported rotation: %Lu\n", rotation);
+		val |= PIPECONF_ROTATION_0;
+	}
+
+	I915_WRITE(reg, val);
+}
+
+static int intel_crtc_set_property(struct drm_crtc *crtc,
+				   struct drm_property *property,
+				   uint64_t val)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (property == dev_priv->rotation_property)
+		intel_crtc_set_rotation(crtc, val);
+	return 0;
+}
+
 static struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.dpms = intel_crtc_dpms,
 	.mode_fixup = intel_crtc_mode_fixup,
@@ -7644,8 +7688,27 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
+	.set_property = intel_crtc_set_property,
 };
 
+static void intel_attach_rotation_property(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property *prop;
+
+	prop = dev_priv->rotation_property;
+	if (prop == NULL) {
+		prop = drm_property_create_range(dev, 0, "rotation", 0, 359);
+		if (prop == NULL)
+			return;
+
+		dev_priv->rotation_property = prop;
+	}
+
+	drm_object_attach_property(&crtc->base, prop, 0);
+}
+
 static void intel_crtc_init(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
@@ -7665,6 +7728,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 		intel_crtc->lut_b[i] = i;
 	}
 
+	if (INTEL_INFO(dev)->gen >= 5)
+		intel_attach_rotation_property(&intel_crtc->base);
+
 	/* Swap pipes & planes for FBC on pre-965 */
 	intel_crtc->pipe = pipe;
 	intel_crtc->plane = pipe;
-- 
1.7.9.1

[PATCH 8/8] drm/i915: add overscan compensation CRTC properties

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

They're named "underscan hborder" and "underscan vborder". The
properties accept values from 0 to 100, where 0 is "don't compensate"
and 100 is "shrink the screen as much as possible" (not necessarily
100 pixels).

V2: Rename to "underscan hborder" and "underscan vborder" to match the
radeon properties. Note that we have a range of 0-100 (not pixels) and
radeon has a range of 0-128 (pixels). Fix compilation on 32 bit
systems.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    2 +
 drivers/gpu/drm/i915/intel_display.c |  117 +++++++++++++++++++++++++++++++++-
 2 files changed, 118 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 98d7d74..0373723 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -780,6 +780,8 @@ typedef struct drm_i915_private {
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
 	struct drm_property *rotation_property;
+	struct drm_property *underscan_hborder_property;
+	struct drm_property *underscan_vborder_property;
 } drm_i915_private_t;
 
 enum hdmi_force_audio {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d621a54..b80c508 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5621,6 +5621,84 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
 	return 120000;
 }
 
+/*
+ * The overscan compensation property (aka underscan property) has values from 0
+ * to 100, where 0 means that the compensation is disabled and 100 means the
+ * screen should shrink as much as possible. The current maximum supported value
+ * (from the specifications) is "src/dst < 1.125".
+ *
+ * In short:
+ * - if val == 0   -> dst = src
+ * - if val == 100 -> dst = src * 8/9
+ * - dst can't be odd
+ * - dst can't be < src * 8 / (double)9
+ * - so the formulae, not considering rounding, should be:
+ *   - dst = 9*src - prop*src/100 / 9
+ */
+static void ironlake_crtc_overscan_compensate(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+	uint64_t prop_x_64 = 0, prop_y_64 = 0;
+	uint32_t prop_x, prop_y;
+	int tot_x, tot_y, src_x, src_y, dst_x, dst_y, pos_x, pos_y;
+	u32 reg;
+
+	drm_object_property_get_value(&crtc->base,
+				      dev_priv->underscan_hborder_property,
+				      &prop_x_64);
+	drm_object_property_get_value(&crtc->base,
+				      dev_priv->underscan_vborder_property,
+				      &prop_y_64);
+
+	/* This is needed to avoid 64 bit division later. Values should be
+	 * between 0 and 100, so no problem. */
+	prop_x = prop_x_64;
+	prop_y = prop_y_64;
+
+	if (prop_x == 0 && prop_y == 0 &&
+	    !(dev_priv->pch_pf_size &&
+	      (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) || HAS_eDP))) {
+		I915_WRITE(PF_CTL(pipe), 0);
+		I915_WRITE(PF_WIN_POS(pipe), 0);
+		I915_WRITE(PF_WIN_SZ(pipe), 0);
+		return;
+	}
+
+	reg = I915_READ(HTOTAL(pipe));
+	tot_x = (reg & 0xFFF) + 1;
+	reg = I915_READ(VTOTAL(pipe));
+	tot_y = (reg & 0xFFF) + 1;
+	reg = I915_READ(PIPESRC(pipe));
+	src_x = ((reg >> 16) & 0xFFF) + 1;
+	src_y = (reg & 0xFFF) + 1;
+
+	dst_x = (src_x * 9 - src_x * prop_x / 100 + 8) / 9;
+	dst_x &= ~1;
+	if (dst_x < ((src_x * 8 + 8) / 9))
+		dst_x += 2;
+
+	dst_y = (src_y * 9 - src_y * prop_y / 100 + 8) / 9;
+	dst_y &= ~1;
+	if (dst_y < ((src_y * 8 + 8) / 9))
+		dst_y += 2;
+
+	pos_x = (tot_x - dst_x) / 2;
+	pos_y = (tot_y - dst_y) / 2;
+
+	if (pos_x == 1)
+		pos_x = 0;
+	reg = I915_READ(PIPECONF(pipe));
+	if ((reg & PIPECONF_INTERLACE_MASK) != PIPECONF_PROGRESSIVE)
+		pos_y &= ~1;
+
+	I915_WRITE(PF_CTL(pipe), PF_ENABLE | PF_FILTER_MED_3x3);
+	I915_WRITE(PF_WIN_POS(pipe), (pos_x << 16) | pos_y);
+	I915_WRITE(PF_WIN_SZ(pipe), (dst_x << 16) | dst_y);
+}
+
 static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 				  struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode,
@@ -6066,6 +6144,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_update_watermarks(dev);
 
+	ironlake_crtc_overscan_compensate(crtc);
+
 	return ret;
 }
 
@@ -7667,6 +7747,11 @@ static int intel_crtc_set_property(struct drm_crtc *crtc,
 
 	if (property == dev_priv->rotation_property)
 		intel_crtc_set_rotation(crtc, val);
+	if (property == dev_priv->underscan_hborder_property ||
+	    property == dev_priv->underscan_vborder_property) {
+		drm_object_property_set_value(&crtc->base, property, val);
+		ironlake_crtc_overscan_compensate(crtc);
+	}
 	return 0;
 }
 
@@ -7709,6 +7794,34 @@ static void intel_attach_rotation_property(struct drm_crtc *crtc)
 	drm_object_attach_property(&crtc->base, prop, 0);
 }
 
+static void intel_attach_underscan_properties(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_property *prop_h, *prop_v;
+
+	prop_h = dev_priv->underscan_hborder_property;
+	if (prop_h == NULL) {
+		prop_h = drm_property_create_range(dev, 0, "underscan hborder",
+						   0, 100);
+		if (prop_h != NULL)
+			dev_priv->underscan_hborder_property = prop_h;
+	}
+
+	prop_v = dev_priv->underscan_vborder_property;
+	if (prop_v == NULL) {
+		prop_v = drm_property_create_range(dev, 0, "underscan vborder",
+						   0, 100);
+		if (prop_v != NULL)
+			dev_priv->underscan_vborder_property = prop_v;
+	}
+
+	if (prop_h)
+		drm_object_attach_property(&crtc->base, prop_h, 0);
+	if (prop_v)
+		drm_object_attach_property(&crtc->base, prop_v, 0);
+}
+
 static void intel_crtc_init(struct drm_device *dev, int pipe)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
@@ -7728,8 +7841,10 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 		intel_crtc->lut_b[i] = i;
 	}
 
-	if (INTEL_INFO(dev)->gen >= 5)
+	if (INTEL_INFO(dev)->gen >= 5) {
 		intel_attach_rotation_property(&intel_crtc->base);
+		intel_attach_underscan_properties(&intel_crtc->base);
+	}
 
 	/* Swap pipes & planes for FBC on pre-965 */
 	intel_crtc->pipe = pipe;
-- 
1.7.9.1

Re: [PATCH 1/8] drm: add drm_property_change_is_valid

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 543 bytes --]

On Thu, Mar 29, 2012 at 18:27, Paulo Zanoni <przanoni@gmail.com> wrote:

> +       if (property->flags & DRM_MODE_PROP_RANGE) {
> +               if (value < property->values[0])
> +                       return false;
> +               if (value > property->values[1])
> +                       return false;
>

Those two checks could probably be combined into one (< values || > values)
for further simplification.

But other than this,
Reviewed-by: Eugeni Dodonov <eugein.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 972 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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