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

2017-03-02 Thread Laurent Pinchart 
Hi Daniel,

Thank you for the patch.

On Thursday 02 Mar 2017 08:19:58 Daniel Vetter wrote:
> 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.
> 
> v2: Spelling and clarifications (Eric).
> 
> v3: Implement suggestion from Gabriel to fix the graph.
> 
> Cc: Gabriel Krisman Bertazi 
> Acked-by: Eric Anholt 
> Cc: Eric Anholt 
> Cc: Laurent Pinchart 
> Cc: Harry Wentland 
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/gpu/drm-kms-helpers.rst |  2 +
>  Documentation/gpu/drm-kms.rst | 84 +++-
>  2 files changed, 85 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 a504d9ee4d94..eb9d29865c41 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -189,8 +189,90 @@ 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
> +===
> +
> +
> +.. 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"]
> +  "duplicated drm_plane_state A" -> "drm_device"[style=invis]
> +   }
> +
> +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 allow setting and rollback of just the software state,
> +  simplifying conversion of existing drivers. But auditing drivers for
> +  correctness of the atomic_check code becomes really hard with that:
> Rolling
> +  back changes in data structures all over the place is hard to get right.
> +
> +- Lastly, for backwards compatibility and to support all use-cases, atomic

s/backwards/backward/ (see my comment to patch 3/6, and not that the margin is 
now slightly larger :-))

> +  updates need to be incremental and be able to execute in parallel.
> Hardware
> +  doesn't always allow it, but where possible plane updates on
> different CRTCs
> +  should not interfere, and not get stalled due to output routing changing
> on
> +  different CRTCs.
> +
> +Taken all together there's two consequences for the atomic design:
> +
> +- The overall state is split up into per-object state structures:
> +  :c:type:`struct drm_plane_state ` for planes,
> :c:type:`struct +  drm_crtc_state ` for CRTCs and
> :c:type:`struct
> +  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

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

2017-03-02 Thread Daniel Vetter 
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.

v2: Spelling and clarifications (Eric).

v3: Implement suggestion from Gabriel to fix the graph.

Cc: Gabriel Krisman Bertazi 
Acked-by: Eric Anholt 
Cc: Eric Anholt 
Cc: Laurent Pinchart 
Cc: Harry Wentland 
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/drm-kms-helpers.rst |  2 +
 Documentation/gpu/drm-kms.rst | 84 ++-
 2 files changed, 85 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 a504d9ee4d94..eb9d29865c41 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -189,8 +189,90 @@ 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
+===
+
+
+.. 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"]
+  "duplicated drm_plane_state A" -> "drm_device"[style=invis]
+   }
+
+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 allow setting and rollback of just the software state,
+  simplifying conversion of existing drivers. But auditing drivers for
+  correctness of the atomic_check code becomes really hard with that: Rolling
+  back changes in data structures all over the place is hard to get right.
+
+- Lastly, for backwards compatibility and to support all use-cases, atomic
+  updates need to be incremental and be able to execute in parallel. Hardware
+  doesn't always allow it, but where possible plane updates on different CRTCs
+  should not interfere, and not get stalled due to output routing changing on
+  different CRTCs.
+
+Taken all together there's two consequences for the atomic design:
+
+- The overall state is split up into per-object state structures:
+  :c:type:`struct drm_plane_state ` for planes, 
:c:type:`struct
+  drm_crtc_state ` for CRTCs and :c:type:`struct
+  drm_connector_state

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

2017-03-01 Thread Eric Anholt 
Daniel Vetter  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.
>
> v2: Spelling and clarifications (Eric).
>
> Cc: Eric Anholt 
> Cc: Laurent Pinchart 
> Cc: Harry Wentland 
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/gpu/drm-kms-helpers.rst |  2 +
>  Documentation/gpu/drm-kms.rst | 86 
> ++-
>  2 files changed, 87 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

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

s/The //

With that,

Acked-by: Eric Anholt 


signature.asc
Description: PGP signature

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

2017-03-01 Thread Daniel Vetter 
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.

v2: Spelling and clarifications (Eric).

Cc: Eric Anholt 
Cc: Laurent Pinchart 
Cc: Harry Wentland 
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/drm-kms-helpers.rst |  2 +
 Documentation/gpu/drm-kms.rst | 86 ++-
 2 files changed, 87 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 a504d9ee4d94..4823df03c773 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -189,8 +189,92 @@ 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"]
+   }
+
+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 allow setting and rollback of just the software state,
+  simplifying conversion of existing drivers. But auditing drivers for
+  correctness of the atomic_check code becomes really hard with that: Rolling
+  back changes in data structures all over the place is hard to get right.
+
+- Lastly, for backwards compatibility and to support all use-cases, atomic
+  updates need to be incremental and be able to execute in parallel. Hardware
+  doesn't always allow it, but where possible plane updates on different CRTCs
+  should not interfere, and not get stalled due to output routing changing on
+  different CRTCs.
+
+Taken all together there's two consequences for the atomic design:
+
+- The overall state is split up into per-object state structures:
+  :c:type:`struct drm_plane_state ` for planes, 
:c:type:`struct
+  drm_crtc_state ` for CRTCs and :c:type:`struct
+  drm_connector_state