On Tue, Apr 29, 2014 at 11:44:34AM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> This fixes up a couple of inconsistencies with kerneldoc comments for
> the EDID related functions. Functions that return a value now use the
> "Return" section consistently. For some exported symbols the kernel-doc
> comments were not in the appropriate format and therefore not included
> in the generated documentation. Others had no kernel-doc at all.
>
> While at it, fix a couple of whitespace issues and capitalize common
> abbreviations (i2c -> I2C, edid -> EDID, ...).
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
Two comments below for further polish, but this looks good imo.
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/drm_edid.c | 128 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 79 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 696186401955..0a52ded1b380 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -984,9 +984,13 @@ static const u8 edid_header[] = {
> 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
> };
>
> - /*
> - * Sanity check the header of the base EDID block. Return 8 if the header
> - * is perfect, down to 0 if it's totally wrong.
> +/**
> + * drm_edid_header_is_valid - sanity check the header of the base EDID block
> + * @raw_edid: pointer to raw base EDID block
> + *
> + * Sanity check the header of the base EDID block.
> + *
> + * Return: 8 if the header is perfect, down to 0 if it's totally wrong.
> */
> int drm_edid_header_is_valid(const u8 *raw_edid)
> {
> @@ -1005,9 +1009,16 @@ module_param_named(edid_fixup, edid_fixup, int, 0400);
> MODULE_PARM_DESC(edid_fixup,
> "Minimum number of valid EDID header bytes (0-8, default 6)");
>
> -/*
> - * Sanity check the EDID block (base or extension). Return 0 if the block
> - * doesn't check out, or 1 if it's valid.
> +/**
> + * drm_edid_block_valid - Sanity check the EDID block (base or extension)
> + * @raw_edid: pointer to raw EDID block
> + * @block: type of block to validate (0 for base, extension otherwise)
> + * @print_bad_edid: if true, dump bad EDID blocks to the console
> + *
> + * Validate a base or extension EDID block and optionally dump bad blocks to
> + * the console.
> + *
> + * Return: True if the block is valid, false otherwise.
> */
> bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
> {
> @@ -1077,6 +1088,8 @@ EXPORT_SYMBOL(drm_edid_block_valid);
> * @edid: EDID data
> *
> * Sanity-check an entire EDID record (including extensions)
> + *
> + * Return: True if the EDID data is valid, false otherwise.
> */
> bool drm_edid_is_valid(struct edid *edid)
> {
> @@ -1096,18 +1109,15 @@ EXPORT_SYMBOL(drm_edid_is_valid);
>
> #define DDC_SEGMENT_ADDR 0x30
> /**
> - * Get EDID information via I2C.
> - *
> - * @adapter : i2c device adaptor
> + * drm_do_probe_ddc_edid() - get EDID information via I2C
> + * @adapter: I2C device adaptor
> * @buf: EDID data buffer to be filled
> * @block: 128 byte EDID block to start fetching from
> * @len: EDID data buffer length to fetch
> *
> - * Returns:
> - *
> - * 0 on success or -1 on failure.
> + * Try to fetch EDID information by calling I2C driver functions.
> *
> - * Try to fetch EDID information by calling i2c driver function.
> + * Return: 0 on success or -1 on failure.
I wonder whether we shouldn't just switch to the common negative error
codes as the return value on failure, with -ENXIO as-is and -EIO when
running out of retry attempts. Just an idea for a follow-up patch.
> */
> static int
> drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
> @@ -1118,7 +1128,8 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
> unsigned char xfers = segment ? 3 : 2;
> int ret, retries = 5;
>
> - /* The core i2c driver will automatically retry the transfer if the
> + /*
> + * The core I2C driver will automatically retry the transfer if the
> * adapter reports EAGAIN. However, we find that bit-banging transfers
> * are susceptible to errors under a heavily loaded machine and
> * generate spurious NAKs and timeouts. Retrying the transfer
> @@ -1144,10 +1155,10 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
> }
> };
>
> - /*
> - * Avoid sending the segment addr to not upset non-compliant ddc
> - * monitors.
> - */
> + /*
> + * Avoid sending the segment addr to not upset non-compliant
> + * DDC monitors.
> + */
> ret = i2c_transfer(adapter, &msgs[3 - xfers], xfers);
>
> if (ret == -ENXIO) {
> @@ -1246,12 +1257,10 @@ out:
> }
>
> /**
> - * Probe DDC presence.
> - * @adapter: i2c adapter to probe
> + * drm_probe_ddc() - probe DDC presence
> + * @adapter: I2C adapter to probe
> *
> - * Returns:
> - *
> - * 1 on success
> + * Return: True on success, false on failure.
> */
> bool
> drm_probe_ddc(struct i2c_adapter *adapter)
> @@ -1265,12 +1274,12 @@ EXPORT_SYMBOL(drm_probe_ddc);
> /**
> * drm_get_edid - get EDID data, if available
> * @connector: connector we're probing
> - * @adapter: i2c adapter to use for DDC
> + * @adapter: I2C adapter to use for DDC
> *
> - * Poke the given i2c channel to grab EDID data if possible. If found,
> + * Poke the given I2C channel to grab EDID data if possible. If found,
> * attach it to the connector.
> *
> - * Return edid data or NULL if we couldn't find any.
> + * Return: Pointer to valid EDID or NULL if we couldn't find any.
> */
> struct edid *drm_get_edid(struct drm_connector *connector,
> struct i2c_adapter *adapter)
> @@ -1288,7 +1297,7 @@ EXPORT_SYMBOL(drm_get_edid);
> * drm_edid_duplicate - duplicate an EDID and the extensions
> * @edid: EDID to duplicate
> *
> - * Return duplicate edid or NULL on allocation failure.
> + * Return: Pointer to duplicated EDID or NULL on allocation failure.
> */
> struct edid *drm_edid_duplicate(const struct edid *edid)
> {
> @@ -1411,7 +1420,8 @@ mode_is_rb(const struct drm_display_mode *mode)
> * @rb: Mode reduced-blanking-ness
> *
> * Walk the DMT mode list looking for a match for the given parameters.
> - * Return a newly allocated copy of the mode, or NULL if not found.
> + *
> + * Return: A newly allocated copy of the mode, or NULL if not found.
> */
> struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
> int hsize, int vsize, int fresh,
> @@ -2139,7 +2149,7 @@ do_established_modes(struct detailed_timing *timing, void *c)
>
> /**
> * add_established_modes - get est. modes from EDID and add them
> - * @connector: connector of for the EDID block
> + * @connector: connector to add mode(s) to
> * @edid: EDID block to scan
> *
> * Each EDID block contains a bitmap of the supported "established modes" list
> @@ -2201,7 +2211,7 @@ do_standard_modes(struct detailed_timing *timing, void *c)
>
> /**
> * add_standard_modes - get std. modes from EDID and add them
> - * @connector: connector of for the EDID block
> + * @connector: connector to add mode(s) to
> * @edid: EDID block to scan
> *
> * Standard modes can be calculated using the appropriate standard (DMT,
> @@ -2422,7 +2432,7 @@ cea_mode_alternate_clock(const struct drm_display_mode *cea_mode)
> * drm_match_cea_mode - look for a CEA mode matching given mode
> * @to_match: display mode
> *
> - * Returns the CEA Video ID (VIC) of the mode or 0 if it isn't a CEA-861
> + * Return: The CEA Video ID (VIC) of the mode or 0 if it isn't a CEA-861
> * mode.
> */
> u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
> @@ -3020,11 +3030,9 @@ monitor_name(struct detailed_timing *t, void *data)
> * @connector: connector corresponding to the HDMI/DP sink
> * @edid: EDID to parse
> *
> - * Fill the ELD (EDID-Like Data) buffer for passing to the audio driver.
> - * Some ELD fields are left to the graphics driver caller:
> - * - Conn_Type
> - * - HDCP
> - * - Port_ID
> + * Fill the ELD (EDID-Like Data) buffer for passing to the audio driver. The
> + * Conn_Type, HDCP and Port_ID ELD fields are left for the graphics driver to
> + * fill in.
> */
> void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> {
> @@ -3108,9 +3116,10 @@ EXPORT_SYMBOL(drm_edid_to_eld);
> * @sads: pointer that will be set to the extracted SADs
> *
> * Looks for CEA EDID block and extracts SADs (Short Audio Descriptors) from it.
> - * Note: returned pointer needs to be kfreed
> *
> - * Return number of found SADs or negative number on error.
> + * Note: The returned pointer needs to be freed using kfree().
> + *
> + * Return: The number of found SADs or negative number on error.
> */
> int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads)
> {
> @@ -3167,9 +3176,11 @@ EXPORT_SYMBOL(drm_edid_to_sad);
> * @sadb: pointer to the speaker block
> *
> * Looks for CEA EDID block and extracts the Speaker Allocation Data Block from it.
> - * Note: returned pointer needs to be kfreed
> *
> - * Return number of found Speaker Allocation Blocks or negative number on error.
> + * Note: The returned pointer needs to be freed using kfree().
> + *
> + * Return: The number of found Speaker Allocation Blocks or negative number on
> + * error.
> */
> int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb)
> {
> @@ -3216,9 +3227,12 @@ int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb)
> EXPORT_SYMBOL(drm_edid_to_speaker_allocation);
>
> /**
> - * drm_av_sync_delay - HDMI/DP sink audio-video sync delay in millisecond
> + * drm_av_sync_delay - compute the HDMI/DP sink audio-video sync delay
> * @connector: connector associated with the HDMI/DP sink
> * @mode: the display mode
> + *
> + * Return: The HDMI/DP sink's audio-video sync delay in milliseconds or 0 if
> + * the sink doesn't support audio or video.
> */
> int drm_av_sync_delay(struct drm_connector *connector,
> struct drm_display_mode *mode)
> @@ -3260,6 +3274,9 @@ EXPORT_SYMBOL(drm_av_sync_delay);
> *
> * It's possible for one encoder to be associated with multiple HDMI/DP sinks.
> * The policy is now hard coded to simply use the first HDMI/DP sink's ELD.
> + *
> + * Return: The connector associated with the first HDMI/DP sink that has ELD
> + * attached to it.
> */
> struct drm_connector *drm_select_eld(struct drm_encoder *encoder,
> struct drm_display_mode *mode)
> @@ -3276,11 +3293,12 @@ struct drm_connector *drm_select_eld(struct drm_encoder *encoder,
> EXPORT_SYMBOL(drm_select_eld);
>
> /**
> - * drm_detect_hdmi_monitor - detect whether monitor is hdmi.
> + * drm_detect_hdmi_monitor - detect whether monitor is HDMI
> * @edid: monitor EDID information
> *
> * Parse the CEA extension according to CEA-861-B.
> - * Return true if HDMI, false if not or unknown.
> + *
> + * Return: True if the monitor is HDMI, false if not or unknown.
> */
> bool drm_detect_hdmi_monitor(struct edid *edid)
> {
> @@ -3318,6 +3336,7 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor);
> * audio format, assume at least 'basic audio' support, even if 'basic
> * audio' is not defined in EDID.
> *
> + * Return: True if the monitor supports audio, false otherwise.
> */
> bool drm_detect_monitor_audio(struct edid *edid)
> {
> @@ -3361,6 +3380,8 @@ EXPORT_SYMBOL(drm_detect_monitor_audio);
> * Check whether the monitor reports the RGB quantization range selection
> * as supported. The AVI infoframe can then be used to inform the monitor
> * which quantization range (full or limited) is used.
> + *
> + * Return: True if the RGB quantization range is selectable, false otherwise.
> */
> bool drm_rgb_quant_range_selectable(struct edid *edid)
> {
> @@ -3465,11 +3486,11 @@ static void drm_add_display_info(struct edid *edid,
> /**
> * drm_add_edid_modes - add modes from EDID data, if available
> * @connector: connector we're probing
> - * @edid: edid data
> + * @edid: EDID data
> *
> * Add the specified modes to the connector's mode list.
> *
> - * Return number of modes added or 0 if we couldn't find any.
> + * Return: The number of modes added or 0 if we couldn't find any.
> */
> int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> {
> @@ -3531,7 +3552,7 @@ EXPORT_SYMBOL(drm_add_edid_modes);
> * Add the specified modes to the connector's mode list. Only when the
> * hdisplay/vdisplay is not beyond the given limit, it will be added.
> *
> - * Return number of modes added or 0 if we couldn't find any.
> + * Return: The number of modes added or 0 if we couldn't find any.
> */
> int drm_add_modes_noedid(struct drm_connector *connector,
> int hdisplay, int vdisplay)
I wonder whether this wouldn't make more sense in drm_modes.c now, but
again follow-up patch material.
> @@ -3570,13 +3591,22 @@ int drm_add_modes_noedid(struct drm_connector *connector,
> }
> EXPORT_SYMBOL(drm_add_modes_noedid);
>
> +/**
> + * drm_set_preferred_mode - Sets the preferred mode of a connector
> + * @connector: connector whose mode list should be processed
> + * @hpref: horizontal resolution of preferred mode
> + * @vpref: vertical resolution of preferred mode
> + *
> + * Marks a mode as preferred if it matches the resolution specified by @hpref
> + * and @vpref.
> + */
> void drm_set_preferred_mode(struct drm_connector *connector,
> int hpref, int vpref)
> {
> struct drm_display_mode *mode;
>
> list_for_each_entry(mode, &connector->probed_modes, head) {
> - if (mode->hdisplay == hpref &&
> + if (mode->hdisplay == hpref &&
> mode->vdisplay == vpref)
> mode->type |= DRM_MODE_TYPE_PREFERRED;
> }
> @@ -3589,7 +3619,7 @@ EXPORT_SYMBOL(drm_set_preferred_mode);
> * @frame: HDMI AVI infoframe
> * @mode: DRM display mode
> *
> - * Returns 0 on success or a negative error code on failure.
> + * Return: 0 on success or a negative error code on failure.
> */
> int
> drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> @@ -3654,7 +3684,7 @@ s3d_structure_from_display_mode(const struct drm_display_mode *mode)
> * 4k or stereoscopic 3D mode. So when giving any other mode as input this
> * function will return -EINVAL, error that can be safely ignored.
> *
> - * Returns 0 on success or a negative error code on failure.
> + * Return: 0 on success or a negative error code on failure.
> */
> int
> drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
> --
> 1.9.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
