Re: [PATCH v7 2/9] drm: Support per-plane async flip configuration
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
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
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
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
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
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
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
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 3/9] drm/amdgpu: Enable async flips on the primary plane
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 2/9] drm: Support per-plane async flip configuration
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 1/9] 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
---
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
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
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
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 8/8] drm/amdgpu: Make it possible to async flip overlay planes
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 1/8] 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
---
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 7/8] drm/vc4: Enable async flips on the primary plane
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
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 5/8] drm/i915: Enable async flips on the primary plane
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 4/8] drm: atmel-hlcdc: Enable async flips on the primary plane
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 3/8] drm/amdgpu: Enable async flips on the primary plane
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 0/8] drm: Support per-plane async flip configuration
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
[PATCH v6 2/8] drm: Support per-plane async flip configuration
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
Re: [Intel-gfx] [PATCH v9 0/4] drm: Add support for atomic async page-flip
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: [Intel-gfx] [PATCH v9 0/4] drm: Add support for atomic async page-flip
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é
[Intel-gfx] [PATCH v9 4/4] drm/doc: Define KMS atomic state set
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
[Intel-gfx] [PATCH v9 3/4] drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
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
[Intel-gfx] [PATCH v9 2/4] drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits
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
[Intel-gfx] [PATCH v9 1/4] drm: Refuse to async flip with atomic prop changes
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,
[Intel-gfx] [PATCH v9 0/4] drm: Add support for atomic async page-flip
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
Re: [Intel-gfx] [PATCH v2 2/2] drm: Replace drm_framebuffer plane size functions with its equivalents
On 9/26/23 16:15, Carlos Eduardo Gallo Filho wrote:
The functions drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} do exactly the same job of its
equivalents drm_format_info_plane_{width,height} from drm_fourcc.
The only reason to have these functions on drm_framebuffer
would be if they would added a abstraction layer to call it just
passing a drm_framebuffer pointer and the desired plane index,
which is not the case, where these functions actually implements
just part of it. In the actual implementation, every call to both
drm_framebuffer_plane_{width,height} and fb_plane_{width,height} should
pass some drm_framebuffer attribute, which is the same as calling the
drm_format_info_plane_{width,height} functions.
The drm_format_info_pane_{width,height} functions are much more
consistent in both its implementation and its location on code. The
kind of calculation that they do is intrinsically derivated from the
drm_format_info struct and has not to do with drm_framebuffer, except
by the potential motivation described above, which is still not a good
justification to have drm_framebuffer functions to calculate it.
So, replace each drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} call to drm_format_info_plane_{width,height}
and remove them.
Signed-off-by: Carlos Eduardo Gallo Filho
Reviewed-by: André Almeida
Re: [Intel-gfx] [PATCH v2 1/2] drm: Remove plane hsub/vsub alignment requirement for core helpers
Hi Carlos,
On 9/26/23 16:15, Carlos Eduardo Gallo Filho wrote:
The drm_format_info_plane_{height,width} functions was implemented using
regular division for the plane size calculation, which cause issues [1][2]
when used on contexts where the dimensions are misaligned with relation
to the subsampling factors. So, replace the regular division by the
DIV_ROUND_UP macro.
This allows these functions to be used in more drivers, making further
work to bring more core presence on them possible.
[1]
http://patchwork.freedesktop.org/patch/msgid/20170321181218.10042-3-ville.syrj...@linux.intel.com
[2]
https://patchwork.freedesktop.org/patch/msgid/20211026225105.2783797-2-imre.d...@intel.com
Prefer to use lore:
https://lore.kernel.org/dri-devel/20170321181218.10042-3-ville.syrj...@linux.intel.com/
https://lore.kernel.org/intel-gfx/20211026225105.2783797-2-imre.d...@intel.com/
Other than that,
Reviewed-by: André Almeida
Signed-off-by: Carlos Eduardo Gallo Filho
---
include/drm/drm_fourcc.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 532ae78ca747..ccf91daa4307 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -22,6 +22,7 @@
#ifndef __DRM_FOURCC_H__
#define __DRM_FOURCC_H__
+#include
#include
#include
@@ -279,7 +280,7 @@ int drm_format_info_plane_width(const struct drm_format_info *info, int width,
if (plane == 0)
return width;
- return width / info->hsub;
+ return DIV_ROUND_UP(width, info->hsub);
}
/**
@@ -301,7 +302,7 @@ int drm_format_info_plane_height(const struct
drm_format_info *info, int height,
if (plane == 0)
return height;
- return height / info->vsub;
+ return DIV_ROUND_UP(height, info->vsub);
}
const struct drm_format_info *__drm_format_info(u32 format);
Re: [Intel-gfx] [PATCH] drm: Replace drm_framebuffer plane size functions with its equivalents
Hi Carlos,
Em 27/06/2023 15:22, Carlos Eduardo Gallo Filho escreveu:
[...]
So, replace each drm_framebuffer_plane_{width,height} and
fb_plane_{width,height} call to drm_format_info_plane_{width,height}
and remove them.
I see that with this replace, there's a small code change from
return DIV_ROUND_UP(width, format->hsub);
to
return width / info->hsub;
is there any case that the replaced function will give different results?
Re: [Intel-gfx] [PATCH v2] drm/ttm: Fix dummy res NULL ptr deref bug
Hi Arunpravin, Às 02:30 de 27/07/22, Arunpravin Paneer Selvam escreveu: > Check the bo->resource value before accessing the resource > mem_type. > > v2: Fix commit description unwrapped warning > > > [ 40.191227][ T184] general protection fault, probably for non-canonical > address 0xdc02: [#1] SMP KASAN PTI > [ 40.192995][ T184] KASAN: null-ptr-deref in range > [0x0010-0x0017] > [ 40.194411][ T184] CPU: 1 PID: 184 Comm: systemd-udevd Not tainted > 5.19.0-rc4-00721-gb297c22b7070 #1 > [ 40.196063][ T184] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS 1.16.0-debian-1.16.0-4 04/01/2014 > [ 40.199605][ T184] RIP: 0010:ttm_bo_validate+0x1b3/0x240 [ttm] > [ 40.200754][ T184] Code: e8 72 c5 ff ff 83 f8 b8 74 d4 85 c0 75 54 49 8b > 9e 58 01 00 00 48 b8 00 00 00 00 00 fc ff df 48 8d 7b 10 48 89 fa 48 c1 ea 03 > <0f> b6 04 02 84 c0 74 04 3c 03 7e 44 8b 53 10 31 c0 85 d2 0f 85 58 > [ 40.203685][ T184] RSP: 0018:c96df0c8 EFLAGS: 00010202 > [ 40.204630][ T184] RAX: dc00 RBX: RCX: > 11102f4bb71b > [ 40.205864][ T184] RDX: 0002 RSI: c96df208 RDI: > 0010 > [ 40.207102][ T184] RBP: 192dbe1a R08: c96df208 R09: > > [ 40.208394][ T184] R10: 88817a5f R11: 0001 R12: > c96df110 > [ 40.209692][ T184] R13: c96df0f0 R14: 88817a5db800 R15: > c96df208 > [ 40.210862][ T184] FS: 7f6b1d16e8c0() GS:88839d70() > knlGS: > [ 40.212250][ T184] CS: 0010 DS: ES: CR0: 80050033 > [ 40.213275][ T184] CR2: 55a1001d4ff0 CR3: 0001700f4000 CR4: > 06e0 > [ 40.214469][ T184] Call Trace: > [ 40.214974][ T184] > [ 40.215438][ T184] ? ttm_bo_bounce_temp_buffer+0x140/0x140 [ttm] > [ 40.216572][ T184] ? mutex_spin_on_owner+0x240/0x240 > [ 40.217456][ T184] ? drm_vma_offset_add+0xaa/0x100 [drm] > [ 40.218457][ T184] ttm_bo_init_reserved+0x3d6/0x540 [ttm] > [ 40.219410][ T184] ? shmem_get_inode+0x744/0x980 > [ 40.220231][ T184] ttm_bo_init_validate+0xb1/0x200 [ttm] > [ 40.221172][ T184] ? bo_driver_evict_flags+0x340/0x340 [drm_vram_helper] > [ 40.222530][ T184] ? ttm_bo_init_reserved+0x540/0x540 [ttm] > [ 40.223643][ T184] ? __do_sys_finit_module+0x11a/0x1c0 > [ 40.224654][ T184] ? __shmem_file_setup+0x102/0x280 > [ 40.234764][ T184] drm_gem_vram_create+0x305/0x480 [drm_vram_helper] > [ 40.235766][ T184] ? bo_driver_evict_flags+0x340/0x340 [drm_vram_helper] > [ 40.236846][ T184] ? __kasan_slab_free+0x108/0x180 > [ 40.237650][ T184] drm_gem_vram_fill_create_dumb+0x134/0x340 > [drm_vram_helper] > [ 40.238864][ T184] ? local_pci_probe+0xdf/0x180 > [ 40.239674][ T184] ? drmm_vram_helper_init+0x400/0x400 [drm_vram_helper] > [ 40.240826][ T184] drm_client_framebuffer_create+0x19c/0x400 [drm] > [ 40.241955][ T184] ? drm_client_buffer_delete+0x200/0x200 [drm] > [ 40.243001][ T184] ? drm_client_pick_crtcs+0x554/0xb80 [drm] > [ 40.244030][ T184] drm_fb_helper_generic_probe+0x23f/0x940 > [drm_kms_helper] > [ 40.245226][ T184] ? __cond_resched+0x1c/0xc0 > [ 40.245987][ T184] ? drm_fb_helper_memory_range_to_clip+0x180/0x180 > [drm_kms_helper] > [ 40.247316][ T184] ? mutex_unlock+0x80/0x100 > [ 40.248005][ T184] ? __mutex_unlock_slowpath+0x2c0/0x2c0 > [ 40.249083][ T184] drm_fb_helper_single_fb_probe+0x907/0xf00 > [drm_kms_helper] > [ 40.250314][ T184] ? drm_fb_helper_check_var+0x1180/0x1180 > [drm_kms_helper] > [ 40.251540][ T184] ? __cond_resched+0x1c/0xc0 > [ 40.252321][ T184] ? mutex_lock+0x9f/0x100 > [ 40.253062][ T184] __drm_fb_helper_initial_config_and_unlock+0xb9/0x2c0 > [drm_kms_helper] > [ 40.254394][ T184] drm_fbdev_client_hotplug+0x56f/0x840 [drm_kms_helper] > [ 40.255477][ T184] drm_fbdev_generic_setup+0x165/0x3c0 [drm_kms_helper] > [ 40.256607][ T184] bochs_pci_probe+0x6b7/0x900 [bochs] > [ 40.257515][ T184] ? _raw_spin_lock_irqsave+0x87/0x100 > [ 40.258312][ T184] ? bochs_hw_init+0x480/0x480 [bochs] > [ 40.259244][ T184] ? bochs_hw_init+0x480/0x480 [bochs] > [ 40.260186][ T184] local_pci_probe+0xdf/0x180 > [ 40.260928][ T184] pci_call_probe+0x15f/0x500 > [ 40.265798][ T184] ? _raw_spin_lock+0x81/0x100 > [ 40.266508][ T184] ? pci_pm_suspend_noirq+0x980/0x980 > [ 40.267322][ T184] ? pci_assign_irq+0x81/0x280 > [ 40.268096][ T184] ? pci_match_device+0x351/0x6c0 > [ 40.268883][ T184] ? kernfs_put+0x18/0x40 > [ 40.269611][ T184] pci_device_probe+0xee/0x240 > [ 40.270352][ T184] really_probe+0x435/0xa80 > [ 40.271021][ T184] __driver_probe_device+0x2ab/0x480 > [ 40.271828][ T184] driver_probe_device+0x49/0x140 > [ 40.272627][ T184] __driver_attach+0x1bd/0x4c0 > [ 40.273372][ T184] ? __device_attach_driver+0x240/0x240 > [
Re: [Intel-gfx] Enabling sample_c optimization for Broadwell GPUs
Hi Rodrigo, Thank you very much for providing that information in a precise manner. Às 07:16 de 05/05/21, Rodrigo Vivi escreveu: Hi Andre, I'm not familiar with the sample c message optimization. Probably Ken can comment. However I could check the internal spec here and I saw this bit only exists with this meaning in Haswell. For all the other platforms, including Broadwell it got re-purposed with a different meaning and a programming note: "This bit should be programmed to zero (0h) at all times." Also, I could not find any workaround documented anywhere recommending this bit to be set. So, I would not recommend to use it in any product, even downstream. Regardless the state of sample c message optimization in later platforms. Thanks, Rodrigo. On Tue, May 04, 2021 at 08:07:14PM -0300, André Almeida wrote: Hi there, While browsing an old downstream kernel, I found a patch[0] that enables sample_c optimizations at Broadwell GPUs. The message from the upstream commit that enables it for Haswell[1] (and presumably where the code at[0] was copied from) states that "[..] later platforms remove this bit, and apparently always enable the optimization". Could you confirm that Broadwell and following architectures enable this optimization by default (and thus, patch[0] is a no-op), or should I upstream it? Thanks, André [0] https://github.com/ValveSoftware/steamos_kernel/commit/198990f13e1d9429864c177d9441a6559771c5e2 [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=944115934436b1ff6cf773a9e9123858ea9ef3da ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
