From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFCv1 11/12] drm: Atomic modeset ioctl Date: Mon, 7 Oct 2013 16:28:12 +0300 Message-ID: <20131007132812.GG9395@intel.com> References: <1381020350-1125-1-git-send-email-robdclark@gmail.com> <1381020350-1125-12-git-send-email-robdclark@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 9D067E744C for ; Mon, 7 Oct 2013 06:28:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1381020350-1125-12-git-send-email-robdclark@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Rob Clark Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Sat, Oct 05, 2013 at 08:45:49PM -0400, Rob Clark wrote: > From: Ville Syrj=E4l=E4 > = > The atomic modeset ioctl cna be used to push any number of new values > for object properties. The driver can then check the full device > configuration as single unit, and try to apply the changes atomically. > = > 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. > = > The caller can demand non-blocking operation from the ioctl, and if the > driver can't satisfy that requirement an error will be returned. > = > The caller can also request to receive asynchronous completion events > after the operation has reached the hardware. An event is sent for each > object specified by the caller, whether or not the actual state of > that object changed. Each event also carries a framebuffer ID, which > indicates to user space that the specified object is no longer > accessing that framebuffer. > = > TODO: detailed error reporting? > = > v1: original > v2: rebase on uapi changes, and drm state structs.. -- Rob Clark > v3: rebase, missing padding in drm_event_atomic.. -- Rob Clark > = > Signed-off-by: Ville Syrj=E4l=E4 > --- > drivers/gpu/drm/drm_crtc.c | 159 ++++++++++++++++++++++++++++++++++++++= ++++++ > drivers/gpu/drm/drm_drv.c | 1 + > include/drm/drmP.h | 6 ++ > include/drm/drm_crtc.h | 2 + > include/uapi/drm/drm.h | 12 ++++ > include/uapi/drm/drm_mode.h | 16 +++++ > 6 files changed, 196 insertions(+) > = > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 09396a9..910e5c6 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -4338,3 +4338,162 @@ void drm_mode_config_cleanup(struct drm_device *d= ev) > idr_destroy(&dev->mode_config.crtc_idr); > } > EXPORT_SYMBOL(drm_mode_config_cleanup); > + > +int drm_mode_atomic_ioctl(struct drm_device *dev, > + void *data, struct drm_file *file_priv) > +{ > + struct drm_mode_atomic *arg =3D data; > + uint32_t __user *objs_ptr =3D (uint32_t __user *)(unsigned long)(arg->o= bjs_ptr); > + uint32_t __user *count_props_ptr =3D (uint32_t __user *)(unsigned long)= (arg->count_props_ptr); > + uint32_t __user *props_ptr =3D (uint32_t __user *)(unsigned long)(arg->= props_ptr); > + uint64_t __user *prop_values_ptr =3D (uint64_t __user *)(unsigned long)= (arg->prop_values_ptr); > + uint64_t __user *blob_values_ptr =3D (uint64_t __user *)(unsigned long)= (arg->blob_values_ptr); > + unsigned int copied_objs =3D 0; > + unsigned int copied_props =3D 0; > + unsigned int copied_blobs =3D 0; > + void *state; > + int ret =3D 0; > + unsigned int i, j; > + > + if (arg->flags & ~(DRM_MODE_ATOMIC_TEST_ONLY | > + DRM_MODE_ATOMIC_EVENT | DRM_MODE_ATOMIC_NONBLOCK)) > + return -EINVAL; > + > + /* can't test and expect an event at the same time. */ > + if ((arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) && > + (arg->flags & DRM_MODE_ATOMIC_EVENT)) > + return -EINVAL; > + > + mutex_lock(&dev->mode_config.mutex); I'm pretty sure I had a version w/ fixed locking... git://gitorious.org/vsyrjala/linux.git rebased_drm_atomic_24 > + > + state =3D dev->driver->atomic_begin(dev, arg->flags); > + if (IS_ERR(state)) { > + ret =3D PTR_ERR(state); > + goto unlock; > + } > + > + for (i =3D 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 =3D -EFAULT; > + goto out; > + } > + > + obj =3D drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY); > + if (!obj || !obj->properties) { > + ret =3D -ENOENT; > + goto out; > + } > + > + if (arg->flags & DRM_MODE_ATOMIC_EVENT) { > + // XXX create atomic event instead.. Some people are apparently more comfortable with a per-crtc event rather than the per-obj events I added. So I think we might want both in the end. > + struct drm_pending_vblank_event *e =3D > + create_vblank_event(dev, file_priv, arg->user_data); > + if (!e) { > + ret =3D -ENOMEM; > + goto out; > + } > + ret =3D dev->driver->atomic_set_event(dev, state, obj, e); > + if (ret) { > + destroy_vblank_event(dev, file_priv, e); > + goto out; > + } > + } > + > + if (get_user(count_props, count_props_ptr + copied_objs)) { > + ret =3D -EFAULT; > + goto out; > + } > + > + copied_objs++; > + > + for (j =3D 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 =3D NULL; > + > + if (get_user(prop_id, props_ptr + copied_props)) { > + ret =3D -EFAULT; > + goto out; > + } > + > + if (!object_has_prop(obj, prop_id)) { > + ret =3D -EINVAL; > + goto out; > + } > + > + prop_obj =3D drm_mode_object_find(dev, prop_id, > + DRM_MODE_OBJECT_PROPERTY); > + if (!prop_obj) { > + ret =3D -ENOENT; > + goto out; > + } > + prop =3D obj_to_property(prop_obj); > + > + if (get_user(prop_value, prop_values_ptr + copied_props)) { > + ret =3D -EFAULT; > + goto out; > + } > + > + if (!drm_property_change_is_valid(prop, prop_value)) { > + ret =3D -EINVAL; > + goto out; > + } > + > + if ((prop->flags & DRM_MODE_PROP_BLOB) && prop_value) { > + uint64_t blob_ptr; > + > + if (get_user(blob_ptr, blob_values_ptr + copied_blobs)) { > + ret =3D -EFAULT; > + goto out; > + } > + > + blob_data =3D kmalloc(prop_value, GFP_KERNEL); > + if (!blob_data) { > + ret =3D -ENOMEM; > + goto out; > + } > + > + if (copy_from_user(blob_data, (void __user *)(unsigned long)blob_ptr= , prop_value)) { > + kfree(blob_data); > + ret =3D -EFAULT; > + goto out; > + } > + } > + > + /* User space sends the blob pointer even if we > + * don't use it (length=3D=3D0). > + */ > + if (prop->flags & DRM_MODE_PROP_BLOB) > + copied_blobs++; > + > + /* The driver will be in charge of blob_data from now on. */ > + ret =3D drm_mode_set_obj_prop(obj, state, prop, > + prop_value, blob_data); > + if (ret) > + goto out; > + > + copied_props++; > + } > + } > + > + ret =3D dev->driver->atomic_check(dev, state); > + if (ret) > + goto out; > + > + if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) > + goto out; > + > + ret =3D dev->driver->atomic_commit(dev, state); > + > + out: > + dev->driver->atomic_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 e572dd2..43e04ae 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -166,6 +166,7 @@ static const struct drm_ioctl_desc drm_ioctls[] =3D { > DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_proper= ties_ioctl, 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_CURSOR2, drm_mode_cursor2_ioctl, DRM_MASTE= R|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/drmP.h b/include/drm/drmP.h > index 4c3de22..f282733 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1158,6 +1158,12 @@ struct drm_pending_vblank_event { > struct drm_event_vblank event; > }; > = > +struct drm_pending_atomic_event { > + struct drm_pending_event base; > + int pipe; > + struct drm_event_atomic event; > +}; > + > /** > * DRM device structure. This structure represent a complete card that > * may contain multiple heads. > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 86a5a00..fa3956e 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1313,6 +1313,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); > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index ece8678..efb1461 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -733,6 +733,7 @@ struct drm_prime_handle { > #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_ob= j_set_property) > #define DRM_IOCTL_MODE_CURSOR2 DRM_IOWR(0xBB, struct drm_mode_cursor2) > +#define DRM_IOCTL_MODE_ATOMIC DRM_IOWR(0xBC, struct drm_mode_atomic) > = > /** > * Device specific ioctls should only be in their respective headers > @@ -774,6 +775,17 @@ struct drm_event_vblank { > __u32 reserved; > }; > = > +struct drm_event_atomic { > + struct drm_event base; > + __u64 user_data; > + __u32 tv_sec; > + __u32 tv_usec; > + __u32 sequence; > + __u32 obj_id; > + __u32 old_fb_id; > + __u32 reserved; > +}; > + > #define DRM_CAP_DUMB_BUFFER 0x1 > #define DRM_CAP_VBLANK_HIGH_CRTC 0x2 > #define DRM_CAP_DUMB_PREFERRED_DEPTH 0x3 > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 6d4f089..03a473c 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -489,4 +489,20 @@ struct drm_mode_destroy_dumb { > uint32_t handle; > }; > = > +#define DRM_MODE_ATOMIC_TEST_ONLY (1<<0) > +#define DRM_MODE_ATOMIC_EVENT (1<<1) > +#define DRM_MODE_ATOMIC_NONBLOCK (1<<2) > + > +/* 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; > + __u64 user_data; > +}; > + > #endif > -- = > 1.8.3.1 -- = Ville Syrj=E4l=E4 Intel OTC