On Tue, Feb 28, 2017 at 06:13:15PM +0100, Daniel Vetter wrote:

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 (by Daniel).

v3: Simplify stuff a bit (by Daniel):

• Remove path detection and setup/check code for that. In Documentation/media/Makefile we already simply use these tools, better to have one consolidated check if we want/need one. Also remove the convertsvg support, we require ImageMagick's convert already in the doc build, no need for a 2nd fallback.

• Use sphinx for depency tracking, remove hand-rolled version.

• Forward stderr from dot and convert, otherwise debugging issues with the diagrams is impossible.

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

Just to avoid confusion: Markus&I chatted a bit in private, and he volunteered to take over, and fix the few issues that need to be fixed in his original patch: - drop the convertsvg python fallback, since we need convert anyway - relative paths - stderr fwd - and the small polish I noticed in docs

I think with my blind hacking I botched the job too much :-)

Anyway, still hoping we could land this for 4.12, I really want the drm-side doc improvements this provides ...

Thanks, Daniel

---

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 | 7 +- Documentation/sphinx/kfigure.py | 442 ++++++++++++++++++++++++++++++++++ 6 files changed, 548 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 f6823cf01275..e3f537ce2935 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..e4f25038ef65 100644 --- a/Documentation/process/changes.rst +++ b/Documentation/process/changes.rst @@ -318,9 +318,10 @@ 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 ``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..8f749d389dae --- /dev/null +++ b/Documentation/sphinx/kfigure.py @@ -0,0 +1,442 @@ +# -*- 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:

•  - ``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 +import sys

+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 mkdir(folder, mode=0o775):

• if not path.isdir(folder):

•    os.makedirs(folder, mode)
  

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

+def setup(app):

• # 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
  

• )

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

•    # ----------
  

•    dst_fname = path.join(out_dir, fname + '.pdf')
  

•    if 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':
  

•        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}
  

•    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', '-T%s' % out_format, dot_fname]
• exit_code = 42
• with open(out_fname, "w") as out:

•    p = subprocess.Popen(
  

•        cmd, stdout = out, stderr = subprocess.PIPE )
  

•    nil, err = p.communicate()
  

•    sys.stderr.write(err)
  

•    exit_code = p.returncode
  

•    out.flush()
  

• return bool(exit_code == 0)

+def svg2pdf(svg_fname, pdf_fname):

• """Converts SVG to PDF with CairoSVG or ``convert(1)`` command.
• Uses ``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``)
• """
• cmd = [convert_cmd, svg_fname, pdf_fname]
• p = subprocess.Popen(

•    cmd, stdout = out, stderr = subprocess.PIPE )
  

• nil, err = p.communicate()
• sys.stderr.write(err)
• exit_code = p.returncode
• return bool(exit_code == 0)

+# 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):

•    env = self.state.document.settings.env
  

•    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))
  

•    # Tell sphinx of the dependency
  

•    env.note_dependency(os.path.abspath(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):

•    env = self.state.document.settings.env
  

•    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))
  

•    # Tell sphinx of the dependency
  

