[PATCH RFC v2 0/2] Async Flip in Atomic ioctl corrections

[PATCH RFC v2 0/2] Async Flip in Atomic ioctl corrections

[Arun R Murthy]

struct drm_crtc_state {
         /**
          * @async_flip:
          *
          * This is set when DRM_MODE_PAGE_FLIP_ASYNC is set in the legacy
          * PAGE_FLIP IOCTL. It's not wired up for the atomic IOCTL
itself yet.
          */
         bool async_flip;

In the existing code the flag async_flip was intended for the legacy
PAGE_FLIP IOCTL. But the same is being used for atomic IOCTL.
As per the hardware feature is concerned, async flip is a plane feature
and is to be treated per plane basis and not per pipe basis.
For a given hardware pipe, among the multiple hardware planes, one can
go with sync flip and other 2/3 can go with async flip.
Tearing affect will be noticed with this and if any policy should be
taken care by the user space. KMD not to include any policy
as to allow async on only one plane for a given pipe as all policy done
in user and KMD exposes what is supported by the hardware.

There would be a bunch of changes to correct this in the atomic path.
Add a new async_flip plane property to allow user enable async flip on
the required plane.
Any restriction checks for async_flip will be taken in atomic_check()
and not in the atomic_ioctl().
Let the preset code reside as is even in the atomic patch until all the
existing drivers and user space implementations move to plane property
for async flips.
Changes include removal of the checks we have in atomic path so as to
reject any changes(different plane, pipe, connector) along with async
flip. This would be replaced with checks so as to reject any change in
that particular plane where async is enabled(reject any change in
pipe/connector as that would have impact on this plane)

With the above changes, the challenge that we have presently so as to
enable async flip on overlays which is handled seperately with if
condition in drm_atomic_uapi.c can be moved to driver specific
atomic_check code.

This series depicts the changes in the drm core and upon getting
feedback on this, driver specific changes for theis will be done in the
next revision on the same series.

Please let us know your opinion on this.

Thanks and Regards,
Arun R Murthy
-------------

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
Arun R Murthy (2):
      drm/atomic/plane: Add plane property for async flip
      drm/atomic: Use async_flip plane property for asynchronous flips

