[PATCH 26/34] drm: polish function kerneldoc for drm_modes.[hc]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



- Tune down yelling RETURNS.
- OCD align all the parameters the same.
- Add missing kerneldoc, which also means that we need to include the
  kerneldoc from the drm_modes.h header now.
- Add missing Returns: sections.
- General polish and clarification - especially the kerneldoc for the
  mode creation helpers seems to have been some good specimen of
  copypasta gone wrong.

All actual code changes have all been extracted into prep patches
since there was simply too much to polish.

v2: More polish for the command line modeline functions.

Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
---
 Documentation/DocBook/drm.tmpl |   1 +
 drivers/gpu/drm/drm_modes.c    | 171 +++++++++++++++++++++++++----------------
 include/drm/drm_modes.h        |   8 ++
 3 files changed, 115 insertions(+), 65 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 50af3298ac1f..4268cbe6f95c 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -965,6 +965,7 @@ int max_width, max_height;</synopsis>
     </itemizedlist>
     <sect2>
       <title>Display Modes Function Reference</title>
+!Iinclude/drm/drm_modes.h
 !Edrivers/gpu/drm/drm_modes.c
     </sect2>
     <sect2>
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index cc352eed0191..8b410576fce4 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -63,9 +63,10 @@ EXPORT_SYMBOL(drm_mode_debug_printmodeline);
  * drm_mode_create - create a new display mode
  * @dev: DRM device
  *
- * Create a new drm_display_mode, give it an ID, and return it.
+ * Create a new, cleared drm_display_mode with kzalloc, allocate an ID for it
+ * and return it.
  *
- * RETURNS:
+ * Returns:
  * Pointer to new mode on success, NULL on error.
  */
 struct drm_display_mode *drm_mode_create(struct drm_device *dev)
@@ -90,7 +91,7 @@ EXPORT_SYMBOL(drm_mode_create);
  * @dev: DRM device
  * @mode: mode to remove
  *
- * Free @mode's unique identifier, then free it.
+ * Release @mode's unique ID, then free it @mode structure itself using kfree.
  */
 void drm_mode_destroy(struct drm_device *dev, struct drm_display_mode *mode)
 {
@@ -104,11 +105,13 @@ void drm_mode_destroy(struct drm_device *dev, struct drm_display_mode *mode)
 EXPORT_SYMBOL(drm_mode_destroy);
 
 /**
- * drm_mode_probed_add - add a mode to a connector's probed mode list
+ * drm_mode_probed_add - add a mode to a connector's probed_mode list
  * @connector: connector the new mode
  * @mode: mode data
  *
- * Add @mode to @connector's mode list for later use.
+ * Add @mode to @connector's probed_mode list for later use. This list should
+ * then in a second step get filtered and all the modes actually supported by
+ * the hardware moved to the @connector's modes list.
  */
 void drm_mode_probed_add(struct drm_connector *connector,
 			 struct drm_display_mode *mode)
@@ -120,16 +123,14 @@ void drm_mode_probed_add(struct drm_connector *connector,
 EXPORT_SYMBOL(drm_mode_probed_add);
 
 /**
- * drm_cvt_mode -create a modeline based on CVT algorithm
- * @dev: DRM device
+ * drm_cvt_mode -create a modeline based on the CVT algorithm
+ * @dev: drm device
  * @hdisplay: hdisplay size
  * @vdisplay: vdisplay size
- * @vrefresh  : vrefresh rate
- * @reduced : Whether the GTF calculation is simplified
- * @interlaced:Whether the interlace is supported
- * @margins: whether to add margins or not
- *
- * return the modeline based on CVT algorithm
+ * @vrefresh: vrefresh rate
+ * @reduced: whether to use reduced blanking
+ * @interlaced: whether to compute an interlaced mode
+ * @margins: whether to add margins (borders)
  *
  * This function is called to generate the modeline based on CVT algorithm
  * according to the hdisplay, vdisplay, vrefresh.
@@ -139,6 +140,11 @@ EXPORT_SYMBOL(drm_mode_probed_add);
  *
  * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
  * What I have done is to translate it by using integer calculation.
+ *
+ * Returns:
+ * The modeline based on the CVT algorithm stored in a drm_display_mode object.
+ * The display mode object is allocated with drm_mode_create(). Returns NULL
+ * when no mode could be allocated.
  */
 struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay,
 				      int vdisplay, int vrefresh,
@@ -338,23 +344,25 @@ struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay,
 EXPORT_SYMBOL(drm_cvt_mode);
 
 /**
- * drm_gtf_mode_complex - create the modeline based on full GTF algorithm
- *
- * @dev		:drm device
- * @hdisplay	:hdisplay size
- * @vdisplay	:vdisplay size
- * @vrefresh	:vrefresh rate.
- * @interlaced	:whether the interlace is supported
- * @margins	:desired margin size
+ * drm_gtf_mode_complex - create the modeline based on the full GTF algorithm
+ * @dev: drm device
+ * @hdisplay: hdisplay size
+ * @vdisplay: vdisplay size
+ * @vrefresh: vrefresh rate.
+ * @interlaced: whether to compute an interlaced mode
+ * @margins: desired margin (borders) size
  * @GTF_M: extended GTF formula parameters
  * @GTF_2C: extended GTF formula parameters
  * @GTF_K: extended GTF formula parameters
  * @GTF_2J: extended GTF formula parameters
  *
- * return the modeline based on full GTF algorithm.
- *
  * GTF feature blocks specify C and J in multiples of 0.5, so we pass them
  * in here multiplied by two.  For a C of 40, pass in 80.
+ *
+ * Returns:
+ * The modeline based on the full GTF algorithm stored in a drm_display_mode object.
+ * The display mode object is allocated with drm_mode_create(). Returns NULL
+ * when no mode could be allocated.
  */
 struct drm_display_mode *
 drm_gtf_mode_complex(struct drm_device *dev, int hdisplay, int vdisplay,
@@ -524,14 +532,13 @@ drm_gtf_mode_complex(struct drm_device *dev, int hdisplay, int vdisplay,
 EXPORT_SYMBOL(drm_gtf_mode_complex);
 
 /**
- * drm_gtf_mode - create the modeline based on GTF algorithm
- *
- * @dev		:drm device
- * @hdisplay	:hdisplay size
- * @vdisplay	:vdisplay size
- * @vrefresh	:vrefresh rate.
- * @interlaced	:whether the interlace is supported
- * @margins	:whether the margin is supported
+ * drm_gtf_mode - create the modeline based on the GTF algorithm
+ * @dev: drm device
+ * @hdisplay: hdisplay size
+ * @vdisplay: vdisplay size
+ * @vrefresh: vrefresh rate.
+ * @interlaced: whether to compute an interlaced mode
+ * @margins: desired margin (borders) size
  *
  * return the modeline based on GTF algorithm
  *
@@ -550,6 +557,11 @@ EXPORT_SYMBOL(drm_gtf_mode_complex);
  * C = 40
  * K = 128
  * J = 20
+ *
+ * Returns:
+ * The modeline based on the GTF algorithm stored in a drm_display_mode object.
+ * The display mode object is allocated with drm_mode_create(). Returns NULL
+ * when no mode could be allocated.
  */
 struct drm_display_mode *
 drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
@@ -562,6 +574,13 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
 EXPORT_SYMBOL(drm_gtf_mode);
 
 #ifdef CONFIG_VIDEOMODE_HELPERS
+/**
+ * drm_display_mode_from_videomode - fill in @dmode using @vm,
+ * @vm: videomode structure to use as source
+ * @dmode: drm_display_mode structure to use as destination
+ *
+ * Fills out @dmode using the display mode specified in @vm.
+ */
 void drm_display_mode_from_videomode(const struct videomode *vm,
 				     struct drm_display_mode *dmode)
 {
@@ -606,6 +625,9 @@ EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
  * This function is expensive and should only be used, if only one mode is to be
  * read from DT. To get multiple modes start with of_get_display_timings and
  * work with that instead.
+ *
+ * Returns:
+ * 0 on success, a negative errno code when no of videomode node was found.
  */
 int of_get_drm_display_mode(struct device_node *np,
 			    struct drm_display_mode *dmode, int index)
@@ -633,7 +655,8 @@ EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
  *
- * Set the name of @mode to a standard format.
+ * Set the name of @mode to a standard format which is <hdisplay>x<vdisplay>
+ * with an optional 'i' suffix for interlaced modes.
  */
 void drm_mode_set_name(struct drm_display_mode *mode)
 {
@@ -648,7 +671,9 @@ EXPORT_SYMBOL(drm_mode_set_name);
 /** drm_mode_hsync - get the hsync of a mode
  * @mode: mode
  *
- * Return @modes's hsync rate in kHz, rounded to the nearest int.
+ * Returns:
+ * @modes's hsync rate in kHz, rounded to the nearest integer. Calculates the
+ * value first if it is not yet set.
  */
 int drm_mode_hsync(const struct drm_display_mode *mode)
 {
@@ -672,14 +697,9 @@ EXPORT_SYMBOL(drm_mode_hsync);
  * drm_mode_vrefresh - get the vrefresh of a mode
  * @mode: mode
  *
- * Return @mode's vrefresh rate in Hz or calculate it if necessary.
- *
- * FIXME: why is this needed?  shouldn't vrefresh be set already?
- *
- * RETURNS:
- * Vertical refresh rate. It will be the result of actual value plus 0.5.
- * If it is 70.288, it will return 70Hz.
- * If it is 59.6, it will return 60Hz.
+ * Returns:
+ * @modes's vrefresh rate in Hz, rounded to the nearest integer. Calculates the
+ * value first if it is not yet set.
  */
 int drm_mode_vrefresh(const struct drm_display_mode *mode)
 {
@@ -708,11 +728,11 @@ int drm_mode_vrefresh(const struct drm_display_mode *mode)
 EXPORT_SYMBOL(drm_mode_vrefresh);
 
 /**
- * drm_mode_set_crtcinfo - set CRTC modesetting parameters
+ * drm_mode_set_crtcinfo - set CRTC modesetting timing parameters
  * @p: mode
  * @adjust_flags: a combination of adjustment flags
  *
- * Setup the CRTC modesetting parameters for @p, adjusting if necessary.
+ * Setup the CRTC modesetting timing parameters for @p, adjusting if necessary.
  *
  * - The CRTC_INTERLACE_HALVE_V flag can be used to halve vertical timings of
  *   interlaced modes.
@@ -780,7 +800,6 @@ void drm_mode_set_crtcinfo(struct drm_display_mode *p, int adjust_flags)
 }
 EXPORT_SYMBOL(drm_mode_set_crtcinfo);
 
-
 /**
  * drm_mode_copy - copy the mode
  * @dst: mode to overwrite
@@ -807,6 +826,9 @@ EXPORT_SYMBOL(drm_mode_copy);
  *
  * Just allocate a new mode, copy the existing mode into it, and return
  * a pointer to it.  Used to create new instances of established modes.
+ *
+ * Returns:
+ * Pointer to duplicated mode on success, NULL on error.
  */
 struct drm_display_mode *drm_mode_duplicate(struct drm_device *dev,
 					    const struct drm_display_mode *mode)
@@ -830,7 +852,7 @@ EXPORT_SYMBOL(drm_mode_duplicate);
  *
  * Check to see if @mode1 and @mode2 are equivalent.
  *
- * RETURNS:
+ * Returns:
  * True if the modes are equal, false otherwise.
  */
 bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2)
@@ -859,7 +881,7 @@ EXPORT_SYMBOL(drm_mode_equal);
  * Check to see if @mode1 and @mode2 are equivalent, but
  * don't check the pixel clocks nor the stereo layout.
  *
- * RETURNS:
+ * Returns:
  * True if the modes are equal, false otherwise.
  */
 bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
@@ -890,9 +912,10 @@ EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
  * @maxX: maximum width
  * @maxY: maximum height
  *
- * The DRM device (@dev) has size and pitch limits.  Here we validate the
- * modes we probed for @dev against those limits and set their status as
- * necessary.
+ * This function is a helper which can be used to validate modes against size
+ * limitations of the DRM device/connector. If a mode is too big its status
+ * memeber is updated with the appropriate validation failure code. The list
+ * itself is not changed.
  */
 void drm_mode_validate_size(struct drm_device *dev,
 			    struct list_head *mode_list,
@@ -916,9 +939,10 @@ EXPORT_SYMBOL(drm_mode_validate_size);
  * @mode_list: list of modes to check
  * @verbose: be verbose about it
  *
- * Once mode list generation is complete, a caller can use this routine to
- * remove invalid modes from a mode list.  If any of the modes have a
- * status other than %MODE_OK, they are removed from @mode_list and freed.
+ * This helper function can be used to prune a display mode list after
+ * validation has been completed. All modes who's status is not MODE_OK will be
+ * removed from the list, and if @verbose the status code and mode name is also
+ * printed to dmesg.
  */
 void drm_mode_prune_invalid(struct drm_device *dev,
 			    struct list_head *mode_list, bool verbose)
@@ -948,7 +972,7 @@ EXPORT_SYMBOL(drm_mode_prune_invalid);
  * Compare two modes, given by @lh_a and @lh_b, returning a value indicating
  * which is better.
  *
- * RETURNS:
+ * Returns:
  * Negative if @lh_a is better than @lh_b, zero if they're equivalent, or
  * positive if @lh_b is better than @lh_a.
  */
@@ -976,9 +1000,9 @@ static int drm_mode_compare(void *priv, struct list_head *lh_a, struct list_head
 
 /**
  * drm_mode_sort - sort mode list
- * @mode_list: list to sort
+ * @mode_list: list of drm_display_mode structures to sort
  *
- * Sort @mode_list by favorability, putting good modes first.
+ * Sort @mode_list by favorability, moving good modes to the head of the list.
  */
 void drm_mode_sort(struct list_head *mode_list)
 {
@@ -992,8 +1016,10 @@ EXPORT_SYMBOL(drm_mode_sort);
  *
  * This moves the modes from the @connector probed_modes list
  * to the actual mode list. It compares the probed mode against the current
- * list and only adds different modes. All modes unverified after this point
- * will be removed by the prune invalid modes.
+ * list and only adds different/new modes.
+ *
+ * This is just a helper functions doesn't validate any modes itself and also
+ * doesn't prune any invalid modes. Callers need to do that themselves.
  */
 void drm_mode_connector_list_update(struct drm_connector *connector)
 {
@@ -1028,18 +1054,25 @@ void drm_mode_connector_list_update(struct drm_connector *connector)
 EXPORT_SYMBOL(drm_mode_connector_list_update);
 
 /**
- * drm_mode_parse_command_line_for_connector - parse command line for connector
- * @mode_option: per connector mode option
- * @connector: connector to parse line for
- * @mode: preallocated mode structure to fill out
+ * drm_mode_parse_command_line_for_connector - parse command line modeline for connector
+ * @mode_option: optional per connector mode option
+ * @connector: connector to parse modeline for
+ * @mode: preallocated drm_cmdline_mode structure to fill out
+ *
+ * This parses @mode_option command line modeline for modes and options to
+ * configure the connector. If @mode_option is NULL the default command line
+ * modeline in fb_mode_option will be parsed instead.
  *
- * This parses the connector specific then generic command lines for
- * modes and options to configure the connector.
+ * This uses the same parameters as the fb modedb.c, except for an extra
+ * force-enable, force-enable-digital and force-disable bit at the end:
  *
- * This uses the same parameters as the fb modedb.c, except for extra
  *	<xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m][eDd]
  *
- * enable/enable Digital/disable bit at the end
+ * The intermediate drm_cmdline_mode structure is required to store additional
+ * options from the command line modline like the force-enabel/disable flag.
+ *
+ * Returns:
+ * True if a valid modeline has been parsed, false otherwise.
  */
 bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 					       struct drm_connector *connector,
@@ -1192,6 +1225,14 @@ done:
 }
 EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector);
 
+/**
+ * drm_mode_create_from_cmdline_mode - convert a command line modeline into a DRM display mode
+ * @dev: DRM device to create the new mode for
+ * @cmd: input command line modeline
+ *
+ * Returns:
+ * Pointer to converted mode on success, NULL on error.
+ */
 struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 				  struct drm_cmdline_mode *cmd)
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index b3507f15d010..995c34d91ef1 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -162,6 +162,14 @@ struct drm_cmdline_mode {
 	enum drm_connector_force force;
 };
 
+/**
+ * drm_mode_is_stereo - check for stereo mode flags
+ * @mode: drm_display_mode to check
+ *
+ * Returns:
+ * True if the mode is one of the stereo modes (like side-by-side), false if
+ * not.
+ */
 static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode)
 {
 	return mode->flags & DRM_MODE_FLAG_3D_MASK;
-- 
1.8.5.2

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux