2024-06-12 19:55:56

by André Almeida

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

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/[email protected]/

Changes from v2:
- Allow IN_FENCE_ID for any driver
- Allow overlay planes again
v2: https://lore.kernel.org/lkml/[email protected]/

Changes from v1:
- Drop overlay planes option for now
v1: https://lore.kernel.org/dri-devel/[email protected]/

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



2024-06-12 19:56:20

by André Almeida

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

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 <[email protected]>
---
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


2024-06-12 20:05:49

by André Almeida

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

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 <[email protected]>
---
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


2024-06-12 20:45:10

by Dmitry Baryshkov

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

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 <[email protected]>
> ---
> 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.

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


--
With best wishes
Dmitry

2024-06-13 16:00:03

by André Almeida

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

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 <[email protected]>
>> ---
>> 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;
>
>