Message-Id: <20170227201144.10970-4-eric@anholt.net>
Date:   Mon, 27 Feb 2017 12:11:43 -0800
From:   Eric Anholt <eric@...olt.net>
To:     dri-devel@...ts.freedesktop.org
Cc:     linux-kernel@...r.kernel.org, Eric Anholt <eric@...olt.net>
Subject: [PATCH 3/4] drm/vc4: Extend and edit documentation for output from the RST

I had written most of my comments as if I was describing the
individual code files the way I used to for doxygen, while for RST we
want to describe things in a more chapter/section way where there's no
obvious relation to .c files.

Additionally, several of the files had stub descriptions that I've
taken this opportunity to extend.

Signed-off-by: Eric Anholt <eric@...olt.net>
---
 drivers/gpu/drm/vc4/vc4_crtc.c             |  7 ++++---
 drivers/gpu/drm/vc4/vc4_dpi.c              |  3 ++-
 drivers/gpu/drm/vc4/vc4_hdmi.c             | 23 ++++++++++++++++++++---
 drivers/gpu/drm/vc4/vc4_hvs.c              | 12 ++++++------
 drivers/gpu/drm/vc4/vc4_render_cl.c        |  4 ++++
 drivers/gpu/drm/vc4/vc4_validate.c         | 24 ++++++++++++++----------
 drivers/gpu/drm/vc4/vc4_validate_shaders.c | 21 +++++++++++++--------
 drivers/gpu/drm/vc4/vc4_vec.c              |  6 ++++++
 8 files changed, 69 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 764320156cce..7fa4c3d5cddf 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -11,12 +11,13 @@
  *
  * In VC4, the Pixel Valve is what most closely corresponds to the
  * DRM's concept of a CRTC.  The PV generates video timings from the
- * output's clock plus its configuration.  It pulls scaled pixels from
+ * encoder's clock plus its configuration.  It pulls scaled pixels from
  * the HVS at that timing, and feeds it to the encoder.
  *
  * However, the DRM CRTC also collects the configuration of all the
- * DRM planes attached to it.  As a result, this file also manages
- * setup of the VC4 HVS's display elements on the CRTC.
+ * DRM planes attached to it.  As a result, the CRTC is also
+ * responsible for writing the display list for the HVS channel that
+ * the CRTC will use.
  *
  * The 2835 has 3 different pixel valves.  pv0 in the audio power
  * domain feeds DSI0 or DPI, while pv1 feeds DS1 or SMI.  pv2 in the
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 3f360cf6cf5a..71435796c710 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -18,7 +18,8 @@
  * DOC: VC4 DPI module
  *
  * The VC4 DPI hardware supports MIPI DPI type 4 and Nokia ViSSI
- * signals, which are routed out to GPIO0-27 with the ALT2 function.
+ * signals.  On BCM2835, these can be routed out to GPIO0-27 with the
+ * ALT2 function.
  */
 
 #include "drm_atomic_helper.h"
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 93d5994f3a04..1be1e8304720 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -20,9 +20,26 @@
 /**
  * DOC: VC4 Falcon HDMI module
  *
- * The HDMI core has a state machine and a PHY.  Most of the unit
- * operates off of the HSM clock from CPRMAN.  It also internally uses
- * the PLLH_PIX clock for the PHY.
+ * The HDMI core has a state machine and a PHY.  On BCM2835, most of
+ * the unit operates off of the HSM clock from CPRMAN.  It also
+ * internally uses the PLLH_PIX clock for the PHY.
+ *
+ * HDMI infoframes are kept within a small packet ram, where each
+ * packet can be individually enabled for including in a frame.
+ *
+ * HDMI audio is implemented entirely within the HDMI IP block.  A
+ * register in the HDMI encoder takes SPDIF frames from the DMA engine
+ * and transfers them over an internal MAI (multi-channel audio
+ * interconnect) bus to the encoder side for insertion into the video
+ * blank regions.
+ *
+ * The driver's HDMI encoder does not yet support power management.
+ * The HDMI encoder's power domain and the HSM/pixel clocks are kept
+ * continuously running, and only the HDMI logic and packet ram are
+ * powered off/on at disable/enable time.
+ *
+ * The driver does not yet support CEC control, though the HDMI
+ * encoder block has CEC support.
  */
 
 #include "drm_atomic_helper.h"
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index f7f7677f6d8d..fd421ba3c5d7 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -9,12 +9,12 @@
 /**
  * DOC: VC4 HVS module.
  *
- * The HVS is the piece of hardware that does translation, scaling,
- * colorspace conversion, and compositing of pixels stored in
- * framebuffers into a FIFO of pixels going out to the Pixel Valve
- * (CRTC).  It operates at the system clock rate (the system audio
- * clock gate, specifically), which is much higher than the pixel
- * clock rate.
+ * The Hardware Video Scaler (HVS) is the piece of hardware that does
+ * translation, scaling, colorspace conversion, and compositing of
+ * pixels stored in framebuffers into a FIFO of pixels going out to
+ * the Pixel Valve (CRTC).  It operates at the system clock rate (the
+ * system audio clock gate, specifically), which is much higher than
+ * the pixel clock rate.
  *
  * There is a single global HVS, with multiple output FIFOs that can
  * be consumed by the PVs.  This file just manages the resources for
diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c
index 08886a309757..8fc2fb461ac3 100644
--- a/drivers/gpu/drm/vc4/vc4_render_cl.c
+++ b/drivers/gpu/drm/vc4/vc4_render_cl.c
@@ -24,6 +24,10 @@
 /**
  * DOC: Render command list generation
  *
+ * In the V3D hardware, render command lists are what load and store
+ * tiles of a framebuffer and optionally call out to binner-generated
+ * command lists to do the 3D drawing for that tile.
+ *
  * In the VC4 driver, render command list generation is performed by the
  * kernel instead of userspace.  We do this because validating a
  * user-submitted command list is hard to get right and has high CPU overhead,
diff --git a/drivers/gpu/drm/vc4/vc4_validate.c b/drivers/gpu/drm/vc4/vc4_validate.c
index d696ed49e9f0..da6f1e138e8d 100644
--- a/drivers/gpu/drm/vc4/vc4_validate.c
+++ b/drivers/gpu/drm/vc4/vc4_validate.c
@@ -24,19 +24,23 @@
 /**
  * DOC: Command list validator for VC4.
  *
- * The VC4 has no IOMMU between it and system memory.  So, a user with
- * access to execute command lists could escalate privilege by
+ * Since the VC4 has no IOMMU between it and system memory, a user
+ * with access to execute command lists could escalate privilege by
  * overwriting system memory (drawing to it as a framebuffer) or
- * reading system memory it shouldn't (reading it as a texture, or
- * uniform data, or vertex data).
+ * reading system memory it shouldn't (reading it as a vertex buffer
+ * or index buffer)
  *
- * This validates command lists to ensure that all accesses are within
- * the bounds of the GEM objects referenced.  It explicitly whitelists
- * packets, and looks at the offsets in any address fields to make
- * sure they're constrained within the BOs they reference.
+ * We validate binner command lists to ensure that all accesses are
+ * within the bounds of the GEM objects referenced by the submitted
+ * job.  It explicitly whitelists packets, and looks at the offsets in
+ * any address fields to make sure they're contained within the BOs
+ * they reference.
  *
- * Note that because of the validation that's happening anyway, this
- * is where GEM relocation processing happens.
+ * Note that because CL validation is already reading the
+ * user-submitted CL and writing the validated copy out to the memory
+ * that the GPU will actually read, this is also where GEM relocation
+ * processing (turning BO references into actual addresses for the GPU
+ * to use) happens.
  */
 
 #include "uapi/drm/vc4_drm.h"
diff --git a/drivers/gpu/drm/vc4/vc4_validate_shaders.c b/drivers/gpu/drm/vc4/vc4_validate_shaders.c
index 5dba13dd1e9b..0b2df5c6efb4 100644
--- a/drivers/gpu/drm/vc4/vc4_validate_shaders.c
+++ b/drivers/gpu/drm/vc4/vc4_validate_shaders.c
@@ -24,16 +24,21 @@
 /**
  * DOC: Shader validator for VC4.
  *
- * The VC4 has no IOMMU between it and system memory, so a user with
- * access to execute shaders could escalate privilege by overwriting
- * system memory (using the VPM write address register in the
- * general-purpose DMA mode) or reading system memory it shouldn't
- * (reading it as a texture, or uniform data, or vertex data).
+ * Since the VC4 has no IOMMU between it and system memory, a user
+ * with access to execute shaders could escalate privilege by
+ * overwriting system memory (using the VPM write address register in
+ * the general-purpose DMA mode) or reading system memory it shouldn't
+ * (reading it as a texture, uniform data, or direct-addressed TMU
+ * lookup).
  *
- * This walks over a shader BO, ensuring that its accesses are
- * appropriately bounded, and recording how many texture accesses are
- * made and where so that we can do relocations for them in the
+ * The shader validator walks over a shader's BO, ensuring that its
+ * accesses are appropriately bounded, and recording where texture
+ * accesses are made so that we can do relocations for them in the
  * uniform stream.
+ *
+ * Shader BO are immutable for their lifetimes (enforced by not
+ * allowing mmaps, GEM prime export, or rendering to from a CL), so
+ * this validation is only performed at BO creation time.
  */
 
 #include "vc4_drv.h"
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index 32bb8ef985fb..09c1e05765fa 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -16,6 +16,12 @@
 
 /**
  * DOC: VC4 SDTV module
+ *
+ * The VEC encoder generates PAL or NTSC composite video output.
+ *
+ * TV mode selection is done by an atomic property on the encoder,
+ * because a drm_mode_modeinfo is insufficient to distinguish between
+ * PAL and PAL-M or NTSC and NTSC-J.
  */
 
 #include <drm/drm_atomic_helper.h>
-- 
2.11.0

Powered by blists - more mailing lists