diff mbox

[05/28] drm: Merge helper docbook into kerneldoc comments

Message ID 1449218769-16577-6-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Dec. 4, 2015, 8:45 a.m. UTC
Duplication is bad, luckily both help texts highlighted different
issues so the kerneldoc gained quite a bit!

While at it also sprinkle more references to the vtable structs around
and make it clear that legacy CRTC helpers are deprecated and which
functions to use instead.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/DocBook/gpu.tmpl     | 80 +-------------------------------------
 drivers/gpu/drm/drm_crtc_helper.c  | 47 +++++++++++++++++++---
 drivers/gpu/drm/drm_probe_helper.c | 27 ++++++++++---
 3 files changed, 65 insertions(+), 89 deletions(-)

Comments

Thierry Reding Dec. 7, 2015, 11:15 a.m. UTC | #1
On Fri, Dec 04, 2015 at 09:45:46AM +0100, Daniel Vetter wrote:
[...]
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 077e48d3cac2..c4fbcf8e6664 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -51,6 +51,11 @@
>   * the same callbacks which drivers can use to e.g. restore the modeset
>   * configuration on resume with drm_helper_resume_force_mode().
>   *
> + * Note that this helper library doesn't track the current power state of CRTCs
> + * and encoders. It can callbacks like ->dpms() even though the hardware is

Perhaps "It can {call,run,execute} callbacks like ->dpms() ..."

> @@ -450,11 +455,33 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
>   * drm_crtc_helper_set_config - set a new config from userspace
>   * @set: mode set configuration
>   *
> - * Setup a new configuration, provided by the upper layers (either an ioctl call
> - * from userspace or internally e.g. from the fbdev support code) in @set, and
> - * enable it. This is the main helper functions for drivers that implement
> - * kernel mode setting with the crtc helper functions and the assorted
> - * ->prepare(), ->modeset() and ->commit() helper callbacks.
> + * The drm_crtc_helper_set_config() helper function implements the set_config
> + * callback of struct &drm_crtc_funcs for drivers using the legacy CRTC helpers.
> + *
> + * It first tries to locate the best encoder for each connector by calling the
> + * connector best_encoder (struct &drm_connector_helper_funcs) helper operation.

Perhaps "->best_encoder()"? Or is the above required to get formatting
right with the new hypertext/markdown additions?

> + *
> + * After locating the appropriate encoders, the helper function will call the
> + * mode_fixup encoder and CRTC helper operations to adjust the requested mode,

Again, "->mode_fixup()"?

> + * or reject it completely in which case an error will be returned to the
> + * application. If the new configuration after mode adjustment is identical to
> + * the current configuration the helper function will return without performing
> + * any other operation.
> + *
> + * If the adjusted mode is identical to the current mode but changes to the
> + * frame buffer need to be applied, the drm_crtc_helper_set_config function will

Parentheses after "drm_crtc_helper_set_config" to get it marked up as
function?

> + * call the CRTC mode_set_base (struct &drm_crtc_helper_funcs) helper operation.

"->mode_set_base()"?

> + *
> + * If the adjusted mode differs from the current mode, or if the mode_set_base

"->mode_set_base()"?

> + * helper operation is not provided, the helper function performs a full mode
> + * set sequence by calling the prepare, mode_set and commit CRTC and encoder

"->prepare(), ->mode_set() and ->commit()"?

> @@ -763,10 +790,18 @@ static int drm_helper_choose_crtc_dpms(struct drm_crtc *crtc)
>   * @connector: affected connector
>   * @mode: DPMS mode
>   *
> + * The drm_helper_connector_dpms() helper function implements the dpms

"->dpms()"?

> + * callback of struct &drm_connector_funcs for drivers using the legacy CRTC helpers.
> + *
>   * This is the main helper function provided by the crtc helper framework for

s/crtc/CRTC/?

>   * implementing the DPMS connector attribute. It computes the new desired DPMS
>   * state for all encoders and crtcs in the output mesh and calls the ->dpms()

