On Mon, 31 Jul 2023 at 04:01, André Almeida wrote:
>
> Em 13/07/2023 04:51, Pekka Paalanen escreveu:
> > On Tue, 11 Jul 2023 10:57:57 +0200
> > Daniel Vetter wrote:
> >
> >> On Fri, Jul 07, 2023 at 07:40:59PM -0300, André Almeida wrote:
> >>> 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
> >>> ---
> >>> 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
> >
> > After talking on IRC yesterday, we realized that the no-op rule is
> > nowhere near as generic as I have believed. Roughly:
> > https://oftc.irclog.whitequark.org/dri-devel/2023-07-12#1689152446-1689157291;
> >
> >
>
> How about:
>
> 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, the kernel can
> not assure that every redundant information will always result in a
> no-op, giving the need to take locks to check par of the state. Giving
> that, some redundant information can lead to a full damage path. This is
> not something bad by itself, but userspace need to be aware of that side
> effect.
I think the addition about damage tracking should be a separate
paragraph, and not mixed in with the general explanation that no-op
updates might lead to oversync/overlocking issues. Because the damage
tracking issue is more a question of efficiency/power usage, but
should not (for most drivers/hw at least) result in delays and missed
updates due to oversynchronization of updates.
Also in my opinion the exact damage update rules are more a plane
property issue, and should probably be clarified there if the current
documentation is not clear enough. Since it's not about whe
