Second version of the atomic mode setting work. Still very much work in progress.
I decided that I can't afford to fight the drm_crtc_helper architecture due to the sheer amount of driver code depending on it. So I changed the code to do things in way that more closely matches drm_crtc_helper.
Next I'll be moving the buffer pinning to happen before any hw state is clobbered. This will avoid having to do actual rollbacks when pinning fails. And a similar treatment needs to be done to the PLL calculations (those should be done before buffer pinning).
I didn't re-post all the i915 specific bits I have in my tree since those didn't really change. I pushed the whole work to the drm_atomic_2 branch at https://gitorious.org/vsyrjala/linux.
From: Ville Syrjälä ville.syrjala@linux.intel.com
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 8 +++++--- include/drm/drm_crtc.h | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 6dafb99..ce0f446 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3044,8 +3044,8 @@ done: return ret; }
-static struct drm_property_blob *drm_property_create_blob(struct drm_device *dev, int length, - void *data) +struct drm_property_blob *drm_property_create_blob(struct drm_device *dev, int length, + void *data) { struct drm_property_blob *blob; int ret; @@ -3070,14 +3070,16 @@ static struct drm_property_blob *drm_property_create_blob(struct drm_device *dev list_add_tail(&blob->head, &dev->mode_config.property_blob_list); return blob; } +EXPORT_SYMBOL(drm_property_create_blob);
-static void drm_property_destroy_blob(struct drm_device *dev, +void drm_property_destroy_blob(struct drm_device *dev, struct drm_property_blob *blob) { drm_mode_object_put(dev, &blob->base); list_del(&blob->head); kfree(blob); } +EXPORT_SYMBOL(drm_property_destroy_blob);
int drm_mode_getblob_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 14bc68d..0a2ebb4 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -952,6 +952,10 @@ struct drm_property *drm_property_create_range(struct drm_device *dev, int flags const char *name, uint64_t min, uint64_t max); extern void drm_property_destroy(struct drm_device *dev, struct drm_property *property); +extern struct drm_property_blob *drm_property_create_blob(struct drm_device *dev, + int length, void *data); +extern void drm_property_destroy_blob(struct drm_device *dev, + struct drm_property_blob *blob); extern int drm_property_add_enum(struct drm_property *property, int index, uint64_t value, const char *name); extern int drm_mode_create_dvi_i_properties(struct drm_device *dev);
From: Ville Syrjälä ville.syrjala@linux.intel.com
Treat a range property as signed when the unsigned minimum value is larger than the unsigned maximum value.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index ce0f446..cc18ae6 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3143,14 +3143,25 @@ 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, +static bool range_property_is_signed(const struct drm_property *property) +{ + return property->values[0] > property->values[1]; +} + +static bool drm_property_change_is_valid(const struct drm_property *property, uint64_t value) { if (property->flags & DRM_MODE_PROP_IMMUTABLE) return false; if (property->flags & DRM_MODE_PROP_RANGE) { - if (value < property->values[0] || value > property->values[1]) - return false; + if (range_property_is_signed(property)) { + if ((int64_t)value < (int64_t)property->values[0] || + (int64_t)value > (int64_t)property->values[1]) + return false; + } else { + if (value < property->values[0] || value > property->values[1]) + return false; + } return true; } else if (property->flags & DRM_MODE_PROP_BITMASK) { int i;
From: Ville Syrjälä ville.syrjala@linux.intel.com
To avoid having to pass object types from userspace for atomic mode setting ioctl, allow drm_mode_object_find() to look up an object of any type. This will only work as long as the all object types share the ID space.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 3 ++- include/drm/drm_crtc.h | 1 + 2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index cc18ae6..9945e4b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -268,7 +268,8 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
mutex_lock(&dev->mode_config.idr_mutex); obj = idr_find(&dev->mode_config.crtc_idr, id); - if (!obj || (obj->type != type) || (obj->id != id)) + if (!obj || (type != DRM_MODE_OBJECT_ANY && obj->type != type) || + (obj->id != id)) obj = NULL; mutex_unlock(&dev->mode_config.idr_mutex);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 0a2ebb4..99bd489 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -47,6 +47,7 @@ struct drm_object_properties; #define DRM_MODE_OBJECT_FB 0xfbfbfbfb #define DRM_MODE_OBJECT_BLOB 0xbbbbbbbb #define DRM_MODE_OBJECT_PLANE 0xeeeeeeee +#define DRM_MODE_OBJECT_ANY 0
struct drm_mode_object { uint32_t id;
From: Ville Syrjälä ville.syrjala@linux.intel.com
--- drivers/gpu/drm/drm_crtc_helper.c | 5 +++-- include/drm/drm_crtc_helper.h | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 3252e70..c4f7e7c 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -279,8 +279,8 @@ EXPORT_SYMBOL(drm_helper_disable_unused_functions); * * Return false if @encoder can't be driven by @crtc, true otherwise. */ -static bool drm_encoder_crtc_ok(struct drm_encoder *encoder, - struct drm_crtc *crtc) +bool drm_encoder_crtc_ok(struct drm_encoder *encoder, + struct drm_crtc *crtc) { struct drm_device *dev; struct drm_crtc *tmp; @@ -300,6 +300,7 @@ static bool drm_encoder_crtc_ok(struct drm_encoder *encoder, return true; return false; } +EXPORT_SYMBOL(drm_encoder_crtc_ok);
/* * Check the CRTC we're going to map each output to vs. its current diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index 7988e55..79bc76b 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -166,4 +166,7 @@ extern void drm_helper_hpd_irq_event(struct drm_device *dev); extern void drm_kms_helper_poll_disable(struct drm_device *dev); extern void drm_kms_helper_poll_enable(struct drm_device *dev);
+extern bool drm_encoder_crtc_ok(struct drm_encoder *encoder, + struct drm_crtc *crtc); + #endif
From: Ville Syrjälä ville.syrjala@linux.intel.com
--- drivers/gpu/drm/drm_crtc_helper.c | 3 ++- include/drm/drm_crtc_helper.h | 1 + 2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index c4f7e7c..ebbfcc6 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -307,7 +307,7 @@ EXPORT_SYMBOL(drm_encoder_crtc_ok); * CRTC. If they don't match, we have to disable the output and the CRTC * since the driver will have to re-route things. */ -static void +void drm_crtc_prepare_encoders(struct drm_device *dev) { struct drm_encoder_helper_funcs *encoder_funcs; @@ -324,6 +324,7 @@ drm_crtc_prepare_encoders(struct drm_device *dev) drm_encoder_disable(encoder); } } +EXPORT_SYMBOL(drm_crtc_prepare_encoders);
/** * drm_crtc_set_mode - set a mode diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index 79bc76b..c88202d 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -168,5 +168,6 @@ extern void drm_kms_helper_poll_enable(struct drm_device *dev);
extern bool drm_encoder_crtc_ok(struct drm_encoder *encoder, struct drm_crtc *crtc); +extern void drm_crtc_prepare_encoders(struct drm_device *dev);
#endif
From: Ville Syrjälä ville.syrjala@linux.intel.com
Refactor the code to check whether an object has a specific property to a new function.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 9945e4b..958a4b0 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3301,6 +3301,19 @@ out: return ret; }
+static bool object_has_prop(const struct drm_mode_object *obj, u32 prop_id) +{ + int i; + + if (!obj->properties) + return false; + + for (i = 0; i < obj->properties->count; i++) + if (obj->properties->ids[i] == prop_id) + return true; + return false; +} + int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -3309,7 +3322,6 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, 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; @@ -3322,11 +3334,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, if (!arg_obj->properties) goto out;
- for (i = 0; i < arg_obj->properties->count; i++) - if (arg_obj->properties->ids[i] == arg->prop_id) - break; - - if (i == arg_obj->properties->count) + if (!object_has_prop(arg_obj, arg->prop_id)) goto out;
prop_obj = drm_mode_object_find(dev, arg->prop_id,
From: Ville Syrjälä ville.syrjala@linux.intel.com
Add a new blob property MODE_IDS to connectors. This property contains a list a mode object IDs attached to the connector (either probed modes, or user attached modes).
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 77 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_crtc_helper.c | 1 + include/drm/drm_crtc.h | 2 + 3 files changed, 80 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 958a4b0..cfef9de 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -508,6 +508,9 @@ int drm_connector_init(struct drm_device *dev, drm_connector_attach_property(connector, dev->mode_config.dpms_property, 0);
+ drm_connector_attach_property(connector, + dev->mode_config.mode_ids_property, 0); + out: mutex_unlock(&dev->mode_config.mutex);
@@ -713,6 +716,7 @@ static int drm_mode_create_standard_connector_properties(struct drm_device *dev) { struct drm_property *edid; struct drm_property *dpms; + struct drm_property *mode_ids;
/* * Standard properties (apply to all connectors) @@ -727,6 +731,11 @@ static int drm_mode_create_standard_connector_properties(struct drm_device *dev) ARRAY_SIZE(drm_dpms_enum_list)); dev->mode_config.dpms_property = dpms;
+ mode_ids = drm_property_create(dev, DRM_MODE_PROP_BLOB | + DRM_MODE_PROP_IMMUTABLE, + "MODE_IDS", 0); + dev->mode_config.mode_ids_property = mode_ids; + return 0; }
@@ -2648,6 +2657,10 @@ int drm_mode_attachmode_ioctl(struct drm_device *dev, }
drm_mode_attachmode(dev, connector, mode); + + ret = drm_mode_connector_update_mode_ids_property(connector); + if (ret) + drm_mode_remove(connector, mode); out: mutex_unlock(&dev->mode_config.mutex); return ret; @@ -3144,6 +3157,70 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, } EXPORT_SYMBOL(drm_mode_connector_update_edid_property);
+int drm_mode_connector_update_mode_ids_property(struct drm_connector *connector) +{ + struct drm_device *dev = connector->dev; + struct drm_property_blob *blob = NULL; + struct drm_property_blob *old_blob = NULL; + struct drm_display_mode *mode; + uint64_t value; + int i = 0; + int ret; + + ret = drm_connector_property_get_value(connector, + dev->mode_config.mode_ids_property, + &value); + if (ret) + return ret; + + if (value) { + struct drm_mode_object *obj = drm_mode_object_find(dev, value, DRM_MODE_OBJECT_BLOB); + if (!obj) + return -ENOENT; + + old_blob = obj_to_blob(obj); + } + + list_for_each_entry(mode, &connector->modes, head) + i++; + list_for_each_entry(mode, &connector->user_modes, head) + i++; + + if (i) { + uint32_t *mode_ids = kcalloc(i, sizeof mode_ids[0], GFP_KERNEL); + if (!mode_ids) + return -ENOMEM; + + i = 0; + list_for_each_entry(mode, &connector->modes, head) + mode_ids[i++] = mode->base.id; + list_for_each_entry(mode, &connector->user_modes, head) + mode_ids[i++] = mode->base.id; + + blob = drm_property_create_blob(dev, i * sizeof mode_ids[0], mode_ids); + + kfree(mode_ids); + + if (!blob) + return -ENOMEM; + } + + ret = drm_connector_property_set_value(connector, + dev->mode_config.mode_ids_property, + blob ? blob->base.id : 0); + if (ret) { + if (blob) + drm_property_destroy_blob(dev, blob); + return ret; + } + + if (old_blob) + drm_property_destroy_blob(dev, old_blob); + + return ret; +} +EXPORT_SYMBOL(drm_mode_connector_update_mode_ids_property); + static bool range_property_is_signed(const struct drm_property *property) { return property->values[0] > property->values[1]; diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index ebbfcc6..bc24c0e 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -131,6 +131,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, goto prune;
drm_mode_connector_list_update(connector); + drm_mode_connector_update_mode_ids_property(connector);
if (maxX && maxY) drm_mode_validate_size(dev, &connector->modes, maxX, diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 99bd489..c8bfdf1 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -790,6 +790,7 @@ struct drm_mode_config { struct list_head property_blob_list; struct drm_property *edid_property; struct drm_property *dpms_property; + struct drm_property *mode_ids_property;
/* DVI-I properties */ struct drm_property *dvi_i_subconnector_property; @@ -910,6 +911,7 @@ extern void drm_mode_set_crtcinfo(struct drm_display_mode *p, extern void drm_mode_connector_list_update(struct drm_connector *connector); extern int drm_mode_connector_update_edid_property(struct drm_connector *connector, struct edid *edid); +extern int drm_mode_connector_update_mode_ids_property(struct drm_connector *connector); extern int drm_connector_property_set_value(struct drm_connector *connector, struct drm_property *property, uint64_t value);
From: Ville Syrjälä ville.syrjala@linux.intel.com
First draft.
The ioctl simply takes a list of object IDs and property IDs and their values. For setting values to blob properties, the property value indicates the length of the data, and the actual data is passed via another blob pointer.
Detailed error reporting is missing, as is completion event support.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 133 ++++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_drv.c | 1 + include/drm/drm.h | 1 + include/drm/drmP.h | 2 + include/drm/drm_crtc.h | 12 ++++ include/drm/drm_mode.h | 14 +++++ 6 files changed, 163 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index cfef9de..35f8882 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4201,3 +4201,136 @@ int drm_calc_vscale(struct drm_region *src, struct drm_region *dst, return vscale; } EXPORT_SYMBOL(drm_calc_vscale); + +int drm_mode_atomic_ioctl(struct drm_device *dev, + void *data, struct drm_file *file_priv) +{ + struct drm_mode_atomic *arg = data; + uint32_t __user *objs_ptr = (uint32_t __user *)(unsigned long)(arg->objs_ptr); + uint32_t __user *count_props_ptr = (uint32_t __user *)(unsigned long)(arg->count_props_ptr); + uint32_t __user *props_ptr = (uint32_t __user *)(unsigned long)(arg->props_ptr); + uint64_t __user *prop_values_ptr = (uint64_t __user *)(unsigned long)(arg->prop_values_ptr); + uint64_t __user *blob_values_ptr = (uint64_t __user *)(unsigned long)(arg->blob_values_ptr); + unsigned int copied_objs = 0; + unsigned int copied_props = 0; + unsigned int copied_blobs = 0; + void *state; + int ret = 0; + unsigned int i, j; + + if (!dev->driver->atomic_funcs || + !dev->driver->atomic_funcs->begin || + !dev->driver->atomic_funcs->set || + !dev->driver->atomic_funcs->check || + !dev->driver->atomic_funcs->commit || + !dev->driver->atomic_funcs->end) + return -ENOSYS; + + if (arg->flags & ~DRM_ATOMIC_TEST_ONLY) + return -EINVAL; + + mutex_lock(&dev->mode_config.mutex); + + state = dev->driver->atomic_funcs->begin(dev, arg->flags); + if (IS_ERR(state)) { + ret = PTR_ERR(state); + goto unlock; + } + + for (i = 0; i < arg->count_objs; i++) { + uint32_t obj_id, count_props; + struct drm_mode_object *obj; + + if (get_user(obj_id, objs_ptr + copied_objs)) { + ret = -EFAULT; + goto out; + } + + obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY); + if (!obj || !obj->properties) { + ret = -ENOENT; + goto out; + } + + if (get_user(count_props, count_props_ptr + copied_objs)) { + ret = -EFAULT; + goto out; + } + + copied_objs++; + + for (j = 0; j < count_props; j++) { + uint32_t prop_id; + uint64_t prop_value; + struct drm_mode_object *prop_obj; + struct drm_property *prop; + void *blob_data = NULL; + + if (get_user(prop_id, props_ptr + copied_props)) { + ret = -EFAULT; + goto out; + } + + if (!object_has_prop(obj, prop_id)) { + ret = -EINVAL; + goto out; + } + + prop_obj = drm_mode_object_find(dev, prop_id, DRM_MODE_OBJECT_PROPERTY); + if (!prop_obj) { + ret = -ENOENT; + goto out; + } + prop = obj_to_property(prop_obj); + + if (get_user(prop_value, prop_values_ptr + copied_props)) { + ret = -EFAULT; + goto out; + } + + if (!drm_property_change_is_valid(prop, prop_value)) { + ret = -EINVAL; + goto out; + } + + if (prop->flags & DRM_MODE_PROP_BLOB && prop_value) { + blob_data = kmalloc(prop_value, GFP_KERNEL); + if (!blob_data) { + ret = -ENOMEM; + goto out; + } + + if (copy_from_user(blob_data, blob_values_ptr + copied_blobs, prop_value)) { + kfree(blob_data); + ret = -EFAULT; + goto out; + } + + copied_blobs++; + } + + /* The driver will be in charge of blob_data from now on. */ + ret = dev->driver->atomic_funcs->set(dev, state, obj, prop, prop_value, blob_data); + if (ret) + goto out; + + copied_props++; + } + } + + ret = dev->driver->atomic_funcs->check(dev, state); + if (ret) + goto out; + + if (arg->flags & DRM_ATOMIC_TEST_ONLY) + goto out; + + ret = dev->driver->atomic_funcs->commit(dev, state); + + out: + dev->driver->atomic_funcs->end(dev, state); + unlock: + mutex_unlock(&dev->mode_config.mutex); + + return ret; +} diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 946bd91..6ca936a 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -166,6 +166,7 @@ static struct drm_ioctl_desc drm_ioctls[] = { 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), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATOMIC, drm_mode_atomic_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 e51035a..b179169 100644 --- a/include/drm/drm.h +++ b/include/drm/drm.h @@ -732,6 +732,7 @@ struct drm_prime_handle { #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) +#define DRM_IOCTL_MODE_ATOMIC DRM_IOWR(0xBB, struct drm_mode_atomic)
/** * Device specific ioctls should only be in their respective headers diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 31ad880..7bff96d 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -964,6 +964,8 @@ struct drm_driver {
/* List of devices hanging off this driver */ struct list_head device_list; + + const struct drm_atomic_funcs *atomic_funcs; };
#define DRM_MINOR_UNASSIGNED 0 diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c8bfdf1..a27b70f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1065,6 +1065,8 @@ 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 int drm_mode_atomic_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); @@ -1101,4 +1103,14 @@ extern int drm_calc_hscale(struct drm_region *src, struct drm_region *dst, extern int drm_calc_vscale(struct drm_region *src, struct drm_region *dst, int min_vscale, int max_vscale);
+struct drm_atomic_funcs { + void *(*begin)(struct drm_device *dev, uint32_t flags); + int (*set)(struct drm_device *dev, void *state, + struct drm_mode_object *obj, struct drm_property *prop, + uint64_t value, void *blob_data); + int (*check)(struct drm_device *dev, void *state); + int (*commit)(struct drm_device *dev, void *state); + void (*end)(struct drm_device *dev, void *state); +}; + #endif /* __DRM_CRTC_H__ */ diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h index 5581980..ed776b4 100644 --- a/include/drm/drm_mode.h +++ b/include/drm/drm_mode.h @@ -459,4 +459,18 @@ struct drm_mode_destroy_dumb { uint32_t handle; };
+ +#define DRM_ATOMIC_TEST_ONLY (1<<0) + +/* FIXME come up with some sane error reporting mechanism? */ +struct drm_mode_atomic { + __u32 flags; + __u32 count_objs; + __u64 objs_ptr; + __u64 count_props_ptr; + __u64 props_ptr; + __u64 prop_values_ptr; + __u64 blob_values_ptr; +}; + #endif
On Wed, Jun 27, 2012 at 5:24 AM, ville.syrjala@linux.intel.com wrote:
Hmm, I'm thinking we may need to define a bit in the way of semantics.. ie. I think we really do want things like enabling/disabling a plane to be asynchronous (after doing a DRM_ATOMIC_TEST_ONLY call), otherwise performance will suck when switching a window surface to/from a plane. But maybe we should define that another DRM_IOCTL_MODE_ATOMIC cannot come until after some sort of event back to userspace?
DRM_IOCTL_MODE_ATOMIC_SET? DRM_IOCTL_MODE_SET_ATOMIC? Seems like there should somehow be a "SET" in the name..
BR, -R
On Fri, Aug 31, 2012 at 05:47:13PM -0500, Rob Clark wrote:
I've discussed things with Kristian Høgsberg (and also Jesse now that he's back from sabbatical) a lot and we've concluded on one big thing:
We should separate the issue of modeset on mutliple crtcs (usually called atomic modeset on irc) from a (usually called nuclear) pageflip on a single crtc that exchanges a bunch of things (framebufferrs, plane parameters, other properties, cursor position, ...).
The reason is that these things solve in large parts separate problems:
- atomic modeset is first and foremost a solution to exposing constraints on global configurations due to shared resources like plls, link lanes (e.g. on many intel chipset if you enable multiple outputs you can use the full bw of the single output config) and similar things. The solution is to tell the kernel the entire config and let the kernel decide how to best make it possible with the hw it has (maybe even re-assigning crtcs internally to give the display pipe with the most bandwidth to the crtc with the highest dotclock*bpp requirement). To be able to expose all this to userspace Jesse proposed a test_flag to check configurations without actually changing anything in the hw state. This way a modeset GUI could grey out everything that won't work (since describing the actual hw constraints is too complex, and users don't really care anyway in most cases why something doesn't work).
- a modeset also usually can take a few vblanks, since establishing new clocks and locking them in the various stages of the pipeline can take a while. And since modeset configuration happens seldomly, we imo can simply specify that a modeset always happens synchronously.
- Pageflip on the _must_ happen on a precise vblank, and be fully asynchronous, always. Furthermore we want a timestamp to know when the pageflip actually happened. Again, due to funny hw constraints we might need a test_flag so that a compositor can figure out whether it can use a plane for a given surface. Compared to the atomic modeset test-flag we have a few complications: - We don't want to walk the plane configuration space in the test mode for every frame. Hence the compositor will just try the current configuration (maybe with some surfaces moved around and a few properties changed). If a given plane config doesn't work out, the compositor could just fall back to gpu rendering and try to come up with a new optimal configuration for the next frame. - There's the issue that some changes require more than one vblank to take effect due to some prep work (e.g. switching a plane to a different crtc). My idea here is that the kernel could return a special "transient -EINVAL" for this case indicating to the compositor that it can't do this configuration for the next vblank, but it has set up things so that eventually (in a few vblanks) the sprite will be available on that crtc. The userspace compositor should then just keep on retrying for each frame with this configuration (until it either works or the kernel returns a "definit -EINVAL" indicating that something changed and it can't do this configuration). That way we should be able to support any steady-state plane/fb/cursor/whatever configuration that the hw can, without forcing the compositor to miss a frame. Obviously, these prepare steps _must_ _not_ result in a rouge plane eventually appearing ;-)
- Also a rather practical one: The lack of nuclear pageflip is the reason that Wayland/Weston can't use sprites atm (since set_plane is can be synchronous and can't be synced with a pageflip). Kristian therefore wants the atomic modeset (which I think is much more invasive) not to stall the nuclear pageflip support. And the implementation effort is imo really a big difference: For i915.ko I expect the nuclear pageflip to mostly boil down to wiring up async plane/cursor updates through the latch register (with keeping all the other setup code around since we only allow at most one outstanding pageflip per crtc currently anyway) and wiring that up with the existing crtc pageflip handler. atomic modeset otoh requires an invasive rewrite of the driver code to properly handle these shared resources (and support the test mode). The beginning of that is happening with the modeset rework, but that's just the first step.
I.e. I'm voting to reduce the scope of this ioctl a bit in the naming of gaining simpler&clearer semantics. In any case I don't see the big&scary work in the ioctl itself, but the actual low-level hw implementation in drivers (imo the crtc helper framework is holy inadequate for this safe for the simplest of drivers) and in testing the ioctl interfaces exhaustively.
Especially the testing is very important, since the entire design hinges on the test-flag to make complex configuration constraints discoverable at run-time. I think we need a few tools that random-walk the configuration space and check whether any configuration that returns -EINVAL in the test mode actually fails (and vice-versa), maybe excluding any other failures in the real modeset like -EIO (e.g. due to some stupid dp link failing to properly lock onto the stream).
Cheers, Daniel
On Sat, Sep 1, 2012 at 6:12 AM, Daniel Vetter daniel@ffwll.ch wrote:
yeah, nuclear-pageflip would be associated only w/ a single crtc. Actually I was kinda assuming atomic-modeset was too.. ie. moving a plane from one crtc to another would be two ioctls, one to remove it from first crtc, one to add to the other. Although maybe one big ioctl is better.
I do think there is a bit of a grey area between the two. Ie. enabling/disabling a crtc, or changing what encoder->connector it is hooked to might be taking several vblank cycles, and is a relatively infrequent operation so ok to block. Enabling/disabling a plane could be much more frequent and should not block. Returning EINVAL or EBUSY in the transient case where it isn't ready quite yet seems like a good approach.. EBUSY might be better for "it might succeed if you try again in next vblank" and EINVAL for "it won't ever work with current setup, don't bother trying again".
Infrequent, can be slow: + enabling/disabling a new output
Semi-frequent, should not block but -EBUSY ok: + enabling/disabling a plane + resizing a plane
More frequent, should not block, should not need extra "test" ioctl step, but should just work + page flip, update plane position
So for that middle group, I'm a bit fuzzy on where those operations should fit in..
Hmm, I was thinking Ville's atomic fxns would be useful, but maybe I can still bits from that and try and put nuclear-pageflip beneath atomic-modeset in the stack.
BR, -R
On Sat, Sep 1, 2012 at 5:58 PM, Rob Clark robdclark@gmail.com wrote:
My idea is that the nuclear pageflip should support those since they only affect a single crtc. The compositor would simply assume it will work out, and the kernel can then succeed, or return either the transient or definite -EINVAL if it doesn't work out. In the later case the compositor can retry in the next frame (and fall back to gpu compositing for the current one).
[snip]
At least in i915 I expect that the atomic modeset and the nuclear pageflip would take rather different paths. I'm not even sure whether we should support enabling planes in the amotic modeset (userspace can always composite the first frame after a modeset with the gpu) and handle enabling planes with only the nuclear pageflip.
I guess we /could/ add a few things to the crtc helper code to facilitate drivers using them to support atomic modeset. I'm thinking of a simple ->check callback at the beginning, which checks whether the global configuration can be supported. This checking would be in addition to the ->mode_fixup checking we already have in place at the encoder/crtc layer.
Then we could add a simple atomic_modeset implementation for using these interfaces.
For the nuclear pageflip I guess we'll just add a new crtc interface (not in the helper func pointer) which gets all the new parameters and let drivers sort things out. Since the flip needs to happen synced to the same vblank for everything, I don't see how we could compose this without some in-depth hw knowledge (which only the driver has) in some generic framework. And we do not yet support pageflip on planes (at least not in full glory with drm events and timestamps) anyway.
Cheers, Daniel
On Sat, Sep 1, 2012 at 11:56 AM, Daniel Vetter daniel@ffwll.ch wrote:
hmm, but this would imply that the nuclear pageflip must also have a 'test' step.. because the compositor needs to know whether or not it will work, *then* kick rendering, and *then* issue the actual pageflip.
But OTOH, some simple things like just flipping or perhaps flipping+moving should not need a test step. I guess userspace could just know that certain operations don't need a test step. Seems a bit weird, though.
hmm, ok, yeah.. I guess we could just make nuclear_page_flip a fxnptr on the crtc funcs.
I suppose I don't mind one way or another about the enabling/disabling planes. It seems ok to render first frame w/ gpu, so if it simplifies things I guess it is ok.
Right, I was thinking to start without helper.. it is easy enough to add helpers later if there are some common patterns emerging between the different drivers.
BR, -R
From: Ville Syrjälä ville.syrjala@linux.intel.com
Split the update_plane() codepath into two separate steps. The first step checkis and clips the plane, and the second step actually commits the changes to the hardware. This allows the atomic modesetting code to perform all checks before clobering hardware state.
The update_plane() hook is reduced to a thin wrapper calling both check and commit functions.
Buffer (un)pinning is still being performed in the commit step. This needs to be changed as well, so that the atomic modesetting code can try to pin all new buffers before touching the hardware.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_drv.h | 15 +- drivers/gpu/drm/i915/intel_sprite.c | 372 ++++++++++++++++++++++------------- 2 files changed, 249 insertions(+), 138 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 08fa52d..3e9506c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -187,6 +187,15 @@ struct intel_crtc { struct intel_pch_pll *pch_pll; };
+struct intel_plane_coords { + /* disabled or fully clipped? */ + bool visible; + /* coordinates clipped against pipe dimensions */ + int32_t crtc_x, crtc_y; + uint32_t crtc_w, crtc_h; + uint32_t src_x, src_y, src_w, src_h; +}; + struct intel_plane { struct drm_plane base; enum pipe pipe; @@ -196,11 +205,7 @@ struct intel_plane { u32 lut_r[1024], lut_g[1024], lut_b[1024]; void (*update_plane)(struct drm_plane *plane, struct drm_framebuffer *fb, - struct drm_i915_gem_object *obj, - int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t x, uint32_t y, - uint32_t src_w, uint32_t src_h); + const struct intel_plane_coords *clip); void (*disable_plane)(struct drm_plane *plane); int (*update_colorkey)(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index e2abae6..f2cdb05 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -36,16 +36,173 @@ #include "i915_drm.h" #include "i915_drv.h"
+static bool +format_is_yuv(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_YUYV: + case DRM_FORMAT_UYVY: + case DRM_FORMAT_VYUY: + case DRM_FORMAT_YVYU: + return true; + default: + return false; + } +} + +static void intel_clip_plane(const struct drm_plane *plane, + const struct drm_crtc *crtc, + const struct drm_framebuffer *fb, + struct intel_plane_coords *coords) +{ + const struct intel_plane *intel_plane = to_intel_plane(plane); + const struct drm_display_mode *mode = &crtc->mode; + int hscale, vscale; + struct drm_region src = { + .x1 = coords->src_x, + .x2 = coords->src_x + coords->src_w, + .y1 = coords->src_y, + .y2 = coords->src_y + coords->src_h, + }; + struct drm_region dst = { + .x1 = coords->crtc_x, + .x2 = coords->crtc_x + coords->crtc_w, + .y1 = coords->crtc_y, + .y2 = coords->crtc_y + coords->crtc_h, + }; + const struct drm_region clip = { + .x2 = mode->hdisplay, + .y2 = mode->vdisplay, + }; + + hscale = drm_calc_hscale(&src, &dst, 1, intel_plane->max_downscale << 16); + vscale = drm_calc_vscale(&src, &dst, 1, intel_plane->max_downscale << 16); + + coords->visible = drm_region_clip_scaled(&src, &dst, &clip, hscale, vscale); + + coords->crtc_x = dst.x1; + coords->crtc_y = dst.y1; + coords->crtc_w = drm_region_width(&dst); + coords->crtc_h = drm_region_height(&dst); + + /* HW doesn't seem to like smaller sprite, even when scaling */ + /* FIXME return an error instead? */ + if (coords->crtc_w < 3 || coords->crtc_h < 3) + coords->visible = false; + + /* + * Hardware doesn't handle subpixel coordinates. + * Round to nearest (macro)pixel boundary. + */ + if (format_is_yuv(fb->pixel_format)) { + coords->src_x = ((src.x1 + 0x10000) >> 17) << 1; + coords->src_w = (((src.x2 + 0x10000) >> 17) << 1) - coords->src_x; + } else { + coords->src_x = (src.x1 + 0x8000) >> 16; + coords->src_w = ((src.x2 + 0x8000) >> 16) - coords->src_x; + } + coords->src_y = (src.y1 + 0x8000) >> 16; + coords->src_h = ((src.y2 + 0x8000) >> 16) - coords->src_y; + + /* Account for minimum source size when scaling */ + if (coords->visible && (coords->src_w != coords->crtc_w || coords->src_h != coords->crtc_h)) { + unsigned int min_w = format_is_yuv(fb->pixel_format) ? 4 : 3; + + if (coords->src_w < min_w) { + coords->src_w = min_w; + if (coords->src_x > fb->width - coords->src_w) + coords->src_x = fb->width - coords->src_w; + } + + /* FIXME interlacing */ + if (coords->src_h < 3) { + coords->src_h = 3; + if (coords->src_y > fb->height - coords->src_h) + coords->src_y = fb->height - coords->src_h; + } + } +} + +int intel_check_plane(const struct drm_plane *plane, + const struct drm_crtc *crtc, + const struct drm_framebuffer *fb, + struct intel_plane_coords *coords) +{ + const struct intel_plane *intel_plane = to_intel_plane(plane); + + if (fb) { + /* FIXME copy-pasted. refactor common code in drm_crtc.c */ + uint32_t fb_width = fb->width << 16; + uint32_t fb_height = fb->height << 16; + int i; + + for (i = 0; i < plane->format_count; i++) { + if (plane->format_types[i] == fb->pixel_format) + break; + } + if (i == plane->format_count) + return -EINVAL; + + if (coords->src_w > fb_width || + coords->src_x > fb_width - coords->src_w || + coords->src_h > fb_height || + coords->src_y > fb_height - coords->src_h) + return -ENOSPC; + + if (coords->crtc_w > INT_MAX || + coords->crtc_x > INT_MAX - (int32_t) coords->crtc_w || + coords->crtc_h > INT_MAX || + coords->crtc_y > INT_MAX - (int32_t) coords->crtc_h) + return -ERANGE; + + if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > 16384) + return -EINVAL; + } + + if (crtc) { + const struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + /* Don't modify another pipe's plane */ + if (intel_plane->pipe != intel_crtc->pipe) + return -EINVAL; + } + + if (!fb || !crtc || !crtc->enabled) { + coords->visible = false; + return 0; + } + + intel_clip_plane(plane, crtc, fb, coords); + + /* Check size restrictions when scaling */ + if (coords->visible && (coords->src_w != coords->crtc_w || coords->src_h != coords->crtc_h)) { + unsigned int cpp = drm_format_plane_cpp(fb->pixel_format, 0); + + if (coords->src_w > 2048 || coords->src_h > 2048 || + coords->src_w * cpp > 4096 - 64 || fb->pitches[0] > 4096) + return -EINVAL; + } + + return 0; +} + static void -ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb, - struct drm_i915_gem_object *obj, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t x, uint32_t y, - uint32_t src_w, uint32_t src_h) +ivb_update_plane(struct drm_plane *plane, + struct drm_framebuffer *fb, + const struct intel_plane_coords *coords) { struct drm_device *dev = plane->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(plane); + const struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj; + int crtc_x = coords->crtc_x; + int crtc_y = coords->crtc_y; + unsigned int crtc_w = coords->crtc_w; + unsigned int crtc_h = coords->crtc_h; + uint32_t x = coords->src_x; + uint32_t y = coords->src_y; + uint32_t src_w = coords->src_w; + uint32_t src_h = coords->src_h; int pipe = intel_plane->pipe; u32 sprctl, sprscale = 0; int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); @@ -209,15 +366,22 @@ ivb_get_colorkey(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key) }
static void -ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb, - struct drm_i915_gem_object *obj, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t x, uint32_t y, - uint32_t src_w, uint32_t src_h) +ilk_update_plane(struct drm_plane *plane, + struct drm_framebuffer *fb, + const struct intel_plane_coords *coords) { struct drm_device *dev = plane->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_plane *intel_plane = to_intel_plane(plane); + const struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj; + int crtc_x = coords->crtc_x; + int crtc_y = coords->crtc_y; + unsigned int crtc_w = coords->crtc_w; + unsigned int crtc_h = coords->crtc_h; + uint32_t x = coords->src_x; + uint32_t y = coords->src_y; + uint32_t src_w = coords->src_w; + uint32_t src_h = coords->src_h; int pipe = intel_plane->pipe; u32 dvscntr, dvsscale; int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0); @@ -383,129 +547,70 @@ ilk_get_colorkey(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key) key->flags = I915_SET_COLORKEY_NONE; }
-static bool -format_is_yuv(uint32_t format) -{ - switch (format) { - case DRM_FORMAT_YUYV: - case DRM_FORMAT_UYVY: - case DRM_FORMAT_VYUY: - case DRM_FORMAT_YVYU: - return true; - default: - return false; - } -} - static int -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) +intel_disable_plane(struct drm_plane *plane) { struct drm_device *dev = plane->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_plane *intel_plane = to_intel_plane(plane); - struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); - struct drm_i915_gem_object *obj = intel_fb->obj; - struct drm_i915_gem_object *old_obj = intel_plane->obj; - int pipe = intel_plane->pipe; int ret = 0; - int primary_w = crtc->mode.hdisplay, primary_h = crtc->mode.vdisplay; - bool disable_primary = false; - bool visible; - int hscale, vscale; - int cpp = drm_format_plane_cpp(fb->pixel_format, 0); - struct drm_region src = { - .x1 = src_x, - .x2 = src_x + src_w, - .y1 = src_y, - .y2 = src_y + src_h, - }; - struct drm_region dst = { - .x1 = crtc_x, - .x2 = crtc_x + crtc_w, - .y1 = crtc_y, - .y2 = crtc_y + crtc_h, - }; - const struct drm_region clip = { - .x2 = crtc->mode.hdisplay, - .y2 = crtc->mode.vdisplay, - }; - - /* Don't modify another pipe's plane */ - if (intel_plane->pipe != intel_crtc->pipe) - return -EINVAL; - - if (fb->width < 3 || fb->height < 3 || fb->pitches[0] > 16384) - return -EINVAL; - - hscale = drm_calc_hscale(&src, &dst, 1, intel_plane->max_downscale << 16); - vscale = drm_calc_vscale(&src, &dst, 1, intel_plane->max_downscale << 16); - - visible = drm_region_clip_scaled(&src, &dst, &clip, hscale, vscale);
- crtc_x = dst.x1; - crtc_y = dst.y1; - crtc_w = drm_region_width(&dst); - crtc_h = drm_region_height(&dst); + if (intel_plane->primary_disabled) { + intel_enable_primary(plane->crtc); + intel_plane->primary_disabled = false; + }
- /* HW doesn't seem to like smaller sprite, even when scaling */ - /* FIXME return an error instead? */ - if (crtc_w < 3 || crtc_h < 3) - visible = false; + intel_plane->disable_plane(plane);
- /* - * Hardware doesn't handle subpixel coordinates. - * Round to nearest (macro)pixel boundary. - */ - if (format_is_yuv(fb->pixel_format)) { - src_x = ((src.x1 + 0x10000) >> 17) << 1; - src_w = (((src.x2 + 0x10000) >> 17) << 1) - src_x; - } else { - src_x = (src.x1 + 0x8000) >> 16; - src_w = ((src.x2 + 0x8000) >> 16) - src_x; - } - src_y = (src.y1 + 0x8000) >> 16; - src_h = ((src.y2 + 0x8000) >> 16) - src_y; + if (!intel_plane->obj) + goto out;
- /* Account for minimum source size when scaling */ - if (visible && (src_w != crtc_w || src_h != crtc_h)) { - unsigned int min_w = format_is_yuv(fb->pixel_format) ? 4 : 3; + mutex_lock(&dev->struct_mutex); + intel_unpin_fb_obj(intel_plane->obj); + intel_plane->obj = NULL; + mutex_unlock(&dev->struct_mutex); +out:
- if (src_w < min_w) { - src_w = min_w; - if (src_x > fb->width - src_w) - src_x = fb->width - src_w; - } + return ret; +}
- /* FIXME interlacing */ - if (src_h < 3) { - src_h = 3; - if (src_y > fb->height - src_h) - src_y = fb->height - src_h; - } - } +int +intel_commit_plane(struct drm_plane *plane, + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + const struct intel_plane_coords *coords) +{ + struct drm_device *dev = plane->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_plane *intel_plane = to_intel_plane(plane); + struct intel_framebuffer *intel_fb; + struct drm_i915_gem_object *obj; + struct drm_i915_gem_object *old_obj = intel_plane->obj; + int pipe = intel_plane->pipe; + int ret; + int primary_w, primary_h; + bool disable_primary = false;
- /* Check size restrictions when scaling */ - if (visible && (src_w != crtc_w || src_h != crtc_h)) { - if (src_w > 2048 || src_h > 2048 || - src_w * cpp > 4096 - 64 || fb->pitches[0] > 4096) - return -EINVAL; + if (!coords->visible) { + intel_disable_plane(plane); + return 0; }
+ /* FIXME this should happen anymore I suppose */ /* Pipe must be running... */ if (!(I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE)) return 0;
+ intel_fb = to_intel_framebuffer(fb); + obj = intel_fb->obj; + primary_w = crtc->mode.hdisplay; + primary_h = crtc->mode.vdisplay; + /* * If the sprite is completely covering the primary plane, * we can disable the primary and save power. */ - if ((crtc_x == 0) && (crtc_y == 0) && - (crtc_w == primary_w) && (crtc_h == primary_h)) + if ((coords->crtc_x == 0) && (coords->crtc_y == 0) && + (coords->crtc_w == primary_w) && (coords->crtc_h == primary_h)) disable_primary = true;
mutex_lock(&dev->struct_mutex); @@ -525,9 +630,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, intel_plane->primary_disabled = false; }
- if (visible) { - intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y, - crtc_w, crtc_h, src_x, src_y, src_w, src_h); + if (coords->visible) { + intel_plane->update_plane(plane, fb, coords);
if (disable_primary) { intel_disable_primary(crtc); @@ -558,29 +662,31 @@ out_unlock: }
static int -intel_disable_plane(struct drm_plane *plane) +intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) { - struct drm_device *dev = plane->dev; - struct intel_plane *intel_plane = to_intel_plane(plane); - int ret = 0; - - if (intel_plane->primary_disabled) { - intel_enable_primary(plane->crtc); - intel_plane->primary_disabled = false; - } - - intel_plane->disable_plane(plane); + int ret; + struct intel_plane_coords coords = { + .crtc_x = crtc_x, + .crtc_y = crtc_y, + .crtc_w = crtc_w, + .crtc_h = crtc_h, + .src_x = src_x, + .src_y = src_y, + .src_w = src_w, + .src_h = src_h, + };
- if (!intel_plane->obj) - goto out; + ret = intel_check_plane(plane, crtc, fb, &coords); + if (ret) + return ret;
- mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(intel_plane->obj); - intel_plane->obj = NULL; - mutex_unlock(&dev->struct_mutex); -out: + intel_commit_plane(plane, crtc, fb, &coords);
- return ret; + return 0; }
static void intel_destroy_plane(struct drm_plane *plane)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Second attempt at the atomic modeset implementation. This versions does things in a way that's closer to the drm_crtc_helper way. Not exactly pretty, but there's too much code depending on that design to change it.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_atomic.c | 1101 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 7 + 3 files changed, 1109 insertions(+), 0 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_atomic.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 2e9268d..f97e7a4 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -15,6 +15,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_tiling.o \ i915_sysfs.o \ i915_trace_points.o \ + intel_atomic.o \ intel_display.o \ intel_crt.o \ intel_lvds.o \ diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c new file mode 100644 index 0000000..173721a --- /dev/null +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -0,0 +1,1101 @@ +/* + */ + +#include <drm/drmP.h> +#include <drm/drm_crtc.h> + +#include "intel_drv.h" + +static struct drm_property *prop_src_x; +static struct drm_property *prop_src_y; +static struct drm_property *prop_src_w; +static struct drm_property *prop_src_h; +static struct drm_property *prop_crtc_x; +static struct drm_property *prop_crtc_y; +static struct drm_property *prop_crtc_w; +static struct drm_property *prop_crtc_h; +static struct drm_property *prop_fb_id; +static struct drm_property *prop_crtc_id; +static struct drm_property *prop_mode_id; +static struct drm_property *prop_connector_ids; + +struct intel_plane_state { + struct drm_plane *plane; + struct intel_plane_coords coords; + bool dirty; +}; + +struct intel_crtc_state { + struct drm_crtc *crtc; + struct drm_framebuffer *old_fb; + bool mode_dirty; + bool fb_dirty; + bool active_dirty; + unsigned long connectors_bitmask; + unsigned long encoders_bitmask; +}; + +struct intel_atomic_state { + struct intel_plane_state *plane; + struct intel_crtc_state *crtc; + bool dirty; + bool restore_hw; + struct drm_plane *saved_planes; + struct drm_crtc *saved_crtcs; + struct drm_connector *saved_connectors; + struct drm_encoder *saved_encoders; +}; + +static void update_connectors_bitmask(struct intel_crtc_state *st) +{ + struct drm_device *dev = st->crtc->dev; + struct drm_connector *connector; + unsigned int i; + + st->connectors_bitmask = 0; + + i = 0; + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { + if (connector->encoder && connector->encoder->crtc == st->crtc) + __set_bit(i, &st->connectors_bitmask); + + i++; + } +} + +static void update_encoders_bitmask(struct intel_crtc_state *st) +{ + struct drm_device *dev = st->crtc->dev; + struct drm_encoder *encoder; + unsigned int i; + + st->encoders_bitmask = 0; + + i = 0; + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { + if (encoder->crtc == st->crtc) + __set_bit(i, &st->encoders_bitmask); + + i++; + } +} + +static int process_connectors(struct intel_crtc_state *s, const uint32_t *ids, int count_ids) +{ + struct drm_crtc *crtc = s->crtc; + struct drm_device *dev = crtc->dev; + struct drm_connector *connectors[count_ids]; + struct drm_connector *connector; + struct drm_encoder *encoder; + int i; + + for (i = 0; i < count_ids; i++) { + struct drm_encoder *encoder; + const struct drm_connector_helper_funcs *connector_funcs; + struct drm_mode_object *obj; + int j; + + /* don't accept duplicates */ + for (j = i + 1; j < count_ids; j++) + if (ids[i] == ids[j]) + return -EINVAL; + + obj = drm_mode_object_find(dev, ids[i], DRM_MODE_OBJECT_CONNECTOR); + if (!obj) + return -ENOENT; + + connector = obj_to_connector(obj); + connector_funcs = connector->helper_private; + + encoder = connector_funcs->best_encoder(connector); + + if (!drm_encoder_crtc_ok(encoder, crtc)) + return -EINVAL; + + connectors[i] = connector; + } + + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { + const struct drm_connector_helper_funcs *connector_funcs = + connector->helper_private; + + for (i = 0; i < count_ids; i++) { + if (connector == connectors[i]) + break; + } + + /* this connector isn't in the set */ + if (i == count_ids) { + /* remove the link to the encoder if this crtc was driving it previously */ + if (connector->encoder && connector->encoder->crtc == crtc) { + s->mode_dirty = true; + connector->encoder = NULL; + } + continue; + } + + encoder = connector_funcs->best_encoder(connector); + + if (encoder != connector->encoder) { + s->mode_dirty = true; + connector->encoder = encoder; + } + + if (crtc != encoder->crtc) { + s->mode_dirty = true; + encoder->crtc = crtc; + } + } + + /* prune dangling encoder->crtc links pointing to this crtc */ + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { + if (encoder->crtc == crtc && !drm_helper_encoder_in_use(encoder)) + encoder->crtc = NULL; + } + + update_connectors_bitmask(s); + update_encoders_bitmask(s); + + return 0; +} + +static size_t intel_atomic_state_size(const struct drm_device *dev) +{ + struct intel_atomic_state *state; + unsigned int num_connector = dev->mode_config.num_connector; + unsigned int num_encoder = dev->mode_config.num_encoder; + unsigned int num_crtc = dev->mode_config.num_crtc; + unsigned int num_plane = dev->mode_config.num_plane; + + return sizeof *state + + num_crtc * sizeof state->crtc[0] + + num_plane * sizeof state->plane[0] + + num_connector * sizeof state->saved_connectors[0] + + num_encoder * sizeof state->saved_encoders[0] + + num_crtc * sizeof state->saved_crtcs[0] + + num_plane * sizeof state->saved_planes[0]; +} + +static void *intel_atomic_begin(struct drm_device *dev, uint32_t flags) +{ + struct intel_atomic_state *state; + struct drm_plane *plane; + struct drm_crtc *crtc; + struct drm_connector *connector; + struct drm_encoder *encoder; + unsigned int num_connector = dev->mode_config.num_connector; + unsigned int num_encoder = dev->mode_config.num_encoder; + unsigned int num_crtc = dev->mode_config.num_crtc; + unsigned int num_plane = dev->mode_config.num_plane; + int i; + + state = kzalloc(intel_atomic_state_size(dev), GFP_KERNEL); + if (!state) + return ERR_PTR(-ENOMEM); + + state->crtc = (struct intel_crtc_state *)(state + 1); + state->plane = (struct intel_plane_state *)(state->crtc + num_crtc); + + state->saved_connectors = (struct drm_connector *)(state->plane + num_plane); + state->saved_encoders = (struct drm_encoder *)(state->saved_connectors + num_connector); + state->saved_crtcs = (struct drm_crtc *)(state->saved_encoders + num_encoder); + state->saved_planes = (struct drm_plane *)(state->saved_crtcs + num_crtc); + + i = 0; + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { + struct intel_crtc_state *s = &state->crtc[i++]; + + s->crtc = crtc; + s->old_fb = crtc->fb; + + update_connectors_bitmask(s); + update_encoders_bitmask(s); + } + + i = 0; + list_for_each_entry(plane, &dev->mode_config.plane_list, head) { + struct intel_plane_state *s = &state->plane[i++]; + + s->plane = plane; + } + + i = 0; + list_for_each_entry(connector, &dev->mode_config.connector_list, head) + state->saved_connectors[i++] = *connector; + i = 0; + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) + state->saved_encoders[i++] = *encoder; + i = 0; + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) + state->saved_crtcs[i++] = *crtc; + i = 0; + list_for_each_entry(plane, &dev->mode_config.plane_list, head) + state->saved_planes[i++] = *plane; + + return state; +} + +static int plane_set(struct intel_atomic_state *s, + struct intel_plane_state *state, + struct drm_property *prop, + uint64_t value) +{ + struct drm_plane *plane = state->plane; + struct drm_mode_object *obj; + + if (prop == prop_src_x) { + if (plane->src_x == value) + return 0; + plane->src_x = value; + } else if (prop == prop_src_y) { + if (plane->src_y == value) + return 0; + plane->src_y = value; + } else if (prop == prop_src_w) { + if (plane->src_w == value) + return 0; + plane->src_w = value; + } else if (prop == prop_src_h) { + if (plane->src_h == value) + return 0; + plane->src_h = value; + } else if (prop == prop_crtc_x) { + if (plane->crtc_x == value) + return 0; + plane->crtc_x = value; + } else if (prop == prop_crtc_y) { + if (plane->crtc_y == value) + return 0; + plane->crtc_y = value; + } else if (prop == prop_crtc_w) { + if (plane->crtc_w == value) + return 0; + plane->crtc_w = value; + } else if (prop == prop_crtc_h) { + if (plane->crtc_h == value) + return 0; + plane->crtc_h = value; + } else if (prop == prop_crtc_id) { + struct drm_crtc *crtc = NULL; + + if (plane->crtc) { + if (value == plane->crtc->base.id) + return 0; + } else { + if (value == 0) + return 0; + } + + if (value) { + obj = drm_mode_object_find(plane->dev, value, DRM_MODE_OBJECT_CRTC); + if (!obj) { + printk("Unknown CRTC ID %llu\n", value); + return -ENOENT; + } + crtc = obj_to_crtc(obj); + } + + plane->crtc = crtc; + } else if (prop == prop_fb_id) { + struct drm_framebuffer *fb = NULL; + + if (plane->fb) { + if (value == plane->fb->base.id) + return 0; + } else { + if (value == 0) + return 0; + } + + if (value) { + obj = drm_mode_object_find(plane->dev, value, DRM_MODE_OBJECT_FB); + if (!obj) { + printk("Unknown framebuffer ID %llu\n", value); + return -ENOENT; + } + fb = obj_to_fb(obj); + } + + plane->fb = fb; + } else + return -ENOENT; + + state->dirty = true; + s->dirty = true; + + return 0; +} + +static int crtc_set(struct intel_atomic_state *s, + struct intel_crtc_state *state, + struct drm_property *prop, + uint64_t value, void *blob_data) +{ + struct drm_crtc *crtc = state->crtc; + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; + struct drm_mode_object *obj; + + if (prop == prop_src_x) { + if (crtc->x == value) + return 0; + crtc->x = value; + if (crtc_funcs->mode_set_base) + state->fb_dirty = true; + else + state->mode_dirty = true; + } else if (prop == prop_src_y) { + if (crtc->y == value) + return 0; + crtc->y = value; + if (crtc_funcs->mode_set_base) + state->fb_dirty = true; + else + state->mode_dirty = true; + } else if (prop == prop_mode_id) { + struct drm_display_mode *mode = NULL; + + if (!crtc->enabled) { + if (value == 0) + return 0; + } + + if (value) { + obj = drm_mode_object_find(crtc->dev, value, DRM_MODE_OBJECT_MODE); + if (!obj) { + printk("Unknown mode ID %llu\n", value); + return -ENOENT; + } + mode = obj_to_mode(obj); + } + + /* FIXME should check just the user timings? */ + if (crtc->enabled && mode && !memcmp(&crtc->mode, mode, sizeof *mode)) + return 0; + + /* turn on/off or active area changed? */ + if (!crtc->enabled || !mode || + crtc->mode.hdisplay != mode->hdisplay || + crtc->mode.vdisplay != mode->vdisplay) + state->active_dirty = true; + + if (mode) { + crtc->mode = *mode; + crtc->enabled = true; + } else + crtc->enabled = false; + state->mode_dirty = true; + } else if (prop == prop_fb_id) { + struct drm_framebuffer *fb = NULL; + + if (crtc->fb) { + if (value == crtc->fb->base.id) + return 0; + } else { + if (value == 0) + return 0; + } + + if (value) { + obj = drm_mode_object_find(crtc->dev, value, DRM_MODE_OBJECT_FB); + if (!obj) { + printk("Unknown framebuffer ID %llu\n", value); + return -ENOENT; + } + fb = obj_to_fb(obj); + } + + crtc->fb = fb; + if (crtc_funcs->mode_set_base) + state->fb_dirty = true; + else + state->mode_dirty = true; + } else if (prop == prop_connector_ids) { + const uint32_t *ids = blob_data; + uint32_t count_ids = value / sizeof(uint32_t); + int ret; + + if (value & 3) + return -EINVAL; + + if (count_ids > crtc->dev->mode_config.num_connector) + return -ERANGE; + + ret = process_connectors(state, ids, count_ids); + if (ret) + return ret; + } else + return -ENOENT; + + s->dirty = true; + + return 0; +} + +static struct intel_plane_state *get_plane_state(const struct drm_device *dev, + struct intel_atomic_state *state, + const struct drm_plane *plane) +{ + int i; + + for (i = 0; i < dev->mode_config.num_plane; i++) + if (plane == state->plane[i].plane) + return &state->plane[i]; + + return NULL; +} + +static struct intel_crtc_state *get_crtc_state(const struct drm_device *dev, + struct intel_atomic_state *state, + const struct drm_crtc *crtc) +{ + int i; + + for (i = 0; i < dev->mode_config.num_crtc; i++) + if (crtc == state->crtc[i].crtc) + return &state->crtc[i]; + + return NULL; +} + +static void crtc_prepare(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; + struct drm_encoder *encoder; + + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { + const struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private; + + if (encoder->crtc != crtc) + continue; + + encoder_funcs->prepare(encoder); + } + + drm_crtc_prepare_encoders(dev); + + crtc_funcs->prepare(crtc); +} + +static int crtc_set_base(struct drm_crtc *crtc, + struct drm_framebuffer *old_fb) +{ + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; + + return crtc_funcs->mode_set_base(crtc, crtc->x, crtc->y, old_fb); +} + +static int crtc_mode_set(struct drm_crtc *crtc, + struct drm_framebuffer *old_fb) +{ + struct drm_device *dev = crtc->dev; + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; + struct drm_encoder *encoder; + int ret; + + ret = crtc_funcs->mode_set(crtc, &crtc->mode, &crtc->hwmode, + crtc->x, crtc->y, old_fb); + if (ret) + return ret; + + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { + const struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private; + + if (encoder->crtc != crtc) + continue; + + encoder_funcs->mode_set(encoder, &crtc->mode, &crtc->hwmode); + } + + return 0; +} + +static void crtc_commit(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; + struct drm_encoder *encoder; + + crtc_funcs->commit(crtc); + + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { + const struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private; + + if (encoder->crtc != crtc) + continue; + + encoder_funcs->commit(encoder); + } +} + +int intel_commit_plane(struct drm_plane *plane, + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + const struct intel_plane_coords *st); + +static int apply_config(struct drm_device *dev, + struct intel_atomic_state *s) +{ + int i, ret; + struct drm_plane *plane; + + for (i = 0; i < dev->mode_config.num_crtc; i++) { + struct intel_crtc_state *st = &s->crtc[i]; + + if (!st->mode_dirty) + continue; + + crtc_prepare(st->crtc); + } + + for (i = 0; i < dev->mode_config.num_crtc; i++) { + struct intel_crtc_state *st = &s->crtc[i]; + + if (st->mode_dirty) { + ret = crtc_mode_set(st->crtc, st->old_fb); + if (ret) + return ret; + } else if (st->fb_dirty) { + ret = crtc_set_base(st->crtc, st->old_fb); + if (ret) + return ret; + } else + continue; + + /* + * Old fb was unpinned and new fb pinned. + * Update the old_fb pointer accordingingly + * in case we need to roll back later. + */ + st->old_fb = st->crtc->fb; + } + + for (i = 0; i < dev->mode_config.num_plane; i++) { + struct intel_plane_state *st = &s->plane[i]; + struct intel_crtc_state *cst; + struct drm_plane *plane = st->plane; + + if (!s->plane[i].dirty) + continue; + + if (!plane->crtc) + continue; + + cst = get_crtc_state(dev, s, plane->crtc); + /* already handled by the CRTC mode set? */ + if (cst->mode_dirty) + continue; + + ret = intel_commit_plane(plane, plane->crtc, plane->fb, &st->coords); + if (ret) + return ret; + } + + for (i = 0; i < dev->mode_config.num_crtc; i++) { + struct intel_crtc_state *st = &s->crtc[i]; + + if (!st->mode_dirty) + continue; + + crtc_commit(st->crtc); + } + + /* + * FIXME perhaps better order would be + * 1. prepare all current objects + * 2. disable unused objects + * 3. set mode for current objects + * 4. commit current objects + */ + drm_helper_disable_unused_functions(dev); + list_for_each_entry(plane, &dev->mode_config.plane_list, head) { + /* planes attached to crtcs already handled in mode_set() */ + if (!plane->crtc || !plane->fb) + plane->funcs->disable_plane(plane); + } + + /* don't restore the old state in end() */ + s->dirty = false; + + return 0; +} + +static void restore_state(struct drm_device *dev, + struct intel_atomic_state *s) +{ + int i; + struct drm_connector *connector; + struct drm_encoder *encoder; + struct drm_crtc *crtc; + struct drm_plane *plane; + + i = 0; + list_for_each_entry(connector, &dev->mode_config.connector_list, head) + *connector = s->saved_connectors[i++]; + i = 0; + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) + *encoder = s->saved_encoders[i++]; + i = 0; + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) + *crtc = s->saved_crtcs[i++]; + i = 0; + list_for_each_entry(plane, &dev->mode_config.plane_list, head) + *plane = s->saved_planes[i++]; + + /* FIXME props etc. */ + + /* was the hardware state clobbered? */ + if (s->restore_hw) + apply_config(dev, s); +} + +static int intel_atomic_set(struct drm_device *dev, void *state, + struct drm_mode_object *obj, + struct drm_property *prop, + uint64_t value, void *blob_data) +{ + struct intel_atomic_state *s = state; + int ret = -EINVAL; + + switch (obj->type) { + case DRM_MODE_OBJECT_PLANE: + ret = plane_set(s, get_plane_state(dev, s, obj_to_plane(obj)), prop, value); + break; + case DRM_MODE_OBJECT_CRTC: + ret = crtc_set(s, get_crtc_state(dev, s, obj_to_crtc(obj)), prop, value, blob_data); + break; + default: + break; + } + + kfree(blob_data); + + return ret; +} + +int intel_check_plane(const struct drm_plane *plane, + const struct drm_crtc *crtc, + const struct drm_framebuffer *fb, + struct intel_plane_coords *st); + +static void dirty_planes(const struct drm_device *dev, + struct intel_atomic_state *state, + const struct drm_crtc *crtc) +{ + int i; + + for (i = 0; i < dev->mode_config.num_plane; i++) { + struct intel_plane_state *s = &state->plane[i]; + + if (s->plane->crtc == crtc) + s->dirty = true; + } +} + +static int check_crtc(struct intel_crtc_state *s, bool mode_dirty) +{ + struct drm_crtc *crtc = s->crtc; + struct drm_device *dev = crtc->dev; + struct drm_encoder *encoder; + struct drm_framebuffer *fb = crtc->fb; + struct drm_display_mode mode, adjusted_mode; + const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; + + /* must have a fb and connectors if we have a mode, and vice versa */ + if (crtc->enabled) { + if (!fb) + return -EINVAL; + if (!drm_helper_crtc_in_use(crtc)) + return -EINVAL; + } else { + if (fb) + return -EINVAL; + if (drm_helper_crtc_in_use(crtc)) + return -EINVAL; + } + + if (crtc->enabled) { + if (crtc->mode.hdisplay > fb->width || + crtc->mode.vdisplay > fb->height || + crtc->x > fb->width - crtc->mode.hdisplay || + crtc->y > fb->height - crtc->mode.vdisplay) + return -ENOSPC; + } + + if (!crtc->enabled || !mode_dirty) + return 0; + + mode = adjusted_mode = crtc->mode; + + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { + const struct drm_encoder_helper_funcs *encoder_funcs = encoder->helper_private; + + if (!encoder_funcs->mode_fixup(encoder, &mode, &adjusted_mode)) + return -EINVAL; + } + + if (!crtc_funcs->mode_fixup(crtc, &mode, &adjusted_mode)) + return -EINVAL; + + adjusted_mode.clock = mode.clock; + + crtc->hwmode = adjusted_mode; + + return 0; +} + +static int intel_atomic_check(struct drm_device *dev, void *state) +{ + struct intel_atomic_state *s = state; + int ret; + int i; + + if (!s->dirty) + return 0; + + for (i = 0; i < dev->mode_config.num_crtc; i++) { + struct intel_crtc_state *st = &s->crtc[i]; + + if (!st->fb_dirty && !st->mode_dirty) + continue; + + ret = check_crtc(st, st->mode_dirty); + if (ret) + return ret; + + /* + * Mark all planes on this CRTC as dirty if the active video + * area changed so that the planes will get reclipped correctly. + */ + if (st->active_dirty) + dirty_planes(dev, s, st->crtc); + } + + /* check for conflicts in encoder/connector assignment */ + for (i = 0; i < dev->mode_config.num_crtc; i++) { + struct intel_crtc_state *st = &s->crtc[i]; + int j; + + for (j = i + 1; j < dev->mode_config.num_crtc; j++) { + struct intel_crtc_state *st2 = &s->crtc[j]; + + if (st->connectors_bitmask & st2->connectors_bitmask) + return -EINVAL; + + if (st->encoders_bitmask & st2->encoders_bitmask) + return -EINVAL; + } + } + + for (i = 0; i < dev->mode_config.num_plane; i++) { + struct intel_plane_state *st = &s->plane[i]; + const struct drm_plane *plane = st->plane; + + if (!st->dirty) + continue; + + st->coords.crtc_x = plane->crtc_x; + st->coords.crtc_y = plane->crtc_y; + st->coords.crtc_w = plane->crtc_w; + st->coords.crtc_h = plane->crtc_h; + + st->coords.src_x = plane->src_x; + st->coords.src_y = plane->src_y; + st->coords.src_w = plane->src_w; + st->coords.src_h = plane->src_h; + + ret = intel_check_plane(plane, plane->crtc, plane->fb, &st->coords); + if (ret) + return ret; + } + + return 0; +} + +static void update_plane_props(struct drm_plane *plane) +{ + struct drm_mode_object *obj = &plane->base; + + drm_object_property_set_value(obj, prop_src_x, plane->src_x); + drm_object_property_set_value(obj, prop_src_y, plane->src_y); + drm_object_property_set_value(obj, prop_src_w, plane->src_w); + drm_object_property_set_value(obj, prop_src_h, plane->src_h); + + drm_object_property_set_value(obj, prop_crtc_x, plane->crtc_x); + drm_object_property_set_value(obj, prop_crtc_y, plane->crtc_y); + drm_object_property_set_value(obj, prop_crtc_w, plane->crtc_w); + drm_object_property_set_value(obj, prop_crtc_h, plane->crtc_h); + + drm_object_property_set_value(obj, prop_fb_id, plane->fb ? plane->fb->base.id : 0); + drm_object_property_set_value(obj, prop_crtc_id, plane->crtc ? plane->crtc->base.id : 0); +} + +static int free_connector_ids(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_mode_object *obj; + uint64_t value; + int ret; + + ret = drm_object_property_get_value(&crtc->base, prop_connector_ids, &value); + if (ret) + return ret; + + if (!value) + return 0; + + obj = drm_mode_object_find(dev, value, DRM_MODE_PROP_BLOB); + if (!obj) + return -ENOENT; + + drm_property_destroy_blob(dev, obj_to_blob(obj)); + + return 0; +} + +static int update_connector_ids(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_connector *connector; + struct drm_property_blob *blob; + uint64_t value = 0; + int i = 0; + uint32_t connector_ids[dev->mode_config.num_connector]; + + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { + if (connector->encoder && connector->encoder->crtc == crtc) + connector_ids[i++] = connector->base.id; + } + + if (i) { + /* + * FIXME make blobs into vectors, and pre-allocate using worst case estimate. + * That way this function could never fail. + */ + blob = drm_property_create_blob(dev, i * sizeof connector_ids[0], connector_ids); + if (!blob) + return -ENOMEM; + + value = blob->base.id; + } + + free_connector_ids(crtc); + + drm_object_property_set_value(&crtc->base, prop_connector_ids, value); + + return 0; +} + +static void update_crtc_props(struct drm_crtc *crtc) +{ + struct drm_mode_object *obj = &crtc->base; + + drm_object_property_set_value(obj, prop_src_x, crtc->x); + drm_object_property_set_value(obj, prop_src_y, crtc->y); + + drm_object_property_set_value(obj, prop_fb_id, crtc->fb ? crtc->fb->base.id : 0); + + /* + * FIXME if mode is user specifiec via old setcrtc ioctl prop value should be -1 + * or perhaps setcrtc should insert an imlicitly created mode into some list... + */ + drm_object_property_set_value(obj, prop_mode_id, + crtc->enabled ? crtc->mode.base.id : 0); + + update_connector_ids(crtc); +} + +static void update_props(const struct drm_device *dev, + struct intel_atomic_state *s) +{ + int i; + + for (i = 0; i < dev->mode_config.num_crtc; i++) { + struct intel_crtc_state *st = &s->crtc[i]; + + if (!st->fb_dirty && !st->mode_dirty) + continue; + + update_crtc_props(st->crtc); + } + + for (i = 0; i < dev->mode_config.num_plane; i++) { + struct intel_plane_state *st = &s->plane[i]; + + if (!st->dirty) + continue; + + update_plane_props(st->plane); + } +} + +static int intel_atomic_commit(struct drm_device *dev, void *state) +{ + struct intel_atomic_state *s = state; + int ret; + + if (!s->dirty) + return 0; + + ret = apply_config(dev, s); + if (ret) { + s->restore_hw = true; + return ret; + } + + update_props(dev, s); + + return 0; +} + +static void intel_atomic_end(struct drm_device *dev, void *state) +{ + struct intel_atomic_state *s = state; + + /* restore the state of all objects */ + if (s->dirty) + restore_state(dev, state); + + kfree(state); +} + +static const struct drm_atomic_funcs intel_atomic_funcs = { + .begin = intel_atomic_begin, + .set = intel_atomic_set, + .check = intel_atomic_check, + .commit = intel_atomic_commit, + .end = intel_atomic_end, +}; + +int intel_atomic_init(struct drm_device *dev) +{ + struct drm_crtc *crtc; + struct drm_plane *plane; + int ret = -ENOMEM; + + prop_src_x = drm_property_create_range(dev, 0, "SRC_X", 0, UINT_MAX); + if (!prop_src_x) + goto out; + prop_src_y = drm_property_create_range(dev, 0, "SRC_Y", 0, UINT_MAX); + if (!prop_src_y) + goto destroy_src_x; + prop_src_w = drm_property_create_range(dev, 0, "SRC_W", 0, UINT_MAX); + if (!prop_src_w) + goto destroy_src_y; + prop_src_h = drm_property_create_range(dev, 0, "SRC_H", 0, UINT_MAX); + if (!prop_src_h) + goto destroy_src_w; + + prop_crtc_x = drm_property_create_range(dev, 0, "CRTC_X", INT_MIN, INT_MAX); + if (!prop_crtc_x) + goto destroy_src_h; + prop_crtc_y = drm_property_create_range(dev, 0, "CRTC_Y", INT_MIN, INT_MAX); + if (!prop_crtc_y) + goto destroy_crtc_x; + prop_crtc_w = drm_property_create_range(dev, 0, "CRTC_W", 0, INT_MAX); + if (!prop_crtc_w) + goto destroy_crtc_y; + prop_crtc_h = drm_property_create_range(dev, 0, "CRTC_H", 0, INT_MAX); + if (!prop_crtc_h) + goto destroy_crtc_w; + + /* FIXME create special object ID property type? */ + prop_fb_id = drm_property_create_range(dev, 0, "FB_ID", 0, UINT_MAX); + if (!prop_fb_id) + goto destroy_crtc_h; + prop_crtc_id = drm_property_create_range(dev, 0, "CRTC_ID", 0, UINT_MAX); + if (!prop_crtc_id) + goto destroy_fb_id; + prop_mode_id = drm_property_create_range(dev, 0, "MODE_ID", 0, UINT_MAX); + if (!prop_mode_id) + goto destroy_crtc_id; + + /* FIXME create special object ID list property type? */ + prop_connector_ids = drm_property_create(dev, DRM_MODE_PROP_BLOB, "CONNECTOR_IDS", 0); + if (!prop_connector_ids) + goto destroy_mode_id; + + list_for_each_entry(plane, &dev->mode_config.plane_list, head) { + struct drm_mode_object *obj = &plane->base; + + drm_object_attach_property(obj, prop_src_x, 0); + drm_object_attach_property(obj, prop_src_y, 0); + drm_object_attach_property(obj, prop_src_w, 0); + drm_object_attach_property(obj, prop_src_h, 0); + + drm_object_attach_property(obj, prop_crtc_x, 0); + drm_object_attach_property(obj, prop_crtc_y, 0); + drm_object_attach_property(obj, prop_crtc_w, 0); + drm_object_attach_property(obj, prop_crtc_h, 0); + + drm_object_attach_property(obj, prop_fb_id, 0); + drm_object_attach_property(obj, prop_crtc_id, 0); + + update_plane_props(plane); + } + + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { + struct drm_mode_object *obj = &crtc->base; + + drm_object_attach_property(obj, prop_src_x, 0); + drm_object_attach_property(obj, prop_src_y, 0); + + drm_object_attach_property(obj, prop_fb_id, 0); + drm_object_attach_property(obj, prop_mode_id, 0); + drm_object_attach_property(obj, prop_connector_ids, 0); + + update_crtc_props(crtc); + } + + dev->driver->atomic_funcs = &intel_atomic_funcs; + + return 0; + + destroy_mode_id: + drm_property_destroy(dev, prop_mode_id); + destroy_crtc_id: + drm_property_destroy(dev, prop_crtc_id); + destroy_fb_id: + drm_property_destroy(dev, prop_fb_id); + destroy_crtc_h: + drm_property_destroy(dev, prop_crtc_h); + destroy_crtc_w: + drm_property_destroy(dev, prop_crtc_w); + destroy_crtc_y: + drm_property_destroy(dev, prop_crtc_y); + destroy_crtc_x: + drm_property_destroy(dev, prop_crtc_x); + destroy_src_h: + drm_property_destroy(dev, prop_src_h); + destroy_src_w: + drm_property_destroy(dev, prop_src_w); + destroy_src_y: + drm_property_destroy(dev, prop_src_y); + destroy_src_x: + drm_property_destroy(dev, prop_src_x); + out: + return ret; +} + +void intel_atomic_fini(struct drm_device *dev) +{ + struct drm_crtc *crtc; + + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) + free_connector_ids(crtc); + + drm_property_destroy(dev, prop_connector_ids); + drm_property_destroy(dev, prop_mode_id); + drm_property_destroy(dev, prop_crtc_id); + drm_property_destroy(dev, prop_fb_id); + + drm_property_destroy(dev, prop_crtc_h); + drm_property_destroy(dev, prop_crtc_w); + drm_property_destroy(dev, prop_crtc_y); + drm_property_destroy(dev, prop_crtc_x); + + drm_property_destroy(dev, prop_src_h); + drm_property_destroy(dev, prop_src_w); + drm_property_destroy(dev, prop_src_y); + drm_property_destroy(dev, prop_src_x); +} diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 83437ee..4b186d6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7053,6 +7053,9 @@ void intel_modeset_init_hw(struct drm_device *dev) ivb_pch_pwm_override(dev); }
+void intel_atomic_init(struct drm_device *dev); +void intel_atomic_fini(struct drm_device *dev); + void intel_modeset_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -7107,6 +7110,8 @@ void intel_modeset_init(struct drm_device *dev) INIT_WORK(&dev_priv->idle_work, intel_idle_update); setup_timer(&dev_priv->idle_timer, intel_gpu_idle_timer, (unsigned long)dev); + + intel_atomic_init(dev); }
void intel_modeset_gem_init(struct drm_device *dev) @@ -7122,6 +7127,8 @@ void intel_modeset_cleanup(struct drm_device *dev) struct drm_crtc *crtc; struct intel_crtc *intel_crtc;
+ intel_atomic_fini(dev); + drm_kms_helper_poll_fini(dev); mutex_lock(&dev->struct_mutex);
On Wed, 27 Jun 2012 13:24:02 +0300 ville.syrjala@linux.intel.com wrote:
:(
OTOH introducing an atomic alternative to the helper stuff and moving drivers over would probably end up looking a lot nicer. But there's no doubt that's a huge chunk of work...
The other thing I'm worried about with atomic mode setting is handling the legacy case properly. We'll still need to handle apps that want to change one CRTC at a time without altering the state of other CRTCs. In an atomic context, drivers should be able to assume they can shut anything off that doesn't come in as part of the atomic state, which means when we build a compat atomic setup, we'll need to read the current state of any existing objects and pass them in to the driver...
On Wed, 2012-06-27 at 09:48 -0700, Jesse Barnes wrote:
Fortunately that's not true, you can flag-day it (per drm device even, if we want). If the first thing userspace asks for is atomic modeset then you simply disallow subsequent non-atomic modesets, and the reverse. Userspace code will need to grow atomic support anyway, so can simply treat -EINVAL from trying it as meaning "no kernel support, revert to the older thing".
This does mean you have to update all of userspace to do the new call instead of the old one, but that's fine, there's only a handful of clients for this API.
- ajax
On Wed, Jun 27, 2012 at 6:11 PM, Adam Jackson ajax@redhat.com wrote:
You'll also need to handles cases like render nodes where we split resources for multi-seat on one gpu.
Dave.
On Wed, Jun 27, 2012 at 09:48:26AM -0700, Jesse Barnes wrote:
I'm thinking that it should be doable. Well, at least for all the "core" features, but I'm not quite sure how it would handle all driver specific properties in a nice way. Basically what I have now in intel_atomic.c could become the new helper, but then it needs somehow to defer to the driver for some of the props.
So either the driver needs to collect the state for those somehow, or we have the core do it the same way as for other objects, ie. overwrite the object state with the new values as we go on, and then roll back later if necessary. But then we potentially need to save/restore all properties in the beginning and end of the operation, or we could try to do it in some lazy fashion.
My current code collects the current state in the beginning (or rather it saves the state of all the objects in the beginning) and starts to modify the current state of the objects as new values are read in by the ioctl handler. Anything not part of the atomic modeset is left untouched.
dri-devel@lists.freedesktop.org
-
Adam Jackson -
Daniel Vetter -
Dave Airlie -
Jesse Barnes -
Rob Clark -
Ville Syrjälä -
ville.syrjala＠linux.intel.com