[PATCH] drm/i915/display: Allow async flips on planes with no framebuffer changes

[PATCH] drm/i915/display: Allow async flips on planes with no framebuffer changes

[Naveen Kumar]

Allow asynchronous page flips on planes that either lack a framebuffer or
retain the same framebuffer, as long as no plane properties are modified.

This avoids unnecessary failures in async flip paths when the update is
effectively a no-op, improving compatibility with some compositors.

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13834
Signed-off-by: Naveen Kumar <naveen1.kumar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 18 +++++++++------
 drivers/gpu/drm/i915/display/intel_plane.c   | 24 ++++++++++++++++++++
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 456fc4b04cda..e0eb0de62ff4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5970,18 +5970,21 @@ static int intel_async_flip_check_uapi(struct intel_atomic_state *state,
 		 * this(vlv/chv and icl+) should be added when async flip is
 		 * enabled in the atomic IOCTL path.
 		 */
-		if (!plane->async_flip) {
+		if (!plane->async_flip && new_plane_state->uapi.fb) {
 			drm_dbg_kms(display->drm,
 				    "[PLANE:%d:%s] async flip not supported\n",
 				    plane->base.base.id, plane->base.name);
 			return -EINVAL;
 		}
 
-		if (!old_plane_state->uapi.fb || !new_plane_state->uapi.fb) {
-			drm_dbg_kms(display->drm,
-				    "[PLANE:%d:%s] no old or new framebuffer\n",
-				    plane->base.base.id, plane->base.name);
-			return -EINVAL;
+		if (plane->base.type != DRM_PLANE_TYPE_CURSOR &&
+		    plane->base.type != DRM_PLANE_TYPE_OVERLAY) {
+			if (!old_plane_state->uapi.fb || !new_plane_state->uapi.fb) {
+				drm_dbg_kms(display->drm,
+					    "[PLANE:%d:%s] no old or new framebuffer\n",
+					    plane->base.base.id, plane->base.name);
+				return -EINVAL;
+			}
 		}
 	}
 
@@ -6034,7 +6037,8 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in
 		 * an async flip. We should never get this far otherwise.
 		 */
 		if (drm_WARN_ON(display->drm,
-				new_crtc_state->do_async_flip && !plane->async_flip))
+				new_crtc_state->do_async_flip && !plane->async_flip &&
+				new_plane_state->hw.fb))
 			return -EINVAL;
 
 		/*
diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
index 36fb07471deb..434be91b57df 100644
--- a/drivers/gpu/drm/i915/display/intel_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_plane.c
@@ -1425,9 +1425,33 @@ static int intel_get_scanout_buffer(struct drm_plane *plane,
 	return 0;
 }
 
+static int
+intel_plane_atomic_async_check(struct drm_plane *plane,
+			       struct drm_atomic_state *_state, bool flip)
+{
+	struct intel_atomic_state *state = to_intel_atomic_state(_state);
+	struct intel_plane *intel_plane;
+	const struct intel_plane_state *old_plane_state;
+	struct intel_plane_state *new_plane_state;
+	int i;
+
+	for_each_oldnew_intel_plane_in_state(state, intel_plane, old_plane_state,
+					     new_plane_state, i) {
+		if (intel_plane->id != to_intel_plane(plane)->id)
+			continue;
+
+		/* no old or new framebuffer */
+		if (flip && !old_plane_state->uapi.fb && !new_plane_state->uapi.fb)
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
 static const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
 	.prepare_fb = intel_prepare_plane_fb,
 	.cleanup_fb = intel_cleanup_plane_fb,
