Nouveau - Oct 2020 - [RFC PATCH v2 0/3] Documentation: nouveau: Introduce some nouveau documentation

Other gpu drivers have some driver-specific documentation, so it would
nice if nouveau did as well.

This adds a bare-bones ReStructured Text document with sections for
module parameter documentation, an overview of the driver architecture,
a section for internal API documentation, and a glossary for
nouveau-specific terms.

To make the document a little less bare, I've included docblocks for
module parameters in nouveau_drm.c, as well as a header (selected at
random) so reviewers can have a better idea of how this looks. The
module parameter documentation is based on the existing documentation on
nouveau.freedesktop.org/wiki/ along with my own research for the
currently undocumented options. The header documentation ranges from
wild speculation to placeholder text.

I'm interested in any and all feedback. Does the layout make sense, or
would folks prefer a multi-page document. How much of the documentation
I wrote is flat out wrong?

If you want to see the HTML output, but don't want to build the docs,
it's at https://www.jcline.org/docs/8fe0b0869644/gpu/nouveau.html.

Signed-off-by: Jeremy Cline <jcline at redhat.com>
---
 Documentation/gpu/drivers.rst                 |   1 +
 Documentation/gpu/nouveau.rst                 | 129 ++++++++++++++
 .../drm/nouveau/include/nvkm/engine/disp.h    |  35 ++++
 drivers/gpu/drm/nouveau/nouveau_drm.c         | 157 ++++++++++++++++++
 4 files changed, 322 insertions(+)
 create mode 100644 Documentation/gpu/nouveau.rst

diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
index b4a0ed3ca961..783c749d12e0 100644
--- a/Documentation/gpu/drivers.rst
+++ b/Documentation/gpu/drivers.rst
@@ -9,6 +9,7 @@ GPU Driver Documentation
    i915
    mcde
    meson
+   nouveau
    pl111
    tegra
    tve200
diff --git a/Documentation/gpu/nouveau.rst b/Documentation/gpu/nouveau.rst
new file mode 100644
index 000000000000..7bbc2cd73e80
--- /dev/null
+++ b/Documentation/gpu/nouveau.rst
@@ -0,0 +1,129 @@
+======+Nouveau
+======+This document provides a general overview of the driver, contribution
details,
+internal API documentation, and glossary. It is strictly for the nouveau kernel
+driver. The userspace side of nouveau is documented at the `nouveau freedesktop
+wiki <https://nouveau.freedesktop.org/wiki/>`_.
+
+If you'd like to improve this documentation, great! Refer to the
:doc:`Sphinx
+introduction </doc-guide/sphinx>` and :doc:`/doc-guide/kernel-doc`
documents
+for details on how to do so.
+
+Module Parameters
+================+A number of module parameters are supported. Each parameter
requires a value.
+These can be passed via the kernel command line using the format
+"nouveau.<parameter-name>=<value>". Boolean values should
use 1 for true and 0
+for false.
+
+.. kernel-doc:: drivers/gpu/drm/nouveau/nouveau_drm.c
+
+
+Driver Overview
+==============+The driver is located in ``drivers/gpu/drm/nouveau/``.
+
+.. note:: The latest pending patches for nouveau are available as an
+   `out-of-tree driver <https://github.com/skeggsb/nouveau>`_.
+
+Within the driver, there are several distinct sections. The reason for this is
+that NVIDIA hardware is partitioned into "privileged" and
"user" blocks.
+
+The general architecture is described in the diagram below.
+
+.. kernel-render:: DOT
+   :alt: Nouveau Architecture Diagram
+   :caption: Nouveau Architecture Diagram
+
+   digraph "Nouveau" {
+      node [shape=box]
+      "Userspace" -> "DRM APIs"
+      "Userspace" -> "NVIF APIs"
+      "DRM APIs" -> "NVIF APIs"
+      "NVIF APIs" -> "NVKM APIs"
+      "NVKM APIs" -> "Hardware-specific Interfaces"
+   }
+
+
+NVKM
+----
+The NVidia Kernel Module (NVKM) is responsible for the low-level resource
+management of the privileged portions of the hardware. Almost all direct
+register access is performed in this layer. The functionality of the underlying
+hardware is exposed by NVKM as objects of a particular class, and in general
+these are identified with the same class IDs that NVIDIA uses.
+
+Some classes, such as :term:`channels`, have a block of registers associated
with
+them that are intended to be directly accessed by an unprivileged client. NVKM
+allows objects to be "mapped" by a client to support this.
+
+The NVKM layer sits closest to the hardware and services requests issued by
+clients.
+
+
+NVIF
+----
+Atop the NVKM sits the NVidia Interface (NVIF) library, which defines a client
+interface that can be used to interact with the NVKM server. NVIF is intended
+to be usable both in the kernel and in userspace. This is accomplished with
+drivers that implement the interface defined in struct nvif_driver. Clients
+within the kernel use with an NVIF client backed with
+struct nvif_driver_nvkm.
+
+This design allows userspace direct access to the registers of :term:`channels`
+it allocates and allows it to submit work to the GPU without going through the
+kernel.
+
+
+DRM
+---
+The DRM layer of nouveau uses the NVIF to implement the interfaces of a DRM
+driver, such as modesetting, command submission to :term:`channels`
+from userspace, synchronization between userspace clients, and so on.
+
+.. note:: All interaction with the NVKM layer inside the kernel should happen
+   through NVIF.  Historically this has not been the case, so there may still
+   be legacy code that bypasses NVIF and directly calls NVKM.
+
+Nouveau's DRM driver is defined in the aptly-named nouveau_drm.c file. The
+files in the driver directory's root provide all the functionality required
for
+the DRM driver. Kernel mode-setting is implemented in the dispnv* directories
+and is abstracted in the ``nouveau_display.h`` interface.
+
+For details on what is required to implement these interfaces interfaces, refer
+to the :doc:`drm-internals`, :doc:`drm-kms`, and :doc:`drm-uapi` documents.
+
+
+API
+==+This section includes the kernel-docs for nouveau APIs.
+
+
+NVKM
+----
+Privileged, low-level resource management interfaces.
+
+engine/disp.h
+~~~~~~~~~~~~~
+.. kernel-doc:: drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
+
+
+Glossary
+=======+There are a number of NVIDIA-specific terms in the code as well as the
+documentation.
+
+.. glossary::
+
+   EVO
+   NVD
+       In pre-Volta architectures, the Evolution (EVO) controller is used to
+       interact with display memory-mapped IO registers via a pushbuffer.  In
+       Volta architectures and newer, the NVDisplay controller takes the place
+       of the EVO controller, although it has slightly different semantics.
+
+   channels
+        Channels are hardware blocks that consumes methods from a
+        direct-memory-accessed command stream.
+
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
b/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
index 5a96c942d912..76b90d7ddfc6 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
@@ -1,22 +1,57 @@
 /* SPDX-License-Identifier: MIT */
+
+/**
+ * DOC: Overview
+ *
+ * Interfaces for working with the display engine.
+ */
+
 #ifndef __NVKM_DISP_H__
 #define __NVKM_DISP_H__
+
+/**
+ * nvkm_disp() - Get a &struct nvkm_disp from a &struct nvkm_engine.
+ *
+ * @p: A &struct nvkm_engine reference.
+ *
+ * Return: The &struct nvkm_disp that contains the given engine.
+ */
 #define nvkm_disp(p) container_of((p), struct nvkm_disp, engine)
 #include <core/engine.h>
 #include <core/event.h>
 
+/**
+ * struct nvkm_disp - Represents a display engine.
+ *
+ * This structure is for <some abstraction here>. It has <some
assumptions
+ * about its usage here>.
+ */
 struct nvkm_disp {
+    /** @func: Chipset-specific vtable for manipulating the display. */
 	const struct nvkm_disp_func *func;
+
+    /** @engine: The engine for this display. */
 	struct nvkm_engine engine;
 
+    /** @head: list of heads attached to this display. */
 	struct list_head head;
+
+    /** @ior: List of IO resources for this display. */
 	struct list_head ior;
+
+    /** @outp: List of outputs for this display. */
 	struct list_head outp;
+
+    /** @conn: List of connectors for this display. */
 	struct list_head conn;
 
+    /** @hpd: An event that fires when something happens I guess. */
 	struct nvkm_event hpd;
+
+    /** @vblank: An event that fires and has some relation to the vblank. */
 	struct nvkm_event vblank;
 
+    /** @client: The oproxy (?) client for this display. */
 	struct nvkm_oproxy *client;
 };
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 22d246acc5e5..c03c18eaee45 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -68,27 +68,184 @@
 #include "nouveau_svm.h"
 #include "nouveau_dmem.h"
 
+/**
+ * DOC: config (string)
+ *
+ * A configuration string used to adjust a variety of module behaviors. The
+ * value of this parameter should be a string of comma-separated key=value
+ * pairs.
+ *
+ * values marked as integers are parsed as hex when they include a leading 0x,
+ * octal when they include a leading 0, otherwise they are parsed as decimals.
+ *
+ * Supported key-value pairs include:
+ *
+ * * GP100MmuLayout (boolean): If false, the old pre-GP100 VMM backend is used;
+ *   defaults to true and is only valid on NV130-NV138 chips.
+ *
+ * * <engine-name> (boolean): disable the given engine; all engines are
enabled
+ *   by default.
+ *
+ * * NvAcrWpr (integer): ; defaults to -1
+ * * NvAcrWprCompare (boolean): ; defaults to false.
+ * * NvAcrWprPrevAddr (integer): ; defaults to 0.
+ *
+ * * NvBar2Halve (boolean): Halve the BAR2 size; defaults to false.
+ *
+ * * NvFanPWM (boolean): Force PWM for the fan; default is to auto-detect.
+ *
+ * * NvForcePost (boolean): Force the device to perform a POST; defaults to
false.
+ *
+ * * NvGrResetWar (boolean): Reset the GR for an extended period to work around
+ *   an issue where GR stops responding; defaults to true on certain Pascal
+ *   boards where it is a known problem.
+ *
+ * * NvGrUseFW (boolean): Load firmware for PGRAPH (valid on the NVC0, NVE0,
+ *   and NV110 families); defaults to false.
+ *
+ * * NvI2C (boolean): Use the nouveau-internal I2C bus driver rather than the
+ *   I2C bit-adapters; defaults to false.
+ *
+ * * NvMemExec (boolean): Perform memory reclocking; defaults to true.
+ *
+ * * NvMSI (boolean): Use MSI interrupts; defaults to true on chipsets that
+ *   support it..
+ *
+ * * NvMXMDCB (boolean): Sanitize DCB outputs from the VBIOS; defaults to true.
+ *
+ * * NvPCIE (boolean): Whether to use the PCI-E GART (valid on NV40 chips
only);
+ *   defaults to true.
+ *
+ * * NvPmShowAll (boolean): Report all perfmon signals in queries; defaults to
+ *   false.
+ *
+ * * NvPmUnnamed (boolean): Use the raw signal number rather than the name in
+ *   perfmon queries; defaults to the value of NvPmShowAll.
+ *
+ * * NvPmEnableGating (boolean): Enable clockgating for chipsets that support
+ *   it; defaults to false.
+ *
+ * * War00C800_0 (boolean): Enables the PGOB work-around on all GK10[467]
+ *   boards; defaults to true.
+ *
+ * * MmuDebugBufferSize (integer): Size of the MUU debug buffers; defaults to
+ *   the framebuffer page size.
+ *
+ * * NvAGP (integer): Force a particular AGP mode (0 to disable).
+ *
+ * * NvChipset (integer): Override the detected chipset; useful for
development.
+ *
+ * * NvFbBigPage (integer): Size of the framebuffer big page in bits
+ *   (e.g. 16 means 64KiB); default is chip-dependent.
+ *
+ * * NvBios (string): Specify the Video BIOS source; options include:
+ *
+ *   * "OpenFirmware"
+ *   * "PRAMIN"
+ *   * "PROM"
+ *   * "ACPI"
+ *   * "PCIROM"
+ *   * "PLATFORM"
+ *   * A file name passed on to request_firmware.
+ *
+ * * NvBoost (integer): Specify the Boost mode for Fermi and newer. Valid
+ *   options are:
+ *
+ *   * 0: base clocks (default)
+ *   * 1: boost clocks
+ *   * 2: max clocks
+ *
+ * * NvClkMode (string): Force a particular clock level on boot. This is
+ *   equivalent to passing both ``NvClkModeAC`` and ``NvClkModeDC``.
+ *
+ * * NvClkModeAC (string): Force a particular clock level when plugged in to a
+ *   power source.
+ *
+ * * NvClkModeDC (string): Force a particular clock level when running on
+ *   battery.
+ */
 MODULE_PARM_DESC(config, "option string to pass to driver core");
 static char *nouveau_config;
 module_param_named(config, nouveau_config, charp, 0400);
 
+/**
+ * DOC: debug (string)
+ *
+ * Like the "config" parameter, this is a string of comma-separated
key=values.
+ * Valid keys include:
+ *
+ * * CLIENT
+ * * <subdevice>
+ *
+ * The list of current sub-device and engine names is in the %nvkm_subdev_name
+ * array and is enumerated in &enum nvkm_devidx.
+ *
+ * Valid values are log levels to use for messages from the given key:
+ *
+ * * fatal
+ * * error
+ * * warn
+ * * info
+ * * debug
+ * * trace
+ * * paranoia
+ * * spam
+ */
+
 MODULE_PARM_DESC(debug, "debug string to pass to driver core");
 static char *nouveau_debug;
 module_param_named(debug, nouveau_debug, charp, 0400);
 
+/**
+ * DOC: noaccel (boolean)
+ *
+ * Set to ``1`` to disable kernel/abi16 acceleration; defaults to ``0``
+ * (false).
+ */
 MODULE_PARM_DESC(noaccel, "disable kernel/abi16 acceleration");
 static int nouveau_noaccel = 0;
 module_param_named(noaccel, nouveau_noaccel, int, 0400);
 
+/**
+ * DOC: modeset (integer)
+ *
+ * Whether to enable the driver; defaults to automatically detecting whether it
+ * should be enabled. Valid values are:
+ *
+ * * ``0`` - The driver is disabled
+ * * ``1`` - The driver is enabled
+ * * ``2`` - The driver runs in headless mode.
+ */
 MODULE_PARM_DESC(modeset, "enable driver (default: auto, "
 		          "0 = disabled, 1 = enabled, 2 = headless)");
 int nouveau_modeset = -1;
 module_param_named(modeset, nouveau_modeset, int, 0400);
 
+/**
+ * DOC: atomic (boolean)
+ *
+ * Set to ``1`` to enable atomic modesetting support; defaults to ``0``
+ * (false).
+ */
 MODULE_PARM_DESC(atomic, "Expose atomic ioctl (default: disabled)");
 static int nouveau_atomic = 0;
 module_param_named(atomic, nouveau_atomic, int, 0400);
 
+/**
+ * DOC: runpm (integer)
+ *
+ * Control whether or not runtime power management is used. Valid values are:
+ *
+ * * ``0`` - Disables runtime power management
+ * * ``1`` - Forces runtime power management to be enabled
+ * * ``-1`` - Enables runtime power management for Optimus-enabled hardware
+ *   (default).
+ *
+ * .. warning:: Power management is a very experimental feature and is not
+ *	expected to work. If you decided to up-clock your GPU, please
+ *	be aware that your card may overheat. Check the temperature of your GPU
+ *	at all times!
+ */
 MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only
default (-1)");
 static int nouveau_runtime_pm = -1;
 module_param_named(runpm, nouveau_runtime_pm, int, 0400);
-- 
2.26.2

On Fri, Sep 11, 2020 at 6:21 PM Jeremy Cline <jcline at redhat.com>
wrote:
>
> Other gpu drivers have some driver-specific documentation, so it would
> nice if nouveau did as well.
>
> This adds a bare-bones ReStructured Text document with sections for
> module parameter documentation, an overview of the driver architecture,
> a section for internal API documentation, and a glossary for
> nouveau-specific terms.
>
> To make the document a little less bare, I've included docblocks for
> module parameters in nouveau_drm.c, as well as a header (selected at
> random) so reviewers can have a better idea of how this looks. The
> module parameter documentation is based on the existing documentation on
> nouveau.freedesktop.org/wiki/ along with my own research for the
> currently undocumented options. The header documentation ranges from
> wild speculation to placeholder text.
>
> I'm interested in any and all feedback. Does the layout make sense, or
> would folks prefer a multi-page document. How much of the documentation
> I wrote is flat out wrong?
>
> If you want to see the HTML output, but don't want to build the docs,
> it's at https://www.jcline.org/docs/8fe0b0869644/gpu/nouveau.html.
>
> Signed-off-by: Jeremy Cline <jcline at redhat.com>
> ---
>  Documentation/gpu/drivers.rst                 |   1 +
>  Documentation/gpu/nouveau.rst                 | 129 ++++++++++++++
>  .../drm/nouveau/include/nvkm/engine/disp.h    |  35 ++++
>  drivers/gpu/drm/nouveau/nouveau_drm.c         | 157 ++++++++++++++++++
>  4 files changed, 322 insertions(+)
>  create mode 100644 Documentation/gpu/nouveau.rst
>
> diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
> index b4a0ed3ca961..783c749d12e0 100644
> --- a/Documentation/gpu/drivers.rst
> +++ b/Documentation/gpu/drivers.rst
> @@ -9,6 +9,7 @@ GPU Driver Documentation
>     i915
>     mcde
>     meson
> +   nouveau
>     pl111
>     tegra
>     tve200
> diff --git a/Documentation/gpu/nouveau.rst b/Documentation/gpu/nouveau.rst
> new file mode 100644
> index 000000000000..7bbc2cd73e80
> --- /dev/null
> +++ b/Documentation/gpu/nouveau.rst
> @@ -0,0 +1,129 @@
> +======> +Nouveau
> +======> +This document provides a general overview of the driver,
contribution details,
> +internal API documentation, and glossary. It is strictly for the nouveau
kernel
> +driver. The userspace side of nouveau is documented at the `nouveau
freedesktop
> +wiki <https://nouveau.freedesktop.org/wiki/>`_.
> +
> +If you'd like to improve this documentation, great! Refer to the
:doc:`Sphinx
> +introduction </doc-guide/sphinx>` and :doc:`/doc-guide/kernel-doc`
documents
> +for details on how to do so.
> +
> +Module Parameters
> +================> +A number of module parameters are supported. Each
parameter requires a value.
> +These can be passed via the kernel command line using the format
> +"nouveau.<parameter-name>=<value>". Boolean values
should use 1 for true and 0
> +for false.
> +
> +.. kernel-doc:: drivers/gpu/drm/nouveau/nouveau_drm.c
> +
> +
> +Driver Overview
> +==============> +The driver is located in ``drivers/gpu/drm/nouveau/``.
> +
> +.. note:: The latest pending patches for nouveau are available as an
> +   `out-of-tree driver <https://github.com/skeggsb/nouveau>`_.
> +
> +Within the driver, there are several distinct sections. The reason for
this is
> +that NVIDIA hardware is partitioned into "privileged" and
"user" blocks.
> +
> +The general architecture is described in the diagram below.
> +
> +.. kernel-render:: DOT
> +   :alt: Nouveau Architecture Diagram
> +   :caption: Nouveau Architecture Diagram
> +
> +   digraph "Nouveau" {
> +      node [shape=box]
> +      "Userspace" -> "DRM APIs"
> +      "Userspace" -> "NVIF APIs"
> +      "DRM APIs" -> "NVIF APIs"
> +      "NVIF APIs" -> "NVKM APIs"
> +      "NVKM APIs" -> "Hardware-specific Interfaces"
> +   }
> +
> +
> +NVKM
> +----
> +The NVidia Kernel Module (NVKM) is responsible for the low-level resource
> +management of the privileged portions of the hardware. Almost all direct
> +register access is performed in this layer. The functionality of the
underlying
> +hardware is exposed by NVKM as objects of a particular class, and in
general
> +these are identified with the same class IDs that NVIDIA uses.
> +
> +Some classes, such as :term:`channels`, have a block of registers
associated with
> +them that are intended to be directly accessed by an unprivileged client.
NVKM
> +allows objects to be "mapped" by a client to support this.
> +
> +The NVKM layer sits closest to the hardware and services requests issued
by
> +clients.
> +
> +
> +NVIF
> +----
> +Atop the NVKM sits the NVidia Interface (NVIF) library, which defines a
client
> +interface that can be used to interact with the NVKM server. NVIF is
intended
> +to be usable both in the kernel and in userspace. This is accomplished
with
> +drivers that implement the interface defined in struct nvif_driver.
Clients
> +within the kernel use with an NVIF client backed with
> +struct nvif_driver_nvkm.
> +
> +This design allows userspace direct access to the registers of
:term:`channels`
> +it allocates and allows it to submit work to the GPU without going through
the
> +kernel.
> +
> +
> +DRM
> +---
> +The DRM layer of nouveau uses the NVIF to implement the interfaces of a
DRM
> +driver, such as modesetting, command submission to :term:`channels`
> +from userspace, synchronization between userspace clients, and so on.
> +
> +.. note:: All interaction with the NVKM layer inside the kernel should
happen
> +   through NVIF.  Historically this has not been the case, so there may
still
> +   be legacy code that bypasses NVIF and directly calls NVKM.
> +
> +Nouveau's DRM driver is defined in the aptly-named nouveau_drm.c file.
The
> +files in the driver directory's root provide all the functionality
required for
> +the DRM driver. Kernel mode-setting is implemented in the dispnv*
directories
> +and is abstracted in the ``nouveau_display.h`` interface.
> +
> +For details on what is required to implement these interfaces interfaces,
refer
> +to the :doc:`drm-internals`, :doc:`drm-kms`, and :doc:`drm-uapi`
documents.
> +
> +
> +API
> +==> +This section includes the kernel-docs for nouveau APIs.
> +
> +
> +NVKM
> +----
> +Privileged, low-level resource management interfaces.
> +
> +engine/disp.h
> +~~~~~~~~~~~~~
> +.. kernel-doc:: drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
> +
> +
> +Glossary
> +=======> +There are a number of NVIDIA-specific terms in the code as
well as the
> +documentation.
> +
> +.. glossary::
> +
> +   EVO
> +   NVD
> +       In pre-Volta architectures, the Evolution (EVO) controller is used
to
> +       interact with display memory-mapped IO registers via a pushbuffer. 
In
> +       Volta architectures and newer, the NVDisplay controller takes the
place
> +       of the EVO controller, although it has slightly different
semantics.
> +
> +   channels
> +        Channels are hardware blocks that consumes methods from a
> +        direct-memory-accessed command stream.
> +
yeah, I think overall this file is a good idea and being able to get a
quick overview over the driver is helpful. I think if we focus on the
user facing things first, like the hwmon or other things users
generally interact with would be helpful. I think if we start to
document areas where there are many low hanging fruits, this could
help random people to start with easier tasks and get more used to the
driver overall, so I'd probably ignore most of the stuff which really
requires a fundamental understanding on how things work and focus more
on vbios parsing (which has annoying interfaces anyway and it might
make sense to make it more consistent and nicer to use) and/or simple
code interfacing with the mmio space.

Eg some users have problems with their fans as they are either always
ramping up to max, or not running at all... GPUs temperature or power
consumption is reporting incorrectly... all those things users hit
regularly, but which isn't really important enough so it just falls
under the table even if it gets reported.
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
b/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
> index 5a96c942d912..76b90d7ddfc6 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
> @@ -1,22 +1,57 @@
>  /* SPDX-License-Identifier: MIT */
> +
> +/**
> + * DOC: Overview
> + *
> + * Interfaces for working with the display engine.
> + */
> +
>  #ifndef __NVKM_DISP_H__
>  #define __NVKM_DISP_H__
> +
> +/**
> + * nvkm_disp() - Get a &struct nvkm_disp from a &struct
nvkm_engine.
> + *
> + * @p: A &struct nvkm_engine reference.
> + *
> + * Return: The &struct nvkm_disp that contains the given engine.
> + */
>  #define nvkm_disp(p) container_of((p), struct nvkm_disp, engine)
>  #include <core/engine.h>
>  #include <core/event.h>
>
> +/**
> + * struct nvkm_disp - Represents a display engine.
> + *
> + * This structure is for <some abstraction here>. It has <some
assumptions
> + * about its usage here>.
> + */
>  struct nvkm_disp {
> +    /** @func: Chipset-specific vtable for manipulating the display. */
>         const struct nvkm_disp_func *func;
> +
> +    /** @engine: The engine for this display. */
>         struct nvkm_engine engine;
>
> +    /** @head: list of heads attached to this display. */
>         struct list_head head;
> +
> +    /** @ior: List of IO resources for this display. */
>         struct list_head ior;
> +
> +    /** @outp: List of outputs for this display. */
>         struct list_head outp;
> +
> +    /** @conn: List of connectors for this display. */
>         struct list_head conn;
>
> +    /** @hpd: An event that fires when something happens I guess. */
>         struct nvkm_event hpd;
> +
> +    /** @vblank: An event that fires and has some relation to the vblank.
*/
>         struct nvkm_event vblank;
>
> +    /** @client: The oproxy (?) client for this display. */
>         struct nvkm_oproxy *client;
>  };
generally not a big fan of "int a; // a is an int" kind of
documentation. But if it would specify constraints or details on how
it's valid to use those fields, then it makes totally sense to add
stuff.
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 22d246acc5e5..c03c18eaee45 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -68,27 +68,184 @@
>  #include "nouveau_svm.h"
>  #include "nouveau_dmem.h"
>
> +/**
> + * DOC: config (string)
> + *
> + * A configuration string used to adjust a variety of module behaviors.
The
> + * value of this parameter should be a string of comma-separated key=value
> + * pairs.
> + *
> + * values marked as integers are parsed as hex when they include a leading
0x,
> + * octal when they include a leading 0, otherwise they are parsed as
decimals.
> + *
> + * Supported key-value pairs include:
> + *
> + * * GP100MmuLayout (boolean): If false, the old pre-GP100 VMM backend is
used;
> + *   defaults to true and is only valid on NV130-NV138 chips.
> + *
> + * * <engine-name> (boolean): disable the given engine; all engines
are enabled
> + *   by default.
> + *
> + * * NvAcrWpr (integer): ; defaults to -1
> + * * NvAcrWprCompare (boolean): ; defaults to false.
> + * * NvAcrWprPrevAddr (integer): ; defaults to 0.
> + *
> + * * NvBar2Halve (boolean): Halve the BAR2 size; defaults to false.
> + *
> + * * NvFanPWM (boolean): Force PWM for the fan; default is to auto-detect.
> + *
> + * * NvForcePost (boolean): Force the device to perform a POST; defaults
to false.
> + *
> + * * NvGrResetWar (boolean): Reset the GR for an extended period to work
around
> + *   an issue where GR stops responding; defaults to true on certain
Pascal
> + *   boards where it is a known problem.
> + *
> + * * NvGrUseFW (boolean): Load firmware for PGRAPH (valid on the NVC0,
NVE0,
> + *   and NV110 families); defaults to false.
> + *
> + * * NvI2C (boolean): Use the nouveau-internal I2C bus driver rather than
the
> + *   I2C bit-adapters; defaults to false.
> + *
> + * * NvMemExec (boolean): Perform memory reclocking; defaults to true.
> + *
> + * * NvMSI (boolean): Use MSI interrupts; defaults to true on chipsets
that
> + *   support it..
> + *
> + * * NvMXMDCB (boolean): Sanitize DCB outputs from the VBIOS; defaults to
true.
> + *
> + * * NvPCIE (boolean): Whether to use the PCI-E GART (valid on NV40 chips
only);
> + *   defaults to true.
> + *
> + * * NvPmShowAll (boolean): Report all perfmon signals in queries;
defaults to
> + *   false.
> + *
> + * * NvPmUnnamed (boolean): Use the raw signal number rather than the name
in
> + *   perfmon queries; defaults to the value of NvPmShowAll.
> + *
> + * * NvPmEnableGating (boolean): Enable clockgating for chipsets that
support
> + *   it; defaults to false.
> + *
> + * * War00C800_0 (boolean): Enables the PGOB work-around on all GK10[467]
> + *   boards; defaults to true.
> + *
> + * * MmuDebugBufferSize (integer): Size of the MUU debug buffers; defaults
to
> + *   the framebuffer page size.
> + *
> + * * NvAGP (integer): Force a particular AGP mode (0 to disable).
> + *
> + * * NvChipset (integer): Override the detected chipset; useful for
development.
> + *
> + * * NvFbBigPage (integer): Size of the framebuffer big page in bits
> + *   (e.g. 16 means 64KiB); default is chip-dependent.
> + *
> + * * NvBios (string): Specify the Video BIOS source; options include:
> + *
> + *   * "OpenFirmware"
> + *   * "PRAMIN"
> + *   * "PROM"
> + *   * "ACPI"
> + *   * "PCIROM"
> + *   * "PLATFORM"
> + *   * A file name passed on to request_firmware.
> + *
> + * * NvBoost (integer): Specify the Boost mode for Fermi and newer. Valid
> + *   options are:
> + *
> + *   * 0: base clocks (default)
> + *   * 1: boost clocks
> + *   * 2: max clocks
> + *
> + * * NvClkMode (string): Force a particular clock level on boot. This is
> + *   equivalent to passing both ``NvClkModeAC`` and ``NvClkModeDC``.
> + *
> + * * NvClkModeAC (string): Force a particular clock level when plugged in
to a
> + *   power source.
> + *
> + * * NvClkModeDC (string): Force a particular clock level when running on
> + *   battery.
> + */
>  MODULE_PARM_DESC(config, "option string to pass to driver
core");
>  static char *nouveau_config;
>  module_param_named(config, nouveau_config, charp, 0400);
>
> +/**
> + * DOC: debug (string)
> + *
> + * Like the "config" parameter, this is a string of
comma-separated key=values.
> + * Valid keys include:
> + *
> + * * CLIENT
> + * * <subdevice>
> + *
> + * The list of current sub-device and engine names is in the
%nvkm_subdev_name
> + * array and is enumerated in &enum nvkm_devidx.
> + *
> + * Valid values are log levels to use for messages from the given key:
> + *
> + * * fatal
> + * * error
> + * * warn
> + * * info
> + * * debug
> + * * trace
> + * * paranoia
> + * * spam
> + */
> +
>  MODULE_PARM_DESC(debug, "debug string to pass to driver core");
>  static char *nouveau_debug;
>  module_param_named(debug, nouveau_debug, charp, 0400);
>
> +/**
> + * DOC: noaccel (boolean)
> + *
> + * Set to ``1`` to disable kernel/abi16 acceleration; defaults to ``0``
> + * (false).
> + */
>  MODULE_PARM_DESC(noaccel, "disable kernel/abi16 acceleration");
>  static int nouveau_noaccel = 0;
>  module_param_named(noaccel, nouveau_noaccel, int, 0400);
>
> +/**
> + * DOC: modeset (integer)
> + *
> + * Whether to enable the driver; defaults to automatically detecting
whether it
> + * should be enabled. Valid values are:
> + *
> + * * ``0`` - The driver is disabled
> + * * ``1`` - The driver is enabled
> + * * ``2`` - The driver runs in headless mode.
> + */
>  MODULE_PARM_DESC(modeset, "enable driver (default: auto, "
>                           "0 = disabled, 1 = enabled, 2 =
headless)");
>  int nouveau_modeset = -1;
>  module_param_named(modeset, nouveau_modeset, int, 0400);
>
> +/**
> + * DOC: atomic (boolean)
> + *
> + * Set to ``1`` to enable atomic modesetting support; defaults to ``0``
> + * (false).
> + */
>  MODULE_PARM_DESC(atomic, "Expose atomic ioctl (default:
disabled)");
>  static int nouveau_atomic = 0;
>  module_param_named(atomic, nouveau_atomic, int, 0400);
>
> +/**
> + * DOC: runpm (integer)
> + *
> + * Control whether or not runtime power management is used. Valid values
are:
> + *
> + * * ``0`` - Disables runtime power management
> + * * ``1`` - Forces runtime power management to be enabled
> + * * ``-1`` - Enables runtime power management for Optimus-enabled
hardware
> + *   (default).
> + *
> + * .. warning:: Power management is a very experimental feature and is not
> + *     expected to work. If you decided to up-clock your GPU, please
> + *     be aware that your card may overheat. Check the temperature of your
GPU
> + *     at all times!
> + */
>  MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only
default (-1)");
>  static int nouveau_runtime_pm = -1;
>  module_param_named(runpm, nouveau_runtime_pm, int, 0400);
not sure if you were aware of it, but we have some documentation on
the module options here:
https://nouveau.freedesktop.org/wiki/KernelModuleParameters/

But I think it makes sense to move it into the source code and link to
the new thing from the wiki then.
> --
> 2.26.2
>

On Wed, Sep 23, 2020 at 09:02:45PM +0200, Karol Herbst
wrote:
> On Fri, Sep 11, 2020 at 6:21 PM Jeremy Cline <jcline at redhat.com>
wrote:
<snip>
> yeah, I think overall this file is a good idea and being able to get a
> quick overview over the driver is helpful. I think if we focus on the
> user facing things first, like the hwmon or other things users
> generally interact with would be helpful. I think if we start to
> document areas where there are many low hanging fruits, this could
> help random people to start with easier tasks and get more used to the
> driver overall, so I'd probably ignore most of the stuff which really
> requires a fundamental understanding on how things work and focus more
> on vbios parsing (which has annoying interfaces anyway and it might
> make sense to make it more consistent and nicer to use) and/or simple
> code interfacing with the mmio space.
> 
I'll admit to being motivated by entirely selfish reasons. I know
practically nothing about nouveau and I'm the type of person who likes
to keep notes about how things work together, both free form and
structured in-line docs. All that to say, I think focusing on the
"low-hanging fruit" stuff will be very beneficial and I'm happy to
do
that, but I'm also interested in documenting everything else I run
across.
> Eg some users have problems with their fans as they are either always
> ramping up to max, or not running at all... GPUs temperature or power
> consumption is reporting incorrectly... all those things users hit
> regularly, but which isn't really important enough so it just falls
> under the table even if it gets reported.
> 
This does bring up an interesting point about organization and target
audiences. Typically when I'm writing documentation I like to organize
things by target audiences, so we could have a layout like:

* General Introduction

* User Guide
    - Overview of supported hardware/features/etc
    - Installation
    - Configuration (module parameters and such)
    - Troubleshooting
    - Getting Involved (bug reporting, running tests, etc)

* Developer Guide
    - Architecture Overview
    - External APIs (include/uapi/drm/nouveau_drm.h, any sysfs stuff)?
    - Internal APIs
    - Debugging and Development Tools
    - Contribution Guide

I'm not sure how much stuff people want to keep on the
nouveau.freedesktop.org wiki vs here.
> > diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
b/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
> > index 5a96c942d912..76b90d7ddfc6 100644
> > --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
> > +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/disp.h
> > @@ -1,22 +1,57 @@
> >  /* SPDX-License-Identifier: MIT */
> > +
> > +/**
> > + * DOC: Overview
> > + *
> > + * Interfaces for working with the display engine.
> > + */
> > +
> >  #ifndef __NVKM_DISP_H__
> >  #define __NVKM_DISP_H__
> > +
> > +/**
> > + * nvkm_disp() - Get a &struct nvkm_disp from a &struct
nvkm_engine.
> > + *
> > + * @p: A &struct nvkm_engine reference.
> > + *
> > + * Return: The &struct nvkm_disp that contains the given engine.
> > + */
> >  #define nvkm_disp(p) container_of((p), struct nvkm_disp, engine)
> >  #include <core/engine.h>
> >  #include <core/event.h>
> >
> > +/**
> > + * struct nvkm_disp - Represents a display engine.
> > + *
> > + * This structure is for <some abstraction here>. It has
<some assumptions
> > + * about its usage here>.
> > + */
> >  struct nvkm_disp {
> > +    /** @func: Chipset-specific vtable for manipulating the display.
*/
> >         const struct nvkm_disp_func *func;
> > +
> > +    /** @engine: The engine for this display. */
> >         struct nvkm_engine engine;
> >
> > +    /** @head: list of heads attached to this display. */
> >         struct list_head head;
> > +
> > +    /** @ior: List of IO resources for this display. */
> >         struct list_head ior;
> > +
> > +    /** @outp: List of outputs for this display. */
> >         struct list_head outp;
> > +
> > +    /** @conn: List of connectors for this display. */
> >         struct list_head conn;
> >
> > +    /** @hpd: An event that fires when something happens I guess. */
> >         struct nvkm_event hpd;
> > +
> > +    /** @vblank: An event that fires and has some relation to the
vblank. */
> >         struct nvkm_event vblank;
> >
> > +    /** @client: The oproxy (?) client for this display. */
> >         struct nvkm_oproxy *client;
> >  };
> 
> generally not a big fan of "int a; // a is an int" kind of
> documentation. But if it would specify constraints or details on how
> it's valid to use those fields, then it makes totally sense to add
> stuff.
Definitely, I think that is not particularly helpful documentation. Of
course, the what and why of a function parameter or struct member is
very likely to be more interesting than that, but it's true that every
once in a while that the variable name can be so clear there's not much
else to say.

I think it's fair to say the documentation I added for the above struct
is not good. I think it's also fair to say that a new-comer such as
myself who stumbles upon this structure has practically no chance of
guessing what it's all about without reading a bunch of additional code.
My first guess was that it represented a display I had plugged in, which
at this point I'm fairly confident is not at all correct. It required me
to look at many users of this struct along with perusing envytools
documentation to guess it represented a display engine.

It may well be I'm an exceptionally slow learner, but even short notes
can be extremely helpful.
> 
> not sure if you were aware of it, but we have some documentation on
> the module options here:
> https://nouveau.freedesktop.org/wiki/KernelModuleParameters/
> 
> But I think it makes sense to move it into the source code and link to
> the new thing from the wiki then.
> 
Indeed, and in fact I started this documentation from the wiki, but
tried my best to fill in the missing parameters and config options (you
don't happen to know what the NvAcrWpr* config options do, do you?)

Thanks!

- Jeremy

Hi folks,

Here's a v2 of the initial nouveau documentation page.

Based on Roy Spliet's feedback, I added a pointer to envytools
documentation, and attempted to clarify that this document is only about
software architecture.

I also noted more clearly it was meant to compliment the nouveau
freedesktop.org documentation.

Finally, for some starter documentation I dropped my vague placeholder
documentation on the display engine struct and added notes on the CRC
work, which I recently became more familiar with.

I've left out sub-sections for which I had no content, but you can
imagine the nouveau-specific ioctls being documented in their own
sub-section of the "Public API" section, or additional sub-sections to
the
"Private APIs" for NVKM and such.

Let me know what you think!

Jeremy Cline (3):
  drm/nouveau/kms/nvd9-: Introduce some kernel-docs for CRC support
  nouveau: Add kernel-docs for module parameters
  Documentation: nouveau: Introduce some nouveau documentation

 Documentation/gpu/drivers.rst          |   1 +
 Documentation/gpu/nouveau.rst          | 173 +++++++++++++++++++++++++
 drivers/gpu/drm/nouveau/dispnv50/crc.c |   8 ++
 drivers/gpu/drm/nouveau/dispnv50/crc.h |  65 ++++++++++
 drivers/gpu/drm/nouveau/nouveau_drm.c  | 162 +++++++++++++++++++++++
 5 files changed, 409 insertions(+)
 create mode 100644 Documentation/gpu/nouveau.rst

-- 
2.28.0

This is not complete documentation, but covers the pieces I read through
while reviewing the IGT CRC tests for nouveau. It covers the
nouveau-specific debugfs file, an overview of why the code is more
complicated than the DRM display CRC support documentation would lead
one to expect, and the nv50_crc_func vtable. Many of the details were
gleaned from Lyude's commit message, but I think it's valuable to have
them with the code rather than in the change log.

The rendered documentation of the functions in nv50_crc_func aren't as
nice as I'd like, but from what I can tell it would require creating a
typedef for each function with proper argument/return value docs.

Cc: Lyude Paul <lyude at redhat.com>
Signed-off-by: Jeremy Cline <jcline at redhat.com>
---
 drivers/gpu/drm/nouveau/dispnv50/crc.c |  8 ++++
 drivers/gpu/drm/nouveau/dispnv50/crc.h | 65 ++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/crc.c
b/drivers/gpu/drm/nouveau/dispnv50/crc.c
index b8c31b697797..84df41e8fb05 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/crc.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/crc.c
@@ -699,6 +699,14 @@ nv50_crc_debugfs_flip_threshold_set(struct file *file,
 	return ret;
 }
 
+/**
+ * DOC: nv_crc/flip_threshold
+ *
+ * The number of CRC entries to place in a :term:`CRC notifier context` before
+ * attempting to flip to the other notifier context. Writing ``-1`` to this
file
+ * will reset the threshold to the default value, which is hardware-dependent.
+ * Values cannot be greater than the default or less than ``-1``.
+ */
 static const struct file_operations nv50_crc_flip_threshold_fops = {
 	.owner = THIS_MODULE,
 	.open = nv50_crc_debugfs_flip_threshold_open,
diff --git a/drivers/gpu/drm/nouveau/dispnv50/crc.h
b/drivers/gpu/drm/nouveau/dispnv50/crc.h
index 4fce871b04c8..04ebb1f936db 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/crc.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/crc.h
@@ -10,6 +10,24 @@
 #include <nvkm/subdev/bios.h>
 #include "nouveau_encoder.h"
 
+/**
+ * DOC: Overview
+ *
+ * This interface is designed to aid in the implementation of the standard DRM
+ * CRC interface that is part of :doc:`drm-uapi`. NVIDIA's hardware has a
+ * peculiar CRC implementation that limits the number of CRCs that can be
+ * captured, which breaks the DRM API's expectations.
+ *
+ * CRCs are reported to a :term:`CRC notifier context` of limited size. Once
the
+ * context fills, an overflow bit is set and no further CRCs are reported. To
+ * work around this, the driver flips between notifier contexts to allow users
+ * to capture an arbitrary number of CRCs.
+ *
+ * .. warning:: The flip requires flushing two separate updates on the
+ *     :term:`EVO/NVD` channel, and this results in the unavoidable loss of a
single
+ *     frame's CRC.
+ */
+
 struct nv50_atom;
 struct nv50_disp;
 struct nv50_head;
@@ -49,16 +67,63 @@ struct nv50_crc_atom {
 	u8 wndw : 4;
 };
 
+/**
+ * struct nv50_crc_func - Functions and data for CRC support
+ *
+ * The interface used to abstract CRC support across card families that support
+ * it.
+ */
 struct nv50_crc_func {
+	/**
+	 * @set_src: Program the hardware to capture CRCs from the given
+	 * &enum nv50_crc_source_type. Using NULL for the source and ctx will
+	 * disable CRC for the given &struct nv50_head.
+	 *
+	 * Return 0 on success, or a negative error code.
+	 */
 	int (*set_src)(struct nv50_head *, int or, enum nv50_crc_source_type,
 		       struct nv50_crc_notifier_ctx *, u32 wndw);
+
+	/**
+	 * @set_ctx: Change the &struct nv50_crc_notifier_ctx used to capture
+	 * CRC entries. Providing a NULL context will remove any existing
+	 * context.
+	 *
+	 * Return 0 on success, or a negative error code.
+	 */
 	int (*set_ctx)(struct nv50_head *, struct nv50_crc_notifier_ctx *);
+
+	/**
+	 * @get_entry: Read the CRC entry at the given index. idx is guaranteed
+	 * to be less than @num_entries so implementations do not need to
+	 * perform a bounds check.
+	 *
+	 * Return the CRC or 0 if there is no CRC for the given index.
+	 */
 	u32 (*get_entry)(struct nv50_head *, struct nv50_crc_notifier_ctx *,
 			 enum nv50_crc_source, int idx);
+
+	/**
+	 * @ctx_finished: Return true when all requested CRCs have been written
+	 * and it is safe to read them.
+	 */
 	bool (*ctx_finished)(struct nv50_head *,
 			     struct nv50_crc_notifier_ctx *);
+
+	/**
+	 * @flip_threshold: The default number of CRC entries at which point the
+	 * notifier context should be flipped. This should be set somewhat lower
+	 * than @num_entries. It is configurable from userspace via DebugFS.
+	 */
 	short flip_threshold;
+
+	/**
+	 * @num_entries: The maximum number of entries supported in the
+	 * notifier context.
+	 */
 	short num_entries;
+
+	/** @notifier_len: The size of the notifier context in bytes. */
 	size_t notifier_len;
 };
 
-- 
2.28.0

In preparation for the nouveau kernel documentation page, add some
documentation for the supported module parameters. It would be ideal if
the value given to MODULE_PARM_DESC could be referenced in Sphinx to
auto-document these parameters for both readers of the kernel's Sphinx
project and users of modinfo, but that appears to require a bit of work
so for now this approach will do.

Signed-off-by: Jeremy Cline <jcline at redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 162 ++++++++++++++++++++++++++
 1 file changed, 162 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 22d246acc5e5..8d5f17326910 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -68,27 +68,189 @@
 #include "nouveau_svm.h"
 #include "nouveau_dmem.h"
 
+/**
+ * DOC: config (string)
+ *
+ * A configuration string used to adjust a variety of module behaviors. The
+ * value of this parameter should be a string of comma-separated key=value
+ * pairs.
+ *
+ * values marked as integers are parsed as hex when they include a leading 0x,
+ * octal when they include a leading 0, otherwise they are parsed as decimals.
+ *
+ * Supported key-value pairs include:
+ *
+ * * GP100MmuLayout (boolean): If false, the old pre-GP100 VMM backend is used;
+ *   defaults to true and is only valid on NV130-NV138 chips.
+ *
+ * * <engine-name> (boolean): disable the given engine; all engines are
enabled
+ *   by default.
+ *
+ * * NvAcrWpr (integer): This configuration option is for debugging signed
+ *   firmware blobs and is not for end users; defaults to -1
+ *
+ * * NvAcrWprCompare (boolean): This configuration option is for debugging
+ *   signed firmware blobs and is not for end users; defaults to false.
+ *
+ * * NvAcrWprPrevAddr (integer): This configuration option is for debugging
signed
+ *   firmware blobs and is not for end users; defaults to 0.
+ *
+ * * NvBar2Halve (boolean): Halve the BAR2 size; defaults to false.
+ *
+ * * NvFanPWM (boolean): Force PWM for the fan; default is to auto-detect.
+ *
+ * * NvForcePost (boolean): Force the device to perform a POST; defaults to
false.
+ *
+ * * NvGrResetWar (boolean): Reset the GR for an extended period to work around
+ *   an issue where GR stops responding; defaults to true on certain Pascal
+ *   boards where it is a known problem.
+ *
+ * * NvGrUseFW (boolean): Load firmware for PGRAPH (valid on the NVC0, NVE0,
+ *   and NV110 families); defaults to false.
+ *
+ * * NvI2C (boolean): Use the nouveau-internal I2C bus driver rather than the
+ *   I2C bit-adapters; defaults to false.
+ *
+ * * NvMemExec (boolean): Perform memory reclocking; defaults to true.
+ *
+ * * NvMSI (boolean): Use MSI interrupts; defaults to true on chipsets that
+ *   support it..
+ *
+ * * NvMXMDCB (boolean): Sanitize DCB outputs from the VBIOS; defaults to true.
+ *
+ * * NvPCIE (boolean): Whether to use the PCI-E GART (valid on NV40 chips
only);
+ *   defaults to true.
+ *
+ * * NvPmShowAll (boolean): Report all perfmon signals in queries; defaults to
+ *   false.
+ *
+ * * NvPmUnnamed (boolean): Use the raw signal number rather than the name in
+ *   perfmon queries; defaults to the value of NvPmShowAll.
+ *
+ * * NvPmEnableGating (boolean): Enable clockgating for chipsets that support
+ *   it; defaults to false.
+ *
+ * * War00C800_0 (boolean): Enables the PGOB work-around on all GK10[467]
+ *   boards; defaults to true.
+ *
+ * * MmuDebugBufferSize (integer): Size of the MUU debug buffers; defaults to
+ *   the framebuffer page size.
+ *
+ * * NvAGP (integer): Force a particular AGP mode (0 to disable).
+ *
+ * * NvChipset (integer): Override the detected chipset; useful for
development.
+ *
+ * * NvFbBigPage (integer): Size of the framebuffer big page in bits
+ *   (e.g. 16 means 64KiB); default is chip-dependent.
+ *
+ * * NvBios (string): Specify the Video BIOS source; options include:
+ *
+ *   * "OpenFirmware"
+ *   * "PRAMIN"
+ *   * "PROM"
+ *   * "ACPI"
+ *   * "PCIROM"
+ *   * "PLATFORM"
+ *   * A file name passed on to request_firmware.
+ *
+ * * NvBoost (integer): Specify the Boost mode for Fermi and newer. Valid
+ *   options are:
+ *
+ *   * 0: base clocks (default)
+ *   * 1: boost clocks
+ *   * 2: max clocks
+ *
+ * * NvClkMode (string): Force a particular clock level on boot. This is
+ *   equivalent to passing both ``NvClkModeAC`` and ``NvClkModeDC``.
+ *
+ * * NvClkModeAC (string): Force a particular clock level when plugged in to a
+ *   power source.
+ *
+ * * NvClkModeDC (string): Force a particular clock level when running on
+ *   battery.
+ */
 MODULE_PARM_DESC(config, "option string to pass to driver core");
 static char *nouveau_config;
 module_param_named(config, nouveau_config, charp, 0400);
 
+/**
+ * DOC: debug (string)
+ *
+ * Like the "config" parameter, this is a string of comma-separated
key=values.
+ * Valid keys include:
+ *
+ * * CLIENT
+ * * <subdevice>
+ *
+ * The list of current sub-device and engine names is in the %nvkm_subdev_name
+ * array and is enumerated in &enum nvkm_devidx.
+ *
+ * Valid values are log levels to use for messages from the given key:
+ *
+ * * fatal
+ * * error
+ * * warn
+ * * info
+ * * debug
+ * * trace
+ * * paranoia
+ * * spam
+ */
+
 MODULE_PARM_DESC(debug, "debug string to pass to driver core");
 static char *nouveau_debug;
 module_param_named(debug, nouveau_debug, charp, 0400);
 
+/**
+ * DOC: noaccel (boolean)
+ *
+ * Set to ``1`` to disable kernel/abi16 acceleration; defaults to ``0``
+ * (false).
+ */
 MODULE_PARM_DESC(noaccel, "disable kernel/abi16 acceleration");
 static int nouveau_noaccel = 0;
 module_param_named(noaccel, nouveau_noaccel, int, 0400);
 
+/**
+ * DOC: modeset (integer)
+ *
+ * Whether to enable the driver; defaults to automatically detecting whether it
+ * should be enabled. Valid values are:
+ *
+ * * ``0`` - The driver is disabled
+ * * ``1`` - The driver is enabled
+ * * ``2`` - The driver runs in headless mode.
+ */
 MODULE_PARM_DESC(modeset, "enable driver (default: auto, "
 		          "0 = disabled, 1 = enabled, 2 = headless)");
 int nouveau_modeset = -1;
 module_param_named(modeset, nouveau_modeset, int, 0400);
 
+/**
+ * DOC: atomic (boolean)
+ *
+ * Set to ``1`` to enable atomic modesetting support; defaults to ``0``
+ * (false).
+ */
 MODULE_PARM_DESC(atomic, "Expose atomic ioctl (default: disabled)");
 static int nouveau_atomic = 0;
 module_param_named(atomic, nouveau_atomic, int, 0400);
 
+/**
+ * DOC: runpm (integer)
+ *
+ * Control whether or not runtime power management is used. Valid values are:
+ *
+ * * ``0`` - Disables runtime power management
+ * * ``1`` - Forces runtime power management to be enabled
+ * * ``-1`` - Enables runtime power management for Optimus-enabled hardware
+ *   (default).
+ *
+ * .. warning:: Power management is a very experimental feature and is not
+ *	expected to work. If you decided to up-clock your GPU, please
+ *	be aware that your card may overheat. Check the temperature of your GPU
+ *	at all times!
+ */
 MODULE_PARM_DESC(runpm, "disable (0), force enable (1), optimus only
default (-1)");
 static int nouveau_runtime_pm = -1;
 module_param_named(runpm, nouveau_runtime_pm, int, 0400);
-- 
2.28.0

Other gpu drivers have some driver-specific documentation, so it would
nice if nouveau did as well.

This adds a bare-bones ReStructured Text document with sections for
module parameter documentation, an overview of the driver architecture,
a section for internal and external API documentation, and a glossary
for nouveau-specific terms.

Signed-off-by: Jeremy Cline <jcline at redhat.com>
---
 Documentation/gpu/drivers.rst |   1 +
 Documentation/gpu/nouveau.rst | 173 ++++++++++++++++++++++++++++++++++
 2 files changed, 174 insertions(+)
 create mode 100644 Documentation/gpu/nouveau.rst

diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
index b4a0ed3ca961..783c749d12e0 100644
--- a/Documentation/gpu/drivers.rst
+++ b/Documentation/gpu/drivers.rst
@@ -9,6 +9,7 @@ GPU Driver Documentation
    i915
    mcde
    meson
+   nouveau
    pl111
    tegra
    tve200
diff --git a/Documentation/gpu/nouveau.rst b/Documentation/gpu/nouveau.rst
new file mode 100644
index 000000000000..84c8d601adca
--- /dev/null
+++ b/Documentation/gpu/nouveau.rst
@@ -0,0 +1,173 @@
+======+Nouveau
+======+Nouveau is the free/libre driver for NVIDIA GPUs. This documentation is
for the
+kernel mode-setting driver, and is meant to complement the general `nouveau`_
+documentation, which contains end user documentation and a general overview of
+the project, and `envytools`_, which covers reverse-engineering and the
+hardware architecture of the individual card families. Finally, there is
+some `NVIDIA-provided documentation`_.
+
+Issues with this driver are tracked on Nouveau's freedesktop.org `issue
+tracker`_.
+
+If you'd like to improve this documentation, great! Refer to the
:doc:`Sphinx
+introduction </doc-guide/sphinx>` and :doc:`/doc-guide/kernel-doc`
documents
+for details on how to do so.
+
+
+Module Parameters
+================+A number of module parameters are provided to tweak the
behavior of the driver.
+These are provided to ease debugging issues and users that need to set a
+parameter to fix an issue they are experiencing should report this as a bug on
+the `issue tracker`_.
+
+Each parameter requires a value. These can be passed via the kernel command
+line using the format "nouveau.<parameter-name>=<value>".
Boolean values should
+use 1 for true and 0 for false.
+
+.. kernel-doc:: drivers/gpu/drm/nouveau/nouveau_drm.c
+
+
+Driver Overview
+==============+The driver is located in ``drivers/gpu/drm/nouveau/``.
+
+.. note:: The latest pending patches for nouveau are available as an
+   `out-of-tree driver <https://github.com/skeggsb/nouveau>`_.
+
+Within the driver, there are several distinct sections. The reason for this is
+that NVIDIA hardware is partitioned into "privileged" and
"user" blocks. For
+more details on the particulars of NVIDIA's hardware, consult `envytools`_.
+
+The general module architecture from userspace to the hardware lay is described
+in the diagram below.
+
+.. kernel-render:: DOT
+   :alt: Nouveau Software Architecture Diagram
+   :caption: Nouveau Software Architecture Diagram
+
+   digraph "Nouveau" {
+      node [shape=box]
+      "Userspace" -> "DRM APIs"
+      "Userspace" -> "NVIF APIs"
+      "DRM APIs" -> "NVIF APIs"
+      "NVIF APIs" -> "NVKM APIs"
+      "NVKM APIs" -> "Hardware-specific Interfaces"
+   }
+
+
+NVKM
+----
+The NVidia Kernel Module (NVKM) is responsible for the low-level resource
+management of the privileged portions of the hardware. Almost all direct
+register access is performed in this layer. The functionality of the underlying
+hardware is exposed by NVKM as objects of a particular class, and in general
+these are identified with the same class IDs that NVIDIA uses.
+
+Some classes, such as :term:`channels`, have a block of registers associated
with
+them that are intended to be directly accessed by an unprivileged client. NVKM
+allows objects to be "mapped" by a client to support this.
+
+The NVKM layer sits closest to the hardware and services requests issued by
+clients.
+
+
+NVIF
+----
+Atop the NVKM sits the NVidia Interface (NVIF) library, which defines a client
+interface that can be used to interact with the NVKM server. NVIF is intended
+to be usable both in the kernel and in userspace. This is accomplished with
+drivers that implement the interface defined in struct nvif_driver. Clients
+within the kernel use with an NVIF client backed with
+struct nvif_driver_nvkm.
+
+This design allows userspace direct access to the registers of :term:`channels`
+it allocates and allows it to submit work to the GPU without going through the
+kernel.
+
+
+DRM
+---
+The DRM layer of nouveau uses the NVIF to implement the interfaces of a DRM
+driver, such as modesetting, command submission to :term:`channels`
+from userspace, synchronization between userspace clients, and so on.
+
+.. note:: All interaction with the NVKM layer inside the kernel should happen
+   through NVIF.  Historically this has not been the case, so there may still
+   be legacy code that bypasses NVIF and directly calls NVKM.
+
+Nouveau's DRM driver is defined in the aptly-named nouveau_drm.c file. The
+files in the driver directory's root provide all the functionality required
for
+the DRM driver. Kernel mode-setting is implemented in the dispnv* directories
+and is abstracted in the ``nouveau_display.h`` interface.
+
+For details on what is required to implement these interfaces interfaces, refer
+to the :doc:`drm-internals`, :doc:`drm-kms`, and :doc:`drm-uapi` documents.
+
+
+Public APIs
+==========+This section covers userspace interfaces provided by nouveau. Like
other DRM
+drivers, much of the interface exposed to userspace is documented in
+:doc:`drm-uapi`, but there are a few nouveau-specific interfaces.
+
+
+debugfs
+-------
+Nouveau exposes the some :doc:`DebugFS </filesystems/debugfs>` files. All
files
+referenced are relative to ``dri/<card-id>``.
+
+
+crtc-N/nv_crc/flip_threshold
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.. kernel-doc:: drivers/gpu/drm/nouveau/dispnv50/crc.c
+   :doc: nv_crc/flip_threshold
+
+
+Internal APIs
+============+The following sections document the internal interfaces and is
useful only to
+nouveau developers.
+
+
+DRM
+---
+Code related to implementing the standard DRM interfaces is documented in this
+section.
+
+
+dispnv50/crc.h
+~~~~~~~~~~~~~~
+.. kernel-doc:: drivers/gpu/drm/nouveau/dispnv50/crc.h
+
+
+Glossary
+=======+There are a number of NVIDIA-specific terms in the code as well as the
+documentation.
+
+.. glossary::
+
+   EVO
+   NVD
+   EVO/NVD
+       In pre-Volta architectures, the Evolution (EVO) controller is used to
+       interact with display memory-mapped IO registers via a pushbuffer.  In
+       Volta architectures and newer, the NVDisplay controller takes the place
+       of the EVO controller, although it has slightly different semantics.
+
+   channels
+        Channels are hardware blocks that consumes methods from a
+        direct-memory-accessed command stream.
+
+   CRC notifier context
+        A shared DMA region programmed through the core :term:`EVO/NVD`
+        channel used to report frame CRCs.
+
+
+.. _nouveau: https://nouveau.freedesktop.org/
+.. _envytools: https://envytools.readthedocs.io/
+.. _issue tracker: https://gitlab.freedesktop.org/drm/nouveau/-/issues
+.. _NVIDIA-provided documentation: https://github.com/NVIDIA/open-gpu-doc
-- 
2.28.0