Re: [PATCH 27/28] drm: Document drm_encoder/crtc_helper_funcs

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

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

On Fri, Dec 04, 2015 at 09:46:08AM +0100, Daniel Vetter wrote:
> Mostly this is about all the callbacks used to modesets by both legacy

"used for modesets"?

> CRTC helpers and atomic helpers and I figured it doesn't make all that
> much sense to split this up.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  include/drm/drm_modeset_helper_vtables.h | 447 +++++++++++++++++++++++++++----
>  1 file changed, 400 insertions(+), 47 deletions(-)
> 
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 22cc51b278fb..d776ef6dd00e 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -46,58 +46,236 @@ enum mode_set_atomic;
>  
>  /**
>   * struct drm_crtc_helper_funcs - helper operations for CRTCs
> - * @dpms: set power state
> - * @prepare: prepare the CRTC, called before @mode_set
> - * @commit: commit changes to CRTC, called after @mode_set
> - * @mode_fixup: try to fixup proposed mode for this CRTC
> - * @mode_set: set this mode
> - * @mode_set_nofb: set mode only (no scanout buffer attached)
> - * @mode_set_base: update the scanout buffer
> - * @mode_set_base_atomic: non-blocking mode set (used for kgdb support)
> - * @load_lut: load color palette
> - * @disable: disable CRTC when no longer in use
> - * @enable: enable CRTC
>   *
> - * The helper operations are called by the mid-layer CRTC helper.
> - *
> - * Note that with atomic helpers @dpms, @prepare and @commit hooks are
> - * deprecated. Used @enable and @disable instead exclusively.
> - *
> - * With legacy crtc helpers there's a big semantic difference between @disable
> - * and the other hooks: @disable also needs to release any resources acquired in
> - * @mode_set (like shared PLLs).
> + * These hooks are used by the legacy CRTC helpers, the transitional plane
> + * helpers and the new atomic modesetting helpers.
>   */
>  struct drm_crtc_helper_funcs {
> -	/*
> -	 * Control power levels on the CRTC.  If the mode passed in is
> -	 * unsupported, the provider must use the next lowest power level.
> +	/**
> +	 * @dpms:
> +	 *
> +	 * Callback to control power levels on the CRTC.  If the mode passed in
> +	 * is unsupported, the provider must use the next lowest power level.
> +	 * This is used by the legacy CRTC helpers to implement DPMS
> +	 * functionality in drm_helper_connector_dpms().
> +	 *
> +	 * This callback is also used to disable a CRTC by calling it with
> +	 * DRM_MODE_DPMS_OFF if the @disable hook isn't used.
> +	 *
> +	 * This callback is used by the legacy CRTC helpers.  Atomic helpers
> +	 * also support using this hook for enabling and disabling a CRTC to
> +	 * facilitate transitions to atomic, but it is deprecated. Instead
> +	 * @enable and @disable should be used.
>  	 */
>  	void (*dpms)(struct drm_crtc *crtc, int mode);
> +
> +	/**
> +	 * @prepare:
> +	 *
> +	 * This callback should prepare the CRTC for a subsequent modeset, which
> +	 * in practice means the driver should disable the CRTC if it is
> +	 * running. Most drivers ended up implementing this by calling their
> +	 * @dpms hook with DRM_MODE_DPMS_OFF.
> +	 *
> +	 * This callback is used by the legacy CRTC helpers.  Atomic helpers
> +	 * also support using this hook for disabling a CRTC to facilitate
> +	 * transitions to atomic, but it is deprecated. Instead @disable should
> +	 * be used.
> +	 */
>  	void (*prepare)(struct drm_crtc *crtc);
> +
> +	/**
> +	 * @commit:
> +	 *
> +	 * This callback should commit the new mode on the CRTC after a modeset,
> +	 * which in practice means the driver should enable the CRTC.  Most
> +	 * drivers ended up implementing this by calling their @dpms hook with
> +	 * DRM_MODE_DPMS_ON.
> +	 *
> +	 * This callback is used by the legacy CRTC helpers.  Atomic helpers
> +	 * also support using this hook for enabling a CRTC to facilitate
> +	 * transitions to atomic, but it is deprecated. Instead @enable should
> +	 * be used.
> +	 */
>  	void (*commit)(struct drm_crtc *crtc);
>  
> -	/* Provider can fixup or change mode timings before modeset occurs */
> +	/**
> +	 * @mode_fixup:
> +	 *
> +	 * This callback is used to validate a mode. The paramater mode is the

"parameter"

[...]
> +	/**
> +	 * @disable:
> +	 *
> +	 * This callback should be used to disable the CRTC. With the atomic
> +	 * drivers it is called after all encoders connected to this CRTC have
> +	 * been shut off already using their own ->disable hook. If that
> +	 * sequence is too simple drivers can just add their own hooks and call
> +	 * it from this CRTC callback here by looping over all encoders
> +	 * connected to it using for_each_encoder_on_crtc().
> +	 *
> +	 * This hook is used both by legacy CRTC helpers and atomic helpers.
> +	 * Atomic drivers don't need to implement it if there's no need to
> +	 * disable anything at the CRTC level. To ensure that runtime PM
> +	 * handling (using either DPMS or the new "ACTIVE" property) works
> +	 * @disable must be the inversve of @enable for atomic drivers.

"inverse"

> +	 *
> +	 * NOTE:
> +	 *
> +	 * With legacy CRTC helpers there's a big semantic difference between
> +	 * @disable other hooks (like @prepare or @dpms) used to shut down a

"and other hooks"

> +	 * CRTC: @disable is only called when also logically disabling the
> +	 * display pipeline and needs to release any resources acquired in
> +	 * @mode_set (like shared PLLs, or again release pinned framebuffers).
> +	 *
> +	 * Therefore @disable must be the inverse of @mode_set plus @commit for
> +	 * drivers still using legacy CRTC helpers, which is different from the
> +	 * rules under atomic.
> +	 */
>  	void (*disable)(struct drm_crtc *crtc);
[...]
>  struct drm_encoder_helper_funcs {
> +	/**
> +	 * @dpms:
> +	 *
> +	 * Callback to control power levels on the encoder.  If the mode passed in
> +	 * is unsupported, the provider must use the next lowest power level.
> +	 * This is used by the legacy encoder helpers to implement DPMS
> +	 * functionality in drm_helper_connector_dpms().
> +	 *
> +	 * This callback is also used to disable a encoder by calling it with

"disable an encoder"

> +	 * DRM_MODE_DPMS_OFF if the @disable hook isn't used.
> +	 *
> +	 * This callback is used by the legacy CRTC helpers.  Atomic helpers
> +	 * also support using this hook for enabling and disabling a encoder to
> +	 * facilitate transitions to atomic, but it is deprecated. Instead
> +	 * @enable and @disable should be used.
> +	 */
>  	void (*dpms)(struct drm_encoder *encoder, int mode);
>  
> +	/**
> +	 * @mode_fixup:
> +	 *
> +	 * This callback is used to validate and adjust a mode. The paramater

"parameter"

> +	/**
> +	 * @disable:
> +	 *
> +	 * This callback should be used to disable the encoder. With the atomic
> +	 * drivers it is called before this encoder's CRTC has been shut off
> +	 * using the CRTC's own ->disable hook.  If that sequence is too simple
> +	 * drivers can just add their own driver private encoder hooks and call
> +	 * them from CRTC's callback by looping over all encoders connected to
> +	 * it using for_each_encoder_on_crtc().
> +	 *
> +	 * This hook is used both by legacy CRTC helpers and atomic helpers.
> +	 * Atomic drivers don't need to implement it if there's no need to
> +	 * disable anything at the encoder level. To ensure that runtime PM
> +	 * handling (using either DPMS or the new "ACTIVE" property) works
> +	 * @disable must be the inversve of @enable for atomic drivers.

"inverse"

> +	 *
> +	 * NOTE:
> +	 *
> +	 * With legacy CRTC helpers there's a big semantic difference between
> +	 * @disable other hooks (like @prepare or @dpms) used to shut down a

"and other hooks", "an encoder"

> +	/**
> +	 * @enable:
> +	 *
> +	 * This callback should be used to enable the encoder. With the atomic
> +	 * drivers it is called after this encoder's CRTC has been enabled using
> +	 * the CRTC's own ->enable hook.  If that sequence is too simple drivers
> +	 * can just add their own driver private encoder hooks and call them
> +	 * from CRTC's callback by looping over all encoders connected to it
> +	 * using for_each_encoder_on_crtc().
> +	 *
> +	 * This hook is used only by atomic helpers, for symmetry with @disable.
> +	 * Atomic drivers don't need to implement it if there's no need to
> +	 * enable anything at the encoder level. To ensure that runtime PM handling
> +	 * (using either DPMS or the new "ACTIVE" property) works
> +	 * @enable must be the inversve of @disable for atomic drivers.

"inverse"

> +	 */
>  	void (*enable)(struct drm_encoder *encoder);
>  
> -	/* atomic helpers */
> +	/**
> +	 * @atomic_check:
> +	 *
> +	 * This callback is used to validate encoder state for atomic drivers.
> +	 * Since the encoder is the object connecting the CRTC and connector it
> +	 * gets passed both states, to be able to validate interactions and
> +	 * update the CRTC to match what the encoder needs for the requested
> +	 * connector.
> +	 *
> +	 * This function is used by the atomic helpers, but it is optional.
> +	 *
> +	 * NOTE:
> +	 *
> +	 * This function is called in the check phase of an atomic update. The
> +	 * driver is not allowed to change anything outside of the free-standing
> +	 * state objects passed-in or assembled in the overall &drm_atomic_state
> +	 * update tracking structure.
> +	 *
> +	 * RETURNS:
> +	 *
> +	 * 0 on success, -EINVAL if the state or the transition can't be
> +	 * support, -ENOMEM on memory allocation failure and -EDEADLK if an

"supported"

Thanks for writing this up, it's great documentation.

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