[03/28] drm: Reorganize helper vtables and their docs

Commit Message

Daniel Vetter Dec. 4, 2015, 8:45 a.m. UTC

Currently we have 4 helper libraries (probe, crtc, plane & atomic)
that all use the same helper vtables. And that's by necessity since we
don't want to litter the core structs with one ops pointer per helper
library. Also often the reuse the same hooks (like atomic does, to
facilite conversion from existing drivers using crtc and plane
helpers).

Given all that it doesn't make sense to put the docs for these next to
specific helpers. Instead extract them into a new header file and
section in the docbook, and add references to them everywhere.

Unfortunately kernel-doc complains when an include directive doesn't
find anything (and it does by dumping crap into the output file). We
have to remove the now empty includes to avoid that, instead of leaving
them in for future proofing.

v2: More OCD in ordering functions.

Signed-off-by: Daniel Vetter <[e-mail]>
---
 Documentation/DocBook/gpu.tmpl           |   6 +-
 drivers/gpu/drm/drm_atomic_helper.c      |   6 +
 drivers/gpu/drm/drm_crtc_helper.c        |   5 +
 drivers/gpu/drm/drm_plane_helper.c       |   4 +
 drivers/gpu/drm/drm_probe_helper.c       |   3 +
 include/drm/drm_crtc_helper.h            | 158 +------------------
 include/drm/drm_modeset_helper_vtables.h | 252 +++++++++++++++++++++++++++++++
 include/drm/drm_plane_helper.h           |  38 +----
 8 files changed, 277 insertions(+), 195 deletions(-)
 create mode 100644 include/drm/drm_modeset_helper_vtables.h

Comments

Thierry Reding Dec. 7, 2015, 11 a.m. UTC | #1

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 <[e-mail]>

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

Daniel Vetter Dec. 7, 2015, 11:59 a.m. UTC | #2

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 <[e-mail]>
> 
> 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

Thierry Reding Dec. 7, 2015, 12:26 p.m. UTC | #3

On Mon, Dec 07, 2015 at 12:59:33PM +0100, Daniel Vetter wrote:
> 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 <[e-mail]>
> > 
> > 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.

For the record, I'm fine with leave the below as-is and fix it up in a
follow-up patch.

Thierry

Patch

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index e6735d2b6ae8..9952d6ff052f 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2303,8 +2303,12 @@  void intel_crt_init(struct drm_device *dev)
 !Edrivers/gpu/drm/drm_atomic_helper.c
     </sect2>
     <sect2>
+      <title>Modeset Helper Reference for Common Vtables</title>
+!Iinclude/drm/drm_modeset_helper_vtables.h
+!Pinclude/drm/drm_modeset_helper_vtables.h overview
+    </sect2>
+    <sect2>
       <title>Modeset Helper Functions Reference</title>
-!Iinclude/drm/drm_crtc_helper.h
 !Edrivers/gpu/drm/drm_crtc_helper.c
 !Pdrivers/gpu/drm/drm_crtc_helper.c overview
     </sect2>
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d9053ebf4903..2cf8ab7dbc8c 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -52,6 +52,12 @@ 
  * drm_atomic_helper_disable_plane(), drm_atomic_helper_disable_plane() and the
  * various functions to implement set_property callbacks. New drivers must not
  * implement these functions themselves but must use the provided helpers.
+ *
+ * The atomic helper uses the same function table structures as all other
+ * modesetting helpers. See the documentation for struct &drm_crtc_helper_funcs,
+ * struct &drm_encoder_helper_funcs and struct &drm_connector_helper_funcs. It
+ * also shares the struct &drm_plane_helper_funcs function table with the plane
+ * helpers.
  */
 static void
 drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
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
+ * all other modesetting helpers. See the documentation for struct
+ * &drm_crtc_helper_funcs, struct &drm_encoder_helper_funcs and struct
+ * &drm_connector_helper_funcs.
  */
 MODULE_AUTHOR("David Airlie, Jesse Barnes");
 MODULE_DESCRIPTION("DRM KMS helper");
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index a6983d41920d..8455e996dd9c 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -57,6 +57,10 @@ 
  * by the atomic helpers.
  *
  * Again drivers are strongly urged to switch to the new interfaces.
+ *
+ * The plane helpers share the function table structures with other helpers,
+ * specifically also the atomic helpers. See struct &drm_plane_helper_funcs for
+ * the details.
  */
 
 /*
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index b7bdf12c54a5..dcd7c0289e03 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -53,6 +53,9 @@ 
  * This helper library can be used independently of the modeset helper library.
  * Drivers can also overwrite different parts e.g. use their own hotplug
  * handling code to avoid probing unrelated outputs.
+ *
+ * The probe helpers share the function table structures with other display
+ * helper libraries. See struct &drm_connector_helper_funcs for the details.
  */
 
 static bool drm_kms_helper_poll = true;
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 26cc978f79e7..f94ff54ae25e 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -40,145 +40,7 @@ 
 #include <linux/fb.h>
 
 #include <drm/drm_crtc.h>
-
-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.
- *
- * 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).
- */
-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.
-	 */
-	void (*dpms)(struct drm_crtc *crtc, int mode);
-	void (*prepare)(struct drm_crtc *crtc);
-	void (*commit)(struct drm_crtc *crtc);
-
-	/* Provider can fixup or change mode timings before modeset occurs */
-	bool (*mode_fixup)(struct drm_crtc *crtc,
-			   const struct drm_display_mode *mode,
-			   struct drm_display_mode *adjusted_mode);
-	/* Actually set the mode */
-	int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode,
-			struct drm_display_mode *adjusted_mode, int x, int y,
-			struct drm_framebuffer *old_fb);
-	/* Actually set the mode for atomic helpers, optional */
-	void (*mode_set_nofb)(struct drm_crtc *crtc);
-
-	/* Move the crtc on the current fb to the given position *optional* */
-	int (*mode_set_base)(struct drm_crtc *crtc, int x, int y,
-			     struct drm_framebuffer *old_fb);
-	int (*mode_set_base_atomic)(struct drm_crtc *crtc,
-				    struct drm_framebuffer *fb, int x, int y,
-				    enum mode_set_atomic);
-
-	/* reload the current crtc LUT */
-	void (*load_lut)(struct drm_crtc *crtc);
-
-	void (*disable)(struct drm_crtc *crtc);
-	void (*enable)(struct drm_crtc *crtc);
-
-	/* atomic helpers */
-	int (*atomic_check)(struct drm_crtc *crtc,
-			    struct drm_crtc_state *state);
-	void (*atomic_begin)(struct drm_crtc *crtc,
-			     struct drm_crtc_state *old_crtc_state);
-	void (*atomic_flush)(struct drm_crtc *crtc,
-			     struct drm_crtc_state *old_crtc_state);
-};
-
-/**
- * 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.
- *
- * 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).
- */
-struct drm_encoder_helper_funcs {
-	void (*dpms)(struct drm_encoder *encoder, int mode);
-	void (*save)(struct drm_encoder *encoder);
-	void (*restore)(struct drm_encoder *encoder);
-
-	bool (*mode_fixup)(struct drm_encoder *encoder,
-			   const struct drm_display_mode *mode,
-			   struct drm_display_mode *adjusted_mode);
-	void (*prepare)(struct drm_encoder *encoder);
-	void (*commit)(struct drm_encoder *encoder);
-	void (*mode_set)(struct drm_encoder *encoder,
-			 struct drm_display_mode *mode,
-			 struct drm_display_mode *adjusted_mode);
-	struct drm_crtc *(*get_crtc)(struct drm_encoder *encoder);
-	/* detect for DAC style encoders */
-	enum drm_connector_status (*detect)(struct drm_encoder *encoder,
-					    struct drm_connector *connector);
-	void (*disable)(struct drm_encoder *encoder);
-
-	void (*enable)(struct drm_encoder *encoder);
-
-	/* atomic helpers */
-	int (*atomic_check)(struct drm_encoder *encoder,
-			    struct drm_crtc_state *crtc_state,
-			    struct drm_connector_state *conn_state);
-};
-
-/**
- * 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.
- */
-struct drm_connector_helper_funcs {
-	int (*get_modes)(struct drm_connector *connector);
-	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
-					   struct drm_display_mode *mode);
-	struct drm_encoder *(*best_encoder)(struct drm_connector *connector);
-	struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector,
-						   struct drm_connector_state *connector_state);
-};
+#include <drm/drm_modeset_helper_vtables.h>
 
 extern void drm_helper_disable_unused_functions(struct drm_device *dev);
 extern int drm_crtc_helper_set_config(struct drm_mode_set *set);
@@ -196,24 +58,6 @@  extern void drm_helper_move_panel_connectors_to_head(struct drm_device *);
 extern void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
 					   const struct drm_mode_fb_cmd2 *mode_cmd);
 
-static inline void drm_crtc_helper_add(struct drm_crtc *crtc,
-				       const struct drm_crtc_helper_funcs *funcs)
-{
-	crtc->helper_private = funcs;
-}
-
-static inline void drm_encoder_helper_add(struct drm_encoder *encoder,
-					  const struct drm_encoder_helper_funcs *funcs)
-{
-	encoder->helper_private = funcs;
-}
-
-static inline void drm_connector_helper_add(struct drm_connector *connector,
-					    const struct drm_connector_helper_funcs *funcs)
-{
-	connector->helper_private = funcs;
-}
-
 extern void drm_helper_resume_force_mode(struct drm_device *dev);
 
 int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
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 <[e-mail]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __DRM_MODESET_HELPER_VTABLES_H__
+#define __DRM_MODESET_HELPER_VTABLES_H__
+
+#include <drm/drm_crtc.h>
+
+/**
+ * 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.
+ */
+
+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.
+ *
+ * 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).
+ */
+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.
+	 */
+	void (*dpms)(struct drm_crtc *crtc, int mode);
+	void (*prepare)(struct drm_crtc *crtc);
+	void (*commit)(struct drm_crtc *crtc);
+
+	/* Provider can fixup or change mode timings before modeset occurs */
+	bool (*mode_fixup)(struct drm_crtc *crtc,
+			   const struct drm_display_mode *mode,
+			   struct drm_display_mode *adjusted_mode);
+	/* Actually set the mode */
+	int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode,
+			struct drm_display_mode *adjusted_mode, int x, int y,
+			struct drm_framebuffer *old_fb);
+	/* Actually set the mode for atomic helpers, optional */
+	void (*mode_set_nofb)(struct drm_crtc *crtc);
+
+	/* Move the crtc on the current fb to the given position *optional* */
+	int (*mode_set_base)(struct drm_crtc *crtc, int x, int y,
+			     struct drm_framebuffer *old_fb);
+	int (*mode_set_base_atomic)(struct drm_crtc *crtc,
+				    struct drm_framebuffer *fb, int x, int y,
+				    enum mode_set_atomic);
+
+	/* reload the current crtc LUT */
+	void (*load_lut)(struct drm_crtc *crtc);
+
+	void (*disable)(struct drm_crtc *crtc);
+	void (*enable)(struct drm_crtc *crtc);
+
+	/* atomic helpers */
+	int (*atomic_check)(struct drm_crtc *crtc,
+			    struct drm_crtc_state *state);
+	void (*atomic_begin)(struct drm_crtc *crtc,
+			     struct drm_crtc_state *old_crtc_state);
+	void (*atomic_flush)(struct drm_crtc *crtc,
+			     struct drm_crtc_state *old_crtc_state);
+};
+
+/**
+ * drm_crtc_helper_add - sets the helper vtable for a crtc
+ * @crtc: DRM CRTC
+ * @funcs: helper vtable to set for @crtc
+ */
+static inline void drm_crtc_helper_add(struct drm_crtc *crtc,
+				       const struct drm_crtc_helper_funcs *funcs)
+{
+	crtc->helper_private = funcs;
+}
+
+/**
+ * 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.
+ *
+ * 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).
+ */
+struct drm_encoder_helper_funcs {
+	void (*dpms)(struct drm_encoder *encoder, int mode);
+	void (*save)(struct drm_encoder *encoder);
+	void (*restore)(struct drm_encoder *encoder);
+
+	bool (*mode_fixup)(struct drm_encoder *encoder,
+			   const struct drm_display_mode *mode,
+			   struct drm_display_mode *adjusted_mode);
+	void (*prepare)(struct drm_encoder *encoder);
+	void (*commit)(struct drm_encoder *encoder);
+	void (*mode_set)(struct drm_encoder *encoder,
+			 struct drm_display_mode *mode,
+			 struct drm_display_mode *adjusted_mode);
+	struct drm_crtc *(*get_crtc)(struct drm_encoder *encoder);
+	/* detect for DAC style encoders */
+	enum drm_connector_status (*detect)(struct drm_encoder *encoder,
+					    struct drm_connector *connector);
+	void (*disable)(struct drm_encoder *encoder);
+
+	void (*enable)(struct drm_encoder *encoder);
+
+	/* atomic helpers */
+	int (*atomic_check)(struct drm_encoder *encoder,
+			    struct drm_crtc_state *crtc_state,
+			    struct drm_connector_state *conn_state);
+};
+
+/**
+ * drm_encoder_helper_add - sets the helper vtable for a encoder
+ * @encoder: DRM encoder
+ * @funcs: helper vtable to set for @encoder
+ */
+static inline void drm_encoder_helper_add(struct drm_encoder *encoder,
+					  const struct drm_encoder_helper_funcs *funcs)
+{
+	encoder->helper_private = funcs;
+}
+
+/**
+ * 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.
+ */
+struct drm_connector_helper_funcs {
+	int (*get_modes)(struct drm_connector *connector);
+	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
+					   struct drm_display_mode *mode);
+	struct drm_encoder *(*best_encoder)(struct drm_connector *connector);
+	struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector,
+						   struct drm_connector_state *connector_state);
+};
+
+/**
+ * drm_connector_helper_add - sets the helper vtable for a connector
+ * @connector: DRM connector
+ * @funcs: helper vtable to set for @connector
+ */
+static inline void drm_connector_helper_add(struct drm_connector *connector,
+					    const struct drm_connector_helper_funcs *funcs)
+{
+	connector->helper_private = funcs;
+}
+
+/**
+ * struct drm_plane_helper_funcs - helper operations for CRTCs
+ * @prepare_fb: prepare a framebuffer for use by the plane
+ * @cleanup_fb: cleanup a framebuffer when it's no longer used by the plane
+ * @atomic_check: check that a given atomic state is valid and can be applied
+ * @atomic_update: apply an atomic state to the plane (mandatory)
+ * @atomic_disable: disable the plane
+ *
+ * The helper operations are called by the mid-layer CRTC helper.
+ */
+struct drm_plane_helper_funcs {
+	int (*prepare_fb)(struct drm_plane *plane,
+			  const struct drm_plane_state *new_state);
+	void (*cleanup_fb)(struct drm_plane *plane,
+			   const struct drm_plane_state *old_state);
+
+	int (*atomic_check)(struct drm_plane *plane,
+			    struct drm_plane_state *state);
+	void (*atomic_update)(struct drm_plane *plane,
+			      struct drm_plane_state *old_state);
+	void (*atomic_disable)(struct drm_plane *plane,
+			       struct drm_plane_state *old_state);
+};
+
+/**
+ * drm_plane_helper_add - sets the helper vtable for a plane
+ * @plane: DRM plane
+ * @funcs: helper vtable to set for @plane
+ */
+static inline void drm_plane_helper_add(struct drm_plane *plane,
+					const struct drm_plane_helper_funcs *funcs)
+{
+	plane->helper_private = funcs;
+}
+
+#endif
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 5a7f9d4efb1d..4421f3f4ca8d 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -26,6 +26,7 @@ 
 
 #include <drm/drm_rect.h>
 #include <drm/drm_crtc.h>
+#include <drm/drm_modeset_helper_vtables.h>
 
 /*
  * Drivers that don't allow primary plane scaling may pass this macro in place
@@ -36,46 +37,9 @@ 
  */
 #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
 
-/**
- * DOC: plane helpers
- *
- * Helper functions to assist with creation and handling of CRTC primary
- * planes.
- */
-
 int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 		  const struct drm_crtc_funcs *funcs);
 
-/**
- * drm_plane_helper_funcs - helper operations for CRTCs
- * @prepare_fb: prepare a framebuffer for use by the plane
- * @cleanup_fb: cleanup a framebuffer when it's no longer used by the plane
- * @atomic_check: check that a given atomic state is valid and can be applied
- * @atomic_update: apply an atomic state to the plane (mandatory)
- * @atomic_disable: disable the plane
- *
- * The helper operations are called by the mid-layer CRTC helper.
- */
-struct drm_plane_helper_funcs {
-	int (*prepare_fb)(struct drm_plane *plane,
-			  const struct drm_plane_state *new_state);
-	void (*cleanup_fb)(struct drm_plane *plane,
-			   const struct drm_plane_state *old_state);
-
-	int (*atomic_check)(struct drm_plane *plane,
-			    struct drm_plane_state *state);
-	void (*atomic_update)(struct drm_plane *plane,
-			      struct drm_plane_state *old_state);
-	void (*atomic_disable)(struct drm_plane *plane,
-			       struct drm_plane_state *old_state);
-};
-
-static inline void drm_plane_helper_add(struct drm_plane *plane,
-					const struct drm_plane_helper_funcs *funcs)
-{
-	plane->helper_private = funcs;
-}
-
 int drm_plane_helper_check_update(struct drm_plane *plane,
 				  struct drm_crtc *crtc,
 				  struct drm_framebuffer *fb,