s/crtcs/CRTCs/?

> - * callback provided by the driver appropriately.
> + * callbacks provided by the driver in struct &drm_crtc_helper_funcs and struct
> + * &drm_encoder_helper_funcs appropriately.

Perhaps s/appropriately./, respectively./?

> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index dcd7c0289e03..62889249cbf8 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -272,15 +272,29 @@ prune:
>   * @maxX: max width for modes
>   * @maxY: max height for modes
>   *
> - * Based on the helper callbacks implemented by @connector try to detect all
> - * valid modes.  Modes will first be added to the connector's probed_modes list,
> - * then culled (based on validity and the @maxX, @maxY parameters) and put into
> - * the normal modes list.
> + * Based on the helper callbacks implemented by @connector in struct
> + * &drm_connector_helper_funcs try to detect all valid modes.  Modes will first
> + * be added to the connector's probed_modes list, then culled (based on validity
> + * and the @maxX, @maxY parameters) and put into the normal modes list.
>   *
>   * Intended to be use as a generic implementation of the ->fill_modes()

s/be use/be used/

>   * @connector vfunc for drivers that use the crtc helpers for output mode

s/crtc/CRTC/

>   * filtering and detection.
>   *
> + * If the helper operation returns no mode, and if the connector status is
> + * connector_status_connected, standard VESA DMT modes up to 1024x768 are
> + * automatically added to the modes list by a call to
> + * drm_add_modes_noedid().
> + *
> + * The function then filters out modes larger than

Why wrap here? There's a lot of empty space left on the above line.

> + * @maxX and maxY if specified. It finally calls the optional connector
> + * mode_valid helper operation for each mode in the probed list to check whether

"->mode_valid()"?

Thierry
diff mbox

Patch

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 9952d6ff052f..8b298fc2dd4d 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -1830,83 +1830,7 @@  void intel_crt_init(struct drm_device *dev)
       entities.
     </para>
     <sect2>
-      <title>Helper Functions</title>
-      <itemizedlist>
-        <listitem>
-          <synopsis>int drm_crtc_helper_set_config(struct drm_mode_set *set);</synopsis>
-          <para>
-            The <function>drm_crtc_helper_set_config</function> helper function
-            is a CRTC <methodname>set_config</methodname> implementation. It
-            first tries to locate the best encoder for each connector by calling
-            the connector <methodname>best_encoder</methodname> helper
-            operation.
-          </para>
-          <para>
-            After locating the appropriate encoders, the helper function will
-            call the <methodname>mode_fixup</methodname> encoder and CRTC helper
-            operations to adjust the requested mode, or reject it completely in
-            which case an error will be returned to the application. If the new
-            configuration after mode adjustment is identical to the current
-            configuration the helper function will return without performing any
-            other operation.
-          </para>
-          <para>
-            If the adjusted mode is identical to the current mode but changes to
-            the frame buffer need to be applied, the
-            <function>drm_crtc_helper_set_config</function> function will call
-            the CRTC <methodname>mode_set_base</methodname> helper operation. If
-            the adjusted mode differs from the current mode, or if the
-            <methodname>mode_set_base</methodname> helper operation is not
-            provided, the helper function performs a full mode set sequence by
-            calling the <methodname>prepare</methodname>,
-            <methodname>mode_set</methodname> and
-            <methodname>commit</methodname> CRTC and encoder helper operations,
-            in that order.
-          </para>
-        </listitem>
-        <listitem>
-          <synopsis>void drm_helper_connector_dpms(struct drm_connector *connector, int mode);</synopsis>
-          <para>
-            The <function>drm_helper_connector_dpms</function> helper function
-            is a connector <methodname>dpms</methodname> implementation that
-            tracks power state of connectors. To use the function, drivers must
-            provide <methodname>dpms</methodname> helper operations for CRTCs
-            and encoders to apply the DPMS state to the device.
-          </para>
-          <para>
-            The mid-layer doesn't track the power state of CRTCs and encoders.
-            The <methodname>dpms</methodname> helper operations can thus be
-            called with a mode identical to the currently active mode.
-          </para>
-        </listitem>
-        <listitem>
-          <synopsis>int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
-                                            uint32_t maxX, uint32_t maxY);</synopsis>
-          <para>
-            The <function>drm_helper_probe_single_connector_modes</function> helper
-            function is a connector <methodname>fill_modes</methodname>
-            implementation that updates the connection status for the connector
-            and then retrieves a list of modes by calling the connector
-            <methodname>get_modes</methodname> helper operation.
-          </para>
-         <para>
-            If the helper operation returns no mode, and if the connector status
-            is connector_status_connected, standard VESA DMT modes up to
-            1024x768 are automatically added to the modes list by a call to
-            <function>drm_add_modes_noedid</function>.
-          </para>
-          <para>
-            The function then filters out modes larger than
-            <parameter>max_width</parameter> and <parameter>max_height</parameter>
-            if specified. It finally calls the optional connector
-            <methodname>mode_valid</methodname> helper operation for each mode in
-            the probed list to check whether the mode is valid for the connector.
-          </para>
-        </listitem>
-      </itemizedlist>
-    </sect2>
-    <sect2>
-      <title>CRTC Helper Operations</title>
+      <title>Legacy CRTC Helper Operations</title>
       <itemizedlist>
         <listitem id="drm-helper-crtc-mode-fixup">
           <synopsis>bool (*mode_fixup)(struct drm_crtc *crtc,
@@ -2308,7 +2232,7 @@  void intel_crt_init(struct drm_device *dev)
 !Pinclude/drm/drm_modeset_helper_vtables.h overview
     </sect2>
     <sect2>
-      <title>Modeset Helper Functions Reference</title>
+      <title>Legacy CRTC/Modeset Helper Functions Reference</title>
 !Edrivers/gpu/drm/drm_crtc_helper.c
 !Pdrivers/gpu/drm/drm_crtc_helper.c overview
     </sect2>
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 077e48d3cac2..c4fbcf8e6664 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -51,6 +51,11 @@ 
  * the same callbacks which drivers can use to e.g. restore the modeset
  * configuration on resume with drm_helper_resume_force_mode().
  *
+ * Note that this helper library doesn't track the current power state of CRTCs
+ * and encoders. It can callbacks like ->dpms() even though the hardware is
+ * already in the desired state. This deficiency has been fixed in the atomic
+ * helpers.
+ *
  * The driver callbacks are mostly compatible with the atomic modeset helpers,
  * except for the handling of the primary plane: Atomic helpers require that the
  * primary plane is implemented as a real standalone plane and not directly tied
@@ -450,11 +455,33 @@  drm_crtc_helper_disable(struct drm_crtc *crtc)
  * drm_crtc_helper_set_config - set a new config from userspace
  * @set: mode set configuration
  *
- * Setup a new configuration, provided by the upper layers (either an ioctl call
- * from userspace or internally e.g. from the fbdev support code) in @set, and
- * enable it. This is the main helper functions for drivers that implement
- * kernel mode setting with the crtc helper functions and the assorted
- * ->prepare(), ->modeset() and ->commit() helper callbacks.
+ * The drm_crtc_helper_set_config() helper function implements the set_config
+ * callback of struct &drm_crtc_funcs for drivers using the legacy CRTC helpers.
+ *
+ * It first tries to locate the best encoder for each connector by calling the
+ * connector best_encoder (struct &drm_connector_helper_funcs) helper operation.
+ *
+ * After locating the appropriate encoders, the helper function will call the
+ * mode_fixup encoder and CRTC helper operations to adjust the requested mode,
+ * or reject it completely in which case an error will be returned to the
+ * application. If the new configuration after mode adjustment is identical to
+ * the current configuration the helper function will return without performing
+ * any other operation.
+ *
+ * If the adjusted mode is identical to the current mode but changes to the
+ * frame buffer need to be applied, the drm_crtc_helper_set_config function will
+ * call the CRTC mode_set_base (struct &drm_crtc_helper_funcs) helper operation.
+ *
+ * If the adjusted mode differs from the current mode, or if the mode_set_base
+ * helper operation is not provided, the helper function performs a full mode
+ * set sequence by calling the prepare, mode_set and commit CRTC and encoder
+ * helper operations, in that order. Alternatively it can also use the dpms and
+ * disable helper operations. For details see struct &drm_crtc_helper_funcs and
+ * struct &drm_encoder_helper_funcs.
+ *
+ * This function is deprecated.  New drivers must implement atomic modeset
+ * support, for which this function is unsuitable. Instead drivers should use
+ * drm_atomic_helper_set_config().
  *
  * Returns:
  * Returns 0 on success, negative errno numbers on failure.
@@ -763,10 +790,18 @@  static int drm_helper_choose_crtc_dpms(struct drm_crtc *crtc)
  * @connector: affected connector
  * @mode: DPMS mode
  *
+ * The drm_helper_connector_dpms() helper function implements the dpms
+ * callback of struct &drm_connector_funcs for drivers using the legacy CRTC helpers.
+ *
  * This is the main helper function provided by the crtc helper framework for
  * implementing the DPMS connector attribute. It computes the new desired DPMS
  * state for all encoders and crtcs in the output mesh and calls the ->dpms()
- * callback provided by the driver appropriately.
+ * callbacks provided by the driver in struct &drm_crtc_helper_funcs and struct
+ * &drm_encoder_helper_funcs appropriately.
+ *
+ * This function is deprecated.  New drivers must implement atomic modeset
+ * support, for which this function is unsuitable. Instead drivers should use
+ * drm_atomic_helper_connector_dpms().
  *
  * Returns:
  * Always returns 0.
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index dcd7c0289e03..62889249cbf8 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -272,15 +272,29 @@  prune:
  * @maxX: max width for modes
  * @maxY: max height for modes
  *
- * Based on the helper callbacks implemented by @connector try to detect all
- * valid modes.  Modes will first be added to the connector's probed_modes list,
- * then culled (based on validity and the @maxX, @maxY parameters) and put into
- * the normal modes list.
+ * Based on the helper callbacks implemented by @connector in struct
+ * &drm_connector_helper_funcs try to detect all valid modes.  Modes will first
+ * be added to the connector's probed_modes list, then culled (based on validity
+ * and the @maxX, @maxY parameters) and put into the normal modes list.
  *
  * Intended to be use as a generic implementation of the ->fill_modes()
  * @connector vfunc for drivers that use the crtc helpers for output mode
  * filtering and detection.
  *
+ * If the helper operation returns no mode, and if the connector status is
+ * connector_status_connected, standard VESA DMT modes up to 1024x768 are
+ * automatically added to the modes list by a call to
+ * drm_add_modes_noedid().
+ *
+ * The function then filters out modes larger than
+ * @maxX and maxY if specified. It finally calls the optional connector
+ * mode_valid helper operation for each mode in the probed list to check whether
+ * the mode is valid for the connector.
+ *
+ * Compared to drm_helper_probe_single_connector_modes_nomerge() this function
+ * merged the mode bits for the preferred mode, as a hack to work around some
+ * quirky issues on funky hardware.
+ *
  * Returns:
  * The number of modes found on @connector.
  */
@@ -297,8 +311,11 @@  EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  * @maxX: max width for modes
  * @maxY: max height for modes
  *
- * This operates like drm_hehlper_probe_single_connector_modes except it
+ * This operates like drm_hehlper_probe_single_connector_modes() except it
  * replaces the mode bits instead of merging them for preferred modes.
+ *
+ * Returns:
+ * The number of modes found on @connector.
  */
 int drm_helper_probe_single_connector_modes_nomerge(struct drm_connector *connector,
 					    uint32_t maxX, uint32_t maxY)