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. The exception
is for mode_id changes, that might be referring to an identical mode (which
would skip a modeset). This is done to make the async API more similar to the
normal API.
Thanks,
André
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/[email protected]/
- 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
André Almeida (2):
drm: Refuse to async flip with atomic prop changes
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 | 19 +++++
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 1 +
drivers/gpu/drm/drm_atomic_helper.c | 5 ++
drivers/gpu/drm/drm_atomic_uapi.c | 80 ++++++++++++++++++--
drivers/gpu/drm/drm_crtc_internal.h | 2 +-
drivers/gpu/drm/drm_ioctl.c | 5 ++
drivers/gpu/drm/drm_mode_object.c | 2 +-
drivers/gpu/drm/i915/display/intel_display.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 +++
12 files changed, 137 insertions(+), 9 deletions(-)
--
2.41.0
From: Simon Ser <[email protected]>
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 <[email protected]>
Reviewed-by: André Almeida <[email protected]>
Reviewed-by: Alex Deucher <[email protected]>
Signed-off-by: André Almeida <[email protected]>
---
v4: no changes
---
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 258461826140..7acd73e5004f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3970,7 +3970,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.41.0
Specify how the atomic state is maintained between userspace and
kernel, plus the special case for async flips.
Signed-off-by: André Almeida <[email protected]>
---
v4: new patch
---
Documentation/gpu/drm-uapi.rst | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 65fb3036a580..5464376051cc 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -486,3 +486,22 @@ and the CRTC index is its position in this array.
.. kernel-doc:: include/uapi/drm/drm_mode.h
:internal:
+
+KMS atomic state
+================
+
+If a userspace using the DRM atomic API would like to change the modeset, it
+needs to do in an atomic way, changing all desired properties in a single
+commit. Following commits may contain the same properties again, as if they were
+new. The kernel can then judge if those properties requires modesetting and real
+changes, or it's just the same state as before. In summary, userspace commits do
+not need to set the minimal state as possible and can commit redundant
+information, and the kernel will ignore it.
+
+An observation must be made for atomic operations with DRM_MODE_PAGE_FLIP_ASYNC.
+In such scenarios properties values can be sent, but the if they change
+something, the kernel will reject the flip. This is done because property
+changes can lead to modesetting, that would defeat the goal of flipping as fast
+as possible. The only exceptions are the framebuffer ID to be flipped to and
+mode IDs changes, which could be referring to an identical mode, thus not
+requiring modeset.
--
2.41.0
On Fri, 30 Jun 2023 23:09:17 -0300
André Almeida <[email protected]> wrote:
> Specify how the atomic state is maintained between userspace and
> kernel, plus the special case for async flips.
>
> Signed-off-by: André Almeida <[email protected]>
> ---
> v4: new patch
> ---
> Documentation/gpu/drm-uapi.rst | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 65fb3036a580..5464376051cc 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -486,3 +486,22 @@ and the CRTC index is its position in this array.
>
> .. kernel-doc:: include/uapi/drm/drm_mode.h
> :internal:
> +
> +KMS atomic state
> +================
> +
> +If a userspace using the DRM atomic API would like to change the modeset, it
s/modeset/video mode/
> +needs to do in an atomic way, changing all desired properties in a single
> +commit.
This applies any and all state changes, not only video modes. Plane
configuration is a good example, where video mode does not change.
> Following commits may contain the same properties again, as if they were
> +new. The kernel can then judge if those properties requires modesetting and real
> +changes, or it's just the same state as before. In summary, userspace commits do
> +not need to set the minimal state as possible and can commit redundant
> +information, and the kernel will ignore it.
Maybe the whole paragraph should be more like... ahem, looks I got a
bit carried away in writing this. Please, take what's useful, I'm not
sure if any of this has been actually documented before:
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 automatically avoid no-operation changes, so it is safe and
even expected for userspace to send redundant property settings.
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.
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 observation must be made for atomic operations with DRM_MODE_PAGE_FLIP_ASYNC.
> +In such scenarios properties values can be sent, but the if they change
> +something, the kernel will reject the flip. This is done because property
> +changes can lead to modesetting, that would defeat the goal of flipping as fast
> +as possible. The only exceptions are the framebuffer ID to be flipped to and
> +mode IDs changes, which could be referring to an identical mode, thus not
> +requiring modeset.
That's a bit... roundabout way to describe it. Doesn't async flip want
to prevent all kinds of other-than-FB changes, not only the modesetting
ones? Modesets are already gated by ALLOW_MODESET anyway. How about
something like:
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.
If you want to take these and need my Sob, that would be
Signed-off-by: Pekka Paalanen <[email protected]>
Thanks,
pq
Em 03/07/2023 05:38, Pekka Paalanen escreveu:
> On Fri, 30 Jun 2023 23:09:17 -0300
> André Almeida <[email protected]> wrote:
>
>> Specify how the atomic state is maintained between userspace and
>> kernel, plus the special case for async flips.
>>
>> Signed-off-by: André Almeida <[email protected]>
[...]
>
> If you want to take these and need my Sob, that would be
> Signed-off-by: Pekka Paalanen <[email protected]>
>
>
Thank you very much! Your version is way better than mine, I'll use it
in my next version.
> Thanks,
> pq