Re: [PATCH 24/28] drm: Document drm_connector_helper_funcs

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

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

On Fri, Dec 04, 2015 at 09:46:05AM +0100, Daniel Vetter wrote:
> Nothing special, except the somewhat awkard split in probe helper

"awkward"

> callbacks between here and drm_crtc_funcs.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  include/drm/drm_modeset_helper_vtables.h | 106 +++++++++++++++++++++++++++++--
>  1 file changed, 101 insertions(+), 5 deletions(-)
> 
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 66b78c14154e..22cc51b278fb 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -264,18 +264,114 @@ static inline void drm_encoder_helper_add(struct drm_encoder *encoder,
>  
>  /**
>   * struct drm_connector_helper_funcs - helper operations for connectors
> - * @get_modes: get mode list for this connector
> - * @mode_valid: is this mode valid on the given connector? (optional)
> - * @best_encoder: return the preferred encoder for this connector
> - * @atomic_best_encoder: atomic version of @best_encoder
>   *
> - * The helper operations are called by the mid-layer CRTC helper.
> + * These functions are used by the atomic and legacy modeset helpers and by the
> + * probe helpers.
>   */
>  struct drm_connector_helper_funcs {
> +	/**
> +	 * @get_modes:
> +	 *
> +	 * This function should fill in all modes currently valid for the sink
> +	 * into the connector->probe_modes function. It should also update the

What's probe_modes? I've never heard of it. Did you mean ->fill_modes()?
Also it's strange to say "fill into the ... function". Perhaps "pass
into the ... function" instead?

> +	 * EDID property by calling drm_mode_connector_update_edid_property().
> +	 *
> +	 * The usual way to implement this is to cache the EDID retrieved in the
> +	 * probe callback somewhere in the driver-private connector structure.
> +	 * In this function drivers then parse the modes in the EDID and add it

"add them"?

> +	 * by calling drm_add_edid_modes(). But connectors that driver a fixed

"drive"

> +	 * panel can also manually add specific modes using
> +	 * drm_mode_probed_add(). Finally drivers that support audio propably

"probably"

> +	/**
> +	 * @mode_valid:
> +	 *
> +	 * Callback to validate a mode for a connector, irrespective of the
> +	 * specific display configuration.
> +	 *
> +	 * This callback is used by the probe helpers to filter the mode list
> +	 * (which is usually derived from the EDID data block from the sink).
> +	 * See e.g. drm_helper_probe_single_connector_modes().
> +	 *
> +	 * NOTE:
> +	 *
> +	 * This only filters the mode list supplied to userspace in the
> +	 * GETCONNECOTR ioctl. Userspace is free to create modes of its own and

"GETCONNECTOR IOCTL"

> +	 * ask the kernel to use it. It this case the atomic helpers or legacy

"to use them"

> +	 * CRTC heleprs will not call this function. Drivers therefore must

"helpers"

> +	 * still fully validate any mode passed in in a modeset request.
> +	 *
> +	 * RETURNS:
> +	 *
> +	 * Either DRM_MODE_OK or one of the failure reasons in enum

The enum value is MODE_OK, without the DRM_ prefix.

Thierry

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

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx