[PATCH 6/6] drm/doc: atomic overview, with graph

[Daniel Vetter]

On Tue, Feb 28, 2017 at 03:48:47PM -0800, Eric Anholt wrote:
> Daniel Vetter <daniel.vetter at ffwll.ch> writes:
> 
> > I want to split up a few more things and document some details better
> > (like how exactly to subclass drm_atomic_state). And maybe also split
> > up the helpers a bit per-topic, but this should be a ok-ish start for
> > better atomic overview.
> >
> > One thing I failed at is getting DOT to layout the overview graph how
> > I want it. The highlevel structure I want is:
> >
> > 	Free-standing State
> >
> > 	Current State
> >
> > i.e. one over the other. Currently it lays it out side-by-side, but
> > not even that really - "Current State" is somewhat offset below. Makes
> > the graph look like garbage, and also way too wide for proper
> > rendering. Ideas appreciated.
> >
> > Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Cc: Harry Wentland <harry.wentland at amd.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> 
> Thanks for writing these docs.  I wish I had them back when I was
> starting vc4's atomic code!  With the two little spelling nits fixed,
> 3-5 are:
> 
> Acked-by: Eric Anholt <eric at anholt.net>
> 
> A few copyedits on this one below, but it sounds like you don't want to
> push quite yet while you sort out the rendering.

I've spent quite some time trying to beat DOT into submission, this is the
best I can do. The FIXME really is just a hint for someone with more clue
to maybe make it better, or if not possible at all, what would look better
when doing a proper diagram with .svg or something like that.

Assuming no one knows how to fix this, I'd still like to push it - it's
still better than nothing imo, you just need to look at the picture
full-screen.
-Daniel

> 
> > ---
> >  Documentation/gpu/drm-kms-helpers.rst |  2 +
> >  Documentation/gpu/drm-kms.rst         | 85 ++++++++++++++++++++++++++++++++++-
> >  2 files changed, 86 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> > index 050ebe81d256..ac53c0b893f6 100644
> > --- a/Documentation/gpu/drm-kms-helpers.rst
> > +++ b/Documentation/gpu/drm-kms-helpers.rst
> > @@ -42,6 +42,8 @@ Modeset Helper Reference for Common Vtables
> >  .. kernel-doc:: include/drm/drm_modeset_helper_vtables.h
> >     :internal:
> >  
> > +.. _drm_atomic_helper:
> > +
> >  Atomic Modeset Helper Functions Reference
> >  =========================================
> >  
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 20378881445f..979cee853bb1 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -189,8 +189,91 @@ multiple times to different objects using :c:func:`drm_object_attach_property()
> >  .. kernel-doc:: drivers/gpu/drm/drm_mode_object.c
> >     :export:
> >  
> > +Atomic Mode Setting
> > +===================
> > +
> > +
> > +.. FIXME: The I want the below graph to be laid out so that the 2 subgraph
> > +   clusters are below each another. But I failed.
> > +
> > +.. kernel-render:: DOT
> > +   :alt: Mode Objects and Properties
> > +   :caption: Mode Objects and Properties
> > +
> > +   digraph {
> > +      node [shape=box]
> > +
> > +      subgraph cluster_state {
> > +          style=dashed
> > +          label="Free-standing state"
> > +
> > +          "drm_atomic_state" -> "duplicated drm_plane_state A"
> > +          "drm_atomic_state" -> "duplicated drm_plane_state B"
> > +          "drm_atomic_state" -> "duplicated drm_crtc_state"
> > +          "drm_atomic_state" -> "duplicated drm_connector_state"
> > +          "drm_atomic_state" -> "duplicated driver private state"
> > +      }
> > +
> > +      subgraph cluster_current {
> > +          style=dashed
> > +          label="Current state"
> > +
> > +          "drm_device" -> "drm_plane A"
> > +          "drm_device" -> "drm_plane B"
> > +          "drm_device" -> "drm_crtc"
> > +          "drm_device" -> "drm_connector"
> > +          "drm_device" -> "driver private object"
> > +
> > +          "drm_plane A" -> "drm_plane_state A"
> > +          "drm_plane B" -> "drm_plane_state B"
> > +          "drm_crtc" -> "drm_crtc_state"
> > +          "drm_connector" -> "drm_connector_state"
> > +          "driver private object" -> "driver private state"
> > +      }
> > +
> > +      "drm_atomic_state" -> "drm_device" [label="atomic_commit"]
> > +   }
> > +
> > +Essentially atomic is transactional modeset (including planes) updates, but
> > +compared to the usual transactional approach of try-commit and rollback on
> > +failure atomic modesetting is a bit different:
> 
> Maybe reword:
> 
> "Atomic provides transactional modeset (including planes) updates, but a
> bit differently from the usual transactional approach of try-commit and
> rollback:"
> 
> > +- Firstly, no hardware changes are allowed when the commit would fail. This
> > +  allows us to implement the DRM_MODE_ATOMIC_TEST_ONLY mode, which allows
> > +  userspace to explore whether certain configurations would work or not.
> > +
> > +- This would still allows setting and rollback of just the software state,
> 
> "allow"
> 
> > +  simplifying conversion of existing drivers. But auditing drivers for
> > +  correctness of the atomic_check code because really hard with that.
> 
> s/because/becomes/?
> 
> > +Taken all together there's two consequence for the atomic design:
> 
> "consequences"
> 
> > +
> > +- The overall state is split up into per-object state structures:
> > +  :c:type:`struct drm_plane_state <drm_plane_state>` for planes, :c:type:`struct
> > +  drm_crtc_state <drm_crtc_state>` for CRTCs and :c:type:`struct
> > +  drm_connector_state <drm_connector_state` for connectors. These are the only
> > +  objects with userspace-visible and settable state. For internal state drivers
> > +  can subclass these structures through embeddeding, or add entirely new state
> > +  structures for their globally shared hardware functions.
> > +
> > +- An atomic update is assembled and validated as an enterily free-standing pile
> > +  of structures within the :c:type:`drm_atomic_state <drm_atomic_state>`
> > +  container. Again drivers can subclass that container for their own state
> > +  structure tracking needs. Only when a state is commit is it applied to the
> 
> "is committed"
> 
> > +  driver and modeset objects. This way rolling back an update boils down to
> > +  releasing memory and unreference objects like framebuffers.
> 
> "unreferencing"



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