On 01-08-2025 18:40, Xaver Hugl wrote:
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,
I dont think this is required. Again the plane states will have to be
fetched outside the set_prop()
Alternate approach
@@ -1091,8 +1091,16 @@ int drm_atomic_set_property(struct
drm_atomic_state *state,
/* ask the driver if this non-primary plane is
supported */
if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
- ret = -EINVAL;
+ /*
+ * continue if no change in prop on
non-supported async planes as well
+ * or when disabling the plane
+ */
+ if (ret == 0 || (prop ==
config->prop_fb_id && prop_value == 0))
+ drm_dbg_atomic(prop->dev,
+ "[PLANE:%d:%s] continue async as there is no prop change\n",
+ obj->id,
plane->name);
+ else
+ ret = -EINVAL;
if (plane_funcs &&
plane_funcs->atomic_async_check)
Thanks and Regards,
Arun R Murthy
--------------------
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.h...@kde.org>
---
drivers/gpu/drm/drm_atomic_uapi.c | 51 +++++++++++++++++++++----------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index c2726af6698e..2ae41a522e92 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1068,7 +1068,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,21 +1083,8 @@ 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);
- }
-
- /* 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;
- }
+ if (ret)
+ break;
}
}
@@ -1394,6 +1380,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
int ret = 0;
unsigned int i, j, num_fences;
bool async_flip = false;
+ struct drm_plane *plane;
+ struct drm_plane_state *old_plane_state = NULL;
+ struct drm_plane_state *new_plane_state = NULL;
+ u64 fb_id = 0;
/* disallow for drivers not supporting atomic: */
if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
@@ -1521,6 +1511,35 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
copied_props++;
}
+ if (async_flip && obj->type == DRM_MODE_OBJECT_PLANE &&
+ obj_to_plane(obj)->type != DRM_PLANE_TYPE_PRIMARY) {
+ /* need to ask the driver if this plane is supported */
+ 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);
+ ret = drm_atomic_plane_get_property(plane,
new_plane_state,
+
dev->mode_config.prop_fb_id,
+ &fb_id);
+ if (ret)
+ break;
+ /*
+ * Only do the check if the plane was or is enabled.
+ * Note that the new state doesn't have "visible" set
yet,
+ * so this uses fb_id instead.
+ */
+ if (old_plane_state->visible || fb_id)
+ 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);
}
