mbox series

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

Message ID 20231025005318.293690-1-andrealmeid@igalia.com
Headers
Series drm: Add support for atomic async page-flip |

Message

André Almeida Oct. 25, 2023, 12:53 a.m. UTC 
  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-andrealmeid@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-andrealmeid@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-andrealmeid@igalia.com/

Changes from v4:
 - Documentation rewrote by Pekka Paalanen

v4: https://lore.kernel.org/dri-devel/20230701020917.143394-1-andrealmeid@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-contact@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(-)

Comments

Simon Ser Nov. 17, 2023, 4:20 p.m. UTC | #1 
It seems like commits were re-ordered at some point. I think we need "drm:
introduce drm_mode_config.atomic_async_page_flip_not_supported" to come before
"drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits" because the latter
uses atomic_async_page_flip_not_supported. Similarly, "drm: Refuse to async flip
with atomic prop changes" should probably come before that same patch because
we don't want to have an intermediary state where we allow user-space to change
arbitrary properties. In general, commits should be constructed so that there
is no "broken state" in-between: each commit needs to build without errors and
not have transient bugs.

While reading this again, I think that now that we only allow primary FB_ID to
change, we don't need atomic_async_page_flip_not_supported anymore? In other
words, allowing async atomic commits on all drivers doesn't require anything
more than they support already, because the atomic codepath can't do anything
more than the legacy codepath.

With that in mind, I also wonder if the new cap is helpful. Since async atomic
commits can fail for pretty much any reason, user-space can just try and
fallback to something else. The cap could be useful to indicate whether async
atomic commits would always fail, but not sure how useful that is? I don't have
strong opinions either way.