•    env.note_dependency(os.path.abspath(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 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

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-con...)

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

On Tue, Feb 28, 2017 at 03:48:47PM -0800, Eric Anholt wrote:

Daniel Vetter daniel.vetter@ffwll.ch writes:

...

I want to split up a few more things and document some details better (like how exactly to subclass drm_atomic_state). And maybe also split up the helpers a bit per-topic, but this should be a ok-ish start for better atomic overview.

One thing I failed at is getting DOT to layout the overview graph how I want it. The highlevel structure I want is:

Free-standing State

Current State

i.e. one over the other. Currently it lays it out side-by-side, but not even that really - "Current State" is somewhat offset below. Makes the graph look like garbage, and also way too wide for proper rendering. Ideas appreciated.

Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Harry Wentland harry.wentland@amd.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com

Thanks for writing these docs. I wish I had them back when I was starting vc4's atomic code! With the two little spelling nits fixed, 3-5 are:

Acked-by: Eric Anholt eric@anholt.net

A few copyedits on this one below, but it sounds like you don't want to push quite yet while you sort out the rendering.

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

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

...

---

Documentation/gpu/drm-kms-helpers.rst | 2 + Documentation/gpu/drm-kms.rst | 85 ++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 050ebe81d256..ac53c0b893f6 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -42,6 +42,8 @@ Modeset Helper Reference for Common Vtables .. kernel-doc:: include/drm/drm_modeset_helper_vtables.h :internal:

+.. _drm_atomic_helper:

Atomic Modeset Helper Functions Reference

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 20378881445f..979cee853bb1 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -189,8 +189,91 @@ multiple times to different objects using :c:func:`drm_object_attach_property() .. kernel-doc:: drivers/gpu/drm/drm_mode_object.c :export:

+Atomic Mode Setting +===================

+.. FIXME: The I want the below graph to be laid out so that the 2 subgraph

• clusters are below each another. But I failed.

+.. kernel-render:: DOT

• :alt: Mode Objects and Properties
• :caption: Mode Objects and Properties
• digraph {

•  node [shape=box]
  

•  subgraph cluster_state {
  

•      style=dashed
  

•      label="Free-standing state"
  

•      "drm_atomic_state" -> "duplicated drm_plane_state A"
  

•      "drm_atomic_state" -> "duplicated drm_plane_state B"
  

•      "drm_atomic_state" -> "duplicated drm_crtc_state"
  

•      "drm_atomic_state" -> "duplicated drm_connector_state"
  

•      "drm_atomic_state" -> "duplicated driver private state"
  

•  }
  

•  subgraph cluster_current {
  

•      style=dashed
  

•      label="Current state"
  

•      "drm_device" -> "drm_plane A"
  

•      "drm_device" -> "drm_plane B"
  

•      "drm_device" -> "drm_crtc"
  

•      "drm_device" -> "drm_connector"
  

•      "drm_device" -> "driver private object"
  

•      "drm_plane A" -> "drm_plane_state A"
  

•      "drm_plane B" -> "drm_plane_state B"
  

•      "drm_crtc" -> "drm_crtc_state"
  

•      "drm_connector" -> "drm_connector_state"
  

•      "driver private object" -> "driver private state"
  

•  }
  

•  "drm_atomic_state" -> "drm_device" [label="atomic_commit"]
  

• }

+Essentially atomic is transactional modeset (including planes) updates, but +compared to the usual transactional approach of try-commit and rollback on +failure atomic modesetting is a bit different:

Maybe reword:

"Atomic provides transactional modeset (including planes) updates, but a bit differently from the usual transactional approach of try-commit and rollback:"

...

+- Firstly, no hardware changes are allowed when the commit would fail. This

• allows us to implement the DRM_MODE_ATOMIC_TEST_ONLY mode, which allows
• userspace to explore whether certain configurations would work or not.

+- This would still allows setting and rollback of just the software state,

"allow"

...

• simplifying conversion of existing drivers. But auditing drivers for
• correctness of the atomic_check code because really hard with that.

s/because/becomes/?

...

+Taken all together there's two consequence for the atomic design:

"consequences"

...

+- The overall state is split up into per-object state structures:

• :c:type:`struct drm_plane_state <drm_plane_state>` for planes, :c:type:`struct
• drm_crtc_state <drm_crtc_state>` for CRTCs and :c:type:`struct
• drm_connector_state <drm_connector_state` for connectors. These are the only
• objects with userspace-visible and settable state. For internal state drivers
• can subclass these structures through embeddeding, or add entirely new state
• structures for their globally shared hardware functions.

+- An atomic update is assembled and validated as an enterily free-standing pile

• of structures within the :c:type:`drm_atomic_state <drm_atomic_state>`
• container. Again drivers can subclass that container for their own state
• structure tracking needs. Only when a state is commit is it applied to the

"is committed"

...

• driver and modeset objects. This way rolling back an update boils down to
• releasing memory and unreference objects like framebuffers.

"unreferencing"

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 krisman@collabora.co.uk Acked-by: Eric Anholt eric@anholt.net Cc: Eric Anholt eric@anholt.net 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 | 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 <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`

Missing >

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

s/enterily/entirely/

With this fixed,

Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com

(this applies to patch 3/6 too, I forgot to mention it there)

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 committed is it applied to the + driver and modeset objects. This way rolling back an update boils down to + releasing memory and unreferencing 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: