[PATCH v9 1/2] drm/atomic: Let drivers decide which planes to async flip

[André Almeida]

Currently, DRM atomic uAPI allows only primary planes to be flipped
asynchronously. However, each driver might be able to perform async
flips in other different plane types. To enable drivers to set their own
restrictions on which type of plane they can or cannot flip, use the
existing atomic_async_check() from struct drm_plane_helper_funcs to
enhance this flexibility, thus allowing different plane types to be able
to do async flips as well.

In order to prevent regressions and such, we keep the current policy: we
skip the driver check for the primary plane, because it is always
allowed to do async flips on it.

Signed-off-by: André Almeida 
---
Changes from v8:
- Rebased on top of 6.12-rc1
---
 drivers/gpu/drm/drm_atomic_uapi.c | 39 +--
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 7936c2023955..b004fa1d2e2e 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -27,8 +27,9 @@
  * Daniel Vetter 
  */
 
-#include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1063,6 +1064,7 @@ 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)) {
@@ -1070,15 +1072,32 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip &&
-   (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY ||
-(prop != config->prop_fb_id &&
- prop != config->prop_in_fence_fd &&
- prop != config->prop_fb_damage_clips))) {
-   ret = drm_atomic_plane_get_property(plane, plane_state,
-   prop, &old_val);
-   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-   break;
+   if (async_flip) {
+   /* check if the prop does a nop change */
+   if ((plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) ||
+   (prop != config->prop_fb_id &&
+prop != config->prop_in_fence_fd &&
+prop != config->prop_fb_damage_clips)) {
+   ret = drm_atomic_plane_get_property(plane, 
plane_state,
+   prop, 
&old_val);
+   ret = drm_atomic_check_prop_changes(ret, 
old_val, prop_value, prop);
+   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);
+
+   if (ret) {
+   drm_dbg_atomic(prop->dev,
+  "[PLANE:%d] does not 
support async flips\n",
+  obj->id);
+   break;
+   }
+   }
}
 
ret = drm_atomic_plane_set_property(plane,

-- 
2.46.0

[PATCH v9 2/2] drm/amdgpu: Enable async flip on overlay planes

[André Almeida]

amdgpu can handle async flips on overlay planes, so allow it for atomic
async checks.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 495e3cd70426..4c6aed5ca777 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1266,8 +1266,7 @@ static int amdgpu_dm_plane_atomic_async_check(struct 
drm_plane *plane,
struct drm_plane_state *new_plane_state;
struct dm_crtc_state *dm_new_crtc_state;
 
-   /* Only support async updates on cursor planes. */
-   if (plane->type != DRM_PLANE_TYPE_CURSOR)
+   if (plane->type != DRM_PLANE_TYPE_CURSOR && plane->type != 
DRM_PLANE_TYPE_OVERLAY)
return -EINVAL;
 
new_plane_state = drm_atomic_get_new_plane_state(state, plane);

-- 
2.46.0

[PATCH v9 0/2] drm/atomic: Ease async flip restrictions

[André Almeida]

Hi,

As per my previous patchsets, the goal of this work is to find a nice way to
allow amdgpu to perform async page flips in the overlay plane as well, not
only on the primary one. Currently, when using the atomic uAPI, this is the only
type of plane allowed to do async flips, and every driver accepts it.

In my last version, I had created a static field `bool async_flip` for
drm_planes. When creating new planes, drivers could tell if such plane was
allowed or not to do async flips. This would be latter checked on the atomic
uAPI whenever the DRM_MODE_PAGE_FLIP_ASYNC was present.

However, Dmitry Baryshkov raised a valid point about getting confused with the 
existing atomic_async_check() code, giving that is a function to do basically
what I want: to let drivers tell DRM whether a giving plane can do async flips
or not. It turns out atomic_async_check() is implemented by drivers to deal with
the legacy cursor update, so it's not wired with the atomic uAPI because is
something that precedes such API.

So my new proposal is to just reuse this same function in the atomic uAPI path.
The plane restrictions defined at atomic_async_check() should work in this
codepath as well. And I will be able to allow overlays planes by modifying
amdgpu_dm_plane_atomic_async_check(), and anyone else have a proper place to
play with async plane restrictions as well.

One note is that currently we always allow async flips for primary planes,
regardless of the drivers, but not every atomic_async_check() implementation
allows primary planes (because they were writing targeting cursor planes
anyway...). To avoid regressions, my patch only calls atomic_async_check() for
non primary planes, and always allows primary ones.

Thoughts?

Changelog
 v8: 
https://lore.kernel.org/lkml/20240806135300.114469-1-andrealm...@igalia.com/
 - Rebased on top of 6.12-rc1 (drm/drm-next)

Changelog
 v7: 
https://lore.kernel.org/dri-devel/20240618030024.500532-1-andrealm...@igalia.com/
 - Complete rewrite

---
André Almeida (2):
  drm/atomic: Let drivers decide which planes to async flip
  drm/amdgpu: Enable async flip on overlay planes

 .../drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c|  3 +-
 drivers/gpu/drm/drm_atomic_uapi.c  | 39 --
 2 files changed, 30 insertions(+), 12 deletions(-)
---
base-commit: e32cde8d2bd7d251a8f9b434143977ddf13dcec6
change-id: 20241002-tonyk-async_flip-828cfe9cf3ca

Best regards,
-- 
André Almeida 

[PATCH RESEND v8 0/2] drm/atomic: Ease async flip restrictions

[André Almeida]

Hi,

As per my previous patchsets, the goal of this work is to find a nice way to
allow amdgpu to perform async page flips in the overlay plane as well, not
only on the primary one. Currently, when using the atomic uAPI, this is the only
type of plane allowed to do async flips, and every driver accepts it.

In my last version, I had created a static field `bool async_flip` for
drm_planes. When creating new planes, drivers could tell if such plane was
allowed or not to do async flips. This would be latter checked on the atomic
uAPI whenever the DRM_MODE_PAGE_FLIP_ASYNC was present.

However, Dmitry Baryshkov raised a valid point about getting confused with the 
existing atomic_async_check() code, giving that is an function to do basically
what I want: to let drivers tell DRM whether a giving plane can do async flips
or not. It turns out atomic_async_check() is implemented by drivers to deal with
the legacy cursor update, so it's not wired with the atomic uAPI because is
something that precedes such API.

So my new proposal is to just reuse this same function in the atomic uAPI path.
The plane restrictions defined at atomic_async_check() should work in this
codepath as well. And I will be able to allow overlays planes by modifying
amdgpu_dm_plane_atomic_async_check(), and anyone else have a proper place to
play with async plane restrictions as well.

One note is that currently we always allow async flips for primary planes,
regardless of the drivers, but not every atomic_async_check() implementation
allows primary planes (because they were writing targeting cursor planes
anyway...). To avoid regressions, my patch only calls atomic_async_check() for
non primary planes, and always allows primary ones.

Thoughts?

Changelog
 v7: 
https://lore.kernel.org/dri-devel/20240618030024.500532-1-andrealm...@igalia.com/
 - Complete rewrite

André Almeida (2):
  drm/atomic: Let drivers decide which planes to async flip
  drm/amdgpu: Enable async flip on overlay planes

 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  3 +--
 drivers/gpu/drm/drm_atomic_uapi.c | 23 ++-
 2 files changed, 18 insertions(+), 8 deletions(-)

-- 
2.46.0

[PATCH RESEND v8 2/2] drm/amdgpu: Enable async flip on overlay planes

[André Almeida]

amdgpu can handle async flips on overlay planes, so allow it for atomic
async checks.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 8a4c40b4c27e..125402964289 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1175,8 +1175,7 @@ static int amdgpu_dm_plane_atomic_check(struct drm_plane 
*plane,
 static int amdgpu_dm_plane_atomic_async_check(struct drm_plane *plane,
  struct drm_atomic_state *state)
 {
-   /* Only support async updates on cursor planes. */
-   if (plane->type != DRM_PLANE_TYPE_CURSOR)
+   if (plane->type != DRM_PLANE_TYPE_CURSOR && plane->type != 
DRM_PLANE_TYPE_OVERLAY)
return -EINVAL;
 
return 0;
-- 
2.46.0

[PATCH RESEND v8 1/2] drm/atomic: Let drivers decide which planes to async flip

[André Almeida]

Currently, DRM atomic uAPI allows only primary planes to be flipped
asynchronously. However, each driver might be able to perform async
flips in other different plane types. To enable drivers to set their own
restrictions on which type of plane they can or cannot flip, use the
existing atomic_async_check() from struct drm_plane_helper_funcs to
enhance this flexibility, thus allowing different plane types to be able
to do async flips as well.

In order to prevent regressions and such, we keep the current policy: we
skip the driver check for the primary plane, because it is always
allowed to do async flips on it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index fc16fddee5c5..8568c2428670 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -27,8 +27,9 @@
  * Daniel Vetter 
  */
 
-#include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1059,6 +1060,7 @@ 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)) {
@@ -1073,11 +1075,20 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
-   drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
-  obj->id);
-   ret = -EINVAL;
+   if (async_flip) {
+   /* we always allow primary planes */
+   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);
+
+   if (ret) {
+   drm_dbg_atomic(prop->dev,
+  "[PLANE:%d] does not 
support async flips\n",
+   obj->id);
+   }
+   }
break;
}
 
-- 
2.46.0

Re: [PATCH v2] drm/atomic: allow no-op FB_ID updates for async flips

[André Almeida]

Em 31/07/2024 16:10, Simon Ser escreveu:

User-space is allowed to submit any property in an async flip as
long as the value doesn't change. However we missed one case:
as things stand, the kernel rejects no-op FB_ID changes on
non-primary planes. Fix this by changing the conditional and
skipping drm_atomic_check_prop_changes() only for FB_ID on the
primary plane (instead of skipping for FB_ID on any plane).

Fixes: 0e26cc72c71c ("drm: Refuse to async flip with atomic prop changes")
Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Tested-by: Xaver Hugl 
Cc: Alex Deucher 
Cc: Christian König 
Cc: Michel Dänzer 
Cc: Ville Syrjälä 
---

André, can you confirm that the R-b still holds?



Yes, it still holds

Re: [PATCH] drm/atomic: allow no-op FB_ID updates for async flips

[André Almeida]

Hi Simon, thanks for the fix!

Em 29/06/2024 12:22, Simon Ser escreveu:

User-space is allowed to submit any property in an async flip as
long as the value doesn't change. However we missed one case:
as things stand, the kernel rejects no-op FB_ID changes on
non-primary planes. Fix this by changing the conditional and
skipping drm_atomic_check_prop_changes() only for FB_ID on the
primary plane (instead of skipping for FB_ID on any plane).

Fixes: 0e26cc72c71c ("drm: Refuse to async flip with atomic prop changes")
Signed-off-by: Simon Ser 


Reviewed-by: André Almeida 

[PATCH v8 2/2] drm/amdgpu: Enable async flip on overlay planes

[André Almeida]

amdgpu can handle async flips on overlay planes, so allow it for atomic
async checks.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 8a4c40b4c27e..125402964289 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1175,8 +1175,7 @@ static int amdgpu_dm_plane_atomic_check(struct drm_plane 
*plane,
 static int amdgpu_dm_plane_atomic_async_check(struct drm_plane *plane,
  struct drm_atomic_state *state)
 {
-   /* Only support async updates on cursor planes. */
-   if (plane->type != DRM_PLANE_TYPE_CURSOR)
+   if (plane->type != DRM_PLANE_TYPE_CURSOR && plane->type != 
DRM_PLANE_TYPE_OVERLAY)
return -EINVAL;
 
return 0;
-- 
2.45.2

[PATCH v8 1/2] drm/atomic: Let drivers decide which planes to async flip

[André Almeida]

Currently, DRM atomic uAPI allows only primary planes to be flipped
asynchronously. However, each driver might be able to perform async
flips in other different plane types. To enable drivers to set their own
restrictions on which type of plane they can or cannot flip, use the
existing atomic_async_check() from struct drm_plane_helper_funcs to
enhance this flexibility, thus allowing different plane types to be able
to do async flips as well.

In order to prevent regressions and such, we keep the current policy: we
skip the driver check for the primary plane, because it is always
allowed to do async flips on it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 7609c798d73d..d888ee8f7a11 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -27,8 +27,9 @@
  * Daniel Vetter 
  */
 
-#include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1063,6 +1064,7 @@ 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)) {
@@ -1080,11 +1082,20 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
-   drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
-  obj->id);
-   ret = -EINVAL;
+   if (async_flip) {
+   /* we always allow primary planes */
+   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);
+
+   if (ret) {
+   drm_dbg_atomic(prop->dev,
+  "[PLANE:%d] does not 
support async flips\n",
+   obj->id);
+   }
+   }
break;
}
 
-- 
2.45.2

[PATCH v8 0/1] drm/atomic: Ease async flip restrictions

[André Almeida]

Hi,

As per my previous patchsets, the goal of this work is to find a nice way to
allow amdgpu to perform async page flips in the overlay plane as well, not
only on the primary one. Currently, when using the atomic uAPI, this is the only
type of plane allowed to do async flips, and every driver accepts it.

In my last version, I had created a static field `bool async_flip` for
drm_planes. When creating new planes, drivers could tell if such plane was
allowed or not to do async flips. This would be latter checked on the atomic
uAPI whenever the DRM_MODE_PAGE_FLIP_ASYNC was present.

However, Dmitry Baryshkov raised a valid point about getting confused with the 
existing atomic_async_check() code, giving that is an function to do basically
what I want: to let drivers tell DRM whether a giving plane can do async flips
or not. It turns out atomic_async_check() is implemented by drivers to deal with
the legacy cursor update, so it's not wired with the atomic uAPI because is
something that precedes such API.

So my new proposal is to just reuse this same function in the atomic uAPI path.
The plane restrictions defined at atomic_async_check() should work in this
codepath as well. And I will be able to allow overlays planes by modifying
amdgpu_dm_plane_atomic_async_check(), and anyone else have a proper place to
play with async plane restrictions as well.

One note is that currently we always allow async flips for primary planes,
regardless of the drivers, but not every atomic_async_check() implementation
allows primary planes (because they were writing targeting cursor planes
anyway...). To avoid regressions, my patch only calls atomic_async_check() for
non primary planes, and always allows primary ones.

Thoughts?

Changelog
 v7: 
https://lore.kernel.org/dri-devel/20240618030024.500532-1-andrealm...@igalia.com/
 - Complete rewrite


André Almeida (2):
  drm/atomic: Let drivers decide which planes to async flip
  drm/amdgpu: Enable async flip on overlay planes

 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  3 +--
 drivers/gpu/drm/drm_atomic_uapi.c | 21 ++-
 2 files changed, 17 insertions(+), 7 deletions(-)

-- 
2.45.2

Re: [ANNOUNCE] Graphics & DRM MC at LPC 2024

[André Almeida]

Em 13/05/2024 13:46, André Almeida escreveu:
The Graphics and DRM Microconference was accepted for Linux Plumbers 
2024! Please, submit your proposal in the following page:


 https://lpc.events/event/18/abstracts/

Remember to select "Graphics & DRM MC" in the Track field. The deadline 
for submissions is Sunday 30th June. The accepted proposals will be 
listed here:




The deadline was extended till July 15th :)


 https://lpc.events/event/18/contributions/1664/

LPC 2024 will happen in Vienna, Austria, on September 18-20:

 https://lpc.events/event/18/

Thanks!

[PATCH v2 2/2] drm/atomic: Allow userspace to use damage clips with async flips

[André Almeida]

Allow userspace to use damage clips with atomic async flips. Damage
clips are useful for partial plane updates, which can be helpful for
clients that want to do flips asynchronously.

Signed-off-by: André Almeida 
---
v2 changes:
- new patch

 drivers/gpu/drm/drm_atomic_uapi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 2e1d9391febe..7609c798d73d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1072,7 +1072,8 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
 
if (async_flip &&
prop != config->prop_fb_id &&
-   prop != config->prop_in_fence_fd) {
+   prop != config->prop_in_fence_fd &&
+   prop != config->prop_fb_damage_clips) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-- 
2.45.2

[PATCH v2 1/2] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.

Signed-off-by: André Almeida 
Reviewed-by: Simon Ser 
---
v2 changes:
- Add r-b

 drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 22bbb2d83e30..2e1d9391febe 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1070,7 +1070,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
+   if (async_flip &&
+   prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-- 
2.45.2

Re: [PATCH 1/1] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Em 29/06/2024 12:32, Simon Ser escreveu:

On Saturday, June 22nd, 2024 at 19:09, André Almeida  
wrote:


Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.


LGTM.


thanks!



Would you mind sending a patch for FB_DAMAGE_CLIPS as well?


sure :)



Reviewed-by: Simon Ser 

[PATCH 1/1] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.

Signed-off-by: André Almeida 
---

This patch is originally from a patchset, but it doesn't really depends on the
rest of the work, so I'm sending it standalone now.

Original thread: 
https://lore.kernel.org/dri-devel/20240618030024.500532-1-andrealm...@igalia.com/

 drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 22bbb2d83e30..2e1d9391febe 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1070,7 +1070,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
+   if (async_flip &&
+   prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-- 
2.45.2

Re: [PATCH v7 2/9] drm: Support per-plane async flip configuration

[André Almeida]

Em 18/06/2024 14:43, Dmitry Baryshkov escreveu:

On Tue, Jun 18, 2024 at 01:18:10PM GMT, André Almeida wrote:

Em 18/06/2024 07:07, Dmitry Baryshkov escreveu:

On Tue, 18 Jun 2024 at 12:38, Jani Nikula  wrote:


On Tue, 18 Jun 2024, André Almeida  wrote:

Drivers have different capabilities on what plane types they can or
cannot perform async flips. Create a plane::async_flip field so each
driver can choose which planes they allow doing async flips.

Signed-off-by: André Almeida 
---
   include/drm/drm_plane.h | 5 +
   1 file changed, 5 insertions(+)

diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 9507542121fa..0bebc72af5c3 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -786,6 +786,11 @@ struct drm_plane {
 * @kmsg_panic: Used to register a panic notifier for this plane
 */
struct kmsg_dumper kmsg_panic;
+
+ /**
+  * @async_flip: indicates if a plane can do async flips
+  */


When is it okay to set or change the value of this member?

If you don't document it, people will find creative uses for this.


Maybe it's better to have a callback instead of a static field? This
way it becomes clear that it's only relevant at the time of the
atomic_check().



So we would have something like bool (*async_flip) for struct
drm_plane_funcs I suppose. Then each driver will implement this function and
check on runtime if it should flip or not, right?

I agree that it makes more clear, but as far as I can see this is not
something that is subject to being changed at runtime at all, so it seems a
bit overkill to me to encapsulate a static information like that. I prefer
to improve the documentation on the struct member to see if this solves the
problem. What do you think of the following comment:


It looks like I keep on mixing async_flips as handled via the
DRM_MODE_PAGE_FLIP_ASYNC and the plane flips that are governed by
.atomic_async_check / .atomic_async_update / drm_atomic_helper_check()
and which end up being used just for legacy cursor updates.

So, yes, those are two different code paths, but with your changes I
think it becomes even easier to get confused between
atomic_async_check() and .async_flip member.



I see, now that I read about atomic_async_check(), it got me confused as 
well :)


I see that drivers define atomic_async_check() to tell DRM whether or 
not such plane is able to do async flips... just like I'm trying to do 
here. amdgpu implementation for that function is almost the opposite of 
the restrictions that I've implemented in this patchset:


int amdgpu_dm_plane_atomic_async_check(...) {
/* Only support async updates on cursor planes. */
if (plane->type != DRM_PLANE_TYPE_CURSOR)
return -EINVAL;

return 0;
}

Anyway, I'll try to see if the legacy cursor path might be incorporated 
somehow in the DRM_MODE_PAGE_FLIP_ASYNC path, or to come up with 
something that makes them more distinguishable.


Thanks!




/**
  * @async_flip: indicates if a plane can perform async flips. The
  * driver should set this true only for planes that the hardware
  * supports flipping asynchronously. It may not be changed during
  * runtime. This field is checked inside drm_mode_atomic_ioctl() to
  * allow only the correct planes to go with DRM_MODE_PAGE_FLIP_ASYNC.
  */

Re: [PATCH v7 2/9] drm: Support per-plane async flip configuration

[André Almeida]

Em 18/06/2024 07:07, Dmitry Baryshkov escreveu:

On Tue, 18 Jun 2024 at 12:38, Jani Nikula  wrote:


On Tue, 18 Jun 2024, André Almeida  wrote:

Drivers have different capabilities on what plane types they can or
cannot perform async flips. Create a plane::async_flip field so each
driver can choose which planes they allow doing async flips.

Signed-off-by: André Almeida 
---
  include/drm/drm_plane.h | 5 +
  1 file changed, 5 insertions(+)

diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 9507542121fa..0bebc72af5c3 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -786,6 +786,11 @@ struct drm_plane {
* @kmsg_panic: Used to register a panic notifier for this plane
*/
   struct kmsg_dumper kmsg_panic;
+
+ /**
+  * @async_flip: indicates if a plane can do async flips
+  */


When is it okay to set or change the value of this member?

If you don't document it, people will find creative uses for this.


Maybe it's better to have a callback instead of a static field? This
way it becomes clear that it's only relevant at the time of the
atomic_check().



So we would have something like bool (*async_flip) for struct 
drm_plane_funcs I suppose. Then each driver will implement this function 
and check on runtime if it should flip or not, right?


I agree that it makes more clear, but as far as I can see this is not 
something that is subject to being changed at runtime at all, so it 
seems a bit overkill to me to encapsulate a static information like 
that. I prefer to improve the documentation on the struct member to see 
if this solves the problem. What do you think of the following comment:


/**
 * @async_flip: indicates if a plane can perform async flips. The
 * driver should set this true only for planes that the hardware
 * supports flipping asynchronously. It may not be changed during
 * runtime. This field is checked inside drm_mode_atomic_ioctl() to
 * allow only the correct planes to go with DRM_MODE_PAGE_FLIP_ASYNC.
 */

[PATCH v7 9/9] drm/amdgpu: Make it possible to async flip overlay planes

[André Almeida]

amdgpu can handle async flips on overlay planes, so mark it as true
during the plane initialization.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 0c126c5609d3..7d508d816f0d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1709,6 +1709,7 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
*dm,
} else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
unsigned int zpos = 1 + drm_plane_index(plane);
drm_plane_create_zpos_property(plane, zpos, 1, 254);
+   plane->async_flip = true;
} else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
drm_plane_create_zpos_immutable_property(plane, 255);
}
-- 
2.45.2

[PATCH v7 8/9] drm: Enable per-plane async flip check

[André Almeida]

Replace the generic "is this plane primary" for a plane::async_flip
check, so DRM follows the plane restrictions set by the driver.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 2e1d9391febe..ed1af3455477 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1079,9 +1079,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
+   if (async_flip && !plane->async_flip) {
drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
+  "[PLANE:%d] does not support async 
flips\n",
   obj->id);
ret = -EINVAL;
break;
-- 
2.45.2

[PATCH v7 7/9] drm/vc4: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/vc4/vc4_plane.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 07caf2a47c6c..e3d41da14e6f 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -1672,8 +1672,10 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
  DRM_COLOR_YCBCR_BT709,
  DRM_COLOR_YCBCR_LIMITED_RANGE);
 
-   if (type == DRM_PLANE_TYPE_PRIMARY)
+   if (type == DRM_PLANE_TYPE_PRIMARY) {
drm_plane_create_zpos_immutable_property(plane, 0);
+   plane->async_flip = true;
+   }
 
return plane;
 }
-- 
2.45.2

[PATCH v7 6/9] drm/nouveau: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4310ad71870b..fd06d46d49ec 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -1285,6 +1285,7 @@ int
 nv04_crtc_create(struct drm_device *dev, int crtc_num)
 {
struct nouveau_display *disp = nouveau_display(dev);
+   struct nouveau_drm *drm = nouveau_drm(dev);
struct nouveau_crtc *nv_crtc;
struct drm_plane *primary;
int ret;
@@ -1338,6 +1339,9 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
if (ret)
return ret;
 
+   if (drm->client.device.info.chipset >= 0x11)
+   primary->async_flip = true;
+
return nvif_head_vblank_event_ctor(&nv_crtc->head, "kmsVbl", 
nv04_crtc_vblank_handler,
   false, &nv_crtc->vblank);
 }
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c 
b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 7a2cceaee6e9..55db0fdf61e7 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -763,6 +763,10 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct 
drm_device *dev,
return ret;
}
 
+   if (type == DRM_PLANE_TYPE_PRIMARY &&
+   drm->client.device.info.chipset >= 0x11)
+   wndw->plane.async_flip = true;
+
return 0;
 }
 
-- 
2.45.2

[PATCH v7 5/9] drm/i915: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/i915/display/i9xx_plane.c  | 3 +++
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c 
b/drivers/gpu/drm/i915/display/i9xx_plane.c
index 0279c8aabdd1..0142beef20dc 100644
--- a/drivers/gpu/drm/i915/display/i9xx_plane.c
+++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
@@ -931,6 +931,9 @@ intel_primary_plane_create(struct drm_i915_private 
*dev_priv, enum pipe pipe)
 
intel_plane_helper_add(plane);
 
+   if (plane->async_flip)
+   plane->base.async_flip = true;
+
return plane;
 
 fail:
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c 
b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 860574d04f88..8d0a9f69709a 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -2371,6 +2371,7 @@ skl_universal_plane_create(struct drm_i915_private 
*dev_priv,
plane->async_flip = skl_plane_async_flip;
plane->enable_flip_done = skl_plane_enable_flip_done;
plane->disable_flip_done = skl_plane_disable_flip_done;
+   plane->base.async_flip = true;
}
 
if (DISPLAY_VER(dev_priv) >= 11)
-- 
2.45.2

[PATCH v7 4/9] drm: atmel-hlcdc: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 4a7ba0918eca..22b8a5c888ef 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -1227,6 +1227,9 @@ static int atmel_hlcdc_plane_create(struct drm_device 
*dev,
if (ret)
return ret;
 
+   if (type == DRM_PLANE_TYPE_PRIMARY)
+   plane->base.async_flip = true;
+
drm_plane_helper_add(&plane->base,
 &atmel_hlcdc_layer_plane_helper_funcs);
 
-- 
2.45.2

[PATCH v7 2/9] drm: Support per-plane async flip configuration

[André Almeida]

Drivers have different capabilities on what plane types they can or
cannot perform async flips. Create a plane::async_flip field so each
driver can choose which planes they allow doing async flips.

Signed-off-by: André Almeida 
---
 include/drm/drm_plane.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 9507542121fa..0bebc72af5c3 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -786,6 +786,11 @@ struct drm_plane {
 * @kmsg_panic: Used to register a panic notifier for this plane
 */
struct kmsg_dumper kmsg_panic;
+
+   /**
+* @async_flip: indicates if a plane can do async flips
+*/
+   bool async_flip;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
2.45.2

[PATCH v7 3/9] drm/amdgpu: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 8a4c40b4c27e..0c126c5609d3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1705,6 +1705,7 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
*dm,
 
if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
drm_plane_create_zpos_immutable_property(plane, 0);
+   plane->async_flip = true;
} else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
unsigned int zpos = 1 + drm_plane_index(plane);
drm_plane_create_zpos_property(plane, zpos, 1, 254);
-- 
2.45.2

[PATCH v7 1/9] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 22bbb2d83e30..2e1d9391febe 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1070,7 +1070,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
+   if (async_flip &&
+   prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-- 
2.45.2

[PATCH v7 0/9] drm: Support per-plane async flip configuration

[André Almeida]

AMD hardware can do async flips with overlay planes, but currently there's no
easy way to enable that in DRM. To solve that, this patchset creates a new
drm_plane field, bool async_flip, that allows drivers to choose which plane can
or cannot do async flips. This is latter used on drm_atomic_set_property when
users want to do async flips.

Patch 1 allows async commits with IN_FENCE_ID in any driver.

Patches 2 to 7 have no functional change. As per current code, every driver that
allows async page flips using the atomic API, allows doing it only in the
primary plane. Those patches then enable it for every driver.

Every driver that I found out capable of doing async flips were changed here.
The drivers that weren't touch don't have any mention to
mode_config::async_page_flip, so they are not currently advertising to userspace
that they can do async flips.

Patch 8 changes the current DRM uAPI check from allowing only primary planes to
allowing any plane that the driver allows flipping asynchronously.

Patch 9 finally enables async flip on overlay planes for amdgpu.

Changes from v6:
- Added async_flip check for i915/skl planes (Rodrigo)
- Commit the plane->async_flip check just after all driver had set their
async_flip capabilities (Dmitry)
https://lore.kernel.org/dri-devel/20240614153535.351689-1-andrealm...@igalia.com/

Changes from v5:
- Instead of enabling plane->async_flip in the common code, move it to driver
code.
- Enable primary plane async flip on every driver
https://lore.kernel.org/dri-devel/20240612193713.167448-1-andrealm...@igalia.com/

André Almeida (9):
  drm/atomic: Allow userspace to use explicit sync with atomic async
flips
  drm: Support per-plane async flip configuration
  drm/amdgpu: Enable async flips on the primary plane
  drm: atmel-hlcdc: Enable async flips on the primary plane
  drm/i915: Enable async flips on the primary plane
  drm/nouveau: Enable async flips on the primary plane
  drm/vc4: Enable async flips on the primary plane
  drm: Enable per-plane async flip check
  drm/amdgpu: Make it possible to async flip overlay planes

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 2 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +++
 drivers/gpu/drm/drm_atomic_uapi.c   | 8 +---
 drivers/gpu/drm/i915/display/i9xx_plane.c   | 3 +++
 drivers/gpu/drm/i915/display/skl_universal_plane.c  | 1 +
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 
 drivers/gpu/drm/vc4/vc4_plane.c | 4 +++-
 include/drm/drm_plane.h | 5 +
 9 files changed, 30 insertions(+), 4 deletions(-)

-- 
2.45.2

Re: [PATCH v6 2/8] drm: Support per-plane async flip configuration

[André Almeida]

Em 14/06/2024 14:32, Dmitry Baryshkov escreveu:> On Fri, Jun 14, 2024 at 
12:35:29PM GMT, André Almeida wrote:

>> Drivers have different capabilities on what plane types they can or
>> cannot perform async flips. Create a plane::async_flip field so each
>> driver can choose which planes they allow doing async flips.
>>
>> Signed-off-by: André Almeida 
>> ---
>>   drivers/gpu/drm/drm_atomic_uapi.c | 4 ++--
>>   include/drm/drm_plane.h   | 5 +
>>   2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c

>> index 2e1d9391febe..ed1af3455477 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -1079,9 +1079,9 @@ int drm_atomic_set_property(struct 
drm_atomic_state *state,

>>break;
>>}
>>
>> -		if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {

>> +  if (async_flip && !plane->async_flip) {
>
> So, after this patch async flips becomes disabled until the driver
> enables that manually. Whether that's desired or not is a separate
> topic, but this definitely should be explicitly mentioned in the commit
> message.
>

You are right, I think I should separate this in the last commit, so we 
don't have any regression commits. Thanks for the feedback

Re: [PATCH v6 0/8] drm: Support per-plane async flip configuration

[André Almeida]

Hi Dmitry,

Em 14/06/2024 14:32, Dmitry Baryshkov escreveu:

On Fri, Jun 14, 2024 at 12:35:27PM GMT, André Almeida wrote:

AMD hardware can do async flips with overlay planes, but currently there's no
easy way to enable that in DRM. To solve that, this patchset creates a new
drm_plane field, bool async_flip, that allows drivers to choose which plane can
or cannot do async flips. This is latter used on drm_atomic_set_property when
users want to do async flips.

Patch 1 allows async commits with IN_FENCE_ID in any driver.

Patches 2 to 7 have no function change. As per current code, every driver that
allows async page flips using the atomic API, allows doing it only in the
primary plane. Those patches then enable it for every driver.

Patch 8 finally enables async flip on overlay planes for amdgpu.

Changes from v5:
- Instead of enabling plane->async_flip in the common code, move it to driver
code.
- Enable primary plane async flip on every driver
https://lore.kernel.org/dri-devel/20240612193713.167448-1-andrealm...@igalia.com/

André Almeida (8):
   drm/atomic: Allow userspace to use explicit sync with atomic async
 flips
   drm: Support per-plane async flip configuration
   drm/amdgpu: Enable async flips on the primary plane
   drm: atmel-hlcdc: Enable async flips on the primary plane
   drm/i915: Enable async flips on the primary plane
   drm/nouveau: Enable async flips on the primary plane
   drm/vc4: Enable async flips on the primary plane
   drm/amdgpu: Make it possible to async flip overlay planes

  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 2 ++
  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +++
  drivers/gpu/drm/drm_atomic_uapi.c   | 8 +---
  drivers/gpu/drm/i915/display/i9xx_plane.c   | 3 +++
  drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 
  drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 
  drivers/gpu/drm/vc4/vc4_plane.c | 4 +++-


The main question is why only these drivers were updated.



According to `git grep async_page_flip`, only those drivers supports 
async page flip. The only corner case is radeon, that does supports 
async but doesn't support planes.


Do you know any other driver that should be updated to?


  include/drm/drm_plane.h | 5 +
  8 files changed, 29 insertions(+), 4 deletions(-)

--
2.45.2

[PATCH v6 5/8] drm/i915: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/i915/display/i9xx_plane.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c 
b/drivers/gpu/drm/i915/display/i9xx_plane.c
index 0279c8aabdd1..0142beef20dc 100644
--- a/drivers/gpu/drm/i915/display/i9xx_plane.c
+++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
@@ -931,6 +931,9 @@ intel_primary_plane_create(struct drm_i915_private 
*dev_priv, enum pipe pipe)
 
intel_plane_helper_add(plane);
 
+   if (plane->async_flip)
+   plane->base.async_flip = true;
+
return plane;
 
 fail:
-- 
2.45.2

[PATCH v6 2/8] drm: Support per-plane async flip configuration

[André Almeida]

Drivers have different capabilities on what plane types they can or
cannot perform async flips. Create a plane::async_flip field so each
driver can choose which planes they allow doing async flips.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 4 ++--
 include/drm/drm_plane.h   | 5 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 2e1d9391febe..ed1af3455477 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1079,9 +1079,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
+   if (async_flip && !plane->async_flip) {
drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
+  "[PLANE:%d] does not support async 
flips\n",
   obj->id);
ret = -EINVAL;
break;
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 9507542121fa..0bebc72af5c3 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -786,6 +786,11 @@ struct drm_plane {
 * @kmsg_panic: Used to register a panic notifier for this plane
 */
struct kmsg_dumper kmsg_panic;
+
+   /**
+* @async_flip: indicates if a plane can do async flips
+*/
+   bool async_flip;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
2.45.2

[PATCH v6 4/8] drm: atmel-hlcdc: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 4a7ba0918eca..22b8a5c888ef 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -1227,6 +1227,9 @@ static int atmel_hlcdc_plane_create(struct drm_device 
*dev,
if (ret)
return ret;
 
+   if (type == DRM_PLANE_TYPE_PRIMARY)
+   plane->base.async_flip = true;
+
drm_plane_helper_add(&plane->base,
 &atmel_hlcdc_layer_plane_helper_funcs);
 
-- 
2.45.2

[PATCH v6 8/8] drm/amdgpu: Make it possible to async flip overlay planes

[André Almeida]

amdgpu can handle async flips on overlay planes, so mark it as true
during the plane initialization.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 0c126c5609d3..7d508d816f0d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1709,6 +1709,7 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
*dm,
} else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
unsigned int zpos = 1 + drm_plane_index(plane);
drm_plane_create_zpos_property(plane, zpos, 1, 254);
+   plane->async_flip = true;
} else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
drm_plane_create_zpos_immutable_property(plane, 255);
}
-- 
2.45.2

[PATCH v6 7/8] drm/vc4: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/vc4/vc4_plane.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 07caf2a47c6c..e3d41da14e6f 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -1672,8 +1672,10 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
  DRM_COLOR_YCBCR_BT709,
  DRM_COLOR_YCBCR_LIMITED_RANGE);
 
-   if (type == DRM_PLANE_TYPE_PRIMARY)
+   if (type == DRM_PLANE_TYPE_PRIMARY) {
drm_plane_create_zpos_immutable_property(plane, 0);
+   plane->async_flip = true;
+   }
 
return plane;
 }
-- 
2.45.2

[PATCH v6 6/8] drm/nouveau: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4310ad71870b..fd06d46d49ec 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -1285,6 +1285,7 @@ int
 nv04_crtc_create(struct drm_device *dev, int crtc_num)
 {
struct nouveau_display *disp = nouveau_display(dev);
+   struct nouveau_drm *drm = nouveau_drm(dev);
struct nouveau_crtc *nv_crtc;
struct drm_plane *primary;
int ret;
@@ -1338,6 +1339,9 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
if (ret)
return ret;
 
+   if (drm->client.device.info.chipset >= 0x11)
+   primary->async_flip = true;
+
return nvif_head_vblank_event_ctor(&nv_crtc->head, "kmsVbl", 
nv04_crtc_vblank_handler,
   false, &nv_crtc->vblank);
 }
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c 
b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 7a2cceaee6e9..55db0fdf61e7 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -763,6 +763,10 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct 
drm_device *dev,
return ret;
}
 