 drivers/gpu/drm/drm_atomic_uapi.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/drm_plane.c       | 31 +++++++++++++++++++++++++++++++
 include/drm/drm_plane.h           | 12 ++++++++++++
 3 files changed, 65 insertions(+)
---
base-commit: ac65b2261663d50cc761c94a40b6093ee714e62f
change-id: 20251010-async-feb09912440b

Best regards,
-- 
Arun R Murthy <arun.r.murthy@intel.com>

[PATCH RFC v2 1/2] drm/atomic/plane: Add plane property for async flip

[Arun R Murthy]

Add a new property for enabling/disabling async flip on a plane for
atomic path. Certain vendors have support for async flip on more than
one plane and with the present implementation using the flag, async flip
can be enabled on only one plane.
Adding a plane property for async flip enables driver to allow async
flip on multiple planes in atomic path.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
 drivers/gpu/drm/drm_plane.c       | 31 +++++++++++++++++++++++++++++++
 include/drm/drm_plane.h           | 12 ++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 85dbdaa4a2e25878c953b9b41539c8566d55c6d9..7773e0057302fccb57df8067f417b23a9cb9fcde 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -550,6 +550,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 		return ret;
 	} else if (property == plane->scaling_filter_property) {
 		state->scaling_filter = val;
+	} else if (property == plane->async_flip_property) {
+		state->async_flip = val;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -627,6 +629,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 			state->fb_damage_clips->base.id : 0;
 	} else if (property == plane->scaling_filter_property) {
 		*val = state->scaling_filter;
+	} else if (property == plane->async_flip_property) {
+		*val = state->async_flip;
 	} else if (plane->funcs->atomic_get_property) {
 		return plane->funcs->atomic_get_property(plane, state, property, val);
 	} else if (property == plane->hotspot_x_property) {
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 38f82391bfda578d532499585066dd85ff573910..da486defed87eeb01dd9ad5f3d791900286f9f34 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1820,3 +1820,34 @@ int drm_plane_add_size_hints_property(struct drm_plane *plane,
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_add_size_hints_property);
+
+/**
+ * drm_plane_create_async_flip_property - create asynchronous flip property
+ *
+ * @plane: drm plane
+ *
+ * Create a property to enable/disable asynchronous flip on the plane.
+ *
+ * RETURNS:
+ * Zero for success or -errno
+ */
+int drm_plane_create_async_flip_property(struct drm_plane *plane)
+{
+	struct drm_property *prop;
+
+	prop = drm_property_create_bool(plane->dev, DRM_MODE_PROP_IMMUTABLE,
+					"async_flip");
+
+	if (!prop)
+		return -ENOMEM;
+
+	drm_object_attach_property(&plane->base, prop, false);
+
+	plane->async_flip_property = prop;
+
+	if (plane->state)
+		plane->state->async_flip = false;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_async_flip_property);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 01479dd94e76a8389a0c9e9d6744400aa2291064..b649c57437a8303be094e652ed5be7a735f6f5e0 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -260,6 +260,12 @@ struct drm_plane_state {
 	 * flow.
 	 */
 	bool color_mgmt_changed : 1;
+
+	/**
+	 * @async_flip: Indicate that the present plane is asynchronous flip
+	 * mode.
+	 */
+	bool async_flip;
 };
 
 static inline struct drm_rect
@@ -799,6 +805,11 @@ struct drm_plane {
 	 */
 	struct drm_property *hotspot_y_property;
 
+	/**
+	 * @async_flip_property: property to set asynchronous flip on the plane
+	 */
+	struct drm_property *async_flip_property;
+
 	/**
 	 * @kmsg_panic: Used to register a panic notifier for this plane
 	 */
@@ -1005,5 +1016,6 @@ int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
 int drm_plane_add_size_hints_property(struct drm_plane *plane,
 				      const struct drm_plane_size_hint *hints,
 				      int num_hints);
+int drm_plane_create_async_flip_property(struct drm_plane *plane);
 
 #endif

-- 
2.25.1

[PATCH RFC v2 2/2] drm/atomic: Use async_flip plane property for asynchronous flips

[Arun R Murthy]

In atomic path, the crtc_state->async_flip is being used. Its commented
in drm_crtc.h that async_flip in crtc_state is to be used with the
legacy page_flip ioctl for async flips and not in atomic path. Morever
this limits of enabling async flip only on one plane even though
hardware supports them on all the planes. Retaining the present
implementation, support for using async_flip plane property is added.
Further checks for async flip will be done in the driver specific
atomic_check()

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 7773e0057302fccb57df8067f417b23a9cb9fcde..111afc8e07aa531d03d237c61cb4b8216d645fb6 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1075,6 +1075,11 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
 		struct drm_mode_config *config = &plane->dev->mode_config;
 		const struct drm_plane_helper_funcs *plane_funcs = plane->helper_private;
 
+		if (async_flip && plane->async_flip_property) {
+			drm_err(prop->dev, "driver has plane async_flip property, dont use the legacy one\n");
+			return -EINVAL;
+		}
+
 		plane_state = drm_atomic_get_plane_state(state, plane);
 		if (IS_ERR(plane_state)) {
 			ret = PTR_ERR(plane_state);
@@ -1377,9 +1382,22 @@ static void
 set_async_flip(struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
+	struct drm_plane *plane;
 	struct drm_crtc_state *crtc_state;
+	struct drm_plane_state *plane_state;
 	int i;
 
+	/*
+	 * Async flip is being set with plane async_flip property then bypass
+	 * the legacy async path in atomic_ioctl.
+	 * TODO: the below legacy path should be removed once all drivers
+	 * start using the async_flip plane property.
+	 */
+	for_each_new_plane_in_state(state, plane, plane_state, i) {
+		if (plane_state->async_flip)
+			return;
+	}
+
 	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
 		crtc_state->async_flip = true;
 	}

-- 
2.25.1

Re: [PATCH RFC v2 0/2] Async Flip in Atomic ioctl corrections

[Ville Syrjälä]

On Fri, Oct 10, 2025 at 02:15:57PM +0530, Arun R Murthy wrote:
> struct drm_crtc_state {
>          /**
>           * @async_flip:
>           *
>           * This is set when DRM_MODE_PAGE_FLIP_ASYNC is set in the legacy
>           * PAGE_FLIP IOCTL. It's not wired up for the atomic IOCTL
> itself yet.
>           */
>          bool async_flip;
> 
> In the existing code the flag async_flip was intended for the legacy
> PAGE_FLIP IOCTL. But the same is being used for atomic IOCTL.
> As per the hardware feature is concerned, async flip is a plane feature
> and is to be treated per plane basis and not per pipe basis.

You can't treat is as a purely per plane thing. The real problem that
neesd solving is that commit completion happens on a per-crtc basis,
and userspace is dumb enough to try to shove both sync and async stuff
into the same commit. That simply cannot work (unless we downgrade
all such async commits into sync commits).

The sort of compromise that people seem to have agreed on is that we
can allow objects and properties in the async commit that would normally
require a sync commit, but only if their value isn't changing. And
then someone is supposed to filter those out so they don't end up
requiring a real sync commit on the hardware.

I think the only sane way forward is to make the drm core do said
filtering, so that in the end the driver will only see the things
in the commit that are actually supposed to take part in the async
commit. I think in practice it would just involve removing all the
planes with zero property changes from the atomic state before
handing it over to the driver. But so far no one has volunteered
to write that code.

There is of course still the remaining problem of what happens when
there are multiple async flips for different planes in the same commit.
The hardware will complete those at different times, so the driver will
still need to make sure it doesn't signal full commit completion until
all the async flips have finished. Maybe we could eventually add
something to the common commit machinery to help with this, but I
guess we should first implement it in a few drivers.

-- 
Ville Syrjälä
Intel

Re: [PATCH RFC v2 0/2] Async Flip in Atomic ioctl corrections

[Murthy, Arun R]

On 10-10-2025 19:02, Ville Syrjälä wrote:
> On Fri, Oct 10, 2025 at 02:15:57PM +0530, Arun R Murthy wrote:
>> struct drm_crtc_state {
>>           /**
>>            * @async_flip:
>>            *
>>            * This is set when DRM_MODE_PAGE_FLIP_ASYNC is set in the legacy
>>            * PAGE_FLIP IOCTL. It's not wired up for the atomic IOCTL
>> itself yet.
>>            */
>>           bool async_flip;
>>
>> In the existing code the flag async_flip was intended for the legacy
>> PAGE_FLIP IOCTL. But the same is being used for atomic IOCTL.
>> As per the hardware feature is concerned, async flip is a plane feature
>> and is to be treated per plane basis and not per pipe basis.
> You can't treat is as a purely per plane thing. The real problem that
> neesd solving is that commit completion happens on a per-crtc basis,
> and userspace is dumb enough to try to shove both sync and async stuff
> into the same commit. That simply cannot work (unless we downgrade
> all such async commits into sync commits).
Yes, the commit completion should happen per-crtc basis. Also as said 
below in
order to handle multiple async flips, this async_flip flag which is in 
crtc_state
should be moved to plane_state. Then we will have to work on the driver
policy for async and sync flips.
> The sort of compromise that people seem to have agreed on is that we
> can allow objects and properties in the async commit that would normally
> require a sync commit, but only if their value isn't changing. And
> then someone is supposed to filter those out so they don't end up
> requiring a real sync commit on the hardware.
Do you mean to allow only fb change for sync flips along with async flip?
Noted, will take care of this while implementing.
> I think the only sane way forward is to make the drm core do said
> filtering, so that in the end the driver will only see the things
> in the commit that are actually supposed to take part in the async
> commit. I think in practice it would just involve removing all the
> planes with zero property changes from the atomic state before
> handing it over to the driver. But so far no one has volunteered
> to write that code.
This will be done in this series. The first set of patches has been 
floated to
get the feedback on these kind of policies. Will take care of this!
>
> There is of course still the remaining problem of what happens when
> there are multiple async flips for different planes in the same commit.
> The hardware will complete those at different times, so the driver will
> still need to make sure it doesn't signal full commit completion until
> all the async flips have finished. Maybe we could eventually add
> something to the common commit machinery to help with this, but I
> guess we should first implement it in a few drivers.
Yes policy will be added for the same to handle multiple async flips 
within a
single commit.
Sure we can take up the implementation with i915 first.
Thanks for the feedback. With all these comments, I can list down the below
tasks
1. Use async_flip flag in plane_state along with a plane property for async
flip enabling. (Done in this RFC)
2. Policy to filter out any changes other than fb. Add-on to this allow even
sync flips if there is no change in the property except fb.
3. For multiple async commits ensure notification/releasing fb after
completion of all the async flips in that crtc.

With these task listed out, was think if we break the patch series into 3
as listed in the above task will be easier to handle rather then coming
up with a huge list of patches in a single series.

Thanks and Regards,
Arun R Murthy
-------------------