[PATCH v2] drm: don't run atomic_async_check for disabled planes

[Xaver Hugl]

It's entirely valid and correct for compositors to include disabled
planes in the atomic commit, and doing that should not prevent async
flips from working. To fix that, this commit moves the plane check
to after all the properties of the object have been set, and skips
the async checks if the plane was and still is not visible.

Fixes: fd40a63c drm/atomic (Let drivers decide which planes to async flip)
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4263

Signed-off-by: Xaver Hugl <xaver.hugl at kde.org>
---
 drivers/gpu/drm/drm_atomic_uapi.c   | 51 +++++++++++++++++++----------
 drivers/gpu/drm/drm_crtc_internal.h |  3 +-
 drivers/gpu/drm/drm_mode_object.c   |  3 +-
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index c2726af6698e..df298ac49dcd 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1011,7 +1011,8 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
 			    struct drm_mode_object *obj,
 			    struct drm_property *prop,
 			    u64 prop_value,
-			    bool async_flip)
+			    bool async_flip,
+			    bool *needs_async_plane_check)
 {
 	struct drm_mode_object *ref;
 	u64 old_val;
@@ -1068,7 +1069,6 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
 		struct drm_plane *plane = obj_to_plane(obj);
 		struct drm_plane_state *plane_state;
 		struct drm_mode_config *config = &plane->dev->mode_config;
-		const struct drm_plane_helper_funcs *plane_funcs = plane->helper_private;
 
 		plane_state = drm_atomic_get_plane_state(state, plane);
 		if (IS_ERR(plane_state)) {
@@ -1084,22 +1084,14 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
 				ret = drm_atomic_plane_get_property(plane, plane_state,
 								    prop, &old_val);
 				ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);
+				if (ret)
+				    break;
 			}
 
-			/* ask the driver if this non-primary plane is supported */
-			if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
-				ret = -EINVAL;
-
-				if (plane_funcs && plane_funcs->atomic_async_check)
-					ret = plane_funcs->atomic_async_check(plane, state, true);
-
-				if (ret) {
-					drm_dbg_atomic(prop->dev,
-						       "[PLANE:%d:%s] does not support async flips\n",
-						       obj->id, plane->name);
-					break;
-				}
-			}
+			/* Need to ask the driver if this non-primary plane is supported.
+			 * Note that this can't happen here, as the full state of the plane
+			 * is not known yet */
+			*needs_async_plane_check |= plane->type != DRM_PLANE_TYPE_PRIMARY;
 		}
 
 		ret = drm_atomic_plane_set_property(plane,
@@ -1394,6 +1386,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 	int ret = 0;
 	unsigned int i, j, num_fences;
 	bool async_flip = false;
+	bool needs_async_plane_check = false;
+	struct drm_plane *plane;
+	struct drm_plane_state *old_plane_state;
+	struct drm_plane_state *new_plane_state;
 
 	/* disallow for drivers not supporting atomic: */
 	if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
@@ -1450,6 +1446,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 	copied_props = 0;
 	fence_state = NULL;
 	num_fences = 0;
+	needs_async_plane_check = false;
 
 	for (i = 0; i < arg->count_objs; i++) {
 		uint32_t obj_id, count_props;
@@ -1512,7 +1509,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 			}
 
 			ret = drm_atomic_set_property(state, file_priv, obj,
-						      prop, prop_value, async_flip);
+						      prop, prop_value, async_flip,
+						      &needs_async_plane_check);
 			if (ret) {
 				drm_mode_object_put(obj);
 				goto out;
@@ -1521,6 +1519,25 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 			copied_props++;
 		}
 
+		if (needs_async_plane_check) {
+			plane = obj_to_plane(obj);
+			old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+			new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+			/* only do the check if the plane was or is enabled */
+			if (old_plane_state->visible || new_plane_state->visible)
+			    ret = -EINVAL;
+			if (ret &&
+			    plane->helper_private &&
+			    plane->helper_private->atomic_async_check) {
+				ret = plane->helper_private->atomic_async_check(plane, state, true);
+			}
+			if (ret) {
+				drm_dbg_atomic(dev, "[PLANE:%d:%s] does not support async flips\n",
+						obj->id, plane->name);
+				break;
+			}
+		}
+
 		drm_mode_object_put(obj);
 	}
 
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 89706aa8232f..111907a55d9b 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -260,7 +260,8 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
 			    struct drm_file *file_priv,
 			    struct drm_mode_object *obj,
 			    struct drm_property *prop,
-			    u64 prop_value, bool async_flip);
+			    u64 prop_value, bool async_flip,
+			    bool *needs_async_plane_check);
 int drm_atomic_get_property(struct drm_mode_object *obj,
 			    struct drm_property *property, uint64_t *val);
 
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index e943205a2394..ec471536cfbc 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -540,7 +540,8 @@ static int set_property_atomic(struct drm_mode_object *obj,
 						       obj_to_connector(obj),
 						       prop_value);
 	} else {
-		ret = drm_atomic_set_property(state, file_priv, obj, prop, prop_value, false);
+		ret = drm_atomic_set_property(state, file_priv, obj, prop, prop_value,
+					      false, NULL);
 		if (ret)
 			goto out;
 		ret = drm_atomic_commit(state);
-- 
2.50.1