+   if (type == DRM_PLANE_TYPE_PRIMARY &&
+   drm->client.device.info.chipset >= 0x11)
+   wndw->plane.async_flip = true;
+
return 0;
 }
 
-- 
2.45.2

[PATCH v6 3/8] drm/amdgpu: Enable async flips on the primary plane

[André Almeida]

This driver can perfom async flips on primary planes, so enable it.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 8a4c40b4c27e..0c126c5609d3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1705,6 +1705,7 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
*dm,
 
if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
drm_plane_create_zpos_immutable_property(plane, 0);
+   plane->async_flip = true;
} else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
unsigned int zpos = 1 + drm_plane_index(plane);
drm_plane_create_zpos_property(plane, zpos, 1, 254);
-- 
2.45.2

[PATCH v6 1/8] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 22bbb2d83e30..2e1d9391febe 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1070,7 +1070,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
+   if (async_flip &&
+   prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-- 
2.45.2

[PATCH v6 0/8] drm: Support per-plane async flip configuration

[André Almeida]

AMD hardware can do async flips with overlay planes, but currently there's no
easy way to enable that in DRM. To solve that, this patchset creates a new
drm_plane field, bool async_flip, that allows drivers to choose which plane can
or cannot do async flips. This is latter used on drm_atomic_set_property when
users want to do async flips.

Patch 1 allows async commits with IN_FENCE_ID in any driver.

Patches 2 to 7 have no function change. As per current code, every driver that
allows async page flips using the atomic API, allows doing it only in the
primary plane. Those patches then enable it for every driver.

Patch 8 finally enables async flip on overlay planes for amdgpu.

Changes from v5:
- Instead of enabling plane->async_flip in the common code, move it to driver
code.
- Enable primary plane async flip on every driver
https://lore.kernel.org/dri-devel/20240612193713.167448-1-andrealm...@igalia.com/

André Almeida (8):
  drm/atomic: Allow userspace to use explicit sync with atomic async
flips
  drm: Support per-plane async flip configuration
  drm/amdgpu: Enable async flips on the primary plane
  drm: atmel-hlcdc: Enable async flips on the primary plane
  drm/i915: Enable async flips on the primary plane
  drm/nouveau: Enable async flips on the primary plane
  drm/vc4: Enable async flips on the primary plane
  drm/amdgpu: Make it possible to async flip overlay planes

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 2 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +++
 drivers/gpu/drm/drm_atomic_uapi.c   | 8 +---
 drivers/gpu/drm/i915/display/i9xx_plane.c   | 3 +++
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 4 
 drivers/gpu/drm/vc4/vc4_plane.c | 4 +++-
 include/drm/drm_plane.h | 5 +
 8 files changed, 29 insertions(+), 4 deletions(-)

-- 
2.45.2

Re: [PATCH v5 2/3] drm: Allow drivers to choose plane types to async flip

[André Almeida]

Hi Dmitry,

Em 12/06/2024 17:45, Dmitry Baryshkov escreveu:

On Wed, Jun 12, 2024 at 04:37:12PM -0300, André Almeida wrote:

Different planes may have different capabilities of doing async flips,
so create a field to let drivers allow async flip per plane type.

Signed-off-by: André Almeida 
---
  drivers/gpu/drm/drm_atomic_uapi.c | 4 ++--
  drivers/gpu/drm/drm_plane.c   | 3 +++
  include/drm/drm_plane.h   | 5 +
  3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 57662a1fd345..bbcec3940636 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -385,6 +385,9 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
  
  	drm_modeset_lock_init(&plane->mutex);
  
+	if (type == DRM_PLANE_TYPE_PRIMARY)

+   plane->async_flip = true;
+


Why? Also note that the commit message writes about adding the field,
not about enabling it for the primary planes.



This is not meant to have any function change actually, just to enable 
per-plane configuration. Currently, any driver that supports async page 
flip in atomic API supports flipping the primary plane.


But as Ville pointed out, that belongs to driver code, so I'll move 
there, hope that it makes more clear



plane->base.properties = &plane->properties;
plane->dev = dev;
plane->funcs = funcs;

[PATCH v5 3/3] drm/amdgpu: Make it possible to async flip overlay planes

[André Almeida]

amdgpu can handle async flips on overlay planes, so mark it as true
during the plane initialization.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 8a4c40b4c27e..dc5392c08a87 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1708,6 +1708,7 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
*dm,
} else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
unsigned int zpos = 1 + drm_plane_index(plane);
drm_plane_create_zpos_property(plane, zpos, 1, 254);
+   plane->async_flip = true;
} else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
drm_plane_create_zpos_immutable_property(plane, 255);
}
-- 
2.45.2

[PATCH v5 2/3] drm: Allow drivers to choose plane types to async flip

[André Almeida]

Different planes may have different capabilities of doing async flips,
so create a field to let drivers allow async flip per plane type.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 4 ++--
 drivers/gpu/drm/drm_plane.c   | 3 +++
 include/drm/drm_plane.h   | 5 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 2e1d9391febe..dd4b1578f141 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1079,9 +1079,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
+   if (async_flip && !plane_state->plane->async_flip) {
drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
+  "[OBJECT:%d] This type of plane cannot 
be changed during async flip\n",
   obj->id);
ret = -EINVAL;
break;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 57662a1fd345..bbcec3940636 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -385,6 +385,9 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
 
drm_modeset_lock_init(&plane->mutex);
 
+   if (type == DRM_PLANE_TYPE_PRIMARY)
+   plane->async_flip = true;
+
plane->base.properties = &plane->properties;
plane->dev = dev;
plane->funcs = funcs;
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 9507542121fa..0bebc72af5c3 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -786,6 +786,11 @@ struct drm_plane {
 * @kmsg_panic: Used to register a panic notifier for this plane
 */
struct kmsg_dumper kmsg_panic;
+
+   /**
+* @async_flip: indicates if a plane can do async flips
+*/
+   bool async_flip;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
2.45.2

[PATCH v5 1/3] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 22bbb2d83e30..2e1d9391febe 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1070,7 +1070,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
+   if (async_flip &&
+   prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-- 
2.45.2

[PATCH v5 0/3] drm/atomic: Allow drivers to write their own plane check for async

[André Almeida]

Hi,

AMD hardware can do async flips with overlay planes, so this patchset does a
small redesign to allow drivers to choose per plane type if they can or cannot
do async flips.

It also allows async commits with IN_FENCE_ID in any driver.

Changes from v4:
- Rebased on top current drm-misc/drm-misc-next

Changes from v3:
- Major patchset redesign 
v3: https://lore.kernel.org/lkml/20240128212515.630345-1-andrealm...@igalia.com/

Changes from v2:
 - Allow IN_FENCE_ID for any driver
 - Allow overlay planes again
v2: https://lore.kernel.org/lkml/20240119181235.255060-1-andrealm...@igalia.com/

Changes from v1:
 - Drop overlay planes option for now
v1: 
https://lore.kernel.org/dri-devel/20240116045159.1015510-1-andrealm...@igalia.com/

André Almeida (3):
  drm/atomic: Allow userspace to use explicit sync with atomic async
flips
  drm: Allow drivers to choose plane types to async flip
  drm/amdgpu: Make it possible to async flip overlay planes

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 drivers/gpu/drm/drm_atomic_uapi.c   | 4 +++-
 include/drm/drm_plane.h | 5 +
 3 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.45.2

[ANNOUNCE] Graphics & DRM MC at LPC 2024

[André Almeida]

The Graphics and DRM Microconference was accepted for Linux Plumbers 
2024! Please, submit your proposal in the following page:


https://lpc.events/event/18/abstracts/

Remember to select "Graphics & DRM MC" in the Track field. The deadline 
for submissions is Sunday 30th June. The accepted proposals will be 
listed here:


https://lpc.events/event/18/contributions/1664/

LPC 2024 will happen in Vienna, Austria, on September 18-20:

https://lpc.events/event/18/

Thanks!

[RESEND PATCH v4 3/3] drm/amdgpu: Make it possible to async flip overlay planes

[André Almeida]

amdgpu can handle async flips on overlay planes, so mark it as true
during the plane initialization.

Signed-off-by: André Almeida 
---
v4: new patch

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 8a4c40b4c27e..dc5392c08a87 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1708,6 +1708,7 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
*dm,
} else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
unsigned int zpos = 1 + drm_plane_index(plane);
drm_plane_create_zpos_property(plane, zpos, 1, 254);
+   plane->async_flip = true;
} else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
drm_plane_create_zpos_immutable_property(plane, 255);
}
-- 
2.43.0

[RESEND PATCH v4 2/3] drm: Allow drivers to choose plane types to async flip

[André Almeida]

Different planes may have different capabilities of doing async flips,
so create a field to let drivers allow async flip per plane type.

Signed-off-by: André Almeida 
---
v4: new patch

 drivers/gpu/drm/drm_atomic_uapi.c | 4 ++--
 drivers/gpu/drm/drm_plane.c   | 3 +++
 include/drm/drm_plane.h   | 5 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 1eecfb9e240d..5c66289188be 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1075,9 +1075,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
+   if (async_flip && !plane_state->plane->async_flip) {
drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
+  "[OBJECT:%d] This type of plane cannot 
be changed during async flip\n",
   obj->id);
ret = -EINVAL;
break;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 672c655c7a8e..71ada690222a 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -366,6 +366,9 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
 
drm_modeset_lock_init(&plane->mutex);
 
+   if (type == DRM_PLANE_TYPE_PRIMARY)
+   plane->async_flip = true;
+
plane->base.properties = &plane->properties;
plane->dev = dev;
plane->funcs = funcs;
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 641fe298052d..5e9efb7321ac 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -779,6 +779,11 @@ struct drm_plane {
 * @hotspot_y_property: property to set mouse hotspot y offset.
 */
struct drm_property *hotspot_y_property;
+
+   /**
+* @async_flip: indicates if a plane can do async flips
+*/
+   bool async_flip;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
2.43.0

[RESEND PATCH v4 1/3] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.

Signed-off-by: André Almeida 
---
v4: no changes

 drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 29d4940188d4..1eecfb9e240d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1066,7 +1066,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
+   if (async_flip &&
+   prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-- 
2.43.0

[RESEND PATCH v4 0/3] drm/atomic: Allow drivers to write their own plane check for async

[André Almeida]

Hi,

AMD hardware can do async flips with overlay planes, so this patchset does a
small redesign to allow drivers to choose per plane type if they can or cannot
do async flips.

It also allows async commits with IN_FENCE_ID in any driver.

Changes from v3:
- Major patchset redesign 
v3: https://lore.kernel.org/lkml/20240128212515.630345-1-andrealm...@igalia.com/

Changes from v2:
 - Allow IN_FENCE_ID for any driver
 - Allow overlay planes again
v2: https://lore.kernel.org/lkml/20240119181235.255060-1-andrealm...@igalia.com/

Changes from v1:
 - Drop overlay planes option for now
v1: 
https://lore.kernel.org/dri-devel/20240116045159.1015510-1-andrealm...@igalia.com/

André Almeida (3):
  drm/atomic: Allow userspace to use explicit sync with atomic async
flips
  drm: Allow drivers to choose plane types to async flip
  drm/amdgpu: Make it possible to async flip overlay planes

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 drivers/gpu/drm/drm_atomic_uapi.c   | 8 +---
 drivers/gpu/drm/drm_plane.c | 3 +++
 include/drm/drm_plane.h | 5 +
 4 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH v4 3/3] drm/amdgpu: Make it possible to async flip overlay planes

[André Almeida]

amdgpu can handle async flips on overlay planes, so mark it as true
during the plane initialization.

Signed-off-by: André Almeida 
---
v4: new patch

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 8a4c40b4c27e..dc5392c08a87 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1708,6 +1708,7 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
*dm,
} else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
unsigned int zpos = 1 + drm_plane_index(plane);
drm_plane_create_zpos_property(plane, zpos, 1, 254);
+   plane->async_flip = true;
} else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
drm_plane_create_zpos_immutable_property(plane, 255);
}
-- 
2.43.0

[PATCH v4 1/3] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.

Signed-off-by: André Almeida 
---
v4: no changes

 drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 29d4940188d4..1eecfb9e240d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1066,7 +1066,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
+   if (async_flip &&
+   prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-- 
2.43.0

[PATCH v4 2/3] drm: Allow drivers to choose plane types to async flip

[André Almeida]

Different planes may have different capabilities of doing async flips,
so create a field to let drivers allow async flip per plane type.

Signed-off-by: André Almeida 
---
v4: new patch

 drivers/gpu/drm/drm_atomic_uapi.c | 4 ++--
 drivers/gpu/drm/drm_plane.c   | 3 +++
 include/drm/drm_plane.h   | 5 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 1eecfb9e240d..5c66289188be 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1075,9 +1075,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
+   if (async_flip && !plane_state->plane->async_flip) {
drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
+  "[OBJECT:%d] This type of plane cannot 
be changed during async flip\n",
   obj->id);
ret = -EINVAL;
break;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 672c655c7a8e..71ada690222a 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -366,6 +366,9 @@ static int __drm_universal_plane_init(struct drm_device 
*dev,
 
drm_modeset_lock_init(&plane->mutex);
 
+   if (type == DRM_PLANE_TYPE_PRIMARY)
+   plane->async_flip = true;
+
plane->base.properties = &plane->properties;
plane->dev = dev;
plane->funcs = funcs;
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 641fe298052d..5e9efb7321ac 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -779,6 +779,11 @@ struct drm_plane {
 * @hotspot_y_property: property to set mouse hotspot y offset.
 */
struct drm_property *hotspot_y_property;
+
+   /**
+* @async_flip: indicates if a plane can do async flips
+*/
+   bool async_flip;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
2.43.0

[PATCH v4 0/3] drm/atomic: Allow drivers to write their own plane check for async

[André Almeida]

Hi,

AMD hardware can do async flips with overlay planes, so this patchset does a
small redesign to allow drivers to choose per plane type if they can or cannot
do async flips.

It also allows async commits with IN_FENCE_ID in any driver.

Changes from v3:
- Major patchset redesign 
v3: https://lore.kernel.org/lkml/20240128212515.630345-1-andrealm...@igalia.com/

Changes from v2:
 - Allow IN_FENCE_ID for any driver
 - Allow overlay planes again
v2: https://lore.kernel.org/lkml/20240119181235.255060-1-andrealm...@igalia.com/

Changes from v1:
 - Drop overlay planes option for now
v1: 
https://lore.kernel.org/dri-devel/20240116045159.1015510-1-andrealm...@igalia.com/

André Almeida (3):
  drm/atomic: Allow userspace to use explicit sync with atomic async
flips
  drm: Allow drivers to choose plane types to async flip
  drm/amdgpu: Make it possible to async flip overlay planes

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 +
 drivers/gpu/drm/drm_atomic_uapi.c   | 8 +---
 drivers/gpu/drm/drm_plane.c | 3 +++
 include/drm/drm_plane.h | 5 +
 4 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.43.0

Re: [PATCH v3 3/3] drm/amdgpu: Implement check_async_props for planes

[André Almeida]

Hi Sima,

Em 30/01/2024 07:56, Daniel Vetter escreveu:

On Sun, Jan 28, 2024 at 06:25:15PM -0300, André Almeida wrote:

AMD GPUs can do async flips with changes on more properties than just
the FB ID, so implement a custom check_async_props for AMD planes.

Allow amdgpu to do async flips with overlay planes as well.

Signed-off-by: André Almeida 
---
v3: allow overlay planes


This comment very much written with a lack of clearly better ideas, but:

Do we really need this much flexibility, especially for the first driver
adding the first few additional properties?

A simple bool on struct drm_plane to indicate whether async flips are ok
or not should also do this job here? Maybe a bit of work to roll that out
to the primary planes for current drivers, but not much. And wouldn't need
drivers to implement some very uapi-marshalling atomic code ...


Right, perhaps I can first write in the way you suggest, and later 
expand to the form I have proposed here if/when new properties arise.




Also we could probably remove the current drm_mode_config.async_flip flag
and entirely replace it with the per-plane one.
-Sima


  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 29 +++
  1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 116121e647ca..ed75b69636b4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -25,6 +25,7 @@
   */
  
  #include 

+#include 
  #include 
  #include 
  #include 
@@ -1430,6 +1431,33 @@ static void 
amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane,
drm_atomic_helper_plane_destroy_state(plane, state);
  }
  
+static int amdgpu_dm_plane_check_async_props(struct drm_property *prop,

+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY &&
+   plane_state->plane->type != DRM_PLANE_TYPE_OVERLAY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary or overlay planes can be 
changed during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
  static const struct drm_plane_funcs dm_plane_funcs = {
.update_plane   = drm_atomic_helper_update_plane,
.disable_plane  = drm_atomic_helper_disable_plane,
@@ -1438,6 +1466,7 @@ static const struct drm_plane_funcs dm_plane_funcs = {
.atomic_duplicate_state = amdgpu_dm_plane_drm_plane_duplicate_state,
.atomic_destroy_state = amdgpu_dm_plane_drm_plane_destroy_state,
.format_mod_supported = amdgpu_dm_plane_format_mod_supported,
+   .check_async_props = amdgpu_dm_plane_check_async_props,
  };
  
  int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,

--
2.43.0

Re: [PATCH v3 1/3] drm/atomic: Allow drivers to write their own plane check for async flips

[André Almeida]

Hi Pekka,

Em 29/01/2024 05:49, Pekka Paalanen escreveu:

On Sun, 28 Jan 2024 18:25:13 -0300
André Almeida  wrote:


Some hardware are more flexible on what they can flip asynchronously, so
rework the plane check so drivers can implement their own check, lifting
up some of the restrictions.

Signed-off-by: André Almeida 
---
v3: no changes

  drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
  include/drm/drm_atomic_uapi.h | 12 ++
  include/drm/drm_plane.h   |  5 +++
  3 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index aee4a65d4959..6d5b9fec90c7 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -620,7 +620,7 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
return 0;
  }
  
-static int

+int
  drm_atomic_plane_get_property(struct drm_plane *plane,
const struct drm_plane_state *state,
struct drm_property *property, uint64_t *val)
@@ -683,6 +683,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
  
  	return 0;

  }
+EXPORT_SYMBOL(drm_atomic_plane_get_property);
  
  static int drm_atomic_set_writeback_fb_for_connector(

struct drm_connector_state *conn_state,
@@ -1026,18 +1027,54 @@ int drm_atomic_connector_commit_dpms(struct 
drm_atomic_state *state,
return ret;
  }
  
-static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value,

+int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,


Hi,

should the word "async" be somewhere in the function name?


 struct drm_property *prop)
  {
if (ret != 0 || old_val != prop_value) {
drm_dbg_atomic(prop->dev,
-  "[PROP:%d:%s] No prop can be changed during async 
flip\n",
+  "[PROP:%d:%s] This prop cannot be changed during 
async flip\n",
   prop->base.id, prop->name);
return -EINVAL;
}
  
  	return 0;

  }
+EXPORT_SYMBOL(drm_atomic_check_prop_changes);
+
+/* plane changes may have exceptions, so we have a special function for them */
+static int drm_atomic_check_plane_changes(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (plane->funcs->check_async_props)
+   return plane->funcs->check_async_props(prop, plane, plane_state,
+obj, prop_value, 
old_val);


Is it really ok to allow drivers to opt-in to also *reject* the FB_ID
and IN_FENCE_FD props on async commits?

Either intentionally or by accident.



Right, perhaps I should write this function in a way that you can only 
lift restrictions, and not add new ones.



+
+   /*
+* if you are trying to change something other than the FB ID, your
+* change will be either rejected or ignored, so we can stop the check
+* here
+*/
+   if (prop != config->prop_fb_id) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);


When I first read this code, it seemed to say: "If the prop is not
FB_ID, then do the usual prop change checking that happens on all
changes, not only async.". Reading this patch a few more times over, I
finally realized drm_atomic_check_prop_changes() is about async
specifically.



I see that the lack of the async word is giving some confusion, so I'll 
add it to the functions.


Thanks,
André


+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary planes can be changed 
during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
  
  int drm_atomic_set_property(struct drm_atomic_state *state,

struct drm_file *file_priv,
@@ -1100,7 +1137,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
-   struct drm_mode_config *config = &plane->dev->mode_config;
  
  		plane_state 

[PATCH v3 3/3] drm/amdgpu: Implement check_async_props for planes

[André Almeida]

AMD GPUs can do async flips with changes on more properties than just
the FB ID, so implement a custom check_async_props for AMD planes.

Allow amdgpu to do async flips with overlay planes as well.

Signed-off-by: André Almeida 
---
v3: allow overlay planes

 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 29 +++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 116121e647ca..ed75b69636b4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -25,6 +25,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1430,6 +1431,33 @@ static void 
amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane,
drm_atomic_helper_plane_destroy_state(plane, state);
 }
 
+static int amdgpu_dm_plane_check_async_props(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY &&
+   plane_state->plane->type != DRM_PLANE_TYPE_OVERLAY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary or overlay planes can 
be changed during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static const struct drm_plane_funcs dm_plane_funcs = {
.update_plane   = drm_atomic_helper_update_plane,
.disable_plane  = drm_atomic_helper_disable_plane,
@@ -1438,6 +1466,7 @@ static const struct drm_plane_funcs dm_plane_funcs = {
.atomic_duplicate_state = amdgpu_dm_plane_drm_plane_duplicate_state,
.atomic_destroy_state = amdgpu_dm_plane_drm_plane_destroy_state,
.format_mod_supported = amdgpu_dm_plane_format_mod_supported,
+   .check_async_props = amdgpu_dm_plane_check_async_props,
 };
 
 int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
-- 
2.43.0

[PATCH v3 0/3] drm/atomic: Allow drivers to write their own plane check for async

[André Almeida]

Hi,

AMD hardware can do more on the async flip path than just the primary plane, so
to lift up the current restrictions, this patchset allows drivers to write their
own check for planes for async flips.

This patchset allows for async commits with IN_FENCE_ID in any driver and
overlay planes on AMD. Userspace can query if a driver supports this with
TEST_ONLY commits.

Changes from v2:
 - Allow IN_FENCE_ID for any driver
 - Allow overlay planes again
v2: https://lore.kernel.org/lkml/20240119181235.255060-1-andrealm...@igalia.com/

Changes from v1:
 - Drop overlay planes option for now
v1: 
https://lore.kernel.org/dri-devel/20240116045159.1015510-1-andrealm...@igalia.com/

André Almeida (3):
  drm/atomic: Allow drivers to write their own plane check for async
flips
  drm/atomic: Allow userspace to use explicit sync with atomic async
flips
  drm/amdgpu: Implement check_async_props for planes

 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 29 +
 drivers/gpu/drm/drm_atomic_uapi.c | 63 ++-
 include/drm/drm_atomic_uapi.h | 12 
 include/drm/drm_plane.h   |  5 ++
 4 files changed, 92 insertions(+), 17 deletions(-)

-- 
2.43.0

[PATCH v3 1/3] drm/atomic: Allow drivers to write their own plane check for async flips

[André Almeida]

Some hardware are more flexible on what they can flip asynchronously, so
rework the plane check so drivers can implement their own check, lifting
up some of the restrictions.

Signed-off-by: André Almeida 
---
v3: no changes

 drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
 include/drm/drm_atomic_uapi.h | 12 ++
 include/drm/drm_plane.h   |  5 +++
 3 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index aee4a65d4959..6d5b9fec90c7 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -620,7 +620,7 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
return 0;
 }
 
-static int
+int
 drm_atomic_plane_get_property(struct drm_plane *plane,
const struct drm_plane_state *state,
struct drm_property *property, uint64_t *val)
@@ -683,6 +683,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 
return 0;
 }
+EXPORT_SYMBOL(drm_atomic_plane_get_property);
 
 static int drm_atomic_set_writeback_fb_for_connector(
struct drm_connector_state *conn_state,
@@ -1026,18 +1027,54 @@ int drm_atomic_connector_commit_dpms(struct 
drm_atomic_state *state,
return ret;
 }
 
-static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
+int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
 struct drm_property *prop)
 {
if (ret != 0 || old_val != prop_value) {
drm_dbg_atomic(prop->dev,
-  "[PROP:%d:%s] No prop can be changed during 
async flip\n",
+  "[PROP:%d:%s] This prop cannot be changed during 
async flip\n",
   prop->base.id, prop->name);
return -EINVAL;
}
 
return 0;
 }
+EXPORT_SYMBOL(drm_atomic_check_prop_changes);
+
+/* plane changes may have exceptions, so we have a special function for them */
+static int drm_atomic_check_plane_changes(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (plane->funcs->check_async_props)
+   return plane->funcs->check_async_props(prop, plane, plane_state,
+obj, prop_value, 
old_val);
+
+   /*
+* if you are trying to change something other than the FB ID, your
+* change will be either rejected or ignored, so we can stop the check
+* here
+*/
+   if (prop != config->prop_fb_id) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary planes can be changed 
during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
 
 int drm_atomic_set_property(struct drm_atomic_state *state,
struct drm_file *file_priv,
@@ -1100,7 +1137,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
-   struct drm_mode_config *config = &plane->dev->mode_config;
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
@@ -1108,19 +1144,11 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
-   ret = drm_atomic_plane_get_property(plane, plane_state,
-   prop, &old_val);
-   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-   break;
-   }
-
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
-   drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
-

[PATCH v3 2/3] drm/atomic: Allow userspace to use explicit sync with atomic async flips

[André Almeida]

Allow userspace to use explicit synchronization with atomic async flips.
That means that the flip will wait for some hardware fence, and then
will flip as soon as possible (async) in regard of the vblank.

Signed-off-by: André Almeida 
---
v3: new patch

 drivers/gpu/drm/drm_atomic_uapi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 6d5b9fec90c7..edae7924ad69 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1060,7 +1060,8 @@ static int drm_atomic_check_plane_changes(struct 
drm_property *prop,
 * change will be either rejected or ignored, so we can stop the check
 * here
 */
-   if (prop != config->prop_fb_id) {
+   if (prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
-- 
2.43.0

Re: [PATCH v2 2/2] drm/amdgpu: Implement check_async_props for planes

[André Almeida]

Hi Ville,

Em 19/01/2024 15:25, Ville Syrjälä escreveu:

On Fri, Jan 19, 2024 at 03:12:35PM -0300, André Almeida wrote:

AMD GPUs can do async flips with changes on more properties than just
the FB ID, so implement a custom check_async_props for AMD planes.

Allow amdgpu to do async flips with IN_FENCE_ID and FB_DAMAGE_CLIPS
properties. For userspace to check if a driver support this two
properties, the strategy for now is to use TEST_ONLY commits.

Signed-off-by: André Almeida 
---
v2: Drop overlay plane option for now

  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 29 +++
  1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 116121e647ca..7afe8c1b62d4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -25,6 +25,7 @@
   */
  
  #include 

+#include 
  #include 
  #include 
  #include 
@@ -1430,6 +1431,33 @@ static void 
amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane,
drm_atomic_helper_plane_destroy_state(plane, state);
  }
  
+static int amdgpu_dm_plane_check_async_props(struct drm_property *prop,

+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd &&


IN_FENCE should just be allowed always.


Do you mean that the common path should allow IN_FENCE_FD for all drivers?

[PATCH v2 2/2] drm/amdgpu: Implement check_async_props for planes

[André Almeida]

AMD GPUs can do async flips with changes on more properties than just
the FB ID, so implement a custom check_async_props for AMD planes.

Allow amdgpu to do async flips with IN_FENCE_ID and FB_DAMAGE_CLIPS
properties. For userspace to check if a driver support this two
properties, the strategy for now is to use TEST_ONLY commits.

Signed-off-by: André Almeida 
---
v2: Drop overlay plane option for now

 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 29 +++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 116121e647ca..7afe8c1b62d4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -25,6 +25,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1430,6 +1431,33 @@ static void 
amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane,
drm_atomic_helper_plane_destroy_state(plane, state);
 }
 
+static int amdgpu_dm_plane_check_async_props(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd &&
+   prop != config->prop_fb_damage_clips) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary planes can be changed 
during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static const struct drm_plane_funcs dm_plane_funcs = {
.update_plane   = drm_atomic_helper_update_plane,
.disable_plane  = drm_atomic_helper_disable_plane,
@@ -1438,6 +1466,7 @@ static const struct drm_plane_funcs dm_plane_funcs = {
.atomic_duplicate_state = amdgpu_dm_plane_drm_plane_duplicate_state,
.atomic_destroy_state = amdgpu_dm_plane_drm_plane_destroy_state,
.format_mod_supported = amdgpu_dm_plane_format_mod_supported,
+   .check_async_props = amdgpu_dm_plane_check_async_props,
 };
 
 int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
-- 
2.43.0

[PATCH v2 0/2] drm/atomic: Allow drivers to write their own plane check for async

[André Almeida]

Hi,

AMD hardware can do more on the async flip path than just the primary plane, so
to lift up the current restrictions, this patchset allows drivers to write their
own check for planes for async flips.

For now this patchset only allow for async commits with IN_FENCE_ID and
FB_DAMAGE_CLIPS. Userspace can query if a driver supports this with TEST_ONLY
commits.

I will left overlay planes for a next iteration.

Changes from v1:
 - Drop overlay planes option for now
v1: 
https://lore.kernel.org/dri-devel/20240116045159.1015510-1-andrealm...@igalia.com/

André

André Almeida (2):
  drm/atomic: Allow drivers to write their own plane check for async
flips
  drm/amdgpu: Implement check_async_props for planes

 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 29 +
 drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
 include/drm/drm_atomic_uapi.h | 12 
 include/drm/drm_plane.h   |  5 ++
 4 files changed, 91 insertions(+), 17 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] drm/atomic: Allow drivers to write their own plane check for async flips

[André Almeida]

Some hardware are more flexible on what they can flip asynchronously, so
rework the plane check so drivers can implement their own check, lifting
up some of the restrictions.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
 include/drm/drm_atomic_uapi.h | 12 ++
 include/drm/drm_plane.h   |  5 +++
 3 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index aee4a65d4959..6d5b9fec90c7 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -620,7 +620,7 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
return 0;
 }
 
-static int
+int
 drm_atomic_plane_get_property(struct drm_plane *plane,
const struct drm_plane_state *state,
struct drm_property *property, uint64_t *val)
@@ -683,6 +683,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 
return 0;
 }
+EXPORT_SYMBOL(drm_atomic_plane_get_property);
 
 static int drm_atomic_set_writeback_fb_for_connector(
struct drm_connector_state *conn_state,
@@ -1026,18 +1027,54 @@ int drm_atomic_connector_commit_dpms(struct 
drm_atomic_state *state,
return ret;
 }
 
-static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
+int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
 struct drm_property *prop)
 {
if (ret != 0 || old_val != prop_value) {
drm_dbg_atomic(prop->dev,
-  "[PROP:%d:%s] No prop can be changed during 
async flip\n",
+  "[PROP:%d:%s] This prop cannot be changed during 
async flip\n",
   prop->base.id, prop->name);
return -EINVAL;
}
 
return 0;
 }
+EXPORT_SYMBOL(drm_atomic_check_prop_changes);
+
+/* plane changes may have exceptions, so we have a special function for them */
+static int drm_atomic_check_plane_changes(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (plane->funcs->check_async_props)
+   return plane->funcs->check_async_props(prop, plane, plane_state,
+obj, prop_value, 
old_val);
+
+   /*
+* if you are trying to change something other than the FB ID, your
+* change will be either rejected or ignored, so we can stop the check
+* here
+*/
+   if (prop != config->prop_fb_id) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary planes can be changed 
during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
 
 int drm_atomic_set_property(struct drm_atomic_state *state,
struct drm_file *file_priv,
@@ -1100,7 +1137,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
-   struct drm_mode_config *config = &plane->dev->mode_config;
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
@@ -1108,19 +1144,11 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
-   ret = drm_atomic_plane_get_property(plane, plane_state,
-   prop, &old_val);
-   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-   break;
-   }
-
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
-   drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
-

Re: [PATCH 0/2] drm/atomic: Allow drivers to write their own plane check for async

[André Almeida]

+ Joshua

Em 16/01/2024 10:14, Pekka Paalanen escreveu:

On Tue, 16 Jan 2024 08:50:59 -0300
André Almeida  wrote:


Hi Pekka,

Em 16/01/2024 06:45, Pekka Paalanen escreveu:

On Tue, 16 Jan 2024 01:51:57 -0300
André Almeida  wrote:
   

Hi,

AMD hardware can do more on the async flip path than just the primary plane, so
to lift up the current restrictions, this patchset allows drivers to write their
own check for planes for async flips.


Hi,

what's the userspace story for this, how could userspace know it could do more?
What kind of userspace would take advantage of this and in what situations?

Or is this not meant for generic userspace?


Sorry, I forgot to document this. So the idea is that userspace will
query what they can do here with DRM_MODE_ATOMIC_TEST_ONLY calls,
instead of having capabilities for each prop.


That's the theory, but do you have a practical example?

What other planes and props would one want change in some specific use
case?

Is it just "all or nothing", or would there be room to choose and pick
which props you change and which you don't based on what the driver
supports? If the latter, then relying on TEST_ONLY might be yet another
combinatorial explosion to iterate through.



That's a good question, maybe Simon, Xaver or Joshua can share how they 
were planning to use this on Gamescope or Kwin.




Thanks,
pq


I'm not sure if adding something new to drm_plane_funcs is the right way to do,
because if we want to expand the other object types (crtc, connector) we would
need to add their own drm_XXX_funcs, so feedbacks are welcome!

André

André Almeida (2):
drm/atomic: Allow drivers to write their own plane check for async
  flips
drm/amdgpu: Implement check_async_props for planes

   .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 30 +
   drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
   include/drm/drm_atomic_uapi.h | 12 
   include/drm/drm_plane.h   |  5 ++
   4 files changed, 92 insertions(+), 17 deletions(-)
  
   

Re: [PATCH 0/2] drm/atomic: Allow drivers to write their own plane check for async

[André Almeida]

Hi Pekka,

Em 16/01/2024 06:45, Pekka Paalanen escreveu:

On Tue, 16 Jan 2024 01:51:57 -0300
André Almeida  wrote:


Hi,

AMD hardware can do more on the async flip path than just the primary plane, so
to lift up the current restrictions, this patchset allows drivers to write their
own check for planes for async flips.


Hi,

what's the userspace story for this, how could userspace know it could do more?
What kind of userspace would take advantage of this and in what situations?

Or is this not meant for generic userspace?


Sorry, I forgot to document this. So the idea is that userspace will 
query what they can do here with DRM_MODE_ATOMIC_TEST_ONLY calls, 
instead of having capabilities for each prop.





Thanks,
pq


I'm not sure if adding something new to drm_plane_funcs is the right way to do,
because if we want to expand the other object types (crtc, connector) we would
need to add their own drm_XXX_funcs, so feedbacks are welcome!

    André

André Almeida (2):
   drm/atomic: Allow drivers to write their own plane check for async
 flips
   drm/amdgpu: Implement check_async_props for planes

  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 30 +
  drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
  include/drm/drm_atomic_uapi.h | 12 
  include/drm/drm_plane.h   |  5 ++
  4 files changed, 92 insertions(+), 17 deletions(-)

[PATCH 2/2] drm/amdgpu: Implement check_async_props for planes

[André Almeida]

AMD GPUs can do async flips with overlay planes and other props rather
than just FB ID, so implement a custom check_async_props for AMD planes.

Signed-off-by: André Almeida 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 30 +++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 116121e647ca..127cae1f9fb4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -25,6 +25,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1430,6 +1431,34 @@ static void 
amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane,
drm_atomic_helper_plane_destroy_state(plane, state);
 }
 
+static int amdgpu_dm_plane_check_async_props(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd &&
+   prop != config->prop_fb_damage_clips) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY
+   && plane_state->plane->type != DRM_PLANE_TYPE_OVERLAY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary or overlay planes can 
be changed during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static const struct drm_plane_funcs dm_plane_funcs = {
.update_plane   = drm_atomic_helper_update_plane,
.disable_plane  = drm_atomic_helper_disable_plane,
@@ -1438,6 +1467,7 @@ static const struct drm_plane_funcs dm_plane_funcs = {
.atomic_duplicate_state = amdgpu_dm_plane_drm_plane_duplicate_state,
.atomic_destroy_state = amdgpu_dm_plane_drm_plane_destroy_state,
.format_mod_supported = amdgpu_dm_plane_format_mod_supported,
+   .check_async_props = amdgpu_dm_plane_check_async_props,
 };
 
 int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
-- 
2.43.0

[PATCH 0/2] drm/atomic: Allow drivers to write their own plane check for async

[André Almeida]

Hi,

AMD hardware can do more on the async flip path than just the primary plane, so
to lift up the current restrictions, this patchset allows drivers to write their
own check for planes for async flips.

I'm not sure if adding something new to drm_plane_funcs is the right way to do,
because if we want to expand the other object types (crtc, connector) we would
need to add their own drm_XXX_funcs, so feedbacks are welcome!

André

André Almeida (2):
  drm/atomic: Allow drivers to write their own plane check for async
flips
  drm/amdgpu: Implement check_async_props for planes

 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 30 +
 drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
 include/drm/drm_atomic_uapi.h | 12 
 include/drm/drm_plane.h   |  5 ++
 4 files changed, 92 insertions(+), 17 deletions(-)

-- 
2.43.0

[PATCH 1/2] drm/atomic: Allow drivers to write their own plane check for async flips

[André Almeida]

Some hardware are more flexible on what they can flip asynchronously, so
rework the plane check so drivers can implement their own check, lifting
up some of the restrictions.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
 include/drm/drm_atomic_uapi.h | 12 ++
 include/drm/drm_plane.h   |  5 +++
 3 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index aee4a65d4959..6d5b9fec90c7 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -620,7 +620,7 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
return 0;
 }
 
-static int
+int
 drm_atomic_plane_get_property(struct drm_plane *plane,
const struct drm_plane_state *state,
struct drm_property *property, uint64_t *val)
@@ -683,6 +683,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 
return 0;
 }
+EXPORT_SYMBOL(drm_atomic_plane_get_property);
 
 static int drm_atomic_set_writeback_fb_for_connector(
struct drm_connector_state *conn_state,
@@ -1026,18 +1027,54 @@ int drm_atomic_connector_commit_dpms(struct 
drm_atomic_state *state,
return ret;
 }
 
-static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
+int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
 struct drm_property *prop)
 {
if (ret != 0 || old_val != prop_value) {
drm_dbg_atomic(prop->dev,
-  "[PROP:%d:%s] No prop can be changed during 
async flip\n",
+  "[PROP:%d:%s] This prop cannot be changed during 
async flip\n",
   prop->base.id, prop->name);
return -EINVAL;
}
 
return 0;
 }
+EXPORT_SYMBOL(drm_atomic_check_prop_changes);
+
+/* plane changes may have exceptions, so we have a special function for them */
+static int drm_atomic_check_plane_changes(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = &plane->dev->mode_config;
+   int ret;
+
+   if (plane->funcs->check_async_props)
+   return plane->funcs->check_async_props(prop, plane, plane_state,
+obj, prop_value, 
old_val);
+
+   /*
+* if you are trying to change something other than the FB ID, your
+* change will be either rejected or ignored, so we can stop the check
+* here
+*/
+   if (prop != config->prop_fb_id) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary planes can be changed 
during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
 
 int drm_atomic_set_property(struct drm_atomic_state *state,
struct drm_file *file_priv,
@@ -1100,7 +1137,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
-   struct drm_mode_config *config = &plane->dev->mode_config;
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
@@ -1108,19 +1144,11 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
-   if (async_flip && prop != config->prop_fb_id) {
-   ret = drm_atomic_plane_get_property(plane, plane_state,
-   prop, &old_val);
-   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
-   break;
-   }
-
-   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
-   drm_dbg_atomic(prop->dev,
-  "[OBJECT:%d] Only primary planes can be 
changed during async flip\n",
-

Re: [PATCH] drm: allow IN_FENCE_FD and FB_DAMAGE_CLIPS to be changed with async commits

[André Almeida]

Em 11/01/2024 14:59, Xaver Hugl escreveu:
Am Do., 11. Jan. 2024 um 18:13 Uhr schrieb Simon Ser 
mailto:cont...@emersion.fr>>:


Are we sure that all drivers handle these two props properly with async
page-flips? This is a new codepath not taken by the legacy uAPI.

I've only tested on amdgpu so far. Afacs the other drivers that would need
testing / that support atomic and async pageflips are
- i915
- noueveau (though atomic is disabled by default, so maybe it doesn't 
matter?)

- vc4
- atmel-hlcdc

The first two I can test, the latter I don't have the hardware for. I 
don't know if I can
extensively test fb_damage_clips either / how I'd even know if it's 
being applied
correctly, but in the worst case I'd expect the driver to not do the 
optimizations the

property allows.

As an alternative to this, would it be okay to expose a driver hook for 
optional
driver-specific checks that drm_atomic_set_property can delegate to, and 
only

allow this with the properties and hardware that's been tested? Then more
properties (like cursor position changes on amdgpu) could be easily 
added later

on too.


I'm working on some mechanism to allow overlay planes on amdgpu, and I 
think I can add your needs to it. I'll share in the mailing list when I 
have something more concrete.

Re: [PATCH] drm: allow IN_FENCE_FD and FB_DAMAGE_CLIPS to be changed with async commits

[André Almeida]

Hi Xaver,

Em 11/01/2024 13:56, Xaver Hugl escreveu:

Like with FB_ID, the driver never has to do bandwidth validation to use
these properties, so there's no reason not to allow them.

Signed-off-by: Xaver Hugl 


Reviewed-by: André Almeida 


---
  drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index aee4a65d4959..06d476f5a746 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1108,7 +1108,9 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
  
-		if (async_flip && prop != config->prop_fb_id) {

+   if (async_flip && prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd &&
+   prop != config->prop_fb_damage_clips) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, &old_val);
ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);

[PATCH] drm/doc: Define KMS atomic state set

[André Almeida]

From: Pekka Paalanen 

Specify how the atomic state is maintained between userspace and
kernel, plus the special case for async flips.

Signed-off-by: Pekka Paalanen 
Signed-off-by: André Almeida 
---

This is a standalone patch from the following serie, the other patches are
already merged:
https://lore.kernel.org/lkml/20231122161941.320564-1-andrealm...@igalia.com/

 Documentation/gpu/drm-uapi.rst | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 370d820be248..d0693f902a5c 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -570,3 +570,50 @@ dma-buf interoperability
 
 Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst for
 information on how dma-buf is integrated and exposed within DRM.
+
+KMS atomic state
+
+
+An atomic commit can change multiple KMS properties in an atomic fashion,
+without ever applying intermediate or partial state changes.  Either the whole
+commit succeeds or fails, and it will never be applied partially. This is the
+fundamental improvement of the atomic API over the older non-atomic API which 
is
+referred to as the "legacy API".  Applying intermediate state could 
unexpectedly
+fail, cause visible glitches, or delay reaching the final state.
+
+An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the
+complete state change is validated but not applied.  Userspace should use this
+flag to validate any state change before asking to apply it. If validation 
fails
+for any reason, userspace should attempt to fall back to another, perhaps
+simpler, final state.  This allows userspace to probe for various 
configurations
+without causing visible glitches on screen and without the need to undo a
+probing change.
+
+The changes recorded in an atomic commit apply on top the current KMS state in
+the kernel. Hence, the complete new KMS state is the complete old KMS state 
with
+the committed property settings done on top. The kernel will try to avoid
+no-operation changes, so it is safe for userspace to send redundant property
+settings.  However, not every situation allows for no-op changes, due to the
+need to acquire locks for some attributes. Userspace needs to be aware that 
some
+redundant information might result in oversynchronization issues.  No-operation
+changes do not count towards actually needed changes, e.g.  setting MODE_ID to 
a
+different blob with identical contents as the current KMS state shall not be a
+modeset on its own. As a special exception for VRR needs, explicitly setting
+FB_ID to its current value is not a no-op.
+
+A "modeset" is a change in KMS state that might enable, disable, or temporarily
+disrupt the emitted video signal, possibly causing visible glitches on screen. 
A
+modeset may also take considerably more time to complete than other kinds of
+changes, and the video sink might also need time to adapt to the new signal
+properties. Therefore a modeset must be explicitly allowed with the flag
+DRM_MODE_ATOMIC_ALLOW_MODESET.  This in combination with
+DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is
+likely to cause visible disruption on screen and avoid such changes when end
+users do not expect them.
+
+An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to
+effectively change only the FB_ID property on any planes. No-operation changes
+are ignored as always. Changing any other property will cause the commit to be
+rejected. Each driver may relax this restriction if they have guarantees that
+such property change doesn't cause modesets. Userspace can use TEST_ONLY 
commits
+to query the driver about this.
-- 
2.43.0

Re: [PATCH v9 0/4] drm: Add support for atomic async page-flip

[André Almeida]

Em 23/11/2023 13:24, Simon Ser escreveu:

Thanks! This iteration of the first 3 patches LGTM, I've pushed them to
drm-misc-next. I've made two adjustments to make checkpatch.pl happy:



Thank you!


- s/uint64_t/u64/
- Fix indentation for a drm_dbg_atomic()


ops :)

Re: [PATCH v9 0/4] drm: Add support for atomic async page-flip

[André Almeida]

Hi Hamza,

Em 22/11/2023 17:23, Hamza Mahfooz escreveu:

Hi André,
On 11/22/23 11:19, André Almeida wrote:

Hi,

This work from me and Simon adds support for DRM_MODE_PAGE_FLIP_ASYNC 
through
the atomic API. This feature is already available via the legacy API. 
The use

case is to be able to present a new frame immediately (or as soon as
possible), even if after missing a vblank. This might result in 
tearing, but

it's useful when a high framerate is desired, such as for gaming.

Differently from earlier versions, this one refuses to flip if any 
prop changes
for async flips. The idea is that the fast path of immediate page 
flips doesn't

play well with modeset changes, so only the fb_id can be changed.

Tested with:
  - Intel TigerLake-LP GT2
  - AMD VanGogh


Have you had a chance to test this with VRR enabled? Since, I suspect
this series might break that feature.



Someone asked this question in an earlier version of this patch, and the 
result is that VRR still works as expected. You can follow the thread at 
this link:


https://lore.kernel.org/lkml/b48bd1fc-fcb0-481b-8413-9210d44d7...@igalia.com/

I should have included this note at my cover letter, my bad.

Thanks,
André

[PATCH v9 4/4] drm/doc: Define KMS atomic state set

[André Almeida]

From: Pekka Paalanen 

Specify how the atomic state is maintained between userspace and
kernel, plus the special case for async flips.

Signed-off-by: Pekka Paalanen 
Signed-off-by: André Almeida 
---
v9:
- no changes
v8:
- no changes
v7:
- add a note that drivers can make exceptions for ad-hoc prop changes
- add a note about flipping the same FB_ID as a no-op
---
---
 Documentation/gpu/drm-uapi.rst | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 370d820be248..d0693f902a5c 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -570,3 +570,50 @@ dma-buf interoperability
 
 Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst for
 information on how dma-buf is integrated and exposed within DRM.
+
+KMS atomic state
+
+
+An atomic commit can change multiple KMS properties in an atomic fashion,
+without ever applying intermediate or partial state changes.  Either the whole
+commit succeeds or fails, and it will never be applied partially. This is the
+fundamental improvement of the atomic API over the older non-atomic API which 
is
+referred to as the "legacy API".  Applying intermediate state could 
unexpectedly
+fail, cause visible glitches, or delay reaching the final state.
+
+An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the
+complete state change is validated but not applied.  Userspace should use this
+flag to validate any state change before asking to apply it. If validation 
fails
+for any reason, userspace should attempt to fall back to another, perhaps
+simpler, final state.  This allows userspace to probe for various 
configurations
+without causing visible glitches on screen and without the need to undo a
+probing change.
+
+The changes recorded in an atomic commit apply on top the current KMS state in
+the kernel. Hence, the complete new KMS state is the complete old KMS state 
with
+the committed property settings done on top. The kernel will try to avoid
+no-operation changes, so it is safe for userspace to send redundant property
+settings.  However, not every situation allows for no-op changes, due to the
+need to acquire locks for some attributes. Userspace needs to be aware that 
some
+redundant information might result in oversynchronization issues.  No-operation
+changes do not count towards actually needed changes, e.g.  setting MODE_ID to 
a
+different blob with identical contents as the current KMS state shall not be a
+modeset on its own. As a special exception for VRR needs, explicitly setting
+FB_ID to its current value is not a no-op.
+
+A "modeset" is a change in KMS state that might enable, disable, or temporarily
+disrupt the emitted video signal, possibly causing visible glitches on screen. 
A
+modeset may also take considerably more time to complete than other kinds of
+changes, and the video sink might also need time to adapt to the new signal
+properties. Therefore a modeset must be explicitly allowed with the flag
+DRM_MODE_ATOMIC_ALLOW_MODESET.  This in combination with
+DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is
+likely to cause visible disruption on screen and avoid such changes when end
+users do not expect them.
+
+An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to
+effectively change only the FB_ID property on any planes. No-operation changes
+are ignored as always. Changing any other property will cause the commit to be
+rejected. Each driver may relax this restriction if they have guarantees that
+such property change doesn't cause modesets. Userspace can use TEST_ONLY 
commits
+to query the driver about this.
-- 
2.42.1

[PATCH v9 3/4] drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP

[André Almeida]

From: Simon Ser 

This new kernel capability indicates whether async page-flips are
supported via the atomic uAPI. DRM clients can use it to check
for support before feeding DRM_MODE_PAGE_FLIP_ASYNC to the kernel.

Make it clear that DRM_CAP_ASYNC_PAGE_FLIP is for legacy uAPI only.

Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Reviewed-by: Alex Deucher 
Signed-off-by: André Almeida 
---
v9: no changes
---
---
 drivers/gpu/drm/drm_ioctl.c |  4 
 include/uapi/drm/drm.h  | 10 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 44fda68c28ae..f461ed862480 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -301,6 +301,10 @@ static int drm_getcap(struct drm_device *dev, void *data, 
struct drm_file *file_
case DRM_CAP_CRTC_IN_VBLANK_EVENT:
req->value = 1;
break;
+   case DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP:
+   req->value = drm_core_check_feature(dev, DRIVER_ATOMIC) &&
+dev->mode_config.async_page_flip;
+   break;
default:
return -EINVAL;
}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 8662b5aeea0c..796de831f4a0 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -713,7 +713,8 @@ struct drm_gem_open {
 /**
  * DRM_CAP_ASYNC_PAGE_FLIP
  *
- * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC.
+ * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for legacy
+ * page-flips.
  */
 #define DRM_CAP_ASYNC_PAGE_FLIP0x7
 /**
@@ -773,6 +774,13 @@ struct drm_gem_open {
  * :ref:`drm_sync_objects`.
  */
 #define DRM_CAP_SYNCOBJ_TIMELINE   0x14
+/**
+ * DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
+ *
+ * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for atomic
+ * commits.
+ */
+#define DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP 0x15
 
 /* DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
-- 
2.42.1

[PATCH v9 2/4] drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits

[André Almeida]

From: Simon Ser 

If the driver supports it, allow user-space to supply the
DRM_MODE_PAGE_FLIP_ASYNC flag to request an async page-flip.
Set drm_crtc_state.async_flip accordingly.

Document that drivers will reject atomic commits if an async
flip isn't possible. This allows user-space to fall back to
something else. For instance, Xorg falls back to a blit.
Another option is to wait as close to the next vblank as
possible before performing the page-flip to reduce latency.

Signed-off-by: Simon Ser 
Reviewed-by: Alex Deucher 
Co-developed-by: André Almeida 
Signed-off-by: André Almeida 
---
v9: dropped atomic_async_page_flip_not_supported
---
 drivers/gpu/drm/drm_atomic_uapi.c | 25 ++---
 include/uapi/drm/drm_mode.h   |  9 +
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index ed46133a2dd7..de4265423ddc 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1368,6 +1368,18 @@ static void complete_signaling(struct drm_device *dev,
kfree(fence_state);
 }
 
+static void
+set_async_flip(struct drm_atomic_state *state)
+{
+   struct drm_crtc *crtc;
+   struct drm_crtc_state *crtc_state;
+   int i;
+
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+   crtc_state->async_flip = true;
+   }
+}
+
 int drm_mode_atomic_ioctl(struct drm_device *dev,
  void *data, struct drm_file *file_priv)
 {
@@ -1409,9 +1421,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
}
 
if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) {
-   drm_dbg_atomic(dev,
-  "commit failed: invalid flag 
DRM_MODE_PAGE_FLIP_ASYNC\n");
-   return -EINVAL;
+   if (!dev->mode_config.async_page_flip) {
+   drm_dbg_atomic(dev,
+  "commit failed: DRM_MODE_PAGE_FLIP_ASYNC 
not supported\n");
+   return -EINVAL;
+   }
+
+   async_flip = true;
}
 
/* can't test and expect an event at the same time. */
@@ -1514,6 +1530,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
if (ret)
goto out;
 
+   if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC)
+   set_async_flip(state);
+
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
ret = drm_atomic_check_only(state);
} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 09e7a471ee30..95630f170110 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -957,6 +957,15 @@ struct hdr_output_metadata {
  * Request that the page-flip is performed as soon as possible, ie. with no
  * delay due to waiting for vblank. This may cause tearing to be visible on
  * the screen.
+ *
+ * When used with atomic uAPI, the driver will return an error if the hardware
+ * doesn't support performing an asynchronous page-flip for this update.
+ * User-space should handle this, e.g. by falling back to a regular page-flip.
+ *
+ * Note, some hardware might need to perform one last synchronous page-flip
+ * before being able to switch to asynchronous page-flips. As an exception,
+ * the driver will return success even though that first page-flip is not
+ * asynchronous.
  */
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-- 
2.42.1

[PATCH v9 1/4] drm: Refuse to async flip with atomic prop changes

[André Almeida]

Given that prop changes may lead to modesetting, which would defeat the
fast path of the async flip, refuse any atomic prop change for async
flips in atomic API. The only exception is the framebuffer ID to flip
to. Currently the only plane type supported is the primary one.

Signed-off-by: André Almeida 
Reviewed-by: Simon Ser 
---
v9: no changes
v8: add a check for plane type, we can only flip primary planes
v7: drop the mode_id exception for prop changes
---
 drivers/gpu/drm/drm_atomic_uapi.c   | 52 +++--
 drivers/gpu/drm/drm_crtc_internal.h |  2 +-
 drivers/gpu/drm/drm_mode_object.c   |  2 +-
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 98d3b10c08ae..ed46133a2dd7 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1006,13 +1006,28 @@ int drm_atomic_connector_commit_dpms(struct 
drm_atomic_state *state,
return ret;
 }
 
+static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
+struct drm_property *prop)
+{
+   if (ret != 0 || old_val != prop_value) {
+   drm_dbg_atomic(prop->dev,
+  "[PROP:%d:%s] No prop can be changed during 
async flip\n",
+  prop->base.id, prop->name);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 int drm_atomic_set_property(struct drm_atomic_state *state,
struct drm_file *file_priv,
struct drm_mode_object *obj,
struct drm_property *prop,
-   uint64_t prop_value)
+   uint64_t prop_value,
+   bool async_flip)
 {
struct drm_mode_object *ref;
+   uint64_t old_val;
int ret;
 
if (!drm_property_change_valid_get(prop, prop_value, &ref))
@@ -1029,6 +1044,13 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip) {
+   ret = drm_atomic_connector_get_property(connector, 
connector_state,
+   prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
ret = drm_atomic_connector_set_property(connector,
connector_state, file_priv,
prop, prop_value);
@@ -1044,6 +1066,13 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip) {
+   ret = drm_atomic_crtc_get_property(crtc, crtc_state,
+  prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
ret = drm_atomic_crtc_set_property(crtc,
crtc_state, prop, prop_value);
break;
@@ -1051,6 +1080,7 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
+   struct drm_mode_config *config = &plane->dev->mode_config;
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
@@ -1058,6 +1088,21 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip && prop != config->prop_fb_id) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
+   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
+   drm_dbg_atomic(prop->dev,
+   "[OBJECT:%d] Only primary planes can be changed 
during async flip\n",
+   obj->id);
+   ret = -EINVAL;
+   break;
+   }
+
ret = drm_atomic_plane_set_property(plane,
plane_state, file_priv,
prop, prop_value);
@@ -1337,6 +1382,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
struct drm_out_fence_state *fence_state;
int ret = 0;
unsigned int i,

[PATCH v9 0/4] drm: Add support for atomic async page-flip

[André Almeida]

Hi,

This work from me and Simon adds support for DRM_MODE_PAGE_FLIP_ASYNC through
the atomic API. This feature is already available via the legacy API. The use
case is to be able to present a new frame immediately (or as soon as
possible), even if after missing a vblank. This might result in tearing, but
it's useful when a high framerate is desired, such as for gaming.

Differently from earlier versions, this one refuses to flip if any prop changes
for async flips. The idea is that the fast path of immediate page flips doesn't
play well with modeset changes, so only the fb_id can be changed.

Tested with:
 - Intel TigerLake-LP GT2
 - AMD VanGogh

Thanks,
André

- User-space patch: https://github.com/Plagman/gamescope/pull/595
- IGT tests: 
https://lore.kernel.org/all/20231110163811.24158-1-andrealm...@igalia.com/

Changes from v8:
- Dropped atomic_async_page_flip_not_supported, giving that current design works
with any driver that support atomic and async at the same time.
- Dropped the patch that disabled atomic_async_page_flip_not_supported for AMD.
- Reordered commits
v8: https://lore.kernel.org/all/20231025005318.293690-1-andrealm...@igalia.com/

Changes from v7:
- Only accept flips to primary planes. If a driver support flips in different
planes, support will be added  later.
v7: 
https://lore.kernel.org/dri-devel/20231017092837.32428-1-andrealm...@igalia.com/

Changes from v6:
- Dropped the exception to allow MODE_ID changes (Simon)
- Clarify what happens when flipping with the same FB_ID (Pekka)

v6: 
https://lore.kernel.org/dri-devel/20230815185710.159779-1-andrealm...@igalia.com/

Changes from v5:
- Add note in the docs that not every redundant attribute will result in no-op,
  some might cause oversynchronization issues.

v5: 
https://lore.kernel.org/dri-devel/20230707224059.305474-1-andrealm...@igalia.com/

Changes from v4:
 - Documentation rewrote by Pekka Paalanen

v4: 
https://lore.kernel.org/dri-devel/20230701020917.143394-1-andrealm...@igalia.com/

Changes from v3:
 - Add new patch to reject prop changes
 - Add a documentation clarifying the KMS atomic state set

v3: 
https://lore.kernel.org/dri-devel/20220929184307.258331-1-cont...@emersion.fr/

André Almeida (1):
  drm: Refuse to async flip with atomic prop changes

Pekka Paalanen (1):
  drm/doc: Define KMS atomic state set

Simon Ser (2):
  drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits
  drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP

 Documentation/gpu/drm-uapi.rst  | 47 ++
 drivers/gpu/drm/drm_atomic_uapi.c   | 77 ++---
 drivers/gpu/drm/drm_crtc_internal.h |  2 +-
 drivers/gpu/drm/drm_ioctl.c |  4 ++
 drivers/gpu/drm/drm_mode_object.c   |  2 +-
 include/uapi/drm/drm.h  | 10 +++-
 include/uapi/drm/drm_mode.h |  9 
 7 files changed, 142 insertions(+), 9 deletions(-)

-- 
2.42.1

[PATCH v2] drm/amd: Document device reset methods

[André Almeida]

Document what each amdgpu driver reset method does.

Signed-off-by: André Almeida 
---
v2: Add more details and small correction (Alex)

 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a79d53bdbe13..c4675572f907 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -504,6 +504,31 @@ struct amdgpu_allowed_register_entry {
bool grbm_indexed;
 };
 
+/**
+ * enum amd_reset_method - Methods for resetting AMD GPU devices
+ *
+ * @AMD_RESET_METHOD_NONE: The device will not be reset.
+ * @AMD_RESET_LEGACY: Method reserved for SI, CIK and VI ASICs.
+ * @AMD_RESET_MODE0: Reset the entire ASIC. Not currently available for the
+ *   any device.
+ * @AMD_RESET_MODE1: Resets all IP blocks on the ASIC (SDMA, GFX, VCN, etc.)
+ *   individually. Suitable only for some discrete GPU, not
+ *   available for all ASICs.
+ * @AMD_RESET_MODE2: Resets a lesser level of IPs compared to MODE1. Which IPs
+ *   are reset depends on the ASIC. Notably doesn't reset IPs
+ *   shared with the CPU on APUs or the memory controllers (so
+ *   VRAM is not lost). Not available on all ASICs.
+ * @AMD_RESET_BACO: BACO (Bus Alive, Chip Off) method powers off and on the 
card
+ *  but without powering off the PCI bus. Suitable only for
+ *  discrete GPUs.
+ * @AMD_RESET_PCI: Does a full bus reset using core Linux subsystem PCI reset
+ * and does a secondary bus reset or FLR, depending on what the
+ * underlying hardware supports.
+ *
+ * Methods available for AMD GPU driver for resetting the device. Not all
+ * methods are suitable for every device. User can overwrite the method using
+ * module parameter `reset_method`.
+ */
 enum amd_reset_method {
AMD_RESET_METHOD_NONE = -1,
AMD_RESET_METHOD_LEGACY = 0,
-- 
2.42.1

[PATCH i-g-t 2/2] tests/kms_async_flips: Add test for atomic prop changes

[André Almeida]

DRM atomic API should reject modesetting during an async flip, so
test if the kernel properly rejects to flip with prop changes.

Signed-off-by: André Almeida 
---
 tests/kms_async_flips.c | 68 +
 1 file changed, 68 insertions(+)

diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
index 6bfcca474..edc19b5ef 100644
--- a/tests/kms_async_flips.c
+++ b/tests/kms_async_flips.c
@@ -305,6 +305,9 @@ static void test_async_flip_atomic(data_t *data)
 
test_init(data);
 
+   igt_plane_set_fb(data->plane, &data->bufs[0]);
+   igt_display_commit_atomic(&data->display, 
DRM_MODE_ATOMIC_ALLOW_MODESET, data);
+
gettimeofday(&start, NULL);
frame = 1;
do {
@@ -326,6 +329,55 @@ static void test_async_flip_atomic(data_t *data)
 "FPS should be significantly higher than the refresh 
rate\n");
 }
 
+static void test_invalid_atomic(data_t *data)
+{
+   int flags = DRM_MODE_PAGE_FLIP_ASYNC | DRM_MODE_PAGE_FLIP_EVENT;
+   int ret;
+
+   test_init(data);
+
+   igt_plane_set_fb(data->plane, &data->bufs[0]);
+   igt_display_commit_atomic(&data->display, 
DRM_MODE_ATOMIC_ALLOW_MODESET, data);
+
+   /* Trying to change plane position  */
+   igt_plane_set_position(data->plane, 15, 15);
+   igt_plane_set_fb(data->plane, &data->bufs[1]);
+   ret = igt_display_try_commit_atomic(&data->display, flags, data);
+   igt_assert(ret == -EINVAL);
+   igt_plane_set_position(data->plane, 0, 0);
+
+   /* Trying to change plane rotation  */
+   igt_plane_set_rotation(data->plane, IGT_ROTATION_180);
+   igt_plane_set_fb(data->plane, &data->bufs[1]);
+   ret = igt_display_try_commit_atomic(&data->display, flags, data);
+   igt_assert(ret == -EINVAL);
+   igt_plane_set_rotation(data->plane, IGT_ROTATION_0);
+}
+
+static void test_atomic_modeset(data_t *data)
+{
+   int flags = DRM_MODE_PAGE_FLIP_ASYNC | DRM_MODE_PAGE_FLIP_EVENT;
+   igt_output_t *output = data->output;
+   int ret;
+
+   test_init(data);
+
+   igt_plane_set_fb(data->plane, &data->bufs[0]);
+   igt_display_commit_atomic(&data->display, 
DRM_MODE_ATOMIC_ALLOW_MODESET, data);
+
+   /*
+* Modesetting is forbidden during atomic async flips. Mode changes that
+* require modeset are rejected.
+*/
+   for_each_connector_mode(output) {
+   drmModeModeInfo *m = &output->config.connector->modes[j__];
+   igt_output_override_mode(output, m);
+   ret = igt_display_try_commit_atomic(&data->display, flags, 
data);
+   igt_assert(ret == -EINVAL);
+   }
+   igt_output_override_mode(output, NULL);
+}
+
 static void wait_for_vblank(data_t *data, unsigned long *vbl_time, unsigned 
int *seq)
 {
drmVBlank wait_vbl;
@@ -757,6 +809,22 @@ igt_main
run_test(&data, test_async_flip_atomic);
}
 
+   igt_describe("Negative case to verify if any atomic changes are 
rejected from kernel as expected");
+   igt_subtest_with_dynamic("invalid-atomic-async-flip") {
+   require_monotonic_timestamp(data.drm_fd);
+   igt_require_f(igt_has_drm_cap(data.drm_fd, 
DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP),
+ "Atomic async page-flips are not supported\n");
+   run_test(&data, test_invalid_atomic);
+   }
+
+   igt_describe("Verify mode changes during atomic async flips");
+   igt_subtest_with_dynamic("modeset-atomic-async-flip") {
+   require_monotonic_timestamp(data.drm_fd);
+   igt_require_f(igt_has_drm_cap(data.drm_fd, 
DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP),
+ "Atomic async page-flips are not supported\n");
+   run_test(&data, test_atomic_modeset);
+   }
+
igt_fixture {
for (i = 0; i < NUM_FBS; i++)
igt_remove_fb(data.drm_fd, &data.bufs[i]);
-- 
2.42.1

[PATCH i-g-t 1/2] tests/kms_async_flips: add atomic test

[André Almeida]

From: Simon Ser 

This adds a simple test for DRM_MODE_PAGE_FLIP_ASYNC with the
atomic uAPI.

Signed-off-by: Simon Ser 
Signed-off-by: André Almeida 
---
 include/drm-uapi/drm.h  |  8 
 tests/kms_async_flips.c | 37 +
 2 files changed, 45 insertions(+)

diff --git a/include/drm-uapi/drm.h b/include/drm-uapi/drm.h
index 02540248d..500019ce0 100644
--- a/include/drm-uapi/drm.h
+++ b/include/drm-uapi/drm.h
@@ -768,6 +768,14 @@ struct drm_gem_open {
  */
 #define DRM_CAP_SYNCOBJ_TIMELINE   0x14
 
+/**
+ * DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
+ *
+ * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for atomic
+ * commits.
+ */
+#define DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP 0x15
+
 /* DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
__u64 capability;
diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
index 6c97558be..6bfcca474 100644
--- a/tests/kms_async_flips.c
+++ b/tests/kms_async_flips.c
@@ -297,6 +297,35 @@ static void test_async_flip(data_t *data)
}
 }
 
+static void test_async_flip_atomic(data_t *data)
+{
+   int frame;
+   long long int fps;
+   struct timeval start, end, diff;
+
+   test_init(data);
+
+   gettimeofday(&start, NULL);
+   frame = 1;
+   do {
+   uint32_t flags = DRM_MODE_PAGE_FLIP_ASYNC | 
DRM_MODE_PAGE_FLIP_EVENT;
+
+   igt_plane_set_fb(data->plane, &data->bufs[frame % 4]);
+   igt_display_commit_atomic(&data->display, flags, data);
+
+   wait_flip_event(data);
+
+   gettimeofday(&end, NULL);
+   timersub(&end, &start, &diff);
+
+   frame++;
+   } while (diff.tv_sec < RUN_TIME);
+
+   fps = frame * 1000 / RUN_TIME;
+   igt_assert_f((fps / 1000) > (data->refresh_rate * MIN_FLIPS_PER_FRAME),
+"FPS should be significantly higher than the refresh 
rate\n");
+}
+
 static void wait_for_vblank(data_t *data, unsigned long *vbl_time, unsigned 
int *seq)
 {
drmVBlank wait_vbl;
@@ -720,6 +749,14 @@ igt_main
run_test(&data, test_crc);
}
 
+   igt_describe("Verify the async flip functionality and the fps during 
atomic async flips");
+   igt_subtest_with_dynamic("atomic-async-flip") {
+   require_monotonic_timestamp(data.drm_fd);
+   igt_require_f(igt_has_drm_cap(data.drm_fd, 
DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP),
+ "Atomic async page-flips are not supported\n");
+   run_test(&data, test_async_flip_atomic);
+   }
+
igt_fixture {
for (i = 0; i < NUM_FBS; i++)
igt_remove_fb(data.drm_fd, &data.bufs[i]);
-- 
2.42.1

[PATCH i-g-t 0/2] Add test for atomic async page flip

[André Almeida]

This series adds tests for async page flip for DRM Atomic API. Kernel patches:
https://lore.kernel.org/dri-devel/20231025005318.293690-1-andrealm...@igalia.com/

André Almeida (1):
  tests/kms_async_flips: Add test for atomic prop changes

Simon Ser (1):
  tests/kms_async_flips: add atomic test

 include/drm-uapi/drm.h  |   8 +++
 tests/kms_async_flips.c | 105 
 2 files changed, 113 insertions(+)

-- 
2.42.1

[PATCH] drm/amd: Document device reset methods

[André Almeida]

Document what each amdgpu driver reset method does.

Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a79d53bdbe13..500f86c79eb7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -504,6 +504,26 @@ struct amdgpu_allowed_register_entry {
bool grbm_indexed;
 };
 
+/**
+ * enum amd_reset_method - Methods for resetting AMD GPU devices
+ *
+ * @AMD_RESET_METHOD_NONE: The device will not be reset.
+ * @AMD_RESET_LEGACY: Method reserved for SI/CIK asics.
+ * @AMD_RESET_MODE0: High level PCIe reset.
+ * @AMD_RESET_MODE1: Resets each IP block (SDMA, GFX, VCN, etc.) individually.
+ *   Suitable only for some discrete GPUs.
+ * @AMD_RESET_MODE2: Resets only the GFX block. Useful for APUs, giving that
+ *   the rest of IP blocks and SMU is shared with the CPU.
+ * @AMD_RESET_BACO: BACO (Bus Alive, Chip Off) method powers off and on the 
card
+ *  but without powering off the PCI bus. Suitable only for
+ *  discrete GPUs.
+ * @AMD_RESET_PCI: Does a full bus reset, including powering on and off the
+ * card.
+ *
+ * Methods available for AMD GPU driver for resetting the device. Not all
+ * methods are suitable for every device. User can overwrite the method using
+ * module parameter `reset_method`.
+ */
 enum amd_reset_method {
AMD_RESET_METHOD_NONE = -1,
AMD_RESET_METHOD_LEGACY = 0,
-- 
2.42.1

[PATCH v8 6/6] amd/display: indicate support for atomic async page-flips on DC

[André Almeida]

From: Simon Ser 

amdgpu_dm_commit_planes() already sets the flip_immediate flag for
async page-flips. This flag is used to set the UNP_FLIP_CONTROL
register. Thus, no additional change is required to handle async
page-flips with the atomic uAPI.

Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Reviewed-by: Alex Deucher 
Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index dec6e43e7198..45b8fd61a044 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4003,7 +4003,6 @@ static int amdgpu_dm_mode_config_init(struct 
amdgpu_device *adev)
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
/* indicates support for immediate flip */
adev_to_drm(adev)->mode_config.async_page_flip = true;
-   adev_to_drm(adev)->mode_config.atomic_async_page_flip_not_supported = 
true;
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
-- 
2.42.0

[PATCH v8 5/6] drm/doc: Define KMS atomic state set

[André Almeida]

From: Pekka Paalanen 

Specify how the atomic state is maintained between userspace and
kernel, plus the special case for async flips.

Signed-off-by: Pekka Paalanen 
Signed-off-by: André Almeida 
---
v8:
- no changes
v7:
- add a note that drivers can make exceptions for ad-hoc prop changes
- add a note about flipping the same FB_ID as a no-op
---
---
 Documentation/gpu/drm-uapi.rst | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 632989df3727..34bd02270ee7 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -570,3 +570,50 @@ dma-buf interoperability
 
 Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst for
 information on how dma-buf is integrated and exposed within DRM.
+
+KMS atomic state
+
+
+An atomic commit can change multiple KMS properties in an atomic fashion,
+without ever applying intermediate or partial state changes.  Either the whole
+commit succeeds or fails, and it will never be applied partially. This is the
+fundamental improvement of the atomic API over the older non-atomic API which 
is
+referred to as the "legacy API".  Applying intermediate state could 
unexpectedly
+fail, cause visible glitches, or delay reaching the final state.
+
+An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the
+complete state change is validated but not applied.  Userspace should use this
+flag to validate any state change before asking to apply it. If validation 
fails
+for any reason, userspace should attempt to fall back to another, perhaps
+simpler, final state.  This allows userspace to probe for various 
configurations
+without causing visible glitches on screen and without the need to undo a
+probing change.
+
+The changes recorded in an atomic commit apply on top the current KMS state in
+the kernel. Hence, the complete new KMS state is the complete old KMS state 
with
+the committed property settings done on top. The kernel will try to avoid
+no-operation changes, so it is safe for userspace to send redundant property
+settings.  However, not every situation allows for no-op changes, due to the
+need to acquire locks for some attributes. Userspace needs to be aware that 
some
+redundant information might result in oversynchronization issues.  No-operation
+changes do not count towards actually needed changes, e.g.  setting MODE_ID to 
a
+different blob with identical contents as the current KMS state shall not be a
+modeset on its own. As a special exception for VRR needs, explicitly setting
+FB_ID to its current value is not a no-op.
+
+A "modeset" is a change in KMS state that might enable, disable, or temporarily
+disrupt the emitted video signal, possibly causing visible glitches on screen. 
A
+modeset may also take considerably more time to complete than other kinds of
+changes, and the video sink might also need time to adapt to the new signal
+properties. Therefore a modeset must be explicitly allowed with the flag
+DRM_MODE_ATOMIC_ALLOW_MODESET.  This in combination with
+DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is
+likely to cause visible disruption on screen and avoid such changes when end
+users do not expect them.
+
+An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to
+effectively change only the FB_ID property on any planes. No-operation changes
+are ignored as always. Changing any other property will cause the commit to be
+rejected. Each driver may relax this restriction if they have guarantees that
+such property change doesn't cause modesets. Userspace can use TEST_ONLY 
commits
+to query the driver about this.
-- 
2.42.0

[PATCH v8 4/6] drm: Refuse to async flip with atomic prop changes

[André Almeida]

Given that prop changes may lead to modesetting, which would defeat the
fast path of the async flip, refuse any atomic prop change for async
flips in atomic API. The only exception is the framebuffer ID to flip
to. Currently the only plane type supported is the primary one.

Reviewed-by: Simon Ser 
Signed-off-by: André Almeida 
---
v8: add a check for plane type, we can only flip primary planes
v7: drop the mode_id exception for prop changes
---
---
 drivers/gpu/drm/drm_atomic_uapi.c   | 54 +++--
 drivers/gpu/drm/drm_crtc_internal.h |  2 +-
 drivers/gpu/drm/drm_mode_object.c   |  2 +-
 3 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index a15121e75a0a..ebaa6413d5a0 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1006,13 +1006,28 @@ int drm_atomic_connector_commit_dpms(struct 
drm_atomic_state *state,
return ret;
 }
 
+static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
+struct drm_property *prop)
+{
+   if (ret != 0 || old_val != prop_value) {
+   drm_dbg_atomic(prop->dev,
+  "[PROP:%d:%s] No prop can be changed during 
async flip\n",
+  prop->base.id, prop->name);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 int drm_atomic_set_property(struct drm_atomic_state *state,
struct drm_file *file_priv,
struct drm_mode_object *obj,
struct drm_property *prop,
-   uint64_t prop_value)
+   uint64_t prop_value,
+   bool async_flip)
 {
struct drm_mode_object *ref;
+   uint64_t old_val;
int ret;
 
if (!drm_property_change_valid_get(prop, prop_value, &ref))
@@ -1029,6 +1044,13 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip) {
+   ret = drm_atomic_connector_get_property(connector, 
connector_state,
+   prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
ret = drm_atomic_connector_set_property(connector,
connector_state, file_priv,
prop, prop_value);
@@ -1044,6 +1066,13 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip) {
+   ret = drm_atomic_crtc_get_property(crtc, crtc_state,
+  prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
ret = drm_atomic_crtc_set_property(crtc,
crtc_state, prop, prop_value);
break;
@@ -1051,6 +1080,7 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
+   struct drm_mode_config *config = &plane->dev->mode_config;
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
@@ -1058,6 +1088,21 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip && prop != config->prop_fb_id) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
+   if (async_flip && plane_state->plane->type != 
DRM_PLANE_TYPE_PRIMARY) {
+   drm_dbg_atomic(prop->dev,
+   "[OBJECT:%d] Only primary planes can be changed 
during async flip\n",
+   obj->id);
+   ret = -EINVAL;
+   break;
+   }
+
ret = drm_atomic_plane_set_property(plane,
plane_state, file_priv,
prop, prop_value);
@@ -1349,6 +1394,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
struct drm_out_fence_state *fence_state;
int ret = 0;
unsigned int i, j, num_fen

[PATCH v8 3/6] drm: introduce drm_mode_config.atomic_async_page_flip_not_supported

[André Almeida]

From: Simon Ser 

This new field indicates whether the driver has the necessary logic
to support async page-flips via the atomic uAPI. This is leveraged by
the next commit to allow user-space to use this functionality.

All atomic drivers setting drm_mode_config.async_page_flip are updated
to also set drm_mode_config.atomic_async_page_flip_not_supported. We
will gradually check and update these drivers to properly handle
drm_crtc_state.async_flip in their atomic logic.

The goal of this negative flag is the same as
fb_modifiers_not_supported: we want to eventually get rid of all
drivers missing atomic support for async flips. New drivers should not
set this flag, instead they should support atomic async flips (if
they support async flips at all). IOW, we don't want more drivers
with async flip support for legacy but not atomic.

Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Reviewed-by: Alex Deucher 
Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c|  1 +
 drivers/gpu/drm/i915/display/intel_display_driver.c |  1 +
 drivers/gpu/drm/nouveau/nouveau_display.c   |  1 +
 include/drm/drm_mode_config.h   | 11 +++
 5 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 45b8fd61a044..dec6e43e7198 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4003,6 +4003,7 @@ static int amdgpu_dm_mode_config_init(struct 
amdgpu_device *adev)
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
/* indicates support for immediate flip */
adev_to_drm(adev)->mode_config.async_page_flip = true;
+   adev_to_drm(adev)->mode_config.atomic_async_page_flip_not_supported = 
true;
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 84c54e8622d1..f1d9bb1d7c34 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -639,6 +639,7 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device 
*dev)
dev->mode_config.max_height = dc->desc->max_height;
dev->mode_config.funcs = &mode_config_funcs;
dev->mode_config.async_page_flip = true;
+   dev->mode_config.atomic_async_page_flip_not_supported = true;
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c 
b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 44b59ac301e6..6142c83fba06 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -126,6 +126,7 @@ static void intel_mode_config_init(struct drm_i915_private 
*i915)
mode_config->helper_private = &intel_mode_config_funcs;
 
mode_config->async_page_flip = HAS_ASYNC_FLIPS(i915);
+   mode_config->atomic_async_page_flip_not_supported = true;
 
/*
 * Maximum framebuffer dimensions, chosen to match
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
b/drivers/gpu/drm/nouveau/nouveau_display.c
index d8c92521226d..586aa51794e8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -720,6 +720,7 @@ nouveau_display_create(struct drm_device *dev)
dev->mode_config.async_page_flip = false;
else
dev->mode_config.async_page_flip = true;
+   dev->mode_config.atomic_async_page_flip_not_supported = true;
 
drm_kms_helper_poll_init(dev);
drm_kms_helper_poll_disable(dev);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 973119a9176b..47b005671e6a 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -918,6 +918,17 @@ struct drm_mode_config {
 */
bool async_page_flip;
 
+   /**
+* @atomic_async_page_flip_not_supported:
+*
+* If true, the driver does not support async page-flips with the
+* atomic uAPI. This is only used by old drivers which haven't yet
+* accomodated for &drm_crtc_state.async_flip in their atomic logic,
+* even if they have &drm_mode_config.async_page_flip set to true.
+* New drivers shall not set this flag.
+*/
+   bool atomic_async_page_flip_not_supported;
+
/**
 * @fb_modifiers_not_supported:
 *
-- 
2.42.0

[PATCH v8 2/6] drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP

[André Almeida]

From: Simon Ser 

This new kernel capability indicates whether async page-flips are
supported via the atomic uAPI. DRM clients can use it to check
for support before feeding DRM_MODE_PAGE_FLIP_ASYNC to the kernel.

Make it clear that DRM_CAP_ASYNC_PAGE_FLIP is for legacy uAPI only.

Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Reviewed-by: Alex Deucher 
Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_ioctl.c |  5 +
 include/uapi/drm/drm.h  | 10 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 77590b0f38fa..a96e7acb9071 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -301,6 +301,11 @@ static int drm_getcap(struct drm_device *dev, void *data, 
struct drm_file *file_
case DRM_CAP_CRTC_IN_VBLANK_EVENT:
req->value = 1;
break;
+   case DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP:
+   req->value = drm_core_check_feature(dev, DRIVER_ATOMIC) &&
+dev->mode_config.async_page_flip &&
+
!dev->mode_config.atomic_async_page_flip_not_supported;
+   break;
default:
return -EINVAL;
}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 794c1d857677..58baefe32c23 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -713,7 +713,8 @@ struct drm_gem_open {
 /**
  * DRM_CAP_ASYNC_PAGE_FLIP
  *
- * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC.
+ * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for legacy
+ * page-flips.
  */
 #define DRM_CAP_ASYNC_PAGE_FLIP0x7
 /**
@@ -773,6 +774,13 @@ struct drm_gem_open {
  * :ref:`drm_sync_objects`.
  */
 #define DRM_CAP_SYNCOBJ_TIMELINE   0x14
+/**
+ * DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
+ *
+ * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for atomic
+ * commits.
+ */
+#define DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP 0x15
 
 /* DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
-- 
2.42.0

[PATCH v8 1/6] drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits

[André Almeida]

From: Simon Ser 

If the driver supports it, allow user-space to supply the
DRM_MODE_PAGE_FLIP_ASYNC flag to request an async page-flip.
Set drm_crtc_state.async_flip accordingly.

Document that drivers will reject atomic commits if an async
flip isn't possible. This allows user-space to fall back to
something else. For instance, Xorg falls back to a blit.
Another option is to wait as close to the next vblank as
possible before performing the page-flip to reduce latency.

Signed-off-by: Simon Ser 
Reviewed-by: Alex Deucher 
Co-developed-by: André Almeida 
Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 28 +---
 include/uapi/drm/drm_mode.h   |  9 +
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 98d3b10c08ae..a15121e75a0a 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1323,6 +1323,18 @@ static void complete_signaling(struct drm_device *dev,
kfree(fence_state);
 }
 
+static void
+set_async_flip(struct drm_atomic_state *state)
+{
+   struct drm_crtc *crtc;
+   struct drm_crtc_state *crtc_state;
+   int i;
+
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+   crtc_state->async_flip = true;
+   }
+}
+
 int drm_mode_atomic_ioctl(struct drm_device *dev,
  void *data, struct drm_file *file_priv)
 {
@@ -1363,9 +1375,16 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
}
 
if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) {
-   drm_dbg_atomic(dev,
-  "commit failed: invalid flag 
DRM_MODE_PAGE_FLIP_ASYNC\n");
-   return -EINVAL;
+   if (!dev->mode_config.async_page_flip) {
+   drm_dbg_atomic(dev,
+  "commit failed: DRM_MODE_PAGE_FLIP_ASYNC 
not supported\n");
+   return -EINVAL;
+   }
+   if (dev->mode_config.atomic_async_page_flip_not_supported) {
+   drm_dbg_atomic(dev,
+  "commit failed: DRM_MODE_PAGE_FLIP_ASYNC 
not supported with atomic\n");
+   return -EINVAL;
+   }
}
 
/* can't test and expect an event at the same time. */
@@ -1468,6 +1487,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
if (ret)
goto out;
 
+   if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC)
+   set_async_flip(state);
+
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
ret = drm_atomic_check_only(state);
} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index ea1b639bcb28..04e6a3caa675 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -957,6 +957,15 @@ struct hdr_output_metadata {
  * Request that the page-flip is performed as soon as possible, ie. with no
  * delay due to waiting for vblank. This may cause tearing to be visible on
  * the screen.
+ *
+ * When used with atomic uAPI, the driver will return an error if the hardware
+ * doesn't support performing an asynchronous page-flip for this update.
+ * User-space should handle this, e.g. by falling back to a regular page-flip.
+ *
+ * Note, some hardware might need to perform one last synchronous page-flip
+ * before being able to switch to asynchronous page-flips. As an exception,
+ * the driver will return success even though that first page-flip is not
+ * asynchronous.
  */
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-- 
2.42.0

[PATCH v8 0/6] drm: Add support for atomic async page-flip

[André Almeida]

Hi,

This work from me and Simon adds support for DRM_MODE_PAGE_FLIP_ASYNC through
the atomic API. This feature is already available via the legacy API. The use
case is to be able to present a new frame immediately (or as soon as
possible), even if after missing a vblank. This might result in tearing, but
it's useful when a high framerate is desired, such as for gaming.

Differently from earlier versions, this one refuses to flip if any prop changes
for async flips. The idea is that the fast path of immediate page flips doesn't
play well with modeset changes, so only the fb_id can be changed.

Thanks,
André

- User-space patch: https://github.com/Plagman/gamescope/pull/595
- IGT tests: 
https://gitlab.freedesktop.org/andrealmeid/igt-gpu-tools/-/tree/atomic_async_page_flip

Changes from v7:
- Only accept flips to primary planes. If a driver support flips in different
planes, support will be added  later.
v7: 
https://lore.kernel.org/dri-devel/20231017092837.32428-1-andrealm...@igalia.com/

Changes from v6:
- Dropped the exception to allow MODE_ID changes (Simon)
- Clarify what happens when flipping with the same FB_ID (Pekka)

v6: 
https://lore.kernel.org/dri-devel/20230815185710.159779-1-andrealm...@igalia.com/

Changes from v5:
- Add note in the docs that not every redundant attribute will result in no-op,
  some might cause oversynchronization issues.

v5: 
https://lore.kernel.org/dri-devel/20230707224059.305474-1-andrealm...@igalia.com/

Changes from v4:
 - Documentation rewrote by Pekka Paalanen

v4: 
https://lore.kernel.org/dri-devel/20230701020917.143394-1-andrealm...@igalia.com/

Changes from v3:
 - Add new patch to reject prop changes
 - Add a documentation clarifying the KMS atomic state set

v3: 
https://lore.kernel.org/dri-devel/20220929184307.258331-1-cont...@emersion.fr/

André Almeida (1):
  drm: Refuse to async flip with atomic prop changes

Pekka Paalanen (1):
  drm/doc: Define KMS atomic state set

Simon Ser (4):
  drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits
  drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
  drm: introduce drm_mode_config.atomic_async_page_flip_not_supported
  amd/display: indicate support for atomic async page-flips on DC

 Documentation/gpu/drm-uapi.rst| 47 +++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |  1 +
 drivers/gpu/drm/drm_atomic_uapi.c | 82 +--
 drivers/gpu/drm/drm_crtc_internal.h   |  2 +-
 drivers/gpu/drm/drm_ioctl.c   |  5 ++
 drivers/gpu/drm/drm_mode_object.c |  2 +-
 .../drm/i915/display/intel_display_driver.c   |  1 +
 drivers/gpu/drm/nouveau/nouveau_display.c |  1 +
 include/drm/drm_mode_config.h | 11 +++
 include/uapi/drm/drm.h| 10 ++-
 include/uapi/drm/drm_mode.h   |  9 ++
 11 files changed, 162 insertions(+), 9 deletions(-)

-- 
2.42.0

[PATCH v7 4/6] drm: Refuse to async flip with atomic prop changes

[André Almeida]

Given that prop changes may lead to modesetting, which would defeat the
fast path of the async flip, refuse any atomic prop change for async
flips in atomic API. The only exceptions are the framebuffer ID to flip
to and the mode ID, that could be referring to an identical mode.

Signed-off-by: André Almeida 
---
v7: drop the mode_id exception for prop changes
---
 drivers/gpu/drm/drm_atomic_uapi.c   | 47 +++--
 drivers/gpu/drm/drm_crtc_internal.h |  2 +-
 drivers/gpu/drm/drm_mode_object.c   |  2 +-
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index a15121e75a0a..b358de1bf4e7 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1006,13 +1006,28 @@ int drm_atomic_connector_commit_dpms(struct 
drm_atomic_state *state,
return ret;
 }
 
+static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,
+struct drm_property *prop)
+{
+   if (ret != 0 || old_val != prop_value) {
+   drm_dbg_atomic(prop->dev,
+  "[PROP:%d:%s] No prop can be changed during 
async flip\n",
+  prop->base.id, prop->name);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 int drm_atomic_set_property(struct drm_atomic_state *state,
struct drm_file *file_priv,
struct drm_mode_object *obj,
struct drm_property *prop,
-   uint64_t prop_value)
+   uint64_t prop_value,
+   bool async_flip)
 {
struct drm_mode_object *ref;
+   uint64_t old_val;
int ret;
 
if (!drm_property_change_valid_get(prop, prop_value, &ref))
@@ -1029,6 +1044,13 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip) {
+   ret = drm_atomic_connector_get_property(connector, 
connector_state,
+   prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
ret = drm_atomic_connector_set_property(connector,
connector_state, file_priv,
prop, prop_value);
@@ -1037,6 +1059,7 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_CRTC: {
struct drm_crtc *crtc = obj_to_crtc(obj);
struct drm_crtc_state *crtc_state;
+   struct drm_mode_config *config = &crtc->dev->mode_config;
 
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state)) {
@@ -1044,6 +1067,13 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip) {
+   ret = drm_atomic_crtc_get_property(crtc, crtc_state,
+  prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
ret = drm_atomic_crtc_set_property(crtc,
crtc_state, prop, prop_value);
break;
@@ -1051,6 +1081,7 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
+   struct drm_mode_config *config = &plane->dev->mode_config;
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
@@ -1058,6 +1089,13 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
break;
}
 
+   if (async_flip && prop != config->prop_fb_id) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, &old_val);
+   ret = drm_atomic_check_prop_changes(ret, old_val, 
prop_value, prop);
+   break;
+   }
+
ret = drm_atomic_plane_set_property(plane,
plane_state, file_priv,
prop, prop_value);
@@ -1349,6 +1387,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
struct drm_out_fence_state *fence_state;
int ret = 0;
unsigned int i, j, num_fences;
+   bool async_flip = false;
 
/* disallow for drivers

[PATCH v7 5/6] drm/doc: Define KMS atomic state set

[André Almeida]

From: Pekka Paalanen 

Specify how the atomic state is maintained between userspace and
kernel, plus the special case for async flips.

Signed-off-by: Pekka Paalanen 
Signed-off-by: André Almeida 
---
v7:
- add a note that drivers can make exceptions for ad-hoc prop changes
- add a note about flipping the same FB_ID as a no-op
---
 Documentation/gpu/drm-uapi.rst | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 632989df3727..34bd02270ee7 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -570,3 +570,50 @@ dma-buf interoperability
 
 Please see Documentation/userspace-api/dma-buf-alloc-exchange.rst for
 information on how dma-buf is integrated and exposed within DRM.
+
+KMS atomic state
+
+
+An atomic commit can change multiple KMS properties in an atomic fashion,
+without ever applying intermediate or partial state changes.  Either the whole
+commit succeeds or fails, and it will never be applied partially. This is the
+fundamental improvement of the atomic API over the older non-atomic API which 
is
+referred to as the "legacy API".  Applying intermediate state could 
unexpectedly
+fail, cause visible glitches, or delay reaching the final state.
+
+An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the
+complete state change is validated but not applied.  Userspace should use this
+flag to validate any state change before asking to apply it. If validation 
fails
+for any reason, userspace should attempt to fall back to another, perhaps
+simpler, final state.  This allows userspace to probe for various 
configurations
+without causing visible glitches on screen and without the need to undo a
+probing change.
+
+The changes recorded in an atomic commit apply on top the current KMS state in
+the kernel. Hence, the complete new KMS state is the complete old KMS state 
with
+the committed property settings done on top. The kernel will try to avoid
+no-operation changes, so it is safe for userspace to send redundant property
+settings.  However, not every situation allows for no-op changes, due to the
+need to acquire locks for some attributes. Userspace needs to be aware that 
some
+redundant information might result in oversynchronization issues.  No-operation
+changes do not count towards actually needed changes, e.g.  setting MODE_ID to 
a
+different blob with identical contents as the current KMS state shall not be a
+modeset on its own. As a special exception for VRR needs, explicitly setting
+FB_ID to its current value is not a no-op.
+
+A "modeset" is a change in KMS state that might enable, disable, or temporarily
+disrupt the emitted video signal, possibly causing visible glitches on screen. 
A
+modeset may also take considerably more time to complete than other kinds of
+changes, and the video sink might also need time to adapt to the new signal
+properties. Therefore a modeset must be explicitly allowed with the flag
+DRM_MODE_ATOMIC_ALLOW_MODESET.  This in combination with
+DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is
+likely to cause visible disruption on screen and avoid such changes when end
+users do not expect them.
+
+An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to
+effectively change only the FB_ID property on any planes. No-operation changes
+are ignored as always. Changing any other property will cause the commit to be
+rejected. Each driver may relax this restriction if they have guarantees that
+such property change doesn't cause modesets. Userspace can use TEST_ONLY 
commits
+to query the driver about this.
-- 
2.42.0

[PATCH v7 6/6] amd/display: indicate support for atomic async page-flips on DC

[André Almeida]

From: Simon Ser 

amdgpu_dm_commit_planes() already sets the flip_immediate flag for
async page-flips. This flag is used to set the UNP_FLIP_CONTROL
register. Thus, no additional change is required to handle async
page-flips with the atomic uAPI.

Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Reviewed-by: Alex Deucher 
Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9d5742923aed..c6fd34bab358 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3997,7 +3997,6 @@ static int amdgpu_dm_mode_config_init(struct 
amdgpu_device *adev)
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
/* indicates support for immediate flip */
adev_to_drm(adev)->mode_config.async_page_flip = true;
-   adev_to_drm(adev)->mode_config.atomic_async_page_flip_not_supported = 
true;
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
-- 
2.42.0

[PATCH v7 2/6] drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP

[André Almeida]

From: Simon Ser 

This new kernel capability indicates whether async page-flips are
supported via the atomic uAPI. DRM clients can use it to check
for support before feeding DRM_MODE_PAGE_FLIP_ASYNC to the kernel.

Make it clear that DRM_CAP_ASYNC_PAGE_FLIP is for legacy uAPI only.

Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Reviewed-by: Alex Deucher 
Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_ioctl.c |  5 +
 include/uapi/drm/drm.h  | 10 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 77590b0f38fa..a96e7acb9071 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -301,6 +301,11 @@ static int drm_getcap(struct drm_device *dev, void *data, 
struct drm_file *file_
case DRM_CAP_CRTC_IN_VBLANK_EVENT:
req->value = 1;
break;
+   case DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP:
+   req->value = drm_core_check_feature(dev, DRIVER_ATOMIC) &&
+dev->mode_config.async_page_flip &&
+
!dev->mode_config.atomic_async_page_flip_not_supported;
+   break;
default:
return -EINVAL;
}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 794c1d857677..58baefe32c23 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -713,7 +713,8 @@ struct drm_gem_open {
 /**
  * DRM_CAP_ASYNC_PAGE_FLIP
  *
- * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC.
+ * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for legacy
+ * page-flips.
  */
 #define DRM_CAP_ASYNC_PAGE_FLIP0x7
 /**
@@ -773,6 +774,13 @@ struct drm_gem_open {
  * :ref:`drm_sync_objects`.
  */
 #define DRM_CAP_SYNCOBJ_TIMELINE   0x14
+/**
+ * DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
+ *
+ * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for atomic
+ * commits.
+ */
+#define DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP 0x15
 
 /* DRM_IOCTL_GET_CAP ioctl argument type */
 struct drm_get_cap {
-- 
2.42.0

[PATCH v7 3/6] drm: introduce drm_mode_config.atomic_async_page_flip_not_supported

[André Almeida]

From: Simon Ser 

This new field indicates whether the driver has the necessary logic
to support async page-flips via the atomic uAPI. This is leveraged by
the next commit to allow user-space to use this functionality.

All atomic drivers setting drm_mode_config.async_page_flip are updated
to also set drm_mode_config.atomic_async_page_flip_not_supported. We
will gradually check and update these drivers to properly handle
drm_crtc_state.async_flip in their atomic logic.

The goal of this negative flag is the same as
fb_modifiers_not_supported: we want to eventually get rid of all
drivers missing atomic support for async flips. New drivers should not
set this flag, instead they should support atomic async flips (if
they support async flips at all). IOW, we don't want more drivers
with async flip support for legacy but not atomic.

Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Reviewed-by: Alex Deucher 
Signed-off-by: André Almeida 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c|  1 +
 drivers/gpu/drm/i915/display/intel_display_driver.c |  1 +
 drivers/gpu/drm/nouveau/nouveau_display.c   |  1 +
 include/drm/drm_mode_config.h   | 11 +++
 5 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c6fd34bab358..9d5742923aed 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3997,6 +3997,7 @@ static int amdgpu_dm_mode_config_init(struct 
amdgpu_device *adev)
adev_to_drm(adev)->mode_config.prefer_shadow = 1;
/* indicates support for immediate flip */
adev_to_drm(adev)->mode_config.async_page_flip = true;
+   adev_to_drm(adev)->mode_config.atomic_async_page_flip_not_supported = 
true;
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 84c54e8622d1..f1d9bb1d7c34 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -639,6 +639,7 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device 
*dev)
dev->mode_config.max_height = dc->desc->max_height;
dev->mode_config.funcs = &mode_config_funcs;
dev->mode_config.async_page_flip = true;
+   dev->mode_config.atomic_async_page_flip_not_supported = true;
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c 
b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 44b59ac301e6..6142c83fba06 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -126,6 +126,7 @@ static void intel_mode_config_init(struct drm_i915_private 
*i915)
mode_config->helper_private = &intel_mode_config_funcs;
 
mode_config->async_page_flip = HAS_ASYNC_FLIPS(i915);
+   mode_config->atomic_async_page_flip_not_supported = true;
 
/*
 * Maximum framebuffer dimensions, chosen to match
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
b/drivers/gpu/drm/nouveau/nouveau_display.c
index d8c92521226d..586aa51794e8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -720,6 +720,7 @@ nouveau_display_create(struct drm_device *dev)
dev->mode_config.async_page_flip = false;
else
dev->mode_config.async_page_flip = true;
+   dev->mode_config.atomic_async_page_flip_not_supported = true;
 
drm_kms_helper_poll_init(dev);
drm_kms_helper_poll_disable(dev);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 973119a9176b..47b005671e6a 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -918,6 +918,17 @@ struct drm_mode_config {
 */
bool async_page_flip;
 
+   /**
+* @atomic_async_page_flip_not_supported:
+*
+* If true, the driver does not support async page-flips with the
+* atomic uAPI. This is only used by old drivers which haven't yet
+* accomodated for &drm_crtc_state.async_flip in their atomic logic,
+* even if they have &drm_mode_config.async_page_flip set to true.
+* New drivers shall not set this flag.
+*/
+   bool atomic_async_page_flip_not_supported;
+
/**
 * @fb_modifiers_not_supported:
 *
-- 
2.42.0

[PATCH v7 0/6] drm: Add support for atomic async page-flip

[André Almeida]

Hi,

This work from me and Simon adds support for DRM_MODE_PAGE_FLIP_ASYNC through
the atomic API. This feature is already available via the legacy API. The use
case is to be able to present a new frame immediately (or as soon as
possible), even if after missing a vblank. This might result in tearing, but
it's useful when a high framerate is desired, such as for gaming.

Differently from earlier versions, this one refuses to flip if any prop changes
for async flips. The idea is that the fast path of immediate page flips doesn't
play well with modeset changes, so only the fb_id can be changed.
Thanks,
André

- User-space patch: https://github.com/Plagman/gamescope/pull/595
- IGT tests: 
https://gitlab.freedesktop.org/andrealmeid/igt-gpu-tools/-/tree/atomic_async_page_flip

Changes from v6:
- Dropped the exception to allow MODE_ID changes (Simon)
- Clarify what happens when flipping with the same FB_ID (Pekka)

v6: 
https://lore.kernel.org/dri-devel/20230815185710.159779-1-andrealm...@igalia.com/

Changes from v5:
- Add note in the docs that not every redundant attribute will result in no-op,
  some might cause oversynchronization issues.

v5: 
https://lore.kernel.org/dri-devel/20230707224059.305474-1-andrealm...@igalia.com/

Changes from v4:
 - Documentation rewrote by Pekka Paalanen

v4: 
https://lore.kernel.org/dri-devel/20230701020917.143394-1-andrealm...@igalia.com/

Changes from v3:
 - Add new patch to reject prop changes
 - Add a documentation clarifying the KMS atomic state set

v3: 
https://lore.kernel.org/dri-devel/20220929184307.258331-1-cont...@emersion.fr/

André Almeida (1):
  drm: Refuse to async flip with atomic prop changes

Pekka Paalanen (1):
  drm/doc: Define KMS atomic state set

Simon Ser (4):
  drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits
  drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
  drm: introduce drm_mode_config.atomic_async_page_flip_not_supported
  amd/display: indicate support for atomic async page-flips on DC

 Documentation/gpu/drm-uapi.rst| 47 
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |  1 +
 drivers/gpu/drm/drm_atomic_uapi.c | 75 +--
 drivers/gpu/drm/drm_crtc_internal.h   |  2 +-
 drivers/gpu/drm/drm_ioctl.c   |  5 ++
 drivers/gpu/drm/drm_mode_object.c |  2 +-
 .../drm/i915/display/intel_display_driver.c   |  1 +
 drivers/gpu/drm/nouveau/nouveau_display.c |  1 +
 include/drm/drm_mode_config.h | 11 +++
 include/uapi/drm/drm.h| 10 ++-
 include/uapi/drm/drm_mode.h   |  9 +++
 11 files changed, 155 insertions(+), 9 deletions(-)

-- 
2.42.0

[PATCH v7 1/6] drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits

[André Almeida]

From: Simon Ser 

If the driver supports it, allow user-space to supply the
DRM_MODE_PAGE_FLIP_ASYNC flag to request an async page-flip.
Set drm_crtc_state.async_flip accordingly.

Document that drivers will reject atomic commits if an async
flip isn't possible. This allows user-space to fall back to
something else. For instance, Xorg falls back to a blit.
Another option is to wait as close to the next vblank as
possible before performing the page-flip to reduce latency.

Signed-off-by: Simon Ser 
Reviewed-by: Alex Deucher 
Co-developed-by: André Almeida 
Signed-off-by: André Almeida 
---
 drivers/gpu/drm/drm_atomic_uapi.c | 28 +---
 include/uapi/drm/drm_mode.h   |  9 +
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 98d3b10c08ae..a15121e75a0a 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1323,6 +1323,18 @@ static void complete_signaling(struct drm_device *dev,
kfree(fence_state);
 }
 
+static void
+set_async_flip(struct drm_atomic_state *state)
+{
+   struct drm_crtc *crtc;
+   struct drm_crtc_state *crtc_state;
+   int i;
+
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+   crtc_state->async_flip = true;
+   }
+}
+
 int drm_mode_atomic_ioctl(struct drm_device *dev,
  void *data, struct drm_file *file_priv)
 {
@@ -1363,9 +1375,16 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
}
 
if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) {
-   drm_dbg_atomic(dev,
-  "commit failed: invalid flag 
DRM_MODE_PAGE_FLIP_ASYNC\n");
-   return -EINVAL;
+   if (!dev->mode_config.async_page_flip) {
+   drm_dbg_atomic(dev,
+  "commit failed: DRM_MODE_PAGE_FLIP_ASYNC 
not supported\n");
+   return -EINVAL;
+   }
+   if (dev->mode_config.atomic_async_page_flip_not_supported) {
+   drm_dbg_atomic(dev,
+  "commit failed: DRM_MODE_PAGE_FLIP_ASYNC 
not supported with atomic\n");
+   return -EINVAL;
+   }
}
 
/* can't test and expect an event at the same time. */
@@ -1468,6 +1487,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
if (ret)
goto out;
 
+   if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC)
+   set_async_flip(state);
+
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
ret = drm_atomic_check_only(state);
} else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index ea1b639bcb28..04e6a3caa675 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -957,6 +957,15 @@ struct hdr_output_metadata {
  * Request that the page-flip is performed as soon as possible, ie. with no
  * delay due to waiting for vblank. This may cause tearing to be visible on
  * the screen.
+ *
+ * When used with atomic uAPI, the driver will return an error if the hardware
+ * doesn't support performing an asynchronous page-flip for this update.
+ * User-space should handle this, e.g. by falling back to a regular page-flip.
+ *
+ * Note, some hardware might need to perform one last synchronous page-flip
+ * before being able to switch to asynchronous page-flips. As an exception,
+ * the driver will return success even though that first page-flip is not
+ * asynchronous.
  */
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
-- 
2.42.0

Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set

[André Almeida]

On 10/16/23 16:52, Pekka Paalanen wrote:

On Mon, 16 Oct 2023 15:42:16 +0200
André Almeida  wrote:


Hi Pekka,

On 10/16/23 14:18, Pekka Paalanen wrote:

On Mon, 16 Oct 2023 12:52:32 +0200
André Almeida  wrote:
  

Hi Michel,

On 8/17/23 12:37, Michel Dänzer wrote:

On 8/15/23 20:57, André Almeida wrote:

From: Pekka Paalanen 

Specify how the atomic state is maintained between userspace and
kernel, plus the special case for async flips.

Signed-off-by: Pekka Paalanen 
Signed-off-by: André Almeida 

[...]
 

+An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to
+effectively change only the FB_ID property on any planes. No-operation changes
+are ignored as always. [...]

During the hackfest in Brno, it was mentioned that a commit which re-sets the 
same FB_ID could actually have an effect with VRR: It could trigger scanout of 
the next frame before vertical blank has reached its maximum duration. Some 
kind of mechanism is required for this in order to allow user space to perform 
low frame rate compensation.
 

Xaver tested this hypothesis in a flipping the same fb in a VRR monitor
and it worked as expected, so this shouldn't be a concern.

Right, so it must have some effect. It cannot be simply ignored like in
the proposed doc wording. Do we special-case re-setting the same FB_ID
as "not a no-op" or "not ignored" or some other way?

There's an effect in the refresh rate, the image won't change but it
will report that a flip had happened asynchronously so the reported
framerate will be increased. Maybe an additional wording could be like:

Flipping to the same FB_ID will result in a immediate flip as if it was
changing to a different one, with no effect on the image but effecting
the reported frame rate.

Re-setting FB_ID to its current value is a special case regardless of
PAGE_FLIP_ASYNC, is it not?

So it should be called out somewhere that applies regardless of
PAGE_FLIP_ASYNC. Maybe to the end of the earlier paragraph:


+The changes recorded in an atomic commit apply on top the current KMS state in
+the kernel. Hence, the complete new KMS state is the complete old KMS state 
with
+the committed property settings done on top. The kernel will try to avoid
+no-operation changes, so it is safe for userspace to send redundant property
+settings.  However, not every situation allows for no-op changes, due to the
+need to acquire locks for some attributes. Userspace needs to be aware that 
some
+redundant information might result in oversynchronization issues.  No-operation
+changes do not count towards actually needed changes, e.g.  setting MODE_ID to 
a
+different blob with identical contents as the current KMS state shall not be a
+modeset on its own.

+As a special exception for VRR needs, explicitly setting FB_ID to its
+current value is not a no-op.

Would that work?


I liked this suggestion, thanks! I'll wrap up a v7


I'd like to try to avoid being more specific about what it does
exactly, because that's not the topic here. Such things can be
documented with the property itself. This is a summary of what is or is
not a no-op or a modeset.


Thanks,
pq

Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set

[André Almeida]

Hi Pekka,

On 10/16/23 14:18, Pekka Paalanen wrote:

On Mon, 16 Oct 2023 12:52:32 +0200
André Almeida  wrote:


Hi Michel,

On 8/17/23 12:37, Michel Dänzer wrote:

On 8/15/23 20:57, André Almeida wrote:

From: Pekka Paalanen 

Specify how the atomic state is maintained between userspace and
kernel, plus the special case for async flips.

Signed-off-by: Pekka Paalanen 
Signed-off-by: André Almeida 

[...]
  

+An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to
+effectively change only the FB_ID property on any planes. No-operation changes
+are ignored as always. [...]

During the hackfest in Brno, it was mentioned that a commit which re-sets the 
same FB_ID could actually have an effect with VRR: It could trigger scanout of 
the next frame before vertical blank has reached its maximum duration. Some 
kind of mechanism is required for this in order to allow user space to perform 
low frame rate compensation.
  

Xaver tested this hypothesis in a flipping the same fb in a VRR monitor
and it worked as expected, so this shouldn't be a concern.

Right, so it must have some effect. It cannot be simply ignored like in
the proposed doc wording. Do we special-case re-setting the same FB_ID
as "not a no-op" or "not ignored" or some other way?
There's an effect in the refresh rate, the image won't change but it 
will report that a flip had happened asynchronously so the reported 
framerate will be increased. Maybe an additional wording could be like:


Flipping to the same FB_ID will result in a immediate flip as if it was 
changing to a different one, with no effect on the image but effecting 
the reported frame rate.





Thanks,
pq

Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set

[André Almeida]

Hi Michel,

On 8/17/23 12:37, Michel Dänzer wrote:

On 8/15/23 20:57, André Almeida wrote:

From: Pekka Paalanen 

Specify how the atomic state is maintained between userspace and
kernel, plus the special case for async flips.

Signed-off-by: Pekka Paalanen 
Signed-off-by: André Almeida 

[...]


+An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to
+effectively change only the FB_ID property on any planes. No-operation changes
+are ignored as always. [...]

During the hackfest in Brno, it was mentioned that a commit which re-sets the 
same FB_ID could actually have an effect with VRR: It could trigger scanout of 
the next frame before vertical blank has reached its maximum duration. Some 
kind of mechanism is required for this in order to allow user space to perform 
low frame rate compensation.

Xaver tested this hypothesis in a flipping the same fb in a VRR monitor 
and it worked as expected, so this shouldn't be a concern.


Thanks,
    André