[PATCH 0/6] diagrams for drm docs

[PATCH 0/6] diagrams for drm docs

[Daniel Vetter]

Hi all,

So this is my first stab at doing some overview diagrams for drm. Pretty much
like what it looks like, both input and output. Well there's one layout issue
where I couldn't convince DOT to do what I want it to do. All based on Markus'
kernel-figures script.

I think the script needs a bit more work&polish, but I'm probably not the best
one to do that. I can take a stab at improving things a bit, but I think better
if Jani/Jon/Markus help out here.

Cheers, Daniel

Daniel Vetter (5):
  HACKS: pass stderr along in kfigure.py
  drm/doc: Add KMS overview graphs
  drm/doc: Consistent kerneldoc include order
  drm/doc: diagram for mode objects and properties
  drm/doc: atomic overview, with graph

Markus Heiser (1):
  docs-rst: automatically convert Graphviz and SVG images

 Documentation/conf.py                 |   2 +-
 Documentation/doc-guide/hello.dot     |   3 +
 Documentation/doc-guide/sphinx.rst    |  90 +++++-
 Documentation/doc-guide/svg_image.svg |  10 +
 Documentation/gpu/drm-internals.rst   |   6 +-
 Documentation/gpu/drm-kms-helpers.rst |  28 +-
 Documentation/gpu/drm-kms.rst         | 269 ++++++++++++++++--
 Documentation/gpu/drm-mm.rst          |  24 +-
 Documentation/process/changes.rst     |   8 +-
 Documentation/sphinx/kfigure.py       | 511 ++++++++++++++++++++++++++++++++++
 10 files changed, 903 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/doc-guide/hello.dot
 create mode 100644 Documentation/doc-guide/svg_image.svg
 create mode 100644 Documentation/sphinx/kfigure.py

-- 
2.11.0

[PATCH 1/6] docs-rst: automatically convert Graphviz and SVG images

[Daniel Vetter]

From: Markus Heiser <markus.heiser@darmarit.de>

This patch brings scalable figure, image handling and a concept to
embed *render* markups:

