* [PATCH 2/5] drm: WARN() when drm_connector_attach_property fails
2012-03-20 14:48 [PATCH 1/5] drm: add drm_property_change_is_valid Paulo Zanoni
@ 2012-03-20 14:48 ` Paulo Zanoni
2012-03-20 14:48 ` [PATCH 3/5] drm: add CRTC properties Paulo Zanoni
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-20 14:48 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 | 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 1cadc11..6260fc3 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2659,7 +2659,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;
@@ -2668,13 +2668,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 2a0872c..21681fe 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -907,8 +907,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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/5] drm: add CRTC properties
2012-03-20 14:48 [PATCH 1/5] drm: add drm_property_change_is_valid Paulo Zanoni
2012-03-20 14:48 ` [PATCH 2/5] drm: WARN() when drm_connector_attach_property fails Paulo Zanoni
@ 2012-03-20 14:48 ` Paulo Zanoni
2012-03-20 15:00 ` Rob Clark
2012-03-20 15:09 ` Alex Deucher
2012-03-20 14:48 ` [PATCH 4/5] drm/i915: add 'rotation' CRTC property Paulo Zanoni
` (2 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-20 14:48 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Code based on the connector properties code.
Two new ioctls:
- DRM_IOCTL_MODE_CRTC_GETPROPERTIES
- DRM_IOCTL_MODE_CRTC_SETPROPERTY
The i915 driver needs this for the rotation and overscan compensation
properties. Other drivers might need this too.
v2: replace BUG_ON() for WARN(), fix bugs, add functions to get/set
the value
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 150 ++++++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_drv.c | 4 +-
include/drm/drm.h | 2 +
include/drm/drm_crtc.h | 28 ++++++++-
include/drm/drm_mode.h | 13 ++++
5 files changed, 195 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 6260fc3..df00c29 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2712,6 +2712,55 @@ int drm_connector_property_get_value(struct drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_property_get_value);
+void drm_crtc_attach_property(struct drm_crtc *crtc,
+ struct drm_property *property, uint64_t init_val)
+{
+ int i;
+
+ for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
+ if (crtc->property_ids[i] == 0) {
+ crtc->property_ids[i] = property->base.id;
+ crtc->property_values[i] = init_val;
+ return;
+ }
+ }
+
+ WARN(1, "Failed to attach crtc property\n");
+}
+EXPORT_SYMBOL(drm_crtc_attach_property);
+
+int drm_crtc_property_set_value(struct drm_crtc *crtc,
+ struct drm_property *property, uint64_t value)
+{
+ int i;
+
+ for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
+ if (crtc->property_ids[i] == property->base.id) {
+ crtc->property_values[i] = value;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL(drm_crtc_property_set_value);
+
+int drm_crtc_property_get_value(struct drm_crtc *crtc,
+ struct drm_property *property, uint64_t *val)
+{
+ int i;
+
+ for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
+ if (crtc->property_ids[i] == property->base.id) {
+ *val = crtc->property_values[i];
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL(drm_crtc_property_get_value);
+
int drm_mode_getproperty_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
@@ -2983,6 +3032,107 @@ out:
return ret;
}
+int drm_mode_crtc_get_properties_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_crtc_get_properties *arg = data;
+ struct drm_mode_object *obj;
+ struct drm_crtc *crtc;
+ 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->crtc_id, DRM_MODE_OBJECT_CRTC);
+ if (!obj) {
+ ret = -EINVAL;
+ goto out;
+ }
+ crtc = obj_to_crtc(obj);
+
+ for (props_count = 0; props_count < DRM_CRTC_MAX_PROPERTY &&
+ crtc->property_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(crtc->property_ids[i],
+ props_ptr + copied)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ if (put_user(crtc->property_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_crtc_set_property_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_crtc_set_property *arg = data;
+ struct drm_mode_object *obj;
+ struct drm_property *property;
+ struct drm_crtc *crtc;
+ 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, arg->crtc_id, DRM_MODE_OBJECT_CRTC);
+ if (!obj)
+ goto out;
+ crtc = obj_to_crtc(obj);
+
+ for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++)
+ if (crtc->property_ids[i] == arg->prop_id)
+ break;
+
+ if (i == DRM_CRTC_MAX_PROPERTY)
+ goto out;
+
+ obj = drm_mode_object_find(dev, arg->prop_id, DRM_MODE_OBJECT_PROPERTY);
+ if (!obj)
+ goto out;
+ property = obj_to_property(obj);
+
+ if (!drm_property_change_is_valid(property, arg->value))
+ goto out;
+
+ if (crtc->funcs->set_property)
+ ret = crtc->funcs->set_property(crtc, property, arg->value);
+ if (!ret) {
+ crtc->property_values[i] = arg->value;
+ }
+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 d166bd0..815e3a9 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_CRTC_GETPROPERTIES, drm_mode_crtc_get_properties_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_CRTC_SETPROPERTY, drm_mode_crtc_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..a397a93 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_CRTC_GETPROPERTIES DRM_IOWR(0xB9, struct drm_mode_crtc_get_properties)
+#define DRM_IOCTL_MODE_CRTC_SETPROPERTY DRM_IOWR(0xBA, struct drm_mode_crtc_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 21681fe..a2d660d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -283,6 +283,8 @@ struct drm_encoder;
struct drm_pending_vblank_event;
struct drm_plane;
+#define DRM_CRTC_MAX_PROPERTY 16
+
/**
* drm_crtc_funcs - control CRTCs for a given device
* @reset: reset CRTC after state has been invalidate (e.g. resume)
@@ -297,7 +299,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
@@ -341,6 +344,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);
};
/**
@@ -360,6 +366,8 @@ struct drm_crtc_funcs {
* @framedur_ns: precise line timing
* @pixeldur_ns: precise pixel timing
* @helper_private: mid-layer private data
+ * @property_ids: property tracking for this CRTC
+ * @property_values: property values
*
* Each CRTC may have one or more connectors associated with it. This structure
* allows the CRTC to be controlled.
@@ -395,6 +403,9 @@ struct drm_crtc {
/* if you are using the helper */
void *helper_private;
+
+ u32 property_ids[DRM_CRTC_MAX_PROPERTY];
+ uint64_t property_values[DRM_CRTC_MAX_PROPERTY];
};
@@ -895,6 +906,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_crtc_property_set_value(struct drm_crtc *crtc,
+ struct drm_property *property,
+ uint64_t value);
+extern int drm_crtc_property_get_value(struct drm_crtc *crtc,
+ 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);
@@ -909,6 +926,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_crtc_attach_property(struct drm_crtc *crtc,
+ 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,
@@ -1019,6 +1039,12 @@ 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_crtc_get_properties_ioctl(struct drm_device *dev,
+ void *data,
+ struct drm_file *file_priv);
+extern int drm_mode_crtc_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..890b930 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -252,6 +252,19 @@ struct drm_mode_connector_set_property {
__u32 connector_id;
};
+struct drm_mode_crtc_get_properties {
+ __u64 props_ptr;
+ __u64 prop_values_ptr;
+ __u32 count_props;
+ __u32 crtc_id;
+};
+
+struct drm_mode_crtc_set_property {
+ __u64 value;
+ __u32 prop_id;
+ __u32 crtc_id;
+};
+
struct drm_mode_get_blob {
__u32 blob_id;
__u32 length;
--
1.7.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] drm: add CRTC properties
2012-03-20 14:48 ` [PATCH 3/5] drm: add CRTC properties Paulo Zanoni
@ 2012-03-20 15:00 ` Rob Clark
2012-03-20 15:09 ` Alex Deucher
1 sibling, 0 replies; 12+ messages in thread
From: Rob Clark @ 2012-03-20 15:00 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel
On Tue, Mar 20, 2012 at 9:48 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Code based on the connector properties code.
>
> Two new ioctls:
> - DRM_IOCTL_MODE_CRTC_GETPROPERTIES
> - DRM_IOCTL_MODE_CRTC_SETPROPERTY
Just fyi, this is useful to me too.. I need a way to set rotation on a
crtc or a drm plane. So possibly we also need PLANE_{SET,GET}PROPERTY
ioctls? Or maybe the ioctl could just apply to either a plane or crtc
depending on the id passed? (Not sure if that is too weird sounding?)
BR,
-R
> The i915 driver needs this for the rotation and overscan compensation
> properties. Other drivers might need this too.
>
> v2: replace BUG_ON() for WARN(), fix bugs, add functions to get/set
> the value
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/drm_crtc.c | 150 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_drv.c | 4 +-
> include/drm/drm.h | 2 +
> include/drm/drm_crtc.h | 28 ++++++++-
> include/drm/drm_mode.h | 13 ++++
> 5 files changed, 195 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 6260fc3..df00c29 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2712,6 +2712,55 @@ int drm_connector_property_get_value(struct drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_connector_property_get_value);
>
> +void drm_crtc_attach_property(struct drm_crtc *crtc,
> + struct drm_property *property, uint64_t init_val)
> +{
> + int i;
> +
> + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> + if (crtc->property_ids[i] == 0) {
> + crtc->property_ids[i] = property->base.id;
> + crtc->property_values[i] = init_val;
> + return;
> + }
> + }
> +
> + WARN(1, "Failed to attach crtc property\n");
> +}
> +EXPORT_SYMBOL(drm_crtc_attach_property);
> +
> +int drm_crtc_property_set_value(struct drm_crtc *crtc,
> + struct drm_property *property, uint64_t value)
> +{
> + int i;
> +
> + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> + if (crtc->property_ids[i] == property->base.id) {
> + crtc->property_values[i] = value;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_crtc_property_set_value);
> +
> +int drm_crtc_property_get_value(struct drm_crtc *crtc,
> + struct drm_property *property, uint64_t *val)
> +{
> + int i;
> +
> + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> + if (crtc->property_ids[i] == property->base.id) {
> + *val = crtc->property_values[i];
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_crtc_property_get_value);
> +
> int drm_mode_getproperty_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv)
> {
> @@ -2983,6 +3032,107 @@ out:
> return ret;
> }
>
> +int drm_mode_crtc_get_properties_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file_priv)
> +{
> + struct drm_mode_crtc_get_properties *arg = data;
> + struct drm_mode_object *obj;
> + struct drm_crtc *crtc;
> + 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->crtc_id, DRM_MODE_OBJECT_CRTC);
> + if (!obj) {
> + ret = -EINVAL;
> + goto out;
> + }
> + crtc = obj_to_crtc(obj);
> +
> + for (props_count = 0; props_count < DRM_CRTC_MAX_PROPERTY &&
> + crtc->property_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(crtc->property_ids[i],
> + props_ptr + copied)) {
> + ret = -EFAULT;
> + goto out;
> + }
> + if (put_user(crtc->property_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_crtc_set_property_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file_priv)
> +{
> + struct drm_mode_crtc_set_property *arg = data;
> + struct drm_mode_object *obj;
> + struct drm_property *property;
> + struct drm_crtc *crtc;
> + 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, arg->crtc_id, DRM_MODE_OBJECT_CRTC);
> + if (!obj)
> + goto out;
> + crtc = obj_to_crtc(obj);
> +
> + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++)
> + if (crtc->property_ids[i] == arg->prop_id)
> + break;
> +
> + if (i == DRM_CRTC_MAX_PROPERTY)
> + goto out;
> +
> + obj = drm_mode_object_find(dev, arg->prop_id, DRM_MODE_OBJECT_PROPERTY);
> + if (!obj)
> + goto out;
> + property = obj_to_property(obj);
> +
> + if (!drm_property_change_is_valid(property, arg->value))
> + goto out;
> +
> + if (crtc->funcs->set_property)
> + ret = crtc->funcs->set_property(crtc, property, arg->value);
> + if (!ret) {
> + crtc->property_values[i] = arg->value;
> + }
> +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 d166bd0..815e3a9 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_CRTC_GETPROPERTIES, drm_mode_crtc_get_properties_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CRTC_SETPROPERTY, drm_mode_crtc_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..a397a93 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_CRTC_GETPROPERTIES DRM_IOWR(0xB9, struct drm_mode_crtc_get_properties)
> +#define DRM_IOCTL_MODE_CRTC_SETPROPERTY DRM_IOWR(0xBA, struct drm_mode_crtc_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 21681fe..a2d660d 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -283,6 +283,8 @@ struct drm_encoder;
> struct drm_pending_vblank_event;
> struct drm_plane;
>
> +#define DRM_CRTC_MAX_PROPERTY 16
> +
> /**
> * drm_crtc_funcs - control CRTCs for a given device
> * @reset: reset CRTC after state has been invalidate (e.g. resume)
> @@ -297,7 +299,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
> @@ -341,6 +344,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);
> };
>
> /**
> @@ -360,6 +366,8 @@ struct drm_crtc_funcs {
> * @framedur_ns: precise line timing
> * @pixeldur_ns: precise pixel timing
> * @helper_private: mid-layer private data
> + * @property_ids: property tracking for this CRTC
> + * @property_values: property values
> *
> * Each CRTC may have one or more connectors associated with it. This structure
> * allows the CRTC to be controlled.
> @@ -395,6 +403,9 @@ struct drm_crtc {
>
> /* if you are using the helper */
> void *helper_private;
> +
> + u32 property_ids[DRM_CRTC_MAX_PROPERTY];
> + uint64_t property_values[DRM_CRTC_MAX_PROPERTY];
> };
>
>
> @@ -895,6 +906,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_crtc_property_set_value(struct drm_crtc *crtc,
> + struct drm_property *property,
> + uint64_t value);
> +extern int drm_crtc_property_get_value(struct drm_crtc *crtc,
> + 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);
> @@ -909,6 +926,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_crtc_attach_property(struct drm_crtc *crtc,
> + 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,
> @@ -1019,6 +1039,12 @@ 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_crtc_get_properties_ioctl(struct drm_device *dev,
> + void *data,
> + struct drm_file *file_priv);
> +extern int drm_mode_crtc_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..890b930 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -252,6 +252,19 @@ struct drm_mode_connector_set_property {
> __u32 connector_id;
> };
>
> +struct drm_mode_crtc_get_properties {
> + __u64 props_ptr;
> + __u64 prop_values_ptr;
> + __u32 count_props;
> + __u32 crtc_id;
> +};
> +
> +struct drm_mode_crtc_set_property {
> + __u64 value;
> + __u32 prop_id;
> + __u32 crtc_id;
> +};
> +
> struct drm_mode_get_blob {
> __u32 blob_id;
> __u32 length;
> --
> 1.7.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] drm: add CRTC properties
2012-03-20 14:48 ` [PATCH 3/5] drm: add CRTC properties Paulo Zanoni
2012-03-20 15:00 ` Rob Clark
@ 2012-03-20 15:09 ` Alex Deucher
2012-03-20 15:48 ` Daniel Vetter
1 sibling, 1 reply; 12+ messages in thread
From: Alex Deucher @ 2012-03-20 15:09 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel
On Tue, Mar 20, 2012 at 10:48 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Code based on the connector properties code.
>
> Two new ioctls:
> - DRM_IOCTL_MODE_CRTC_GETPROPERTIES
> - DRM_IOCTL_MODE_CRTC_SETPROPERTY
>
> The i915 driver needs this for the rotation and overscan compensation
> properties. Other drivers might need this too.
>
> v2: replace BUG_ON() for WARN(), fix bugs, add functions to get/set
> the value
>
Is there any reason why these can't just be exposed as connector
properties? While the hw features you are exposing are technically
part of the crtc block, so are most of the existing connector
properties (scalers, etc.). Plus, while the scalers/transforms are
part of the crtc hw, they only really make sense on certain
connectors. E.g., underscan isn't particularly useful on non-TMDS
capable connectors.
Alex
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/drm_crtc.c | 150 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_drv.c | 4 +-
> include/drm/drm.h | 2 +
> include/drm/drm_crtc.h | 28 ++++++++-
> include/drm/drm_mode.h | 13 ++++
> 5 files changed, 195 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 6260fc3..df00c29 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2712,6 +2712,55 @@ int drm_connector_property_get_value(struct drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_connector_property_get_value);
>
> +void drm_crtc_attach_property(struct drm_crtc *crtc,
> + struct drm_property *property, uint64_t init_val)
> +{
> + int i;
> +
> + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> + if (crtc->property_ids[i] == 0) {
> + crtc->property_ids[i] = property->base.id;
> + crtc->property_values[i] = init_val;
> + return;
> + }
> + }
> +
> + WARN(1, "Failed to attach crtc property\n");
> +}
> +EXPORT_SYMBOL(drm_crtc_attach_property);
> +
> +int drm_crtc_property_set_value(struct drm_crtc *crtc,
> + struct drm_property *property, uint64_t value)
> +{
> + int i;
> +
> + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> + if (crtc->property_ids[i] == property->base.id) {
> + crtc->property_values[i] = value;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_crtc_property_set_value);
> +
> +int drm_crtc_property_get_value(struct drm_crtc *crtc,
> + struct drm_property *property, uint64_t *val)
> +{
> + int i;
> +
> + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> + if (crtc->property_ids[i] == property->base.id) {
> + *val = crtc->property_values[i];
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_crtc_property_get_value);
> +
> int drm_mode_getproperty_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv)
> {
> @@ -2983,6 +3032,107 @@ out:
> return ret;
> }
>
> +int drm_mode_crtc_get_properties_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file_priv)
> +{
> + struct drm_mode_crtc_get_properties *arg = data;
> + struct drm_mode_object *obj;
> + struct drm_crtc *crtc;
> + 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->crtc_id, DRM_MODE_OBJECT_CRTC);
> + if (!obj) {
> + ret = -EINVAL;
> + goto out;
> + }
> + crtc = obj_to_crtc(obj);
> +
> + for (props_count = 0; props_count < DRM_CRTC_MAX_PROPERTY &&
> + crtc->property_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(crtc->property_ids[i],
> + props_ptr + copied)) {
> + ret = -EFAULT;
> + goto out;
> + }
> + if (put_user(crtc->property_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_crtc_set_property_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file_priv)
> +{
> + struct drm_mode_crtc_set_property *arg = data;
> + struct drm_mode_object *obj;
> + struct drm_property *property;
> + struct drm_crtc *crtc;
> + 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, arg->crtc_id, DRM_MODE_OBJECT_CRTC);
> + if (!obj)
> + goto out;
> + crtc = obj_to_crtc(obj);
> +
> + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++)
> + if (crtc->property_ids[i] == arg->prop_id)
> + break;
> +
> + if (i == DRM_CRTC_MAX_PROPERTY)
> + goto out;
> +
> + obj = drm_mode_object_find(dev, arg->prop_id, DRM_MODE_OBJECT_PROPERTY);
> + if (!obj)
> + goto out;
> + property = obj_to_property(obj);
> +
> + if (!drm_property_change_is_valid(property, arg->value))
> + goto out;
> +
> + if (crtc->funcs->set_property)
> + ret = crtc->funcs->set_property(crtc, property, arg->value);
> + if (!ret) {
> + crtc->property_values[i] = arg->value;
> + }
> +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 d166bd0..815e3a9 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_CRTC_GETPROPERTIES, drm_mode_crtc_get_properties_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CRTC_SETPROPERTY, drm_mode_crtc_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..a397a93 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_CRTC_GETPROPERTIES DRM_IOWR(0xB9, struct drm_mode_crtc_get_properties)
> +#define DRM_IOCTL_MODE_CRTC_SETPROPERTY DRM_IOWR(0xBA, struct drm_mode_crtc_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 21681fe..a2d660d 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -283,6 +283,8 @@ struct drm_encoder;
> struct drm_pending_vblank_event;
> struct drm_plane;
>
> +#define DRM_CRTC_MAX_PROPERTY 16
> +
> /**
> * drm_crtc_funcs - control CRTCs for a given device
> * @reset: reset CRTC after state has been invalidate (e.g. resume)
> @@ -297,7 +299,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
> @@ -341,6 +344,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);
> };
>
> /**
> @@ -360,6 +366,8 @@ struct drm_crtc_funcs {
> * @framedur_ns: precise line timing
> * @pixeldur_ns: precise pixel timing
> * @helper_private: mid-layer private data
> + * @property_ids: property tracking for this CRTC
> + * @property_values: property values
> *
> * Each CRTC may have one or more connectors associated with it. This structure
> * allows the CRTC to be controlled.
> @@ -395,6 +403,9 @@ struct drm_crtc {
>
> /* if you are using the helper */
> void *helper_private;
> +
> + u32 property_ids[DRM_CRTC_MAX_PROPERTY];
> + uint64_t property_values[DRM_CRTC_MAX_PROPERTY];
> };
>
>
> @@ -895,6 +906,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_crtc_property_set_value(struct drm_crtc *crtc,
> + struct drm_property *property,
> + uint64_t value);
> +extern int drm_crtc_property_get_value(struct drm_crtc *crtc,
> + 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);
> @@ -909,6 +926,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_crtc_attach_property(struct drm_crtc *crtc,
> + 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,
> @@ -1019,6 +1039,12 @@ 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_crtc_get_properties_ioctl(struct drm_device *dev,
> + void *data,
> + struct drm_file *file_priv);
> +extern int drm_mode_crtc_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..890b930 100644
> --- a/include/drm/drm_mode.h
> +++ b/include/drm/drm_mode.h
> @@ -252,6 +252,19 @@ struct drm_mode_connector_set_property {
> __u32 connector_id;
> };
>
> +struct drm_mode_crtc_get_properties {
> + __u64 props_ptr;
> + __u64 prop_values_ptr;
> + __u32 count_props;
> + __u32 crtc_id;
> +};
> +
> +struct drm_mode_crtc_set_property {
> + __u64 value;
> + __u32 prop_id;
> + __u32 crtc_id;
> +};
> +
> struct drm_mode_get_blob {
> __u32 blob_id;
> __u32 length;
> --
> 1.7.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] drm: add CRTC properties
2012-03-20 15:09 ` Alex Deucher
@ 2012-03-20 15:48 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2012-03-20 15:48 UTC (permalink / raw)
To: Alex Deucher; +Cc: Paulo Zanoni, dri-devel
On Tue, Mar 20, 2012 at 11:09:42AM -0400, Alex Deucher wrote:
> On Tue, Mar 20, 2012 at 10:48 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > Code based on the connector properties code.
> >
> > Two new ioctls:
> > - DRM_IOCTL_MODE_CRTC_GETPROPERTIES
> > - DRM_IOCTL_MODE_CRTC_SETPROPERTY
> >
> > The i915 driver needs this for the rotation and overscan compensation
> > properties. Other drivers might need this too.
> >
> > v2: replace BUG_ON() for WARN(), fix bugs, add functions to get/set
> > the value
> >
>
> Is there any reason why these can't just be exposed as connector
> properties? While the hw features you are exposing are technically
> part of the crtc block, so are most of the existing connector
> properties (scalers, etc.). Plus, while the scalers/transforms are
> part of the crtc hw, they only really make sense on certain
> connectors. E.g., underscan isn't particularly useful on non-TMDS
> capable connectors.
The reasons for this pretty much boil down to that we have quite a few
properties that belong to the crtc. Currently we abuse connector
properties for that because no one uses cloning, but imo that's not a
Great Idea. Other examples than rotation are blending/Z-order when using
planes, special options for color conversion (probably only on planes),
...
-Daniel
>
> Alex
>
>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/drm_crtc.c | 150 ++++++++++++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/drm_drv.c | 4 +-
> > include/drm/drm.h | 2 +
> > include/drm/drm_crtc.h | 28 ++++++++-
> > include/drm/drm_mode.h | 13 ++++
> > 5 files changed, 195 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 6260fc3..df00c29 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -2712,6 +2712,55 @@ int drm_connector_property_get_value(struct drm_connector *connector,
> > }
> > EXPORT_SYMBOL(drm_connector_property_get_value);
> >
> > +void drm_crtc_attach_property(struct drm_crtc *crtc,
> > + struct drm_property *property, uint64_t init_val)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> > + if (crtc->property_ids[i] == 0) {
> > + crtc->property_ids[i] = property->base.id;
> > + crtc->property_values[i] = init_val;
> > + return;
> > + }
> > + }
> > +
> > + WARN(1, "Failed to attach crtc property\n");
> > +}
> > +EXPORT_SYMBOL(drm_crtc_attach_property);
> > +
> > +int drm_crtc_property_set_value(struct drm_crtc *crtc,
> > + struct drm_property *property, uint64_t value)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> > + if (crtc->property_ids[i] == property->base.id) {
> > + crtc->property_values[i] = value;
> > + return 0;
> > + }
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +EXPORT_SYMBOL(drm_crtc_property_set_value);
> > +
> > +int drm_crtc_property_get_value(struct drm_crtc *crtc,
> > + struct drm_property *property, uint64_t *val)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++) {
> > + if (crtc->property_ids[i] == property->base.id) {
> > + *val = crtc->property_values[i];
> > + return 0;
> > + }
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +EXPORT_SYMBOL(drm_crtc_property_get_value);
> > +
> > int drm_mode_getproperty_ioctl(struct drm_device *dev,
> > void *data, struct drm_file *file_priv)
> > {
> > @@ -2983,6 +3032,107 @@ out:
> > return ret;
> > }
> >
> > +int drm_mode_crtc_get_properties_ioctl(struct drm_device *dev, void *data,
> > + struct drm_file *file_priv)
> > +{
> > + struct drm_mode_crtc_get_properties *arg = data;
> > + struct drm_mode_object *obj;
> > + struct drm_crtc *crtc;
> > + 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->crtc_id, DRM_MODE_OBJECT_CRTC);
> > + if (!obj) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + crtc = obj_to_crtc(obj);
> > +
> > + for (props_count = 0; props_count < DRM_CRTC_MAX_PROPERTY &&
> > + crtc->property_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(crtc->property_ids[i],
> > + props_ptr + copied)) {
> > + ret = -EFAULT;
> > + goto out;
> > + }
> > + if (put_user(crtc->property_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_crtc_set_property_ioctl(struct drm_device *dev, void *data,
> > + struct drm_file *file_priv)
> > +{
> > + struct drm_mode_crtc_set_property *arg = data;
> > + struct drm_mode_object *obj;
> > + struct drm_property *property;
> > + struct drm_crtc *crtc;
> > + 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, arg->crtc_id, DRM_MODE_OBJECT_CRTC);
> > + if (!obj)
> > + goto out;
> > + crtc = obj_to_crtc(obj);
> > +
> > + for (i = 0; i < DRM_CRTC_MAX_PROPERTY; i++)
> > + if (crtc->property_ids[i] == arg->prop_id)
> > + break;
> > +
> > + if (i == DRM_CRTC_MAX_PROPERTY)
> > + goto out;
> > +
> > + obj = drm_mode_object_find(dev, arg->prop_id, DRM_MODE_OBJECT_PROPERTY);
> > + if (!obj)
> > + goto out;
> > + property = obj_to_property(obj);
> > +
> > + if (!drm_property_change_is_valid(property, arg->value))
> > + goto out;
> > +
> > + if (crtc->funcs->set_property)
> > + ret = crtc->funcs->set_property(crtc, property, arg->value);
> > + if (!ret) {
> > + crtc->property_values[i] = arg->value;
> > + }
> > +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 d166bd0..815e3a9 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_CRTC_GETPROPERTIES, drm_mode_crtc_get_properties_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> > + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CRTC_SETPROPERTY, drm_mode_crtc_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..a397a93 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_CRTC_GETPROPERTIES DRM_IOWR(0xB9, struct drm_mode_crtc_get_properties)
> > +#define DRM_IOCTL_MODE_CRTC_SETPROPERTY DRM_IOWR(0xBA, struct drm_mode_crtc_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 21681fe..a2d660d 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -283,6 +283,8 @@ struct drm_encoder;
> > struct drm_pending_vblank_event;
> > struct drm_plane;
> >
> > +#define DRM_CRTC_MAX_PROPERTY 16
> > +
> > /**
> > * drm_crtc_funcs - control CRTCs for a given device
> > * @reset: reset CRTC after state has been invalidate (e.g. resume)
> > @@ -297,7 +299,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
> > @@ -341,6 +344,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);
> > };
> >
> > /**
> > @@ -360,6 +366,8 @@ struct drm_crtc_funcs {
> > * @framedur_ns: precise line timing
> > * @pixeldur_ns: precise pixel timing
> > * @helper_private: mid-layer private data
> > + * @property_ids: property tracking for this CRTC
> > + * @property_values: property values
> > *
> > * Each CRTC may have one or more connectors associated with it. This structure
> > * allows the CRTC to be controlled.
> > @@ -395,6 +403,9 @@ struct drm_crtc {
> >
> > /* if you are using the helper */
> > void *helper_private;
> > +
> > + u32 property_ids[DRM_CRTC_MAX_PROPERTY];
> > + uint64_t property_values[DRM_CRTC_MAX_PROPERTY];
> > };
> >
> >
> > @@ -895,6 +906,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_crtc_property_set_value(struct drm_crtc *crtc,
> > + struct drm_property *property,
> > + uint64_t value);
> > +extern int drm_crtc_property_get_value(struct drm_crtc *crtc,
> > + 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);
> > @@ -909,6 +926,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_crtc_attach_property(struct drm_crtc *crtc,
> > + 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,
> > @@ -1019,6 +1039,12 @@ 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_crtc_get_properties_ioctl(struct drm_device *dev,
> > + void *data,
> > + struct drm_file *file_priv);
> > +extern int drm_mode_crtc_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..890b930 100644
> > --- a/include/drm/drm_mode.h
> > +++ b/include/drm/drm_mode.h
> > @@ -252,6 +252,19 @@ struct drm_mode_connector_set_property {
> > __u32 connector_id;
> > };
> >
> > +struct drm_mode_crtc_get_properties {
> > + __u64 props_ptr;
> > + __u64 prop_values_ptr;
> > + __u32 count_props;
> > + __u32 crtc_id;
> > +};
> > +
> > +struct drm_mode_crtc_set_property {
> > + __u64 value;
> > + __u32 prop_id;
> > + __u32 crtc_id;
> > +};
> > +
> > struct drm_mode_get_blob {
> > __u32 blob_id;
> > __u32 length;
> > --
> > 1.7.9.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] drm/i915: add 'rotation' CRTC property
2012-03-20 14:48 [PATCH 1/5] drm: add drm_property_change_is_valid Paulo Zanoni
2012-03-20 14:48 ` [PATCH 2/5] drm: WARN() when drm_connector_attach_property fails Paulo Zanoni
2012-03-20 14:48 ` [PATCH 3/5] drm: add CRTC properties Paulo Zanoni
@ 2012-03-20 14:48 ` Paulo Zanoni
2012-03-20 16:48 ` Marcus Lorentzon
2012-03-20 14:48 ` [PATCH 5/5] drm/i915: add overscan compensation CRTC properties Paulo Zanoni
2012-03-20 15:37 ` [PATCH 1/5] drm: add drm_property_change_is_valid Ville Syrjälä
4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-20 14:48 UTC (permalink / raw)
To: dri-devel; +Cc: 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
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(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e7a00b7..7994c4f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -780,6 +780,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 52a06be..79e8b12 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 683002fb..4842de8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7625,6 +7625,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,
@@ -7643,8 +7687,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_crtc_attach_property(crtc, prop, 0);
+}
+
static void intel_crtc_init(struct drm_device *dev, int pipe)
{
drm_i915_private_t *dev_priv = dev->dev_private;
@@ -7664,6 +7727,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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/5] drm/i915: add 'rotation' CRTC property
2012-03-20 14:48 ` [PATCH 4/5] drm/i915: add 'rotation' CRTC property Paulo Zanoni
@ 2012-03-20 16:48 ` Marcus Lorentzon
2012-03-20 18:49 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Marcus Lorentzon @ 2012-03-20 16:48 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel@lists.freedesktop.org
Hi Paulo,
thanks for a quick response posting the patches.
In my use of CRTC rotation properties, I need the change to be
synchronized with modeset. Your implementation activate the property
change immediately instead of staging it for next modeset. Do you think
it is ok for different drivers to behave differently when properties are
set? I can't allow glitches in the output when changing the rotation in
one frame and flipping the framebuffer to one of correct size and
content in another. This would mean I will have to stage the setting and
apply it at next modeset (different behavior). Or do I just have to wait
until we have an atomic modeset that can also set properties (Jesse can
you post your latest proposal ...)?
Do you think you could export the call registering the rotation
property? That way other drivers can reuse it and we don't risk multiple
properties with different ranges etc for the same logical property (like
400 mil degrees or 0-3, 1-4, UR,UD,CCW,CW ...). So some sort of standard
property create.
Then it would also be nice to define some rules in KMS of what should be
modeset params and what should be properties. Modeset params seems hard
to add (struct change) and properties seems very loosely defined and
device specific. Having something in between would be nice (standard and
easy to add), to make it easier to add things like plane z-order
(instead of driver specific ioctls or properties).
Maybe a new "atomic" modeset with just a list of {object_id,
property_id, value} making all params into properties and modeset more
dynamic. It would also solve the atomic modset, even with device
specific properties and future features.
Sorry for multiple topics, but they are sort of related.
/BR
/Marcus
On 03/20/2012 03:48 PM, Paulo Zanoni wrote:
> 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
>
> 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(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e7a00b7..7994c4f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -780,6 +780,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 52a06be..79e8b12 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 683002fb..4842de8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7625,6 +7625,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,
> @@ -7643,8 +7687,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_crtc_attach_property(crtc, prop, 0);
> +}
> +
> static void intel_crtc_init(struct drm_device *dev, int pipe)
> {
> drm_i915_private_t *dev_priv = dev->dev_private;
> @@ -7664,6 +7727,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;
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 4/5] drm/i915: add 'rotation' CRTC property
2012-03-20 16:48 ` Marcus Lorentzon
@ 2012-03-20 18:49 ` Ville Syrjälä
0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2012-03-20 18:49 UTC (permalink / raw)
To: Marcus Lorentzon; +Cc: Paulo Zanoni, dri-devel@lists.freedesktop.org
On Tue, Mar 20, 2012 at 05:48:14PM +0100, Marcus Lorentzon wrote:
> Then it would also be nice to define some rules in KMS of what should be
> modeset params and what should be properties. Modeset params seems hard
> to add (struct change) and properties seems very loosely defined and
> device specific. Having something in between would be nice (standard and
> easy to add), to make it easier to add things like plane z-order
> (instead of driver specific ioctls or properties).
> Maybe a new "atomic" modeset with just a list of {object_id,
> property_id, value} making all params into properties and modeset more
> dynamic. It would also solve the atomic modset, even with device
> specific properties and future features.
Great minds think alike ;) Exactly what I've suggested a few times
before. Unfortunately I haven't had the time to do any prototyping
yet.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] drm/i915: add overscan compensation CRTC properties
2012-03-20 14:48 [PATCH 1/5] drm: add drm_property_change_is_valid Paulo Zanoni
` (2 preceding siblings ...)
2012-03-20 14:48 ` [PATCH 4/5] drm/i915: add 'rotation' CRTC property Paulo Zanoni
@ 2012-03-20 14:48 ` Paulo Zanoni
2012-03-20 15:00 ` Alex Deucher
2012-03-20 15:37 ` [PATCH 1/5] drm: add drm_property_change_is_valid Ville Syrjälä
4 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2012-03-20 14:48 UTC (permalink / raw)
To: dri-devel; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
They're named "underscan x" and "underscan y". The properties accept
values from 0 to 100, where 0 is "don't compensate" and 100 is "shrink
the screen as much as possible".
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 | 110 +++++++++++++++++++++++++++++++++-
2 files changed, 111 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7994c4f..fb9062d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -781,6 +781,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 *x_underscan_property;
+ struct drm_property *y_underscan_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 4842de8..b36572d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5620,6 +5620,77 @@ 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 = 0, prop_y = 0;
+ int tot_x, tot_y, src_x, src_y, dst_x, dst_y, pos_x, pos_y;
+ u32 reg;
+
+ drm_crtc_property_get_value(crtc, dev_priv->x_underscan_property,
+ &prop_x);
+ drm_crtc_property_get_value(crtc, dev_priv->y_underscan_property,
+ &prop_y);
+
+ 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,
@@ -6065,6 +6136,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
intel_update_watermarks(dev);
+ ironlake_crtc_overscan_compensate(crtc);
+
return ret;
}
@@ -7666,6 +7739,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->x_underscan_property ||
+ property == dev_priv->y_underscan_property) {
+ drm_crtc_property_set_value(crtc, property, val);
+ ironlake_crtc_overscan_compensate(crtc);
+ }
return 0;
}
@@ -7708,6 +7786,34 @@ static void intel_attach_rotation_property(struct drm_crtc *crtc)
drm_crtc_attach_property(crtc, 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_x, *prop_y;
+
+ prop_x = dev_priv->x_underscan_property;
+ if (prop_x == NULL) {
+ prop_x = drm_property_create_range(dev, 0, "underscan x",
+ 0, 100);
+ if (prop_x != NULL)
+ dev_priv->x_underscan_property = prop_x;
+ }
+
+ prop_y = dev_priv->y_underscan_property;
+ if (prop_y == NULL) {
+ prop_y = drm_property_create_range(dev, 0, "underscan y",
+ 0, 100);
+ if (prop_y != NULL)
+ dev_priv->y_underscan_property = prop_y;
+ }
+
+ if (prop_x)
+ drm_crtc_attach_property(crtc, prop_x, 0);
+ if (prop_y)
+ drm_crtc_attach_property(crtc, prop_y, 0);
+}
+
static void intel_crtc_init(struct drm_device *dev, int pipe)
{
drm_i915_private_t *dev_priv = dev->dev_private;
@@ -7727,8 +7833,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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 5/5] drm/i915: add overscan compensation CRTC properties
2012-03-20 14:48 ` [PATCH 5/5] drm/i915: add overscan compensation CRTC properties Paulo Zanoni
@ 2012-03-20 15:00 ` Alex Deucher
0 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2012-03-20 15:00 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel
On Tue, Mar 20, 2012 at 10:48 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> They're named "underscan x" and "underscan y". The properties accept
> values from 0 to 100, where 0 is "don't compensate" and 100 is "shrink
> the screen as much as possible".
FWIW, the radeon driver already exposes something similar. It might
be worthwhile to try and use consistent naming and property options.
We have an "underscan" property:
static struct drm_prop_enum_list radeon_underscan_enum_list[] =
{ { UNDERSCAN_OFF, "off" },
{ UNDERSCAN_ON, "on" },
{ UNDERSCAN_AUTO, "auto" },
};
"off" disables underscan
"on" forces it on
"auto" tries to enable/disable underscan automatically based on EDID
information. RIght now we just check if the monitor is HDMI and the
mode is an standard HD mode, but IIRC, there is also a CEA extension
block that is supposed to tell you whether the monitor is overscanning
or not.
Then we have "underscan hborder" and "underscan vborder" to adjust the
amount of underscan. See radeon_display.c in the kernel.
Alex
>
> 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 | 110 +++++++++++++++++++++++++++++++++-
> 2 files changed, 111 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7994c4f..fb9062d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -781,6 +781,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 *x_underscan_property;
> + struct drm_property *y_underscan_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 4842de8..b36572d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5620,6 +5620,77 @@ 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 = 0, prop_y = 0;
> + int tot_x, tot_y, src_x, src_y, dst_x, dst_y, pos_x, pos_y;
> + u32 reg;
> +
> + drm_crtc_property_get_value(crtc, dev_priv->x_underscan_property,
> + &prop_x);
> + drm_crtc_property_get_value(crtc, dev_priv->y_underscan_property,
> + &prop_y);
> +
> + 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,
> @@ -6065,6 +6136,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>
> intel_update_watermarks(dev);
>
> + ironlake_crtc_overscan_compensate(crtc);
> +
> return ret;
> }
>
> @@ -7666,6 +7739,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->x_underscan_property ||
> + property == dev_priv->y_underscan_property) {
> + drm_crtc_property_set_value(crtc, property, val);
> + ironlake_crtc_overscan_compensate(crtc);
> + }
> return 0;
> }
>
> @@ -7708,6 +7786,34 @@ static void intel_attach_rotation_property(struct drm_crtc *crtc)
> drm_crtc_attach_property(crtc, 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_x, *prop_y;
> +
> + prop_x = dev_priv->x_underscan_property;
> + if (prop_x == NULL) {
> + prop_x = drm_property_create_range(dev, 0, "underscan x",
> + 0, 100);
> + if (prop_x != NULL)
> + dev_priv->x_underscan_property = prop_x;
> + }
> +
> + prop_y = dev_priv->y_underscan_property;
> + if (prop_y == NULL) {
> + prop_y = drm_property_create_range(dev, 0, "underscan y",
> + 0, 100);
> + if (prop_y != NULL)
> + dev_priv->y_underscan_property = prop_y;
> + }
> +
> + if (prop_x)
> + drm_crtc_attach_property(crtc, prop_x, 0);
> + if (prop_y)
> + drm_crtc_attach_property(crtc, prop_y, 0);
> +}
> +
> static void intel_crtc_init(struct drm_device *dev, int pipe)
> {
> drm_i915_private_t *dev_priv = dev->dev_private;
> @@ -7727,8 +7833,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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/5] drm: add drm_property_change_is_valid
2012-03-20 14:48 [PATCH 1/5] drm: add drm_property_change_is_valid Paulo Zanoni
` (3 preceding siblings ...)
2012-03-20 14:48 ` [PATCH 5/5] drm/i915: add overscan compensation CRTC properties Paulo Zanoni
@ 2012-03-20 15:37 ` Ville Syrjälä
4 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2012-03-20 15:37 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Paulo Zanoni, dri-devel
On Tue, Mar 20, 2012 at 11:48:28AM -0300, Paulo Zanoni wrote:
> 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.
>
> 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 6fdaf6f..1cadc11 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2910,6 +2910,26 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_mode_connector_update_edid_property);
>
> +static int drm_property_change_is_valid(struct drm_property *property,
> + __u64 value)
static bool ...
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 12+ messages in thread