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

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5377 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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