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

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Jul 07, 2023 at 07:40:59PM -0300, André Almeida wrote:
> From: Pekka Paalanen <pekka.paalanen@collabora.com>
> 
> Specify how the atomic state is maintained between userspace and
> kernel, plus the special case for async flips.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
> v4: total rework by Pekka
> ---
>  Documentation/gpu/drm-uapi.rst | 41 ++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 65fb3036a580..6a1662c08901 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -486,3 +486,44 @@ and the CRTC index is its position in this array.
>  
>  .. kernel-doc:: include/uapi/drm/drm_mode.h
>     :internal:
> +
> +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 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.

Small clarification: The kernel indeed tries very hard to make redundant
changes a no-op, and I think we should consider any issues here bugs. But
it still has to check, which means it needs to acquire the right locks and
put in the right (cross-crtc) synchronization points, and due to
implmentation challenges it's very hard to try to avoid that in all cases.
So adding redundant changes especially across crtc (and their connected
planes/connectors) might result in some oversynchronization issues, and
userspace should therefore avoid them if feasible.

With some sentences added to clarify this:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +
> +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.
> -- 
> 2.41.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch