| Message ID | 1384980493-25499-13-git-send-email-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Rob and Ville, 2013/11/21 Rob Clark <robdclark@gmail.com>: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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 > v4: drop atomic event, align flags w/ pageflip (atomic flags should be > a strick superset of pageflip flags to keep things easier for the > drivers) > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_crtc.c | 157 +++++++++++++++++++++++++++++++++++++++++++- > 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 | 21 ++++++ > 6 files changed, 198 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 4b40a39..8368ef5 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -4037,7 +4037,8 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, > return -ENOENT; > crtc = obj_to_crtc(obj); > > - state = dev->driver->atomic_begin(dev, page_flip->flags); > + state = dev->driver->atomic_begin(dev, > + page_flip->flags | DRM_MODE_ATOMIC_NONBLOCK); > if (IS_ERR(state)) > return PTR_ERR(state); > > @@ -4451,3 +4452,157 @@ void drm_mode_config_cleanup(struct drm_device *dev) > 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) > +{ Build error at this function like below, drivers/built-in.o: In function `drm_mode_atomic_ioctl': of_iommu.c:(.text+0x6d854): undefined reference to `__get_user_bad' of_iommu.c:(.text+0x6d940): undefined reference to `__get_user_bad' And built correctly with the below change, diff --git a/arch/arm/lib/getuser.S b/arch/arm/lib/getuser.S index 9b06bb4..0f4f469 100644 --- a/arch/arm/lib/getuser.S +++ b/arch/arm/lib/getuser.S @@ -66,7 +66,7 @@ ENTRY(__get_user_4) mov pc, lr ENDPROC(__get_user_4) -__get_user_bad: +ENTRY(__get_user_bad) mov r2, #0 mov r0, #-EFAULT mov pc, lr I guess __get_user_bad is not global but drm_mode_atomic_ioctl function tries to refer this function as extern. Is it build error only I can see? Thanks, Inki Dae
On Fri, Nov 22, 2013 at 3:35 AM, Inki Dae <inki.dae@samsung.com> wrote: > Hi Rob and Ville, > > > 2013/11/21 Rob Clark <robdclark@gmail.com>: >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> 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 >> v4: drop atomic event, align flags w/ pageflip (atomic flags should be >> a strick superset of pageflip flags to keep things easier for the >> drivers) >> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> --- >> drivers/gpu/drm/drm_crtc.c | 157 +++++++++++++++++++++++++++++++++++++++++++- >> 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 | 21 ++++++ >> 6 files changed, 198 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >> index 4b40a39..8368ef5 100644 >> --- a/drivers/gpu/drm/drm_crtc.c >> +++ b/drivers/gpu/drm/drm_crtc.c >> @@ -4037,7 +4037,8 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, >> return -ENOENT; >> crtc = obj_to_crtc(obj); >> >> - state = dev->driver->atomic_begin(dev, page_flip->flags); >> + state = dev->driver->atomic_begin(dev, >> + page_flip->flags | DRM_MODE_ATOMIC_NONBLOCK); >> if (IS_ERR(state)) >> return PTR_ERR(state); >> >> @@ -4451,3 +4452,157 @@ void drm_mode_config_cleanup(struct drm_device *dev) >> 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) >> +{ > > Build error at this function like below, > > drivers/built-in.o: In function `drm_mode_atomic_ioctl': > of_iommu.c:(.text+0x6d854): undefined reference to `__get_user_bad' > of_iommu.c:(.text+0x6d940): undefined reference to `__get_user_bad' > > And built correctly with the below change, > > diff --git a/arch/arm/lib/getuser.S b/arch/arm/lib/getuser.S > index 9b06bb4..0f4f469 100644 > --- a/arch/arm/lib/getuser.S > +++ b/arch/arm/lib/getuser.S > @@ -66,7 +66,7 @@ ENTRY(__get_user_4) > mov pc, lr > ENDPROC(__get_user_4) > > -__get_user_bad: > +ENTRY(__get_user_bad) > mov r2, #0 > mov r0, #-EFAULT > mov pc, lr > > I guess __get_user_bad is not global but drm_mode_atomic_ioctl > function tries to refer this function as extern. no, actually __get_user_bad is (I think) supposed to be a build error. You need this patch: http://cgit.freedesktop.org/~robclark/linux/commit/?h=global-thermonuclear-war-5&id=1e94fad78b44d38ee3deca8f0694391e7db0e4cc (Or I think Russell King had a slightly updated version of that.. I need to work out with him about getting some version of that patch merged.) BR, -R > Is it build error only I can see? > > Thanks, > Inki Dae
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 4b40a39..8368ef5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -4037,7 +4037,8 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, return -ENOENT; crtc = obj_to_crtc(obj); - state = dev->driver->atomic_begin(dev, page_flip->flags); + state = dev->driver->atomic_begin(dev, + page_flip->flags | DRM_MODE_ATOMIC_NONBLOCK); if (IS_ERR(state)) return PTR_ERR(state); @@ -4451,3 +4452,157 @@ void drm_mode_config_cleanup(struct drm_device *dev) 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 = 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 (arg->flags & ~DRM_MODE_ATOMIC_FLAGS) + 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_PAGE_FLIP_EVENT)) + return -EINVAL; + + state = dev->driver->atomic_begin(dev, arg->flags); + if (IS_ERR(state)) { + ret = PTR_ERR(state); + goto out; + } + + 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 ((obj->type == DRM_MODE_OBJECT_CRTC) && + (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) { + struct drm_pending_vblank_event *e = + create_vblank_event(dev, file_priv, arg->user_data); + if (!e) { + ret = -ENOMEM; + goto out; + } + ret = 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 = -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) { + uint64_t blob_ptr; + + if (get_user(blob_ptr, blob_values_ptr + copied_blobs)) { + ret = -EFAULT; + goto out; + } + + blob_data = kmalloc(prop_value, GFP_KERNEL); + if (!blob_data) { + ret = -ENOMEM; + goto out; + } + + if (copy_from_user(blob_data, (void __user *)(unsigned long)blob_ptr, prop_value)) { + kfree(blob_data); + ret = -EFAULT; + goto out; + } + } + + /* User space sends the blob pointer even if we + * don't use it (length==0). + */ + if (prop->flags & DRM_MODE_PROP_BLOB) + copied_blobs++; + + /* The driver will be in charge of blob_data from now on. */ + ret = drm_mode_set_obj_prop(obj, state, prop, + prop_value, blob_data); + if (ret) + goto out; + + copied_props++; + } + } + + ret = dev->driver->atomic_check(dev, state); + if (ret) + goto out; + + if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) + goto out; + + ret = dev->driver->atomic_commit(dev, state); + + out: + dev->driver->atomic_end(dev, state); + + return ret; +} diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index d9137e4..cdadc1c 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -167,6 +167,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_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_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/drmP.h b/include/drm/drmP.h index 4c54c88..0a372b2 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1157,6 +1157,12 @@ struct drm_vblank_crtc { once per disable */ }; +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 2531658..6aae449 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1301,6 +1301,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 9b24d65..148c6c0 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -759,6 +759,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_obj_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 @@ -800,6 +801,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; +}; + /* typedef area */ #ifndef __KERNEL__ typedef struct drm_clip_rect drm_clip_rect_t; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index a913953..7d34ee0 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -508,6 +508,27 @@ struct drm_mode_destroy_dumb { uint32_t handle; }; +/* page-flip flags are valid, plus: */ +#define DRM_MODE_ATOMIC_TEST_ONLY 0x0100 +#define DRM_MODE_ATOMIC_NONBLOCK 0x0200 #define DRM_MODE_ATOMIC_NOLOCK 0x8000 /* only used internally */ +#define DRM_MODE_ATOMIC_FLAGS (\ + DRM_MODE_PAGE_FLIP_EVENT |\ + DRM_MODE_PAGE_FLIP_ASYNC |\ + DRM_MODE_ATOMIC_TEST_ONLY |\ + DRM_MODE_ATOMIC_NONBLOCK) + +/* 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