[PATCH v3 0/6] Add support for atomic async page-flips

Simon Ser contact at emersion.fr
Thu Oct 13 16:02:39 UTC 2022


> > > 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);


More information about the dri-devel mailing list