> > > So no tests that actually verify that the kernel properly rejects
> > > stuff stuff like modesets, gamma LUT updates, plane movement,
> > > etc.?
> >
> > Pondering this a bit more, it just occurred to me the current driver
> > level checks might easily lead to confusing behaviour. Eg. is
> > the ioctl going to succeed if you ask for an async change of some
> > random property while the crtc disabled, but fails if you ask for
> > the same async property change when the crtc is active?
> >
> > So another reason why rejecting most properties already at
> > the uapi level might be a good idea.
> 
> And just to be clear this is pretty much what I suggest:
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 79730fa1dd8e..471a2c703847 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1392,6 +1392,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>                               goto out;
>                       }
> 
> +                     if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC &&
> +                         prop != dev->mode_config.prop_fb_id) {
> +                             drm_mode_object_put(obj);
> +                             ret = -EINVAL;
> +                             goto out;
> +                     }
> +
>                       if (copy_from_user(&prop_value,
>                                          prop_values_ptr + copied_props,
>                                          sizeof(prop_value))) {
> 
> 
> That would actively discourage people from even attempting the
> "just dump all the state into the ioctl" approach with async flips
> since even the props whose value isn't even changing would be rejected.

How does this sound?

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 945761968428..ffd16bdc7b83 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -972,14 +972,26 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
                            struct drm_file *file_priv,
                            struct drm_mode_object *obj,
                            struct drm_property *prop,
-                           uint64_t prop_value)
+                           uint64_t prop_value,
+                           bool async_flip)
 {
        struct drm_mode_object *ref;
        int ret;
+       uint64_t old_val;
 
        if (!drm_property_change_valid_get(prop, prop_value, &ref))
                return -EINVAL;
 
+       if (async_flip && prop != prop->dev->mode_config.prop_fb_id) {
+               ret = drm_atomic_get_property(obj, prop, &old_val);
+               if (ret != 0 || old_val != prop_value) {
+                       drm_dbg_atomic(prop->dev,
+                                      "[PROP:%d:%s] cannot be changed during 
async flip\n",
+                                      prop->base.id, prop->name);
+                       return -EINVAL;
+               }
+       }
+
        switch (obj->type) {
        case DRM_MODE_OBJECT_CONNECTOR: {
                struct drm_connector *connector = obj_to_connector(obj);

Reply via email to