* [PATCH] drm/atomic: introduce PAGE_FLIP_EVENT property
@ 2026-05-30 17:14 Simon Ser
2026-05-31 9:10 ` Xaver Hugl
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Simon Ser @ 2026-05-30 17:14 UTC (permalink / raw)
To: dri-devel
Cc: wayland-devel, Daniel Stone, Michel Dänzer, Pekka Paalanen,
Christian König, Simona Vetter
Currently user-space can only request page-flip events for all CRTCs included
in an atomic commit. This is cumbersome with multi-CRTC commits: when moving a
plane from a different CRTC, or when disabling a plane, user-space has no way
to opt-out of the page-flip events.
Additionally, page-flip events are invalid for already-disable planes, so
user-space needs to add special cases [1]. Libraries cannot add planes to the
commit without risking causing unexpected page-flip events for the
compositor [2]. Figuring out what CRTCs get implicitly included in a commit is
non-trivial [3].
Introduce a new PAGE_FLIP_EVENT CRTC property so that user-space can select
explicitly which CRTCs will get page-flip events. The property is very
similar to IN_FENCE_FD/OUT_FENCE_PTR: it always reads as zero, and 1 can
be written to request an event.
A user-space implementation is available at [4].
[1]: https://github.com/ValveSoftware/gamescope/blob/67fcb3aebae0ae1eb2d1cff60f29e6d9fe1d128f/src/Backends/DRMBackend.cpp#L2897
[2]: https://gitlab.freedesktop.org/emersion/libliftoff/-/merge_requests/85
[3]: https://lore.kernel.org/dri-devel/20250501112945.6448-1-contact@emersion.fr/
[4]: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/5387
Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Michel Dänzer <michel.daenzer@mailbox.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Simona Vetter <simona@ffwll.ch>
---
drivers/gpu/drm/drm_atomic_uapi.c | 17 +++++++++++++++--
drivers/gpu/drm/drm_crtc.c | 7 +++++++
drivers/gpu/drm/drm_mode_config.c | 6 ++++++
include/drm/drm_crtc.h | 6 ++++++
include/drm/drm_mode_config.h | 4 ++++
5 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 6441b55cc274..21f2b0df4f63 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -466,6 +466,14 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
return -EFAULT;
set_out_fence_for_crtc(state->state, crtc, fence_ptr);
+ } else if (property == config->prop_page_flip_event) {
+ if (val != 1) {
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] PAGE_FLIP_EVENT can only be set to 1",
+ crtc->base.id, crtc->name);
+ return -EINVAL;
+ }
+ state->page_flip_event_requested = val;
} else if (property == crtc->scaling_filter_property) {
state->scaling_filter = val;
} else if (property == crtc->sharpness_strength_property) {
@@ -507,6 +515,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = state->background_color;
else if (property == config->prop_out_fence_ptr)
*val = 0;
+ else if (property == config->prop_page_flip_event)
+ *val = 0;
else if (property == crtc->scaling_filter_property)
*val = state->scaling_filter;
else if (property == crtc->sharpness_strength_property)
@@ -1385,6 +1395,7 @@ static int prepare_signaling(struct drm_device *dev,
struct drm_connector *conn;
struct drm_connector_state *conn_state;
int i, c = 0, ret;
+ bool page_flip_event_requested;
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
return 0;
@@ -1392,9 +1403,11 @@ static int prepare_signaling(struct drm_device *dev,
for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
s32 __user *fence_ptr;
+ page_flip_event_requested = arg->flags & DRM_MODE_PAGE_FLIP_EVENT ||
+ crtc_state->page_flip_event_requested;
fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
- if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
+ if (page_flip_event_requested || fence_ptr) {
struct drm_pending_vblank_event *e;
e = create_vblank_event(crtc, arg->user_data);
@@ -1404,7 +1417,7 @@ static int prepare_signaling(struct drm_device *dev,
crtc_state->event = e;
}
- if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
+ if (page_flip_event_requested) {
struct drm_pending_vblank_event *e = crtc_state->event;
if (!file_priv)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 63ead8ba6756..5ad6681bd47c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -248,6 +248,11 @@ struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc)
* The sharpness effect takes place post blending on the final composed output.
* If the feature is disabled, the content remains same without any sharpening effect
* and when this feature is applied, it enhances the clarity of the content.
+ * PAGE_FLIP_EVENT:
+ * Atomic property for requesting a page-flip event on this CRTC.
+ *
+ * The value of this property is an integer value which always reads as
+ * zero and can be written with 1 to request the event.
*/
__printf(6, 0)
@@ -322,6 +327,8 @@ static int __drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *
config->prop_out_fence_ptr, 0);
drm_object_attach_property(&crtc->base,
config->prop_vrr_enabled, 0);
+ drm_object_attach_property(&crtc->base,
+ config->prop_page_flip_event, 0);
}
return 0;
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index f432f485a914..688937d2e1ec 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -496,6 +496,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_out_fence_ptr = prop;
+ prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+ "PAGE_FLIP_EVENT", 0, 1);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_page_flip_event = prop;
+
prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
"CRTC_ID", DRM_MODE_OBJECT_CRTC);
if (!prop)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 152349f973e3..b0f9130d263a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -311,6 +311,12 @@ struct drm_crtc_state {
*/
bool vrr_enabled;
+ /**
+ * @page_flip_event_requested: indicates whether a page-flip event has
+ * been requested on this CRTC.
+ */
+ bool page_flip_event_requested;
+
/**
* @self_refresh_active:
*
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index d8f5b7e9673e..13f39677c32f 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -670,6 +670,10 @@ struct drm_mode_config {
* value of type s32, and then cast that pointer to u64.
*/
struct drm_property *prop_out_fence_ptr;
+ /**
+ * @page_flip_event_property: property to request page-flip events.
+ */
+ struct drm_property *prop_page_flip_event;
/**
* @prop_crtc_id: Default atomic plane property to specify the
* &drm_crtc.
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] drm/atomic: introduce PAGE_FLIP_EVENT property
2026-05-30 17:14 [PATCH] drm/atomic: introduce PAGE_FLIP_EVENT property Simon Ser
@ 2026-05-31 9:10 ` Xaver Hugl
2026-05-31 9:26 ` Simon Ser
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Xaver Hugl @ 2026-05-31 9:10 UTC (permalink / raw)
To: Simon Ser
Cc: dri-devel, wayland-devel, Daniel Stone, Michel Dänzer,
Pekka Paalanen, Christian König, Simona Vetter
+1, this is very useful. A KWin implementation for this is at
https://invent.kde.org/plasma/kwin/-/merge_requests/9308
- Xaver
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/atomic: introduce PAGE_FLIP_EVENT property
2026-05-30 17:14 [PATCH] drm/atomic: introduce PAGE_FLIP_EVENT property Simon Ser
2026-05-31 9:10 ` Xaver Hugl
@ 2026-05-31 9:26 ` Simon Ser
2026-06-04 5:29 ` Claude review: " Claude Code Review Bot
2026-06-04 5:29 ` Claude Code Review Bot
3 siblings, 0 replies; 5+ messages in thread
From: Simon Ser @ 2026-05-31 9:26 UTC (permalink / raw)
To: dri-devel
Cc: wayland-devel, Daniel Stone, Michel Dänzer, Pekka Paalanen,
Christian König, Simona Vetter
IGT test available here:
https://lists.freedesktop.org/archives/igt-dev/2026-May/111026.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Claude review: drm/atomic: introduce PAGE_FLIP_EVENT property
2026-05-30 17:14 [PATCH] drm/atomic: introduce PAGE_FLIP_EVENT property Simon Ser
2026-05-31 9:10 ` Xaver Hugl
2026-05-31 9:26 ` Simon Ser
@ 2026-06-04 5:29 ` Claude Code Review Bot
2026-06-04 5:29 ` Claude Code Review Bot
3 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-06-04 5:29 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Bug: Missing reset in `__drm_atomic_helper_crtc_duplicate_state()`**
Looking at `drivers/gpu/drm/drm_atomic_state_helper.c:135-161`, when a new CRTC state is duplicated from the current state, several per-commit fields are explicitly zeroed:
```c
state->event = NULL;
state->async_flip = false;
```
The new `page_flip_event_requested` field is a per-commit property (it always reads as zero, must be explicitly set each commit), but it is **not** cleared in `__drm_atomic_helper_crtc_duplicate_state()`. After `memcpy(state, crtc->state, sizeof(*state))`, if the previous committed state had `page_flip_event_requested = true`, the new state would inherit that value. This would cause spurious page-flip events on subsequent commits that don't set the property.
This is the most important fix needed — add `state->page_flip_event_requested = false;` alongside the other cleared fields.
**Validation of `val != 1` in set_property:**
```c
+ } else if (property == config->prop_page_flip_event) {
+ if (val != 1) {
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] PAGE_FLIP_EVENT can only be set to 1",
+ crtc->base.id, crtc->name);
+ return -EINVAL;
+ }
+ state->page_flip_event_requested = val;
```
This rejects `val == 0`. The property is created as a range `[0, 1]`, so userspace can set it to 0 or 1. Setting it to 0 should be a valid no-op (it's the default). Rejecting 0 means userspace can't explicitly include this property in a commit with value 0, which is inconsistent with how other properties work. Consider:
```c
if (val > 1) {
...
return -EINVAL;
}
state->page_flip_event_requested = val;
```
Or alternatively, if 0 should truly be rejected, the range should be created as `(1, 1)` — but that seems unnecessarily restrictive.
**Interaction between flag and property:**
```c
+ page_flip_event_requested = arg->flags & DRM_MODE_PAGE_FLIP_EVENT ||
+ crtc_state->page_flip_event_requested;
```
The OR logic means the global flag still works as before, and the per-CRTC property adds granularity on top. This is correct for backwards compatibility. However, what happens if userspace uses *both* the flag and the property? The commit message doesn't discuss this. It works fine because the result is the same (event is requested), but it might be worth documenting that the property takes precedence/supplements the flag.
**Missing `c == 0` check update:**
At the end of `prepare_signaling()`:
```c
if (c == 0 && (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) {
drm_dbg_atomic(dev, "need at least one CRTC for DRM_MODE_PAGE_FLIP_EVENT");
return -EINVAL;
}
```
This check only looks at `arg->flags & DRM_MODE_PAGE_FLIP_EVENT` for the "no CRTCs in commit" error. With the new property, `c` will always be > 0 if any CRTC has `page_flip_event_requested` set (because the CRTC is in the state), so this is probably fine. But it's worth verifying the intent — can userspace set the property on a CRTC that's not otherwise part of the commit? The property setter adds it to the state, so `c` would be incremented, meaning this check is correct.
**Property documentation:**
```c
+ * PAGE_FLIP_EVENT:
+ * Atomic property for requesting a page-flip event on this CRTC.
+ *
+ * The value of this property is an integer value which always reads as
+ * zero and can be written with 1 to request the event.
```
The documentation is clear but could mention the relationship with the existing `DRM_MODE_PAGE_FLIP_EVENT` flag — i.e., that this is a per-CRTC alternative to the global flag, and both can be used together (the result is OR'd).
**Struct member docstring mismatch:**
In `drm_mode_config.h`:
```c
+ /**
+ * @page_flip_event_property: property to request page-flip events.
+ */
+ struct drm_property *prop_page_flip_event;
```
The doc name `@page_flip_event_property` doesn't match the field name `prop_page_flip_event`. It should be `@prop_page_flip_event`.
**Overall:** A clean and well-motivated patch. The main issue to fix before merging is the missing state reset in `__drm_atomic_helper_crtc_duplicate_state()`. The val==0 rejection is worth reconsidering for consistency with other range properties. The docstring field name mismatch is minor but should be fixed.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread* Claude review: drm/atomic: introduce PAGE_FLIP_EVENT property
2026-05-30 17:14 [PATCH] drm/atomic: introduce PAGE_FLIP_EVENT property Simon Ser
` (2 preceding siblings ...)
2026-06-04 5:29 ` Claude review: " Claude Code Review Bot
@ 2026-06-04 5:29 ` Claude Code Review Bot
3 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-06-04 5:29 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/atomic: introduce PAGE_FLIP_EVENT property
Author: Simon Ser <contact@emersion.fr>
Patches: 3
Reviewed: 2026-06-04T15:29:59.502018
---
This is a single patch from Simon Ser that introduces a new `PAGE_FLIP_EVENT` atomic CRTC property, allowing userspace to request page-flip events on a per-CRTC basis rather than the current all-or-nothing `DRM_MODE_PAGE_FLIP_EVENT` flag. The motivation is well-explained and addresses real pain points in compositors (gamescope, libliftoff, wlroots).
The approach is sound and follows established patterns (modeled after `OUT_FENCE_PTR` — always reads as zero, write-only semantic). The implementation is compact and mostly correct, but there is one significant bug around state duplication/reset and a few smaller issues worth discussing.
**Key concern:** The new `page_flip_event_requested` field in `drm_crtc_state` is not cleared in `__drm_atomic_helper_crtc_duplicate_state()`, which means it could carry over from a previous committed state if the memcpy copies a stale `true` value. This is unlike `event`, `async_flip`, and other per-commit fields that are explicitly cleared there.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-04 5:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-30 17:14 [PATCH] drm/atomic: introduce PAGE_FLIP_EVENT property Simon Ser
2026-05-31 9:10 ` Xaver Hugl
2026-05-31 9:26 ` Simon Ser
2026-06-04 5:29 ` Claude review: " Claude Code Review Bot
2026-06-04 5:29 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox