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

Thierry Reding
2015-12-07T15:21:43Z

[-- 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