+	.atomic_async_check = intel_plane_atomic_async_check,
 };
 
 static const struct drm_plane_helper_funcs intel_primary_plane_helper_funcs = {
-- 
2.48.1

Re: [PATCH] drm/i915/display: Allow async flips on planes with no framebuffer changes

[Ville Syrjälä]

On Mon, Jul 07, 2025 at 03:41:20PM +0000, Naveen Kumar wrote:
> Allow asynchronous page flips on planes that either lack a framebuffer or
> retain the same framebuffer, as long as no plane properties are modified.
> 
> This avoids unnecessary failures in async flip paths when the update is
> effectively a no-op, improving compatibility with some compositors.

IMO what we want to do is make the drm core filter out all the garbage
from the commit. That way the driver would only see the planes that are
supposed to actually participate in the async flip.

> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13834
> Signed-off-by: Naveen Kumar <naveen1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 18 +++++++++------
>  drivers/gpu/drm/i915/display/intel_plane.c   | 24 ++++++++++++++++++++
>  2 files changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 456fc4b04cda..e0eb0de62ff4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5970,18 +5970,21 @@ static int intel_async_flip_check_uapi(struct intel_atomic_state *state,
>  		 * this(vlv/chv and icl+) should be added when async flip is
>  		 * enabled in the atomic IOCTL path.
>  		 */
> -		if (!plane->async_flip) {
> +		if (!plane->async_flip && new_plane_state->uapi.fb) {
>  			drm_dbg_kms(display->drm,
>  				    "[PLANE:%d:%s] async flip not supported\n",
>  				    plane->base.base.id, plane->base.name);
>  			return -EINVAL;
>  		}
>  
> -		if (!old_plane_state->uapi.fb || !new_plane_state->uapi.fb) {
> -			drm_dbg_kms(display->drm,
> -				    "[PLANE:%d:%s] no old or new framebuffer\n",
> -				    plane->base.base.id, plane->base.name);
> -			return -EINVAL;
> +		if (plane->base.type != DRM_PLANE_TYPE_CURSOR &&
> +		    plane->base.type != DRM_PLANE_TYPE_OVERLAY) {
> +			if (!old_plane_state->uapi.fb || !new_plane_state->uapi.fb) {
> +				drm_dbg_kms(display->drm,
> +					    "[PLANE:%d:%s] no old or new framebuffer\n",
> +					    plane->base.base.id, plane->base.name);
> +				return -EINVAL;
> +			}
>  		}
>  	}
>  
> @@ -6034,7 +6037,8 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in
>  		 * an async flip. We should never get this far otherwise.
>  		 */
>  		if (drm_WARN_ON(display->drm,
> -				new_crtc_state->do_async_flip && !plane->async_flip))
> +				new_crtc_state->do_async_flip && !plane->async_flip &&
> +				new_plane_state->hw.fb))
>  			return -EINVAL;
>  
>  		/*
> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
> index 36fb07471deb..434be91b57df 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane.c
> @@ -1425,9 +1425,33 @@ static int intel_get_scanout_buffer(struct drm_plane *plane,
>  	return 0;
>  }
>  
> +static int
> +intel_plane_atomic_async_check(struct drm_plane *plane,
> +			       struct drm_atomic_state *_state, bool flip)
> +{
> +	struct intel_atomic_state *state = to_intel_atomic_state(_state);
> +	struct intel_plane *intel_plane;
> +	const struct intel_plane_state *old_plane_state;
> +	struct intel_plane_state *new_plane_state;
> +	int i;
> +
> +	for_each_oldnew_intel_plane_in_state(state, intel_plane, old_plane_state,
> +					     new_plane_state, i) {
> +		if (intel_plane->id != to_intel_plane(plane)->id)
> +			continue;
> +
> +		/* no old or new framebuffer */
> +		if (flip && !old_plane_state->uapi.fb && !new_plane_state->uapi.fb)
> +			return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>  	.prepare_fb = intel_prepare_plane_fb,
>  	.cleanup_fb = intel_cleanup_plane_fb,
> +	.atomic_async_check = intel_plane_atomic_async_check,
>  };
>  
>  static const struct drm_plane_helper_funcs intel_primary_plane_helper_funcs = {
> -- 
> 2.48.1

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/i915/display: Allow async flips on planes with no framebuffer changes

[Xaver Hugl]

Am Do., 10. Juli 2025 um 22:21 Uhr schrieb Ville Syrjälä
<ville.syrjala@linux.intel.com>:
>
> On Mon, Jul 07, 2025 at 03:41:20PM +0000, Naveen Kumar wrote:
> > Allow asynchronous page flips on planes that either lack a framebuffer or
> > retain the same framebuffer, as long as no plane properties are modified.
> >
> > This avoids unnecessary failures in async flip paths when the update is
> > effectively a no-op, improving compatibility with some compositors.
>
> IMO what we want to do is make the drm core filter out all the garbage
> from the commit. That way the driver would only see the planes that are
> supposed to actually participate in the async flip.

That sounds like a good goal, but I think it'll need some extra care
to avoid regressions. For example, "no-op" pageflips must still
trigger pageflip events, and affect the refresh rate with adaptive
sync.

Re: [PATCH] drm/i915/display: Allow async flips on planes with no framebuffer changes

[Ville Syrjälä]

On Sat, Jul 19, 2025 at 02:16:22AM +0200, Xaver Hugl wrote:
> Am Do., 10. Juli 2025 um 22:21 Uhr schrieb Ville Syrjälä
> <ville.syrjala@linux.intel.com>:
> >
> > On Mon, Jul 07, 2025 at 03:41:20PM +0000, Naveen Kumar wrote:
> > > Allow asynchronous page flips on planes that either lack a framebuffer or
> > > retain the same framebuffer, as long as no plane properties are modified.
> > >
> > > This avoids unnecessary failures in async flip paths when the update is
> > > effectively a no-op, improving compatibility with some compositors.
> >
> > IMO what we want to do is make the drm core filter out all the garbage
> > from the commit. That way the driver would only see the planes that are
> > supposed to actually participate in the async flip.
> 
> That sounds like a good goal, but I think it'll need some extra care
> to avoid regressions. For example, "no-op" pageflips must still
> trigger pageflip events, and affect the refresh rate with adaptive
> sync.

Not for async commit via the atomic ioctl. That's where I'd want
the filtering.

For sync commit nothing can really be filtered by the core code
because side effects must be observed, and only the driver knows
what all the side effects are.

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/i915/display: Allow async flips on planes with no framebuffer changes

[Michel Dänzer]

On 19.07.25 17:52, Ville Syrjälä wrote:
> On Sat, Jul 19, 2025 at 02:16:22AM +0200, Xaver Hugl wrote:
>> Am Do., 10. Juli 2025 um 22:21 Uhr schrieb Ville Syrjälä
>> <ville.syrjala@linux.intel.com>:
>>>
>>> On Mon, Jul 07, 2025 at 03:41:20PM +0000, Naveen Kumar wrote:
>>>> Allow asynchronous page flips on planes that either lack a framebuffer or
>>>> retain the same framebuffer, as long as no plane properties are modified.
>>>>
>>>> This avoids unnecessary failures in async flip paths when the update is
>>>> effectively a no-op, improving compatibility with some compositors.
>>>
>>> IMO what we want to do is make the drm core filter out all the garbage
>>> from the commit. That way the driver would only see the planes that are
>>> supposed to actually participate in the async flip.
>>
>> That sounds like a good goal, but I think it'll need some extra care
>> to avoid regressions. For example, "no-op" pageflips must still
>> trigger pageflip events, and affect the refresh rate with adaptive
>> sync.
> 
> Not for async commit via the atomic ioctl. That's where I'd want
> the filtering.
> 
> For sync commit nothing can really be filtered by the core code
> because side effects must be observed, and only the driver knows
> what all the side effects are.

I agree with Xaver.


In general, the DRM_MODE_PAGE_FLIP_ASYNC flag should affect only whether or not a plane assignment can take effect outside of vertical blank. That's true even with VRR. Current UAPI is that any commit with any state for a CRTC (or a plane bound to it) must trigger a refresh cycle on it ASAP with VRR.


With DRM_MODE_PAGE_FLIP_EVENT, an event must be sent even for "no-op" changes, or user space may get stuck waiting for the event.


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast