Message-ID: <20230713105142.122a0cc1@eldfell>
Date:   Thu, 13 Jul 2023 10:51:42 +0300
From:   Pekka Paalanen <ppaalanen@...il.com>
To:     André Almeida <andrealmeid@...lia.com>
Cc:     Daniel Vetter <daniel@...ll.ch>, dri-devel@...ts.freedesktop.org,
        amd-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        wayland-devel@...ts.freedesktop.org, kernel-dev@...lia.com,
        alexander.deucher@....com, christian.koenig@....com,
        pierre-eric.pelloux-prayer@....com,
        Simon Ser <contact@...rsion.fr>,
        Rob Clark <robdclark@...il.com>,
        Daniel Stone <daniel@...ishbar.org>,
        'Marek Olšák' <maraeo@...il.com>,
        Dave Airlie <airlied@...il.com>,
        Michel Dänzer <michel.daenzer@...lbox.org>,
        Randy Dunlap <rdunlap@...radead.org>, hwentlan@....com,
        joshua@...ggi.es, ville.syrjala@...ux.intel.com
Subject: Re: [PATCH v5 6/6] drm/doc: Define KMS atomic state set

On Tue, 11 Jul 2023 10:57:57 +0200
Daniel Vetter <daniel@...ll.ch> wrote:

> On Fri, Jul 07, 2023 at 07:40:59PM -0300, André Almeida wrote:
> > From: Pekka Paalanen <pekka.paalanen@...labora.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@...labora.com>
> > Signed-off-by: André Almeida <andrealmeid@...lia.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@...ll.ch>

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;


Thanks,
pq

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


Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists