On Fri, Dec 04, 2015 at 09:45:44AM +0100, Daniel Vetter wrote:
[...]
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c 
> b/drivers/gpu/drm/drm_crtc_helper.c
> index 10d0989db273..077e48d3cac2 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -62,6 +62,11 @@
>   * converting to the plane helpers). New drivers must not use these functions
>   * but need to implement the atomic interface instead, potentially using the
>   * atomic helpers for that.
> + *
> + * The these legacy modeset helpers use the same function table structures as

s/The these/The/

> diff --git a/include/drm/drm_modeset_helper_vtables.h 
> b/include/drm/drm_modeset_helper_vtables.h
> new file mode 100644
> index 000000000000..35c5a1c4e7ba
> --- /dev/null
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -0,0 +1,252 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *   Daniel Vetter <daniel.vet...@ffwll.ch>

Perhaps inherit the copyright statements from the includes that you
extracted these from?

> +/**
> + * DOC: overview
> + *
> + * The DRM mode setting helper functions are common code for drivers to use 
> if
> + * they wish.  Drivers are not forced to use this code in their
> + * implementations but it would be useful if the code they do use at least
> + * provides a consistent interface and operation to userspace. Therefore it 
> is
> + * highly recommended to use the provided helpers as much as possible.
> + *
> + * Because there is only one pointer per modeset object to hold a vfunc table
> + * for helper libraries they are by necessity shared among the different
> + * helpers.
> + *
> + * To make this clear all the helper vtables are pulled together in this 
> location here.

Perhaps drop the "here" at the end of that sentence. Also maybe wrap the
last line because it stands out as much longer than the above.

> + */
> +
> +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
> + * @atomic_check: check for validity of an atomic state
> + * @atomic_begin: begin atomic update
> + * @atomic_flush: flush atomic update
> + *
> + * 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.

I /think/ it would be more correct to say: "Use @enable and @disable
exclusively instead."

> + *
> + * With legacy crtc helpers there's a big semantic difference between 
> @disable

s/crtc/CRTC/, there's a couple more places where the casing is
inconsistent, I'll refrain from pointing them out explicitly since your
editor will be much better at finding them.

> +/**
> + * struct drm_encoder_helper_funcs - helper operations for encoders
> + * @dpms: set power state
> + * @save: save connector state
> + * @restore: restore connector state
> + * @mode_fixup: try to fixup proposed mode for this connector
> + * @prepare: part of the disable sequence, called before the CRTC modeset
> + * @commit: called after the CRTC modeset
> + * @mode_set: set this mode, optional for atomic helpers
> + * @get_crtc: return CRTC that the encoder is currently attached to
> + * @detect: connection status detection
> + * @disable: disable encoder when not in use (overrides DPMS off)
> + * @enable: enable encoder
> + * @atomic_check: check for validity of an atomic update
> + *
> + * 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.

Same comment as for the CRTC helper functions.

Thierry

Attachment: signature.asc
Description: PGP signature

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

Reply via email to