[RFC][PATCH 08/10] WIP: drm: Atomic modeset ioctl
Rob Clark
robdclark at gmail.com
Fri Aug 31 15:47:13 PDT 2012
On Wed, Jun 27, 2012 at 5:24 AM, <ville.syrjala at linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala at 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.
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?
> Signed-off-by: Ville Syrjälä <ville.syrjala at 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),
DRM_IOCTL_MODE_ATOMIC_SET? DRM_IOCTL_MODE_SET_ATOMIC? Seems like
there should somehow be a "SET" in the name..
BR,
-R
> };
>
> #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
> --
> 1.7.3.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list