* DOT (http://www.graphviz.org)
* SVG

For image handling use the 'image' replacement::

    .. kernel-image::  svg_image.svg
       :alt:    simple SVG image

For figure handling use the 'figure' replacement::

    .. kernel-figure::  svg_image.svg
       :alt:    simple SVG image

       SVG image example

Embed *render* markups (or languages) like Graphviz's **DOT** is
provided by the *render* directive.::

  .. kernel-render:: DOT
     :alt: foobar digraph
     :caption: Embedded **DOT** (Graphviz) code.

     digraph foo {
      "bar" -> "baz";
     }

The *render* directive is a concept to integrate *render* markups and
languages, yet supported markups:

* DOT: render embedded Graphviz's **DOT**
* SVG: render embedded Scalable Vector Graphics (**SVG**)

v2: s/DOC/DOT/ in a few places.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Markus Heiser <markus.heiser@darmarit.de> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/conf.py                 |   2 +-
 Documentation/doc-guide/hello.dot     |   3 +
 Documentation/doc-guide/sphinx.rst    |  90 +++++-
 Documentation/doc-guide/svg_image.svg |  10 +
 Documentation/process/changes.rst     |   8 +-
 Documentation/sphinx/kfigure.py       | 505 ++++++++++++++++++++++++++++++++++
 6 files changed, 612 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/doc-guide/hello.dot
 create mode 100644 Documentation/doc-guide/svg_image.svg
 create mode 100644 Documentation/sphinx/kfigure.py

diff --git a/Documentation/conf.py b/Documentation/conf.py
index bb5a9082598d..23f291a8f857 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -34,7 +34,7 @@ from load_config import loadConfig
 # Add any Sphinx extension module names here, as strings. They can be
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
-extensions = ['kerneldoc', 'rstFlatTable', 'kernel_include', 'cdomain']
+extensions = ['kerneldoc', 'rstFlatTable', 'kernel_include', 'cdomain', 'kfigure']
 
 # The name of the math extension changed on Sphinx 1.4
 if major == 1 and minor > 3:
diff --git a/Documentation/doc-guide/hello.dot b/Documentation/doc-guide/hello.dot
new file mode 100644
index 000000000000..504621dfc595
--- /dev/null
+++ b/Documentation/doc-guide/hello.dot
@@ -0,0 +1,3 @@
+graph G {
+      Hello -- World
+}
diff --git a/Documentation/doc-guide/sphinx.rst b/Documentation/doc-guide/sphinx.rst
index 532d65b70500..b902744ce7dd 100644
--- a/Documentation/doc-guide/sphinx.rst
+++ b/Documentation/doc-guide/sphinx.rst
@@ -34,8 +34,10 @@ format-specific subdirectories under ``Documentation/output``.
 
 To generate documentation, Sphinx (``sphinx-build``) must obviously be
 installed. For prettier HTML output, the Read the Docs Sphinx theme
-(``sphinx_rtd_theme``) is used if available. For PDF output, ``rst2pdf`` is also
-needed. All of these are widely available and packaged in distributions.
+(``sphinx_rtd_theme``) is used if available. For PDF output you'll also need
+``XeLaTeX`` and CairoSVG (http://cairosvg.org) or alternatively ``convert(1)``
+from ImageMagick (https://www.imagemagick.org). All of these are widely
+available and packaged in distributions.
 
 To pass extra options to Sphinx, you can use the ``SPHINXOPTS`` make
 variable. For example, use ``make SPHINXOPTS=-v htmldocs`` to get more verbose
@@ -232,3 +234,87 @@ Rendered as:
       * .. _`last row`:
 
         - column 3
+
+
+Figures & Images
+================
+
+If you want to add an image, you should use the ``kernel-figure`` and
+``kernel-image`` directives. E.g. to insert a figure with a scalable
+image format use SVG::
+
+    .. kernel-figure::  svg_image.svg
+       :alt:    simple SVG image
+
+       SVG image example
+
+.. kernel-figure::  svg_image.svg
+   :alt:    simple SVG image
+
+   SVG image example
+
+The kernel figure (and image) directive support **DOT** formated files, see
+
+* DOT: http://graphviz.org/pdf/dotguide.pdf
+* Graphviz: http://www.graphviz.org/content/dot-language
+
+A simple example::
+
+  .. kernel-figure::  hello.dot
+     :alt:    hello world
+
+     DOT's hello world example
+
+.. kernel-figure::  hello.dot
+   :alt:    hello world
+
+   DOT's hello world example
+
+Embed *render* markups (or languages) like Graphviz's **DOT** is provided by the
+``kernel-render`` directives.::
+
+  .. kernel-render:: DOT
+     :alt: foobar digraph
+     :caption: Embedded **DOT** (Graphviz) code.
+
+     digraph foo {
+      "bar" -> "baz";
+     }
+
+How this will be rendered depends on the installed tools. If Graphviz is
+installed, you will see an vector image. If not the raw markup is inserted as
+*literal-block*.
+
+.. kernel-render:: DOT
+   :alt: foobar digraph
+   :caption: Embedded **DOT** (Graphviz) code.
+
+   digraph foo {
+      "bar" -> "baz";
+   }
+
+The *render* directive has all the options known from the *figure* directive,
+plus option ``caption``.  If ``caption`` has a value, a *figure* node is
+inserted. If not, a *image* node is inserted.
+
+Embedded **SVG**::
+
+  .. kernel-render:: SVG
+     :caption: Embedded **SVG** markup.
+     :alt: so-nw-arrow
+
+     <?xml version="1.0" encoding="UTF-8"?>
+     <svg xmlns="http://www.w3.org/2000/svg" version="1.1" ...>
+        ...
+     </svg>
+
+.. kernel-render:: SVG
+   :caption: Embedded **SVG** markup.
+   :alt: so-nw-arrow
+
+   <?xml version="1.0" encoding="UTF-8"?>
+   <svg xmlns="http://www.w3.org/2000/svg"
+     version="1.1" baseProfile="full" width="70px" height="40px" viewBox="0 0 700 400">
+   <line x1="180" y1="370" x2="500" y2="50" stroke="black" stroke-width="15px"/>
+   <polygon points="585 0 525 25 585 50" transform="rotate(135 525 25)"/>
+   </svg>
diff --git a/Documentation/doc-guide/svg_image.svg b/Documentation/doc-guide/svg_image.svg
new file mode 100644
index 000000000000..5405f85b8137
--- /dev/null
+++ b/Documentation/doc-guide/svg_image.svg
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- originate: https://commons.wikimedia.org/wiki/File:Variable_Resistor.svg -->
+<svg xmlns="http://www.w3.org/2000/svg"
+	version="1.1" baseProfile="full"
+	width="70px" height="40px" viewBox="0 0 700 400">
+	<line x1="0" y1="200" x2="700" y2="200" stroke="black" stroke-width="20px"/>
+	<rect x="100" y="100" width="500" height="200" fill="white" stroke="black" stroke-width="20px"/>
+	<line x1="180" y1="370" x2="500" y2="50" stroke="black" stroke-width="15px"/>
+	<polygon points="585 0 525 25 585 50" transform="rotate(135 525 25)"/>
+</svg>
diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
index 56ce66114665..de284ca05587 100644
--- a/Documentation/process/changes.rst
+++ b/Documentation/process/changes.rst
@@ -318,9 +318,11 @@ PDF outputs, it is recommended to use version 1.4.6.
 .. note::
 
   Please notice that, for PDF and LaTeX output, you'll also need ``XeLaTeX``
-  version 3.14159265. Depending on the distribution, you may also need
-  to install a series of ``texlive`` packages that provide the minimal
-  set of functionalities required for ``XeLaTex`` to work.
+  version 3.14159265. Depending on the distribution, you may also need to
+  install a series of ``texlive`` packages that provide the minimal set of
+  functionalities required for ``XeLaTex`` to work. For PDF output you'll also
+  need CairoSVG (http://cairosvg.org) or alternatively ``convert(1)`` from
+  ImageMagick (https://www.imagemagick.org).
 
 Other tools
 -----------
diff --git a/Documentation/sphinx/kfigure.py b/Documentation/sphinx/kfigure.py
new file mode 100644
index 000000000000..76425b461a40
--- /dev/null
+++ b/Documentation/sphinx/kfigure.py
@@ -0,0 +1,505 @@
+# -*- coding: utf-8; mode: python -*-
+# pylint: disable=C0103
+u"""
+    scalable figure and image handling
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    Sphinx extension which implements scalable image handling.
+
+    :copyright:  Copyright (C) 2016  Markus Heiser
+    :license:    GPL Version 2, June 1991 see Linux/COPYING for details.
+
+    The build for image formats depence on image's source format and output's
+    destination format. This extension implement methods to simplify image
+    handling from the author's POV. Directives like ``kernel-figure`` implement
+    methods *to* always get the best output-format even if some tools are not
+    installed.For more details take a look at ``convert_image(...)`` which is
+    the core of all conversions.
+
+    * ``.. kernel-image``: for image handling / ``.. image::`` replacement
+
+    * ``.. kernel-figure``: for figure handling / ``.. figure::`` replacement
+
+    * ``.. kernel-render``: for render markup / a concept to embed *render*
+      markups (or languages). Supported markups (see ``RENDER_MARKUP_EXT``)
+
+      + ``DOT``: render embedded Graphviz's **DOC**
+      + ``SVG``: render embedded Scalable Vector Graphics (**SVG**)
+      + ... *developable*
+
+    Used tools:
+
+    * ``dot(1)``: Graphviz (http://www.graphviz.org). If Graphviz is not
+      available, the DOT language is inserted as literal-block.
+
+    * SVG to PDF: To generate PDF, you need at least one of this tools:
+
+      - CairoSVG (http://cairosvg.org) if installed or alternatively
+      - ``convert(1)``: ImageMagick (https://www.imagemagick.org)
+
+    List of customizations:
+
+    * generate PDF from SVG / used by PDF (LaTeX) builder
+
+    * generate SVG (html-builder) and PDF (latex-builder) from DOT files.
+      DOT: see http://www.graphviz.org/content/dot-language
+
+    """
+
+import os
+from os import path
+import subprocess
+from hashlib import sha1
+
+from docutils import nodes
+from docutils.statemachine import ViewList
+from docutils.parsers.rst import directives
+from docutils.parsers.rst.directives import images
+
+from sphinx.directives import patches
+
+__version__  = '1.0'
+
+# simple helper
+# -------------
+
+def which(cmd):
+    """Searches the ``cmd`` in the ``PATH`` enviroment.
+
+    This *which* searches the PATH for executable ``cmd`` . First match is
+    returned, if nothing is found, ``None` is returned.
+    """
+    envpath = os.environ.get('PATH', None) or os.defpath
+    for folder in envpath.split(os.pathsep):
+        fname = folder + os.sep + cmd
+        if path.isfile(fname):
+            return fname
+
+def mkdir(folder, mode=0o775):
+    if not path.isdir(folder):
+        os.makedirs(folder, mode)
+
+def isNewer(path1, path2):
+    """Returns True if ``path1`` is newer than ``path2``
+
+    If ``path1`` exists and is newer than ``path2`` the function returns
+    ``True`` is returned otherwise ``False``
+    """
+    return (path.exists(path1)
+            and os.stat(path1).st_ctime > os.stat(path2).st_ctime)
+
+# def debug_handle(self, node):           # pylint: disable=W0613
+#     from linuxdoc.kernel_doc import CONSOLE
+#     CONSOLE()
+
+def pass_handle(self, node):           # pylint: disable=W0613
+    pass
+
+# setup conversion tools and sphinx extension
+# -------------------------------------------
+
+# Graphviz's dot(1) support
+dot_cmd = which('dot')
+
+# ImageMagick' convert(1) support
+convert_cmd = which('convert')
+
+# cairosvg support
+try:
+    import cairosvg # pylint: disable=C0413
+except ImportError:
+    cairosvg = None
+
+
+def setup(app):
+    # check toolchain first
+    app.connect('builder-inited', checkTools)
+
+    # image handling
+    app.add_directive("kernel-image",  KernelImage)
+    app.add_node(kernel_image,
+                 html    = (visit_kernel_image, pass_handle),
+                 latex   = (visit_kernel_image, pass_handle),
+                 texinfo = (visit_kernel_image, pass_handle),
+                 text    = (visit_kernel_image, pass_handle),
+                 man     = (visit_kernel_image, pass_handle), )
+
+    # figure handling
+    app.add_directive("kernel-figure", KernelFigure)
+    app.add_node(kernel_figure,
+                 html    = (visit_kernel_figure, pass_handle),
+                 latex   = (visit_kernel_figure, pass_handle),
+                 texinfo = (visit_kernel_figure, pass_handle),
+                 text    = (visit_kernel_figure, pass_handle),
+                 man     = (visit_kernel_figure, pass_handle), )
+
+    # render handling
+    app.add_directive('kernel-render', KernelRender)
+    app.add_node(kernel_render,
+                 html    = (visit_kernel_render, pass_handle),
+                 latex   = (visit_kernel_render, pass_handle),
+                 texinfo = (visit_kernel_render, pass_handle),
+                 text    = (visit_kernel_render, pass_handle),
+                 man     = (visit_kernel_render, pass_handle), )
+
+    return dict(
+        version = __version__,
+        parallel_read_safe = True,
+        parallel_write_safe = True
+    )
+
+
+def checkTools(app):
+    u"""
+    Check available build tools and log some *verbose* messages.
+
+    This function is called once, when the builder is initiated.
+    """
+    app.verbose("kfigure: check installed tools ...")
+    if dot_cmd:
+        app.verbose("use dot(1) from: " + dot_cmd)
+    else:
+        app.warn("dot(1) not found, for better output quality install "
+                 "graphviz from http://www.graphviz.org")
+    if cairosvg:
+        app.verbose("use CairoSVG from: " + str(cairosvg))
+    if convert_cmd:
+        app.verbose("use ImageMagick: " + convert_cmd)
+    if cairosvg is None and convert_cmd is None:
+        app.warn("no SVG to PDF conversion available, "
+                 "install CairoSVG (http://cairosvg.org) or alternatively "
+                 "``convert(1)`` from ImageMagick (https://www.imagemagick.org)")
+
+
+# integrate conversion tools
+# --------------------------
+
+RENDER_MARKUP_EXT = {
+    # The '.ext' must be handled by convert_image(..) function's *in_ext* input.
+    # <name> : <.ext>
+    'DOT'   : '.dot'
+    , 'SVG' : '.svg'
+}
+
+def convert_image(img_node, translator):  # pylint: disable=R0912
+    """Convert an image node for the builder.
+
+    Different builder prefer different image formats, e.g. *latex* builder
+    prefer PDF while *html* builder prefer SVG format for images.
+
+    This function handles outputs image formats in depence of source the format
+    of the image and the translator's output format. This also means to
+    manipulate/update the *image* dictionary of the builder (``builder.images``)
+
+    """
+    fname, in_ext = path.splitext(path.basename(img_node['uri']))
+    src_fname     = path.join(translator.builder.srcdir, img_node['uri'])
+    src_folder    = path.dirname(img_node['uri'])
+    out_dir       = translator.builder.outdir
+    dst_fname     = None
+
+    # in kernel builds, use 'make SPHINXOPTS=-v' to see verbose messages
+    verbose = translator.builder.app.verbose
+    warn    = translator.builder.warn
+
+    verbose('assert best format for: ' + img_node['uri'])
+
+    if in_ext == '.dot':
+
+        # ----------
+        # handle DOT
+        # ----------
+
+        if not dot_cmd:
+            verbose("dot from graphviz not available / include DOT raw.")
+            with open(src_fname, "r") as dot:
+                data = dot.read()
+                node = nodes.literal_block(data, data)
+                img_node.replace_self(node)
+
+        elif translator.builder.format == 'latex':
+            dst_fname = path.join(out_dir, fname + '.pdf')
+
+        elif translator.builder.format == 'html':
+            dst_fname = path.join(out_dir, src_folder, fname + '.svg')
+        else:
+            # all other builder formats will include DOT as raw
+            with open(src_fname, "r") as dot:
+                data = dot.read()
+                node = nodes.literal_block(data, data)
+                img_node.replace_self(node)
+
+
+    elif in_ext == '.svg':
+
+        # ----------
+        # handle SVG
+        # ----------
+
+        if translator.builder.format == 'latex':
+            if cairosvg is None and convert_cmd is None:
+                warn("no SVG to PDF conversion available")
+            else:
+                dst_fname = path.join(out_dir, fname + '.pdf')
+
+    if dst_fname:
+        name = dst_fname[len(out_dir) + 1:]
+        # the builder needs not to copy one more time, so pop it if exists.
+        translator.builder.images.pop(img_node['uri'], None)
+        img_node['uri'] = dst_fname
+        img_node['candidates'] = {'*': dst_fname}
+
+        if isNewer(dst_fname, src_fname):
+            verbose("convert: %s allready exists and is newer" % name)
+        else:
+            mkdir(path.dirname(dst_fname))
+
+            if in_ext == '.dot':
+                verbose('convert DOT to: {out}/' + name)
+                dot2format(src_fname, dst_fname)
+
+            elif in_ext == '.svg':
+                verbose('convert SVG to: {out}/' + name)
+                svg2pdf(src_fname, dst_fname)
+
+def dot2format(dot_fname, out_fname):
+    """Converts DOT file to ``out_fname`` using ``dot(1)``.
+
+    * ``dot_fname`` pathname of the input DOT file, including extension ``.dot``
+    * ``out_fname`` pathname of the output file, including format extension
+
+    The *format extension* depends on the ``dot`` command (see ``man dot``
+    option ``-Txxx``). Normally you will use one of the following extensions:
+
+    - ``.ps`` for PostScript,
+    - ``.svg`` or ``svgz`` for Structured Vector Graphics,
+    - ``.fig`` for XFIG graphics and
+    - ``.png`` or ``gif`` for common bitmap graphics.
+
+    """
+    out_format = path.splitext(out_fname)[1][1:]
+    cmd = [dot_cmd, '-T%s' % out_format, dot_fname]
+    exit_code = 42
+    with open(out_fname, "w") as out:
+        exit_code = subprocess.call(
+            cmd, stdout = out, stderr = subprocess.PIPE )
+        out.flush()
+    return bool(exit_code == 0)
+
+def svg2pdf(svg_fname, pdf_fname):
+    """Converts SVG to PDF with CairoSVG or ``convert(1)`` command.
+
+    Uses CairoSVG (http://cairosvg.org) if installed or alternatively
+    ``convert(1)`` from ImageMagick (https://www.imagemagick.org) for
+    conversion.  Returns ``True`` on success and ``False`` if an error occurred
+    (e.g. none of the conversion tool is available).
+
+    * ``svg_fname`` pathname of the input SVG file with extension (``.svg``)
+    * ``pdf_name``  pathname of the output PDF file with extension (``.pdf``)
+
+    """
+    if cairosvg:
+        # pylint: disable=E1101
+        cairosvg.svg2pdf(url = svg_fname, write_to = pdf_fname)
+        return True
+    if convert_cmd:
+        cmd = [convert_cmd, svg_fname, pdf_fname]
+        exit_code = subprocess.call(
+            cmd, stdout = subprocess.PIPE, stderr = subprocess.PIPE )
+        return bool(exit_code == 0)
+    return False
+
+
+# image handling
+# ---------------------
+
+def visit_kernel_image(self, node):    # pylint: disable=W0613
+    """Visitor of the ``kernel_image`` Node.
+
+    Handles the ``image`` child-node with the ``convert_image(...)``.
+    """
+    img_node = node[0]
+    convert_image(img_node, self)
+
+class kernel_image(nodes.General, nodes.Element):
+    """Node for ``kernel-image`` directive."""
+    pass
+
+class KernelImage(images.Image):
+    u"""KernelImage directive
+
+    Earns everything from ``.. image::`` directive, except *remote URI* and
+    *glob* pattern. The KernelImage wraps a image node into a
+    kernel_image node. See ``visit_kernel_image``.
+    """
+
+    def run(self):
+        uri = self.arguments[0]
+        if uri.endswith('.*') or uri.find('://') != -1:
+            raise self.severe(
+                'Error in "%s: %s": glob pattern and remote images are not allowed'
+                % (self.name, uri))
+        result = images.Image.run(self)
+        if len(result) == 2 or isinstance(result[0], nodes.system_message):
+            return result
+        (image_node,) = result
+        # wrap image node into a kernel_image node / see visitors
+        node = kernel_image('', image_node)
+        return [node]
+
+# figure handling
+# ---------------------
+
+def visit_kernel_figure(self, node):   # pylint: disable=W0613
+    """Visitor of the ``kernel_figure`` Node.
+
+    Handles the ``image`` child-node with the ``convert_image(...)``.
+    """
+    img_node = node[0][0]
+    convert_image(img_node, self)
+
+class kernel_figure(nodes.General, nodes.Element):
+    """Node for ``kernel-figure`` directive."""
+
+class KernelFigure(patches.Figure):
+    u"""KernelImage directive
+
+    Earns everything from ``.. figure::`` directive, except *remote URI* and
+    *glob* pattern.  The KernelFigure wraps a figure node into a kernel_figure
+    node. See ``visit_kernel_figure``.
+    """
+
+    def run(self):
+        uri = self.arguments[0]
+        if uri.endswith('.*') or uri.find('://') != -1:
+            raise self.severe(
+                'Error in "%s: %s":'
+                ' glob pattern and remote images are not allowed'
+                % (self.name, uri))
+        result = patches.Figure.run(self)
+        if len(result) == 2 or isinstance(result[0], nodes.system_message):
+            return result
+        (figure_node,) = result
+        # wrap figure node into a kernel_figure node / see visitors
+        node = kernel_figure('', figure_node)
+        return [node]
+
+
+# render handling
+# ---------------------
+
+def visit_kernel_render(self, node):
+    """Visitor of the ``kernel_render`` Node.
+
+    If rendering tools available, save the markup of the ``literal_block`` child
+    node into a file and replace the ``literal_block`` node with a new created
+    ``image`` node, pointing to the saved markup file. Afterwards, handle the
+    image child-node with the ``convert_image(...)``.
+    """
+
+    verbose = self.builder.app.verbose
+    warn    = self.builder.warn
+    srclang = node.get('srclang')
+
+    verbose('visit kernel-render node lang: "%s"' % (srclang))
+
+    tmp_ext = RENDER_MARKUP_EXT.get(srclang, None)
+    if tmp_ext is None:
+        warn('kernel-render: "%s" unknow / include raw.' % (srclang))
+        return
+
+    literal_block = node[0]
+    code = literal_block.astext()
+
+    if not dot_cmd and tmp_ext == '.dot':
+        verbose("dot from graphviz not available / include raw.")
+        tmp_ext = None
+
+    if tmp_ext:
+        hashobj   = code.encode('utf-8') #  str(node.attributes)
+        fname     = '%s-%s' % (srclang, sha1(hashobj).hexdigest())
+        tmp_fname = path.join(
+            self.builder.outdir, self.builder.imagedir, fname + tmp_ext)
+
+        if not path.isfile(tmp_fname):
+            mkdir(path.dirname(tmp_fname))
+            with open(tmp_fname, "w") as out:
+                out.write(code)
+
+        image_node = nodes.image(node.rawsource, **node.attributes)
+        image_node['uri'] = tmp_fname
+
+        literal_block.replace_self(image_node)
+        convert_image(image_node, self)
+
+
+class kernel_render(nodes.General, nodes.Inline, nodes.Element):
+    """Node for ``kernel-render`` directive."""
+    pass
+
+class KernelRender(patches.Figure):
+    u"""KernelRender directive
+
+    Render content by external tool.  Has all the options known from the
+    *figure*  directive, plus option ``caption``.  If ``caption`` has a
+    value, a figure node with the *caption* is inserted. If not, a image node is
+    inserted.
+
+    The KernelRender directive wraps the text of the directive into a
+    literal_block node and wraps it into a kernel_render node. See
+    ``visit_kernel_render``.
+    """
+    has_content = True
+    required_arguments = 1
+    optional_arguments = 0
+    final_argument_whitespace = False
+
+    # earn options from 'figure'
+    option_spec = patches.Figure.option_spec.copy()
+    option_spec['caption'] = directives.unchanged
+
+    def run(self):
+        return [self.build_node()]
+
+    def build_node(self):
+
+        srclang = self.arguments[0].strip()
+        if srclang not in RENDER_MARKUP_EXT.keys():
+            return [self.state_machine.reporter.warning(
+                'Unknow source language "%s", use one of: %s.' % (
+                    srclang, ",".join(RENDER_MARKUP_EXT.keys())),
+                line=self.lineno)]
+
+        code = '\n'.join(self.content)
+        if not code.strip():
+            return [self.state_machine.reporter.warning(
+                'Ignoring "%s" directive without content.' % (
+                    self.name),
+                line=self.lineno)]
+
+        node = kernel_render()
+        node['alt'] = self.options.get('alt','')
+        node['srclang'] = srclang
+        literal_node = nodes.literal_block(code, code)
+        node += literal_node
+
+        caption = self.options.get('caption')
+        if caption:
+            # parse cation's content
+            parsed = nodes.Element()
+            self.state.nested_parse(
+                ViewList([caption], source=''), self.content_offset, parsed)
+            caption_node = nodes.caption(
+                parsed[0].rawsource, '', *parsed[0].children)
+            caption_node.source = parsed[0].source
+            caption_node.line = parsed[0].line
+
+            figure_node = nodes.figure('', node)
+            for k,v in self.options.items():
+                figure_node[k] = v
+            figure_node += caption_node
+
+            node = figure_node
+
+        return node
+
-- 
2.11.0

[PATCH 2/6] HACKS: pass stderr along in kfigure.py

[Daniel Vetter]

Some addition comments on the implementation:
- Also needs to be hacked up for svg conversion.
- Inline kernel-render creates temp files and caches them, doesn't
  seem that good an idea. Probably should switch over to using pipes.
- Reimplements sphinx' depency tracking. Might be fixable if we just
  use pipes (and using the sphinx functions to open/read files).
- Linenumbers are off, but no idea how to fix that.
- Script looks a bit complicated to me, but I don't have python clue.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/sphinx/kfigure.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/sphinx/kfigure.py b/Documentation/sphinx/kfigure.py
index 76425b461a40..d6819337edf6 100644
--- a/Documentation/sphinx/kfigure.py
+++ b/Documentation/sphinx/kfigure.py
@@ -50,6 +50,7 @@ import os
 from os import path
 import subprocess
 from hashlib import sha1
+import sys
 
 from docutils import nodes
 from docutils.statemachine import ViewList
@@ -278,11 +279,16 @@ def dot2format(dot_fname, out_fname):
 
     """
     out_format = path.splitext(out_fname)[1][1:]
-    cmd = [dot_cmd, '-T%s' % out_format, dot_fname]
+    cmd = [dot_cmd, '-T%s' % out_format, dot_fname, '-v']
     exit_code = 42
     with open(out_fname, "w") as out:
-        exit_code = subprocess.call(
+        p = subprocess.Popen(
             cmd, stdout = out, stderr = subprocess.PIPE )
+        meh, err = p.communicate()
+
+        sys.stderr.write(err)
+
+        exit_code = p.returncode
         out.flush()
     return bool(exit_code == 0)
 
-- 
2.11.0

[PATCH 3/6] drm/doc: Add KMS overview graphs

[Daniel Vetter]

Oh, the shiny and pretties!

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-kms-helpers.rst |   4 ++
 Documentation/gpu/drm-kms.rst         | 132 ++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 03040aa14fe8..012b6ff3ec3f 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -114,6 +114,8 @@ Framebuffer CMA Helper Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_fb_cma_helper.c
    :export:
 
+.. _drm_bridges:
+
 Bridges
 =======
 
@@ -139,6 +141,8 @@ Bridge Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
    :export:
 
+.. _drm_panel_helper:
+
 Panel Helper Reference
 ======================
 
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 0c9abdc0ee31..2bdd266da4a9 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -17,6 +17,138 @@ be setup by initializing the following fields.
 
 Mode Configuration
 
+Overview
+========
+
+.. kernel-render:: DOT
+   :alt: KMS Display Pipeline
+   :caption: KMS Display Pipeline Overview
+
+   digraph "KMS" {
+      node [shape=box]
+
+      subgraph cluster_static {
+          style=dashed
+          label="Static Objects"
+
+          node [bgcolor=grey style=filled]
+          "drm_plane A" -> "drm_crtc"
+          "drm_plane B" -> "drm_crtc"
+          "drm_crtc" -> "drm_encoder A"
+          "drm_crtc" -> "drm_encoder B"
+      }
+
+      subgraph cluster_user_created {
+          style=dashed
+          label="Userspace-Created"
+
+          node [shape=oval]
+          "drm_framebuffer 1" -> "drm_plane A"
+          "drm_framebuffer 2" -> "drm_plane B"
+      }
+
+      subgraph cluster_connector {
+          style=dashed
+          label="Hotpluggable"
+
+          "drm_encoder A" -> "drm_connector A"
+          "drm_encoder B" -> "drm_connector B"
+      }
+   }
+
+The basic object structure KMS presents to userspace is fairly simple.
+Framebuffers (represented by :c:type:`struct drm_framebuffer <drm_framebuffer>`,
+see `Frame Buffer Abstraction`_) feed into planes. Multiple (or just one, or
+even no) planes feed their pixel data into a CRTC (represented by
+:c:type:`struct drm_crtc <drm_crtc>`, see `CRTC Abstraction`_) for blending. The
+precise blending step is explained in more detail in `Plane Composition
+Properties`_ and related chapter.
+
+For the output routing the first step are Encoders (represented by
+:c:type:`struct drm_encoder <drm_encoder>`, see `Encoder Abstraction`_). Those
+are really just internal artifacts of the helper libraries used to implement KMS
+drivers. But unfortunately encoders have been exposed to userspace. Besides that
+they make it unecessarily more complicated for userspace to figure out which
+connections between a CRTC and a connector, and what kind of cloning is
+supported, they serve no purpose in the userspace API. Futhermore the exposed
+restrictions are often wrongly set by drivers, and in many cases not powerful
+enough to express the real restrictions. A CRTC can be connected to multiple
+Encoders, but for an active CRTC there must be at least one encoder.
+
+The final, and real, endpoint in the display chain is the connector (represented
+by :c:type:`struct drm_connector <drm_connector>`, see `Connector
+Abstraction`_). Connectors can have different possible encoders, but the kernel
+driver does this selection. The use case is DVI, which could switch between an
+analog and a digital encoder. There is only ever one active connector for any
+encoder.
+
+Internally the output pipeline is a bit more complex and matches todays hardware
+more closely:
+
+.. kernel-render:: DOT
+   :alt: KMS Output Pipeline
+   :caption: KMS Output Pipeline
+
+   digraph "Output Pipeline" {
+      node [shape=box]
+
+      subgraph {
+          "drm_crtc" [bgcolor=grey style=filled]
+      }
+
+      subgraph cluster_internal {
+          style=dashed
+          label="Internal Pipeline"
+          {
+              node [bgcolor=grey style=filled]
+              "drm_encoder A";
+              "drm_encoder B";
+              "drm_encoder C";
+          }
+
+          {
+              node [bgcolor=grey style=filled]
+              "drm_encoder B" -> "drm_bridge B"
+              "drm_encoder C" -> "drm_bridge C1"
+              "drm_bridge C1" -> "drm_bridge C2";
+          }
+      }
+
+      "drm_crtc" -> "drm_encoder A"
+      "drm_crtc" -> "drm_encoder B"
+      "drm_crtc" -> "drm_encoder C"
+
+
+      subgraph cluster_output {
+          style=dashed
+          label="Outputs"
+
+          "drm_encoder A" -> "drm_connector A";
+          "drm_bridge B" -> "drm_connector B";
+          "drm_bridge C2" -> "drm_connector C";
+
+          "drm_panel"
+      }
+   }
+
+Internally two additional helper objects come into play. First, to be able to
+share code for encoders (sometimes on the same SoC, sometimes off-chip) one or
+more :ref:`drm_bridges` (represented by :c:type:`struct drm_bridge
+<drm_bridge>`) can be linked to an encoder. This link is static and cannot be
+changed, which means the cross-bar (if there is any) needs to be mapped between
+the CRTC and any encoders. Often for drivers with bridges there's no code left
+at the encoder level. Atomic drivers can leave out all the encoder callbacks to
+essentially only leave a dummy routing object behind, which is needed for
+backwards compatibility since encoders are exposed to userspace.
+
+The second objects are panels, represented by :c:type:`struct drm_panel
+<drm_panel>`, see :ref:`drm_panel_helper`. Panels do not have a fixed binding
+point, but are generally linked to the driver private structure which embeds
+:c:type:`struct drm_connector <drm_connector>`.
+
+Note that currently the bridge chaining and interactions with connectors and
+panels are still in-flux and not really fully sorted out yet.
+
 KMS Core Structures and Functions
 =================================
 
-- 
2.11.0

[PATCH 4/6] drm/doc: Consistent kerneldoc include order

[Daniel Vetter]

First overview text (if there is any), then headers (since generally
you want to start out with the data structures), then all the other
stuff with functions.

Most of this is pre-shpinx, since with the old docbook only the
overview stuff was pulled in directly. Everything else was put in a
per-section index, so include order didn't really matter.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-internals.rst   |  6 +++---
 Documentation/gpu/drm-kms-helpers.rst | 22 +++++++++++-----------
 Documentation/gpu/drm-kms.rst         | 30 +++++++++++++++---------------
 Documentation/gpu/drm-mm.rst          | 24 ++++++++++++------------
 4 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
index e35920db1f4c..29d6bf7bb1ac 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -140,12 +140,12 @@ Device Instance and Driver Handling
 .. kernel-doc:: drivers/gpu/drm/drm_drv.c
    :doc: driver instance overview
 
-.. kernel-doc:: drivers/gpu/drm/drm_drv.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_drv.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_drv.c
+   :export:
+
 Driver Load
 -----------
 
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 012b6ff3ec3f..050ebe81d256 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -37,10 +37,10 @@ Modeset Helper Reference for Common Vtables
 ===========================================
 
 .. kernel-doc:: include/drm/drm_modeset_helper_vtables.h
-   :internal:
+   :doc: overview
 
 .. kernel-doc:: include/drm/drm_modeset_helper_vtables.h
-   :doc: overview
+   :internal:
 
 Atomic Modeset Helper Functions Reference
 =========================================
@@ -84,27 +84,27 @@ Legacy CRTC/Modeset Helper Functions Reference
 Simple KMS Helper Reference
 ===========================
 
+.. kernel-doc:: drivers/gpu/drm/drm_simple_kms_helper.c
+   :doc: overview
+
 .. kernel-doc:: include/drm/drm_simple_kms_helper.h
    :internal:
 
 .. kernel-doc:: drivers/gpu/drm/drm_simple_kms_helper.c
    :export:
 
-.. kernel-doc:: drivers/gpu/drm/drm_simple_kms_helper.c
-   :doc: overview
-
 fbdev Helper Functions Reference
 ================================
 
 .. kernel-doc:: drivers/gpu/drm/drm_fb_helper.c
    :doc: fbdev helpers
 
-.. kernel-doc:: drivers/gpu/drm/drm_fb_helper.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_fb_helper.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_fb_helper.c
+   :export:
+
 Framebuffer CMA Helper Functions Reference
 ==========================================
 
@@ -146,15 +146,15 @@ Bridge Helper Reference
 Panel Helper Reference
 ======================
 
+.. kernel-doc:: drivers/gpu/drm/drm_panel.c
+   :doc: drm panel
+
 .. kernel-doc:: include/drm/drm_panel.h
    :internal:
 
 .. kernel-doc:: drivers/gpu/drm/drm_panel.c
    :export:
 
-.. kernel-doc:: drivers/gpu/drm/drm_panel.c
-   :doc: drm panel
-
 Display Port Helper Functions Reference
 =======================================
 
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 2bdd266da4a9..05a346492e07 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -152,12 +152,12 @@ panels are still in-flux and not really fully sorted out yet.
 KMS Core Structures and Functions
 =================================
 
-.. kernel-doc:: drivers/gpu/drm/drm_mode_config.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_mode_config.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_mode_config.c
+   :export:
+
 Modeset Base Object Abstraction
 ===============================
 
@@ -170,21 +170,21 @@ Modeset Base Object Abstraction
 Atomic Mode Setting Function Reference
 ======================================
 
-.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_atomic.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_atomic.c
+   :export:
+
 CRTC Abstraction
 ================
 
-.. kernel-doc:: drivers/gpu/drm/drm_crtc.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_crtc.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_crtc.c
+   :export:
+
 Frame Buffer Abstraction
 ========================
 
@@ -194,12 +194,12 @@ Frame Buffer Abstraction
 Frame Buffer Functions Reference
 --------------------------------
 
-.. kernel-doc:: drivers/gpu/drm/drm_framebuffer.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_framebuffer.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_framebuffer.c
+   :export:
+
 DRM Format Handling
 ===================
 
@@ -502,8 +502,8 @@ operation handler.
 Vertical Blanking and Interrupt Handling Functions Reference
 ------------------------------------------------------------
 
-.. kernel-doc:: drivers/gpu/drm/drm_irq.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_irq.h
    :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_irq.c
+   :export:
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index cb5daffcd6be..1ac599559e28 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -342,36 +342,36 @@ from the client in libdrm.
 GEM Function Reference
 ----------------------
 
-.. kernel-doc:: drivers/gpu/drm/drm_gem.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_gem.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_gem.c
+   :export:
+
 GEM CMA Helper Functions Reference
 ----------------------------------
 
 .. kernel-doc:: drivers/gpu/drm/drm_gem_cma_helper.c
    :doc: cma helpers
 
-.. kernel-doc:: drivers/gpu/drm/drm_gem_cma_helper.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_gem_cma_helper.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_gem_cma_helper.c
+   :export:
+
 VMA Offset Manager
 ==================
 
 .. kernel-doc:: drivers/gpu/drm/drm_vma_manager.c
    :doc: vma offset manager
 
-.. kernel-doc:: drivers/gpu/drm/drm_vma_manager.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_vma_manager.h
    :internal:
 
+.. kernel-doc:: drivers/gpu/drm/drm_vma_manager.c
+   :export:
+
 PRIME Buffer Sharing
 ====================
 
@@ -447,8 +447,8 @@ LRU Scan/Eviction Support
 DRM MM Range Allocator Function References
 ------------------------------------------
 
-.. kernel-doc:: drivers/gpu/drm/drm_mm.c
-   :export:
-
 .. kernel-doc:: include/drm/drm_mm.h
    :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_mm.c
+   :export:
-- 
2.11.0

[PATCH 5/6] drm/doc: diagram for mode objects and properties

[Daniel Vetter]

Resulted in confusion a few times in the past.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/gpu/drm-kms.rst | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 05a346492e07..07d288fa37b2 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -161,6 +161,28 @@ KMS Core Structures and Functions
 Modeset Base Object Abstraction
 ===============================
 
+.. kernel-render:: DOT
+   :alt: Mode Objects and Properties
+   :caption: Mode Objects and Properties
+
+   digraph {
+      node [shape=box]
+
+      "drm_property A" -> "drm_mode_object A"
+      "drm_property A" -> "drm_mode_object B"
+      "drm_property B" -> "drm_mode_object A"
+   }
+
+The base structure for all KMS objects is :c:type:`struct drm_mode_object
+<drm_mode_object>`. One of the base services is provides is tracking properties,
+which are especially important for the atomic IOCTL (see `Atomic Mode
+Setting`_). The somewhat surprising part here is that properties are not
+directly instantiated on each object, but free-standing mode objects themselves,
+represented by :c:type:`struct drm_property <drm_property>`, which only specify
+the type and value range of a property. Any given property can be attached
+multiple times to different objects using :c:func:`drm_object_attach_property()
+<drm_object_attach_property>`.
+
 .. kernel-doc:: include/drm/drm_mode_object.h
    :internal:
 
-- 
2.11.0

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

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

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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 07d288fa37b2..35c41cf84a1b 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:
+
+- 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,
+  simplifying conversion of existing drivers. But auditing drivers for
+  correctness of the atomic_check code because really hard with that.
+
+- 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 consequence for the atomic design:
+
+- 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
+  driver and modeset objects. This way rolling back an update boils down to
+  releasing memory and unreference objects like framebuffers.
+
+Read on in this chapter, and also in :ref:`drm_atomic_helper` for more detailed
+coverage of specific topics.
+
 Atomic Mode Setting Function Reference
-======================================
+--------------------------------------
 
 .. kernel-doc:: include/drm/drm_atomic.h
    :internal:
-- 
2.11.0

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

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

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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 07d288fa37b2..35c41cf84a1b 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:
+
+- 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,
+  simplifying conversion of existing drivers. But auditing drivers for
+  correctness of the atomic_check code because really hard with that.
+
+- 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 consequence for the atomic design:
+
+- 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
+  driver and modeset objects. This way rolling back an update boils down to
+  releasing memory and unreference objects like framebuffers.
+
+Read on in this chapter, and also in :ref:`drm_atomic_helper` for more detailed
+coverage of specific topics.
+
 Atomic Mode Setting Function Reference
-======================================
+--------------------------------------
 
 .. kernel-doc:: include/drm/drm_atomic.h
    :internal:
-- 
2.11.0

[PATCH 3/6] drm/doc: Add KMS overview graphs

[Daniel Vetter]

Oh, the shiny and pretties!

v2: Review from Laurent.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-kms-helpers.rst |   4 +
 Documentation/gpu/drm-kms.rst         | 134 +++++++++++++++++++++++++++++++++-
 2 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 03040aa14fe8..012b6ff3ec3f 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -114,6 +114,8 @@ Framebuffer CMA Helper Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_fb_cma_helper.c
    :export:
 
+.. _drm_bridges:
+
 Bridges
 =======
 
@@ -139,6 +141,8 @@ Bridge Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
    :export:
 
+.. _drm_panel_helper:
+
 Panel Helper Reference
 ======================
 
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4d4068855ec4..bcf475f86e83 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -15,7 +15,139 @@ be setup by initializing the following fields.
 -  struct drm_mode_config_funcs \*funcs;
    Mode setting functions.
 
-Mode Configuration
+Overview
+========
+
+.. kernel-render:: DOT
+   :alt: KMS Display Pipeline
+   :caption: KMS Display Pipeline Overview
+
+   digraph "KMS" {
+      node [shape=box]
+
+      subgraph cluster_static {
+          style=dashed
+          label="Static Objects"
+
+          node [bgcolor=grey style=filled]
+          "drm_plane A" -> "drm_crtc"
+          "drm_plane B" -> "drm_crtc"
+          "drm_crtc" -> "drm_encoder A"
+          "drm_crtc" -> "drm_encoder B"
+      }
+
+      subgraph cluster_user_created {
+          style=dashed
+          label="Userspace-Created"
+
+          node [shape=oval]
+          "drm_framebuffer 1" -> "drm_plane A"
+          "drm_framebuffer 2" -> "drm_plane B"
+      }
+
+      subgraph cluster_connector {
+          style=dashed
+          label="Hotpluggable"
+
+          "drm_encoder A" -> "drm_connector A"
+          "drm_encoder B" -> "drm_connector B"
+      }
+   }
+
+The basic object structure KMS presents to userspace is fairly simple.
+Framebuffers (represented by :c:type:`struct drm_framebuffer <drm_framebuffer>`,
+see `Frame Buffer Abstraction`_) feed into planes. One or more (or even no)
+planes feed their pixel data into a CRTC (represented by :c:type:`struct
+drm_crtc <drm_crtc>`, see `CRTC Abstraction`_) for blending. The precise
+blending step is explained in more detail in `Plane Composition Properties`_ and
+related chapters.
+
+For the output routing the first step is encoders (represented by
+:c:type:`struct drm_encoder <drm_encoder>`, see `Encoder Abstraction`_). Those
+are really just internal artifacts of the helper libraries used to implement KMS
+drivers. Besides that they make it unecessarily more complicated for userspace
+to figure out which connections between a CRTC and a connector are possible, and
+what kind of cloning is supported, they serve no purpose in the userspace API.
+Unfortunately encoders have been exposed to userspace, hence can't remove them
+at this point.  Futhermore the exposed restrictions are often wrongly set by
+drivers, and in many cases not powerful enough to express the real restrictions.
+A CRTC can be connected to multiple encoders, and for an active CRTC there must
+be at least one encoder.
+
+The final, and real, endpoint in the display chain is the connector (represented
+by :c:type:`struct drm_connector <drm_connector>`, see `Connector
+Abstraction`_). Connectors can have different possible encoders, but the kernel
+driver selects which encoder to use for each connector. The use case is DVI,
+which could switch between an analog and a digital encoder. Encoders can also
+drive multiple different connectors. There is exactly one active connector for
+every active encoder.
+
+Internally the output pipeline is a bit more complex and matches today's
+hardware more closely:
+
+.. kernel-render:: DOT
+   :alt: KMS Output Pipeline
+   :caption: KMS Output Pipeline
+
+   digraph "Output Pipeline" {
+      node [shape=box]
+
+      subgraph {
+          "drm_crtc" [bgcolor=grey style=filled]
+      }
+
+      subgraph cluster_internal {
+          style=dashed
+          label="Internal Pipeline"
+          {
+              node [bgcolor=grey style=filled]
+              "drm_encoder A";
+              "drm_encoder B";
+              "drm_encoder C";
+          }
+
+          {
+              node [bgcolor=grey style=filled]
+              "drm_encoder B" -> "drm_bridge B"
+              "drm_encoder C" -> "drm_bridge C1"
+              "drm_bridge C1" -> "drm_bridge C2";
+          }
+      }
+
+      "drm_crtc" -> "drm_encoder A"
+      "drm_crtc" -> "drm_encoder B"
+      "drm_crtc" -> "drm_encoder C"
+
+
+      subgraph cluster_output {
+          style=dashed
+          label="Outputs"
+
+          "drm_encoder A" -> "drm_connector A";
+          "drm_bridge B" -> "drm_connector B";
+          "drm_bridge C2" -> "drm_connector C";
+
+          "drm_panel"
+      }
+   }
+
+Internally two additional helper objects come into play. First, to be able to
+share code for encoders (sometimes on the same SoC, sometimes off-chip) one or
+more :ref:`drm_bridges` (represented by :c:type:`struct drm_bridge
+<drm_bridge>`) can be linked to an encoder. This link is static and cannot be
+changed, which means the cross-bar (if there is any) needs to be mapped between
+the CRTC and any encoders. Often for drivers with bridges there's no code left
+at the encoder level. Atomic drivers can leave out all the encoder callbacks to
+essentially only leave a dummy routing object behind, which is needed for
+backwards compatibility since encoders are exposed to userspace.
+
+The second object is for panels, represented by :c:type:`struct drm_panel
+<drm_panel>`, see :ref:`drm_panel_helper`. Panels do not have a fixed binding
+point, but are generally linked to the driver private structure that embeds
+:c:type:`struct drm_connector <drm_connector>`.
+
+Note that currently the bridge chaining and interactions with connectors and
+panels are still in-flux and not really fully sorted out yet.
 
 KMS Core Structures and Functions
 =================================
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm/doc: Add KMS overview graphs

[Daniel Vetter]

On Thu, Mar 02, 2017 at 04:34:18PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Tuesday 28 Feb 2017 18:13:16 Daniel Vetter wrote:
> > Oh, the shiny and pretties!
> > 
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Overall this looks good to me, please see below for a few minor issues.
> 
> > ---
> >  Documentation/gpu/drm-kms-helpers.rst |   4 ++
> >  Documentation/gpu/drm-kms.rst         | 132 +++++++++++++++++++++++++++++++
> >  2 files changed, 136 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-kms-helpers.rst
> > b/Documentation/gpu/drm-kms-helpers.rst index 03040aa14fe8..012b6ff3ec3f
> > 100644
> > --- a/Documentation/gpu/drm-kms-helpers.rst
> > +++ b/Documentation/gpu/drm-kms-helpers.rst
> > @@ -114,6 +114,8 @@ Framebuffer CMA Helper Functions Reference
> >  .. kernel-doc:: drivers/gpu/drm/drm_fb_cma_helper.c
> > 
> >     :export:
> > +.. _drm_bridges:
> > +
> >  Bridges
> >  =======
> > 
> > @@ -139,6 +141,8 @@ Bridge Helper Reference
> >  .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
> > 
> >     :export:
> > +.. _drm_panel_helper:
> > +
> >  Panel Helper Reference
> >  ======================
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 4d4068855ec4..87d8162c9a34 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -17,6 +17,138 @@ be setup by initializing the following fields.
> > 
> >  Mode Configuration
> 
> Not part of this patch, but this line doesn't feel like it's where it shoul 
> dbe.

Yeah that's an accident from a previous patch, I've removed it.
> 
> > +Overview
> > +========
> > +
> > +.. kernel-render:: DOT
> > +   :alt: KMS Display Pipeline
> > +   :caption: KMS Display Pipeline Overview
> > +
> > +   digraph "KMS" {
> > +      node [shape=box]
> > +
> > +      subgraph cluster_static {
> > +          style=dashed
> > +          label="Static Objects"
> > +
> > +          node [bgcolor=grey style=filled]
> > +          "drm_plane A" -> "drm_crtc"
> > +          "drm_plane B" -> "drm_crtc"
> > +          "drm_crtc" -> "drm_encoder A"
> > +          "drm_crtc" -> "drm_encoder B"
> > +      }
> > +
> > +      subgraph cluster_user_created {
> > +          style=dashed
> > +          label="Userspace-Created"
> > +
> > +          node [shape=oval]
> > +          "drm_framebuffer 1" -> "drm_plane A"
> > +          "drm_framebuffer 2" -> "drm_plane B"
> > +      }
> > +
> > +      subgraph cluster_connector {
> > +          style=dashed
> > +          label="Hotpluggable"
> > +
> > +          "drm_encoder A" -> "drm_connector A"
> > +          "drm_encoder B" -> "drm_connector B"
> > +      }
> > +   }
> > +
> > +The basic object structure KMS presents to userspace is fairly simple.
> > +Framebuffers (represented by :c:type:`struct drm_framebuffer
> > <drm_framebuffer>`, +see `Frame Buffer Abstraction`_) feed into planes.
> > Multiple (or just one, or +even no) planes
> 
> I'd say "One or multiple (or even no) planes", but that's up to you.
> 
> > feed their pixel data into a
> > CRTC (represented by +:c:type:`struct drm_crtc <drm_crtc>`, see `CRTC
> > Abstraction`_) for blending. The +precise blending step is explained in
> > more detail in `Plane Composition +Properties`_ and related chapter.
> 
> s/chapter/chapters/ ? Or /related chapter/the related chapter/ ?
> 
> > +
> > +For the output routing the first step are Encoders (represented by
> 
> s/are/is/
> 
> > +:c:type:`struct drm_encoder <drm_encoder>`, see `Encoder Abstraction`_).
> > Those +are really just internal artifacts of the helper libraries used to
> > implement KMS +drivers. But unfortunately encoders have been exposed to
> 
> s/But u/U/
> 
> (http://blog.oxforddictionaries.com/2012/01/can-i-start-a-sentence-with-a-conjunction/)
> 
> I'd actually move the sentence towards the end of the paragraph and modify it 
> to
> 
> "Unfortunately connectors have been exposed to userspace, so we can't remove 
> them at this point."
> 
> or something similar.
> 
> > userspace. Besides that +they make it unecessarily more complicated for
> > userspace to figure out which +connections between a CRTC and a connector,
> 
> I think you're missing a verb. s/which connections/which connections are 
> possible/ ?
> 
> > and what kind of cloning is +supported, they serve no purpose in the
> > userspace API. Futhermore the exposed +restrictions are often wrongly set
> > by drivers, and in many cases not powerful +enough to express the real
> > restrictions.
> 
> I'd move the "But" sentence here, and possible start a new paragraph.
> 
> > A CRTC can be connected to multiple +Encoders, but for an
> 
> s/but/and/
> 
> > active CRTC there must be at least one encoder.
> > +
> > +The final, and real, endpoint in the display chain is the connector
> > (represented +by :c:type:`struct drm_connector <drm_connector>`, see
> > `Connector +Abstraction`_). Connectors can have different possible
> > encoders, but the kernel +driver does this selection.
> 
> s/but/and/
> s/does this selection/selects which encoder to use for each connector/ ?
> 
> > The use case is DVI,
> > which could switch between an +analog and a digital encoder. There is only
> > ever one active connector for any +encoder.
> 
> Isn't it the other way around, a single encoder for any connector ?

For each active connector theres exactly one active encoder. The possible
linking is n:m. I've clarified this.

All other suggestions applied, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm/doc: Add KMS overview graphs

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Tuesday 28 Feb 2017 18:13:16 Daniel Vetter wrote:
> Oh, the shiny and pretties!
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Overall this looks good to me, please see below for a few minor issues.

> ---
>  Documentation/gpu/drm-kms-helpers.rst |   4 ++
>  Documentation/gpu/drm-kms.rst         | 132 +++++++++++++++++++++++++++++++
>  2 files changed, 136 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst
> b/Documentation/gpu/drm-kms-helpers.rst index 03040aa14fe8..012b6ff3ec3f
> 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -114,6 +114,8 @@ Framebuffer CMA Helper Functions Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_fb_cma_helper.c
> 
>     :export:
> +.. _drm_bridges:
> +
>  Bridges
>  =======
> 
> @@ -139,6 +141,8 @@ Bridge Helper Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
> 
>     :export:
> +.. _drm_panel_helper:
> +
>  Panel Helper Reference
>  ======================
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 4d4068855ec4..87d8162c9a34 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -17,6 +17,138 @@ be setup by initializing the following fields.
> 
>  Mode Configuration

Not part of this patch, but this line doesn't feel like it's where it shoul 
dbe.

> +Overview
> +========
> +
> +.. kernel-render:: DOT
> +   :alt: KMS Display Pipeline
> +   :caption: KMS Display Pipeline Overview
> +
> +   digraph "KMS" {
> +      node [shape=box]
> +
> +      subgraph cluster_static {
> +          style=dashed
> +          label="Static Objects"
> +
> +          node [bgcolor=grey style=filled]
> +          "drm_plane A" -> "drm_crtc"
> +          "drm_plane B" -> "drm_crtc"
> +          "drm_crtc" -> "drm_encoder A"
> +          "drm_crtc" -> "drm_encoder B"
> +      }
> +
> +      subgraph cluster_user_created {
> +          style=dashed
> +          label="Userspace-Created"
> +
> +          node [shape=oval]
> +          "drm_framebuffer 1" -> "drm_plane A"
> +          "drm_framebuffer 2" -> "drm_plane B"
> +      }
> +
> +      subgraph cluster_connector {
> +          style=dashed
> +          label="Hotpluggable"
> +
> +          "drm_encoder A" -> "drm_connector A"
> +          "drm_encoder B" -> "drm_connector B"
> +      }
> +   }
> +
> +The basic object structure KMS presents to userspace is fairly simple.
> +Framebuffers (represented by :c:type:`struct drm_framebuffer
> <drm_framebuffer>`, +see `Frame Buffer Abstraction`_) feed into planes.
> Multiple (or just one, or +even no) planes

I'd say "One or multiple (or even no) planes", but that's up to you.

> feed their pixel data into a
> CRTC (represented by +:c:type:`struct drm_crtc <drm_crtc>`, see `CRTC
> Abstraction`_) for blending. The +precise blending step is explained in
> more detail in `Plane Composition +Properties`_ and related chapter.

s/chapter/chapters/ ? Or /related chapter/the related chapter/ ?

> +
> +For the output routing the first step are Encoders (represented by

s/are/is/

> +:c:type:`struct drm_encoder <drm_encoder>`, see `Encoder Abstraction`_).
> Those +are really just internal artifacts of the helper libraries used to
> implement KMS +drivers. But unfortunately encoders have been exposed to

s/But u/U/

(http://blog.oxforddictionaries.com/2012/01/can-i-start-a-sentence-with-a-conjunction/)

I'd actually move the sentence towards the end of the paragraph and modify it 
to

"Unfortunately connectors have been exposed to userspace, so we can't remove 
them at this point."

or something similar.

> userspace. Besides that +they make it unecessarily more complicated for
> userspace to figure out which +connections between a CRTC and a connector,

I think you're missing a verb. s/which connections/which connections are 
possible/ ?

> and what kind of cloning is +supported, they serve no purpose in the
> userspace API. Futhermore the exposed +restrictions are often wrongly set
> by drivers, and in many cases not powerful +enough to express the real
> restrictions.

I'd move the "But" sentence here, and possible start a new paragraph.

> A CRTC can be connected to multiple +Encoders, but for an

s/but/and/

> active CRTC there must be at least one encoder.
> +
> +The final, and real, endpoint in the display chain is the connector
> (represented +by :c:type:`struct drm_connector <drm_connector>`, see
> `Connector +Abstraction`_). Connectors can have different possible
> encoders, but the kernel +driver does this selection.

s/but/and/
s/does this selection/selects which encoder to use for each connector/ ?

> The use case is DVI,
> which could switch between an +analog and a digital encoder. There is only
> ever one active connector for any +encoder.

Isn't it the other way around, a single encoder for any connector ?

> +Internally the output pipeline is a bit more complex and matches todays

s/todays/today's/

> hardware +more closely:
> +
> +.. kernel-render:: DOT
> +   :alt: KMS Output Pipeline
> +   :caption: KMS Output Pipeline
> +
> +   digraph "Output Pipeline" {
> +      node [shape=box]
> +
> +      subgraph {
> +          "drm_crtc" [bgcolor=grey style=filled]
> +      }
> +
> +      subgraph cluster_internal {
> +          style=dashed
> +          label="Internal Pipeline"
> +          {
> +              node [bgcolor=grey style=filled]
> +              "drm_encoder A";
> +              "drm_encoder B";
> +              "drm_encoder C";
> +          }
> +
> +          {
> +              node [bgcolor=grey style=filled]
> +              "drm_encoder B" -> "drm_bridge B"
> +              "drm_encoder C" -> "drm_bridge C1"
> +              "drm_bridge C1" -> "drm_bridge C2";
> +          }
> +      }
> +
> +      "drm_crtc" -> "drm_encoder A"
> +      "drm_crtc" -> "drm_encoder B"
> +      "drm_crtc" -> "drm_encoder C"
> +
> +
> +      subgraph cluster_output {
> +          style=dashed
> +          label="Outputs"
> +
> +          "drm_encoder A" -> "drm_connector A";
> +          "drm_bridge B" -> "drm_connector B";
> +          "drm_bridge C2" -> "drm_connector C";
> +
> +          "drm_panel"
> +      }
> +   }
> +
> +Internally two additional helper objects come into play. First, to be able
> to +share code for encoders (sometimes on the same SoC, sometimes off-chip)
> one or +more :ref:`drm_bridges` (represented by :c:type:`struct drm_bridge
> +<drm_bridge>`) can be linked to an encoder. This link is static and cannot
> be +changed, which means the cross-bar (if there is any) needs to be mapped
> between +the CRTC and any encoders. Often for drivers with bridges there's
> no code left +at the encoder level. Atomic drivers can leave out all the
> encoder callbacks to +essentially only leave a dummy routing object behind,
> which is needed for +backwards compatibility since encoders are exposed to

I would have written "backward compatibility" but after checking I'm not too 
sure anymore. Documentation/ contains the same number of occurrences of each 
(at least after this patch, so it's a win for "backward compatibility" with a 
very thin margin in current mainline :-)).

> userspace.
> +
> +The second objects are panels, represented by :c:type:`struct drm_panel

Using a plural here with "second" seems strange to me but I might be wrong.

> +<drm_panel>`, see :ref:`drm_panel_helper`. Panels do not have a fixed
> binding +point, but are generally linked to the driver private structure
> which embeds +:c:type:`struct drm_connector <drm_connector>`.

s/which/that/

> +
> +Note that currently the bridge chaining and interactions with connectors
> and +panels are still in-flux and not really fully sorted out yet.
> +
>  KMS Core Structures and Functions
>  =================================

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm/doc: Add KMS overview graphs

[Gabriel Krisman Bertazi]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Oh, the shiny and pretties!
>

Very nice!

Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>


-- 
Gabriel Krisman Bertazi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] drm/doc: Add KMS overview graphs

[Daniel Vetter]

Oh, the shiny and pretties!

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-kms-helpers.rst |   4 ++
 Documentation/gpu/drm-kms.rst         | 132 ++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 03040aa14fe8..012b6ff3ec3f 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -114,6 +114,8 @@ Framebuffer CMA Helper Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_fb_cma_helper.c
    :export:
 
+.. _drm_bridges:
+
 Bridges
 =======
 
@@ -139,6 +141,8 @@ Bridge Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_bridge.c
    :export:
 
+.. _drm_panel_helper:
+
 Panel Helper Reference
 ======================
 
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4d4068855ec4..87d8162c9a34 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -17,6 +17,138 @@ be setup by initializing the following fields.
 
 Mode Configuration
 
+Overview
+========
+
+.. kernel-render:: DOT
+   :alt: KMS Display Pipeline
+   :caption: KMS Display Pipeline Overview
+
+   digraph "KMS" {
+      node [shape=box]
+
+      subgraph cluster_static {
+          style=dashed
+          label="Static Objects"
+
+          node [bgcolor=grey style=filled]
+          "drm_plane A" -> "drm_crtc"
+          "drm_plane B" -> "drm_crtc"
+          "drm_crtc" -> "drm_encoder A"
+          "drm_crtc" -> "drm_encoder B"
+      }
+
+      subgraph cluster_user_created {
+          style=dashed
+          label="Userspace-Created"
+
+          node [shape=oval]
+          "drm_framebuffer 1" -> "drm_plane A"
+          "drm_framebuffer 2" -> "drm_plane B"
+      }
+
+      subgraph cluster_connector {
+          style=dashed
+          label="Hotpluggable"
+
+          "drm_encoder A" -> "drm_connector A"
+          "drm_encoder B" -> "drm_connector B"
+      }
+   }
+
+The basic object structure KMS presents to userspace is fairly simple.
+Framebuffers (represented by :c:type:`struct drm_framebuffer <drm_framebuffer>`,
+see `Frame Buffer Abstraction`_) feed into planes. Multiple (or just one, or
+even no) planes feed their pixel data into a CRTC (represented by
+:c:type:`struct drm_crtc <drm_crtc>`, see `CRTC Abstraction`_) for blending. The
+precise blending step is explained in more detail in `Plane Composition
+Properties`_ and related chapter.
+
+For the output routing the first step are Encoders (represented by
+:c:type:`struct drm_encoder <drm_encoder>`, see `Encoder Abstraction`_). Those
+are really just internal artifacts of the helper libraries used to implement KMS
+drivers. But unfortunately encoders have been exposed to userspace. Besides that
+they make it unecessarily more complicated for userspace to figure out which
+connections between a CRTC and a connector, and what kind of cloning is
+supported, they serve no purpose in the userspace API. Futhermore the exposed
+restrictions are often wrongly set by drivers, and in many cases not powerful
+enough to express the real restrictions. A CRTC can be connected to multiple
+Encoders, but for an active CRTC there must be at least one encoder.
+
+The final, and real, endpoint in the display chain is the connector (represented
+by :c:type:`struct drm_connector <drm_connector>`, see `Connector
+Abstraction`_). Connectors can have different possible encoders, but the kernel
+driver does this selection. The use case is DVI, which could switch between an
+analog and a digital encoder. There is only ever one active connector for any
+encoder.
+
+Internally the output pipeline is a bit more complex and matches todays hardware
+more closely:
+
+.. kernel-render:: DOT
+   :alt: KMS Output Pipeline
+   :caption: KMS Output Pipeline
+
+   digraph "Output Pipeline" {
+      node [shape=box]
+
+      subgraph {
+          "drm_crtc" [bgcolor=grey style=filled]
+      }
+
+      subgraph cluster_internal {
+          style=dashed
+          label="Internal Pipeline"
+          {
+              node [bgcolor=grey style=filled]
+              "drm_encoder A";
+              "drm_encoder B";
+              "drm_encoder C";
+          }
+
+          {
+              node [bgcolor=grey style=filled]
+              "drm_encoder B" -> "drm_bridge B"
+              "drm_encoder C" -> "drm_bridge C1"
+              "drm_bridge C1" -> "drm_bridge C2";
+          }
+      }
+
+      "drm_crtc" -> "drm_encoder A"
+      "drm_crtc" -> "drm_encoder B"
+      "drm_crtc" -> "drm_encoder C"
+
+
+      subgraph cluster_output {
+          style=dashed
+          label="Outputs"
+
+          "drm_encoder A" -> "drm_connector A";
+          "drm_bridge B" -> "drm_connector B";
+          "drm_bridge C2" -> "drm_connector C";
+
+          "drm_panel"
+      }
+   }
+
+Internally two additional helper objects come into play. First, to be able to
+share code for encoders (sometimes on the same SoC, sometimes off-chip) one or
+more :ref:`drm_bridges` (represented by :c:type:`struct drm_bridge
+<drm_bridge>`) can be linked to an encoder. This link is static and cannot be
+changed, which means the cross-bar (if there is any) needs to be mapped between
+the CRTC and any encoders. Often for drivers with bridges there's no code left
+at the encoder level. Atomic drivers can leave out all the encoder callbacks to
+essentially only leave a dummy routing object behind, which is needed for
+backwards compatibility since encoders are exposed to userspace.
+
+The second objects are panels, represented by :c:type:`struct drm_panel
+<drm_panel>`, see :ref:`drm_panel_helper`. Panels do not have a fixed binding
+point, but are generally linked to the driver private structure which embeds
+:c:type:`struct drm_connector <drm_connector>`.
+
+Note that currently the bridge chaining and interactions with connectors and
+panels are still in-flux and not really fully sorted out yet.
+
 KMS Core Structures and Functions
 =================================
 
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel