On Mon, Dec 07, 2015 at 12:00:09PM +0100, Thierry Reding wrote: > 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.vetter@xxxxxxxx> > > Perhaps inherit the copyright statements from the includes that you > extracted these from? Done for the above two - all the stuff below is just moved and would conflict massively with later patches. So left that as per our irc discussion. -Daniel > > > +/** > > + * 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
