[PATCH 1/2] drm: add helper to sort panels to the head of the connector list

[PATCH 1/2] drm: add helper to sort panels to the head of the connector list

[Daniel Vetter]

Userspace seems to like this, see

commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
Author: Adam Jackson <ajax@redhat.com>
Date:   Fri Jul 16 14:46:29 2010 -0400

    drm/i915: Initialize LVDS and eDP outputs before anything else

    This makes them sort to the front in X, which makes them likely to be
    the primary outputs if you haven't specified a preference in your DE,
    which is likely to be what you want.

    Signed-off-by: Adam Jackson <ajax@redhat.com>
    Signed-off-by: Eric Anholt <eric@anholt.net>

Sorting the connector list after the fact is much easier than trying
to be clever with the init sequence.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c | 18 ++++++++++++++++++
 include/drm/drm_crtc_helper.h     |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 1227adf..7105168 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -39,6 +39,24 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_edid.h>
 
+void drm_helper_move_panel_connectors_to_head(struct drm_device *dev)
+{
+	struct drm_connector *connector, *tmp;
+	struct list_head panel_list;
+
+	INIT_LIST_HEAD(&panel_list);
+
+	list_for_each_entry_safe(connector, tmp,
+				 &dev->mode_config.connector_list, head) {
+		if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS ||
+		    connector->connector_type == DRM_MODE_CONNECTOR_eDP)
+			list_move_tail(&connector->head, &panel_list);
+	}
+
+	list_splice(&panel_list, &dev->mode_config.connector_list);
+}
+EXPORT_SYMBOL(drm_helper_move_panel_connectors_to_head);
+
 static bool drm_kms_helper_poll = true;
 module_param_named(poll, drm_kms_helper_poll, bool, 0600);
 
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index e01cc80..defee28 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -137,6 +137,8 @@ extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
 
 extern void drm_helper_connector_dpms(struct drm_connector *connector, int mode);
 
+extern void drm_helper_move_panel_connectors_to_head(struct drm_device *);
+
 extern int drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
 					  struct drm_mode_fb_cmd2 *mode_cmd);
 
-- 
1.7.11.7

[PATCH 2/2] drm/i915: move panel connectors to the front

[Daniel Vetter]

This essentially reverts

commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
Author: Adam Jackson <ajax@redhat.com>
Date:   Fri Jul 16 14:46:29 2010 -0400

    drm/i915: Initialize LVDS and eDP outputs before anything else

simply because it doesn't scale: It misses SDVO and DVO panels,
and now with DDI encoders on haswell this is becoming unmanageable.

Instead we simply sort the connector list after everything is
set up.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc9676a..d91ee4b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7862,16 +7862,6 @@ static void intel_setup_outputs(struct drm_device *dev)
 		I915_WRITE(PFIT_CONTROL, 0);
 	}
 
-	if (HAS_PCH_SPLIT(dev)) {
-		dpd_is_edp = intel_dpd_is_edp(dev);
-
-		if (has_edp_a(dev))
-			intel_dp_init(dev, DP_A, PORT_A);
-
-		if (dpd_is_edp && (I915_READ(PCH_DP_D) & DP_DETECTED))
-			intel_dp_init(dev, PCH_DP_D, PORT_D);
-	}
-
 	intel_crt_init(dev);
 
 	if (IS_HASWELL(dev)) {
@@ -7895,6 +7885,10 @@ static void intel_setup_outputs(struct drm_device *dev)
 			intel_ddi_init(dev, PORT_D);
 	} else if (HAS_PCH_SPLIT(dev)) {
 		int found;
+		dpd_is_edp = intel_dpd_is_edp(dev);
+
+		if (has_edp_a(dev))
+			intel_dp_init(dev, DP_A, PORT_A);
 
 		if (I915_READ(HDMIB) & PORT_DETECTED) {
 			/* PCH SDVOB multiplex with HDMIB */
@@ -7914,7 +7908,7 @@ static void intel_setup_outputs(struct drm_device *dev)
 		if (I915_READ(PCH_DP_C) & DP_DETECTED)
 			intel_dp_init(dev, PCH_DP_C, PORT_C);
 
-		if (!dpd_is_edp && (I915_READ(PCH_DP_D) & DP_DETECTED))
+		if (I915_READ(PCH_DP_D) & DP_DETECTED)
 			intel_dp_init(dev, PCH_DP_D, PORT_D);
 	} else if (IS_VALLEYVIEW(dev)) {
 		int found;
@@ -7990,6 +7984,8 @@ static void intel_setup_outputs(struct drm_device *dev)
 
 	if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
 		ironlake_init_pch_refclk(dev);
+
+	drm_helper_move_panel_connectors_to_head(dev);
 }
 
 static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
-- 
1.7.11.7

Re: [Intel-gfx] [PATCH 2/2] drm/i915: move panel connectors to the front

[Chris Wilson]

On Sat, 27 Oct 2012 15:52:05 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> This essentially reverts
> 
> commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
> Author: Adam Jackson <ajax@redhat.com>
> Date:   Fri Jul 16 14:46:29 2010 -0400
> 
>     drm/i915: Initialize LVDS and eDP outputs before anything else
> 
> simply because it doesn't scale: It misses SDVO and DVO panels,
> and now with DDI encoders on haswell this is becoming unmanageable.
> 
> Instead we simply sort the connector list after everything is
> set up.

It's reasonable as any other arrangement. On the other hand, userspace
should understand that the kernel has no policy regarding connector
ordering, which may change on the fly if we ever do hotplug, and do the
sorting itself if it cared about making its legacy clients work better.

The code looks fine so it's a mixture of r-b and a-b.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/2] drm: add helper to sort panels to the head of the connector list

[Laurent Pinchart]

Hi Daniel,

Thanks for the patch.

On Saturday 27 October 2012 15:52:04 Daniel Vetter wrote:
> Userspace seems to like this, see
> 
> commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
> Author: Adam Jackson <ajax@redhat.com>
> Date:   Fri Jul 16 14:46:29 2010 -0400
> 
>     drm/i915: Initialize LVDS and eDP outputs before anything else
> 
>     This makes them sort to the front in X, which makes them likely to be
>     the primary outputs if you haven't specified a preference in your DE,
>     which is likely to be what you want.
> 
>     Signed-off-by: Adam Jackson <ajax@redhat.com>
>     Signed-off-by: Eric Anholt <eric@anholt.net>
> 
> Sorting the connector list after the fact is much easier than trying
> to be clever with the init sequence.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_crtc_helper.c | 18 ++++++++++++++++++
>  include/drm/drm_crtc_helper.h     |  2 ++
>  2 files changed, 20 insertions(+)

You forgot Documentation/DocBook/drm.tmpl :-)

> 
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c index 1227adf..7105168 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -39,6 +39,24 @@
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_edid.h>
> 
> +void drm_helper_move_panel_connectors_to_head(struct drm_device *dev)
> +{
> +	struct drm_connector *connector, *tmp;
> +	struct list_head panel_list;
> +
> +	INIT_LIST_HEAD(&panel_list);
> +
> +	list_for_each_entry_safe(connector, tmp,
> +				 &dev->mode_config.connector_list, head) {
> +		if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS ||
> +		    connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> +			list_move_tail(&connector->head, &panel_list);
> +	}
> +
> +	list_splice(&panel_list, &dev->mode_config.connector_list);
> +}
> +EXPORT_SYMBOL(drm_helper_move_panel_connectors_to_head);
> +
>  static bool drm_kms_helper_poll = true;
>  module_param_named(poll, drm_kms_helper_poll, bool, 0600);
> 
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index e01cc80..defee28 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -137,6 +137,8 @@ extern bool drm_helper_encoder_in_use(struct drm_encoder
> *encoder);
> 
>  extern void drm_helper_connector_dpms(struct drm_connector *connector, int
> mode);
> 
> +extern void drm_helper_move_panel_connectors_to_head(struct drm_device *);
> +
>  extern int drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
>  					  struct drm_mode_fb_cmd2 *mode_cmd);
-- 
Regards,

Laurent Pinchart

[PATCH 1/4] drm/doc: Helpers are not a Midlayer!

[Daniel Vetter]

I'm devoting all my wrath to that fight, so don't misname it ;-)

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index b030052..ca45155 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -1621,10 +1621,10 @@ void intel_crt_init(struct drm_device *dev)
     </sect2>
   </sect1>
 
-  <!-- Internals: mid-layer helper functions -->
+  <!-- Internals: helper functions -->
 
   <sect1>
-    <title>Mid-layer Helper Functions</title>
+    <title>Helper Functions</title>
     <para>
       The CRTC, encoder and connector functions provided by the drivers
       implement the DRM API. They're called by the DRM core and ioctl handlers
-- 
1.7.11.7

[PATCH 2/4] drm/doc: integrate crtc helper api into docbook

[Daniel Vetter]

- Add the missing doc for drm_helper_move_panel_connectors_to_head.
- Fixup any outdated stuff in existing sections. I've only looked at
  those kerneldoc headers that actually resulted in a complaint from
  the kerneldoc parser tool.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl    |  4 +++
 drivers/gpu/drm/drm_crtc_helper.c | 54 ++++++++++++++++++++++-----------------
 2 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index ca45155..43e706e 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2106,6 +2106,10 @@ void intel_crt_init(struct drm_device *dev)
         </listitem>
       </itemizedlist>
     </sect2>
+    <sect2>
+      <title>Modeset Helper Functions Reference</title>
+!Edrivers/gpu/drm/drm_crtc_helper.c
+    </sect2>
   </sect1>
 
   <!-- Internals: vertical blanking -->
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 1021f505..e2e19ef 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -92,22 +92,21 @@ static void drm_mode_validate_flag(struct drm_connector *connector,
 
 /**
  * drm_helper_probe_single_connector_modes - get complete set of display modes
- * @dev: DRM device
+ * @connector: connector to probe
  * @maxX: max width for modes
  * @maxY: max height for modes
  *
  * LOCKING:
  * Caller must hold mode config lock.
  *
- * Based on @dev's mode_config layout, scan all the connectors and try to detect
- * modes on them.  Modes will first be added to the connector's probed_modes
- * list, then culled (based on validity and the @maxX, @maxY parameters) and
- * put into the normal modes list.
- *
- * Intended to be used either at bootup time or when major configuration
- * changes have occurred.
+ * Based on the helper callbacks implemented by @connector try to detect all
+ * valid modes.  Modes will first be added to the connector's probed_modes list,
+ * then culled (based on validity and the @maxX, @maxY parameters) and put into
+ * the normal modes list.
  *
- * FIXME: take into account monitor limits
+ * Intended to be use as a generic implementation of the ->probe() @connector
+ * callback for drivers that use the crtc helpers for output mode filtering and
+ * conector detection.
  *
  * RETURNS:
  * Number of modes found on @connector.
@@ -353,17 +352,23 @@ drm_crtc_prepare_encoders(struct drm_device *dev)
 }
 
 /**
- * drm_crtc_set_mode - set a mode
+ * drm_crtc_set_mode - internal helper to set a mode
  * @crtc: CRTC to program
  * @mode: mode to use
  * @x: width of mode
  * @y: height of mode
+ * @old_fb: old framebuffer, for cleanup
  *
  * LOCKING:
  * Caller must hold mode config lock.
  *
  * Try to set @mode on @crtc.  Give @crtc and its associated connectors a chance
- * to fixup or reject the mode prior to trying to set it.
+ * to fixup or reject the mode prior to trying to set it. This is an internal
+ * helper that drivers could e.g. use to update properties that require the
+ * entire output pipe to be disabled and re-enabled in a new configuration. For
+ * example for changing whether audio is enabled on a hdmi link or for changing
+ * panel fitter or dither attributes. It is also called by the
+ * drm_crtc_helper_set_config() helper function.
  *
  * RETURNS:
  * True if the mode was set successfully, or false otherwise.
@@ -519,20 +524,19 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
 
 /**
  * drm_crtc_helper_set_config - set a new config from userspace
- * @crtc: CRTC to setup
- * @crtc_info: user provided configuration
- * @new_mode: new mode to set
- * @connector_set: set of connectors for the new config
- * @fb: new framebuffer
+ * @set: mode set configuration
  *
  * LOCKING:
  * Caller must hold mode config lock.
  *
- * Setup a new configuration, provided by the user in @crtc_info, and enable
- * it.
+ * Setup a new configuration, provided by the upper layers (either an ioctl call
+ * from userspace or internally e.g. from the fbdev suppport code) in @set, and
+ * enable it. This is the main helper functions for drivers that implement
+ * kernel mode setting with the crtc helper functions and the assorted
+ * ->prepare(), ->modeset() and ->commit() interfaces.
  *
  * RETURNS:
- * Zero. (FIXME)
+ * Returns 0 on success, -ERRNO on failure.
  */
 int drm_crtc_helper_set_config(struct drm_mode_set *set)
 {
@@ -828,12 +832,14 @@ static int drm_helper_choose_crtc_dpms(struct drm_crtc *crtc)
 }
 
 /**
- * drm_helper_connector_dpms
- * @connector affected connector
- * @mode DPMS mode
+ * drm_helper_connector_dpms() - connector dpms helper implementation
+ * @connector: affected connector
+ * @mode: DPMS mode
  *
- * Calls the low-level connector DPMS function, then
- * calls appropriate encoder and crtc DPMS functions as well
+ * This is the main helper function provided by the crtc helper framework for
+ * implementing the DPMS connector attribute. It computes the new desired DPMS
+ * state for all encoders and crtcs in the output mesh and calls the ->dpsm()
+ * callback provided by the driver appropriately.
  */
 void drm_helper_connector_dpms(struct drm_connector *connector, int mode)
 {
-- 
1.7.11.7

[PATCH 3/4] drm/doc: integrate fb helper reference into docs

[Daniel Vetter]

Again only minimal changes to make kerneldoc no longer shout. Plus a
little introduction in the form of a inline DOC: section to quickly
explain what this is all about.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl  |  5 +++++
 drivers/gpu/drm/drm_fb_helper.c | 19 ++++++++++++++-----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 43e706e..15729eb 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2110,6 +2110,11 @@ void intel_crt_init(struct drm_device *dev)
       <title>Modeset Helper Functions Reference</title>
 !Edrivers/gpu/drm/drm_crtc_helper.c
     </sect2>
+    <sect2>
+      <title>fbdev Helper Functions Reference</title>
+!Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers
+!Edrivers/gpu/drm/drm_fb_helper.c
+    </sect2>
   </sect1>
 
   <!-- Internals: vertical blanking -->
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4d58d7e..b6e9893 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -41,7 +41,14 @@ MODULE_AUTHOR("David Airlie, Jesse Barnes");
 MODULE_DESCRIPTION("DRM KMS helper");
 MODULE_LICENSE("GPL and additional rights");
 
-static LIST_HEAD(kernel_fb_helper_list);
+/**
+ * DOC: fbdev helpers
+ *
+ * The fb helper functions are useful to provide an fbdev on top of a drm kernel
+ * mode setting driver. They can be used mostly independantely from the crtc
+ * helper functions used by many drivers to implement the kernel mode setting
+ * interfaces.  static LIST_HEAD(kernel_fb_helper_list).
+ */
 
 /* simple single crtc case helper function */
 int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
@@ -1291,12 +1298,14 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
 
 /**
  * drm_helper_initial_config - setup a sane initial connector configuration
- * @dev: DRM device
+ * @fb_helper: fb_helper device struct
+ * @bpp_sel: bpp value to use for the framebuffer configuration
  *
  * LOCKING:
- * Called at init time, must take mode config lock.
+ * Called at init time by the driver to set up the @fb_helper initial
+ * configuration, must take mode config lock.
  *
- * Scan the CRTCs and connectors and try to put together an initial setup.
+ * Scans the CRTCs and connectors and tries to put together an initial setup.
  * At the moment, this is a cloned configuration across all heads with
  * a new framebuffer object as the backing store.
  *
@@ -1330,7 +1339,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
 
 /**
  * drm_fb_helper_hotplug_event - respond to a hotplug notification by
- *                               probing all the outputs attached to the fb.
+ *                               probing all the outputs attached to the fb
  * @fb_helper: the drm_fb_helper
  *
  * LOCKING:
-- 
1.7.11.7

[PATCH 4/4] drm/doc: add new dp helpers into drm DocBook

[Daniel Vetter]

I didn't bother with documenting the really trivial new "extract
something from dpcd" helpers, but the i2c over aux ch is now
documented a bit.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl  |  6 ++++++
 drivers/gpu/drm/drm_dp_helper.c | 20 ++++++++++++++++++++
 include/drm/drm_dp_helper.h     |  7 +++++++
 3 files changed, 33 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 15729eb..71edb57 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2115,6 +2115,12 @@ void intel_crt_init(struct drm_device *dev)
 !Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers
 !Edrivers/gpu/drm/drm_fb_helper.c
     </sect2>
+    <sect2>
+      <title>Display Port Helper Functions Reference</title>
+!Pdrivers/gpu/drm/drm_dp_helper.c dp helpers
+!Iinclude/drm/drm_dp_helper.h
+!Edrivers/gpu/drm/drm_dp_helper.c
+    </sect2>
   </sect1>
 
   <!-- Internals: vertical blanking -->
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 3c4cccd..2ff6482 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -30,6 +30,15 @@
 #include <drm/drm_dp_helper.h>
 #include <drm/drmP.h>
 
+/**
+ * DOC: dp helpers
+ *
+ * These functions contain some come logic and helpers at various abstraction
+ * levels to deal with Display Port sink devices and related things like DP aux
+ * channel transfers, EDID reading over DP aux channels, decoding certain DPCD
+ * blocks, ...
+ */
+
 /* Run a single AUX_CH I2C transaction, writing/reading data as necessary */
 static int
 i2c_algo_dp_aux_transaction(struct i2c_adapter *adapter, int mode,
@@ -193,6 +202,17 @@ i2c_dp_aux_prepare_bus(struct i2c_adapter *adapter)
 	return 0;
 }
 
+/**
+ * i2c_dp_aux_add_bus() - register a i2c adaptar using the aux ch helper
+ * @adapter: i2c adapter to register
+ *
+ * This registers an i2c adapater that uses dp aux channel as it's underlaying
+ * transport. The driver needs to fill out the &i2c_algo_dp_aux_data structure
+ * which will be used by the the i2c over dp aux algo.
+ *
+ * RETURNS:
+ * 0 on success, -ERRNO on failure.
+ */
 int
 i2c_dp_aux_add_bus(struct i2c_adapter *adapter)
 {
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index c09d367..3f94ede 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -312,6 +312,13 @@
 #define MODE_I2C_READ	4
 #define MODE_I2C_STOP	8
 
+/**
+ * struct i2c_algo_dp_aux_data - driver interface structure for i2c over dp aux algo
+ * @running: set by the algo indicating whether an i2c is ongoing or whether the
+ * i2c bus is quiescent
+ * @address: i2c target address for the currently ongoing transfer
+ * @aux_ch: driver callback to transfer a single byte of the i2c payload
+ */
 struct i2c_algo_dp_aux_data {
 	bool running;
 	u16 address;
-- 
1.7.11.7

[PATCH 1/3] drm/doc: integrate crtc helper api into docbook

[Daniel Vetter]

- Add the missing doc for drm_helper_move_panel_connectors_to_head.
- Fixup any outdated stuff in existing sections. I've only looked at
  those kerneldoc headers that actually resulted in a complaint from
  the kerneldoc parser tool.

v2:
- Actually include the docbook snippet in the right patch.
- Fix spelling fail.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl    |  4 +++
 drivers/gpu/drm/drm_crtc_helper.c | 66 +++++++++++++++++++++++++--------------
 2 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index ca45155..43e706e 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2106,6 +2106,10 @@ void intel_crt_init(struct drm_device *dev)
         </listitem>
       </itemizedlist>
     </sect2>
+    <sect2>
+      <title>Modeset Helper Functions Reference</title>
+!Edrivers/gpu/drm/drm_crtc_helper.c
+    </sect2>
   </sect1>
 
   <!-- Internals: vertical blanking -->
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 7105168..df4aba1 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -39,6 +39,17 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_edid.h>
 
+/**
+ * drm_helper_move_panel_connectors_to_head() - move panels to the front in the
+ * 						connector list
+ * @dev: drm device to operate on
+ *
+ * Some userspace presumes that the first connected connector is the main
+ * display, where it's supposed to display e.g. the login screen. For
+ * laptops, this should be the main panel. Use this function to sort all
+ * (eDP/LVDS) panels to the front of the connector list, instead of
+ * painstakingly trying to initialize them in the right order.
+ */
 void drm_helper_move_panel_connectors_to_head(struct drm_device *dev)
 {
 	struct drm_connector *connector, *tmp;
@@ -82,22 +93,21 @@ static void drm_mode_validate_flag(struct drm_connector *connector,
 
 /**
  * drm_helper_probe_single_connector_modes - get complete set of display modes
- * @dev: DRM device
+ * @connector: connector to probe
  * @maxX: max width for modes
  * @maxY: max height for modes
  *
  * LOCKING:
  * Caller must hold mode config lock.
  *
- * Based on @dev's mode_config layout, scan all the connectors and try to detect
- * modes on them.  Modes will first be added to the connector's probed_modes
- * list, then culled (based on validity and the @maxX, @maxY parameters) and
- * put into the normal modes list.
- *
- * Intended to be used either at bootup time or when major configuration
- * changes have occurred.
+ * Based on the helper callbacks implemented by @connector try to detect all
+ * valid modes.  Modes will first be added to the connector's probed_modes list,
+ * then culled (based on validity and the @maxX, @maxY parameters) and put into
+ * the normal modes list.
  *
- * FIXME: take into account monitor limits
+ * Intended to be use as a generic implementation of the ->probe() @connector
+ * callback for drivers that use the crtc helpers for output mode filtering and
+ * detection.
  *
  * RETURNS:
  * Number of modes found on @connector.
@@ -343,17 +353,24 @@ drm_crtc_prepare_encoders(struct drm_device *dev)
 }
 
 /**
- * drm_crtc_set_mode - set a mode
+ * drm_crtc_set_mode - internal helper to set a mode
  * @crtc: CRTC to program
  * @mode: mode to use
  * @x: width of mode
  * @y: height of mode
+ * @old_fb: old framebuffer, for cleanup
  *
  * LOCKING:
  * Caller must hold mode config lock.
  *
  * Try to set @mode on @crtc.  Give @crtc and its associated connectors a chance
- * to fixup or reject the mode prior to trying to set it.
+ * to fixup or reject the mode prior to trying to set it. This is an internal
+ * helper that drivers could e.g. use to update properties that require the
+ * entire output pipe to be disabled and re-enabled in a new configuration. For
+ * example for changing whether audio is enabled on a hdmi link or for changing
+ * panel fitter or dither attributes. It is also called by the
+ * drm_crtc_helper_set_config() helper function to drive the mode setting
+ * sequence.
  *
  * RETURNS:
  * True if the mode was set successfully, or false otherwise.
@@ -509,20 +526,19 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
 
 /**
  * drm_crtc_helper_set_config - set a new config from userspace
- * @crtc: CRTC to setup
- * @crtc_info: user provided configuration
- * @new_mode: new mode to set
- * @connector_set: set of connectors for the new config
- * @fb: new framebuffer
+ * @set: mode set configuration
  *
  * LOCKING:
  * Caller must hold mode config lock.
  *
- * Setup a new configuration, provided by the user in @crtc_info, and enable
- * it.
+ * Setup a new configuration, provided by the upper layers (either an ioctl call
+ * from userspace or internally e.g. from the fbdev suppport code) in @set, and
+ * enable it. This is the main helper functions for drivers that implement
+ * kernel mode setting with the crtc helper functions and the assorted
+ * ->prepare(), ->modeset() and ->commit() helper callbacks.
  *
  * RETURNS:
- * Zero. (FIXME)
+ * Returns 0 on success, -ERRNO on failure.
  */
 int drm_crtc_helper_set_config(struct drm_mode_set *set)
 {
@@ -818,12 +834,14 @@ static int drm_helper_choose_crtc_dpms(struct drm_crtc *crtc)
 }
 
 /**
- * drm_helper_connector_dpms
- * @connector affected connector
- * @mode DPMS mode
+ * drm_helper_connector_dpms() - connector dpms helper implementation
+ * @connector: affected connector
+ * @mode: DPMS mode
  *
- * Calls the low-level connector DPMS function, then
- * calls appropriate encoder and crtc DPMS functions as well
+ * This is the main helper function provided by the crtc helper framework for
+ * implementing the DPMS connector attribute. It computes the new desired DPMS
+ * state for all encoders and crtcs in the output mesh and calls the ->dpms()
+ * callback provided by the driver appropriately.
  */
 void drm_helper_connector_dpms(struct drm_connector *connector, int mode)
 {
-- 
1.7.11.7

[PATCH 2/3] drm/doc: integrate fb helper reference into docs

[Daniel Vetter]

Again only minimal changes to make kerneldoc no longer shout. Plus a
little introduction in the form of a inline DOC: section to quickly
explain what this is all about.

v2: Fixup spelling fail.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl  |  5 +++++
 drivers/gpu/drm/drm_fb_helper.c | 19 ++++++++++++++-----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 43e706e..15729eb 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2110,6 +2110,11 @@ void intel_crt_init(struct drm_device *dev)
       <title>Modeset Helper Functions Reference</title>
 !Edrivers/gpu/drm/drm_crtc_helper.c
     </sect2>
+    <sect2>
+      <title>fbdev Helper Functions Reference</title>
+!Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers
+!Edrivers/gpu/drm/drm_fb_helper.c
+    </sect2>
   </sect1>
 
   <!-- Internals: vertical blanking -->
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4d58d7e..320dcbc 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -41,7 +41,14 @@ MODULE_AUTHOR("David Airlie, Jesse Barnes");
 MODULE_DESCRIPTION("DRM KMS helper");
 MODULE_LICENSE("GPL and additional rights");
 
-static LIST_HEAD(kernel_fb_helper_list);
+/**
+ * DOC: fbdev helpers
+ *
+ * The fb helper functions are useful to provide an fbdev on top of a drm kernel
+ * mode setting driver. They can be used mostly independantely from the crtc
+ * helper functions used by many drivers to implement the kernel mode setting
+ * interfaces.
+ */
 
 /* simple single crtc case helper function */
 int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
@@ -1291,12 +1298,14 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
 
 /**
  * drm_helper_initial_config - setup a sane initial connector configuration
- * @dev: DRM device
+ * @fb_helper: fb_helper device struct
+ * @bpp_sel: bpp value to use for the framebuffer configuration
  *
  * LOCKING:
- * Called at init time, must take mode config lock.
+ * Called at init time by the driver to set up the @fb_helper initial
+ * configuration, must take the mode config lock.
  *
- * Scan the CRTCs and connectors and try to put together an initial setup.
+ * Scans the CRTCs and connectors and tries to put together an initial setup.
  * At the moment, this is a cloned configuration across all heads with
  * a new framebuffer object as the backing store.
  *
@@ -1330,7 +1339,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
 
 /**
  * drm_fb_helper_hotplug_event - respond to a hotplug notification by
- *                               probing all the outputs attached to the fb.
+ *                               probing all the outputs attached to the fb
  * @fb_helper: the drm_fb_helper
  *
  * LOCKING:
-- 
1.7.11.7

[PATCH 3/3] drm/doc: add new dp helpers into drm DocBook

[Daniel Vetter]

I didn't bother with documenting the really trivial new "extract
something from dpcd" helpers, but the i2c over aux ch is now
documented a bit.

v2: Clarify the comment for i2c_dp_aux_add_bus a bit.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl  |  6 ++++++
 drivers/gpu/drm/drm_dp_helper.c | 21 +++++++++++++++++++++
 include/drm/drm_dp_helper.h     |  7 +++++++
 3 files changed, 34 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 15729eb..71edb57 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2115,6 +2115,12 @@ void intel_crt_init(struct drm_device *dev)
 !Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers
 !Edrivers/gpu/drm/drm_fb_helper.c
     </sect2>
+    <sect2>
+      <title>Display Port Helper Functions Reference</title>
+!Pdrivers/gpu/drm/drm_dp_helper.c dp helpers
+!Iinclude/drm/drm_dp_helper.h
+!Edrivers/gpu/drm/drm_dp_helper.c
+    </sect2>
   </sect1>
 
   <!-- Internals: vertical blanking -->
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 3c4cccd..77dbbaf 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -30,6 +30,15 @@
 #include <drm/drm_dp_helper.h>
 #include <drm/drmP.h>
 
+/**
+ * DOC: dp helpers
+ *
+ * These functions contain some common logic and helpers at various abstraction
+ * levels to deal with Display Port sink devices and related things like DP aux
+ * channel transfers, EDID reading over DP aux channels, decoding certain DPCD
+ * blocks, ...
+ */
+
 /* Run a single AUX_CH I2C transaction, writing/reading data as necessary */
 static int
 i2c_algo_dp_aux_transaction(struct i2c_adapter *adapter, int mode,
@@ -193,6 +202,18 @@ i2c_dp_aux_prepare_bus(struct i2c_adapter *adapter)
 	return 0;
 }
 
+/**
+ * i2c_dp_aux_add_bus() - register a i2c adaptar using the aux ch helper
+ * @adapter: i2c adapter to register
+ *
+ * This registers an i2c adapater that uses dp aux channel as it's underlaying
+ * transport. The driver needs to fill out the &i2c_algo_dp_aux_data structure
+ * and store it in the algo_data member of the @adapter argument. This will be
+ * used by the the i2c over dp aux algo to drive the hardware.
+ *
+ * RETURNS:
+ * 0 on success, -ERRNO on failure.
+ */
 int
 i2c_dp_aux_add_bus(struct i2c_adapter *adapter)
 {
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index c09d367..3f94ede 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -312,6 +312,13 @@
 #define MODE_I2C_READ	4
 #define MODE_I2C_STOP	8
 
+/**
+ * struct i2c_algo_dp_aux_data - driver interface structure for i2c over dp aux algo
+ * @running: set by the algo indicating whether an i2c is ongoing or whether the
+ * i2c bus is quiescent
+ * @address: i2c target address for the currently ongoing transfer
+ * @aux_ch: driver callback to transfer a single byte of the i2c payload
+ */
 struct i2c_algo_dp_aux_data {
 	bool running;
 	u16 address;
-- 
1.7.11.7

Re: [PATCH 1/2] drm: add helper to sort panels to the head of the connector list

[Adam Jackson]

On 10/27/12 9:52 AM, Daniel Vetter wrote:
> Userspace seems to like this, see
>
> commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
> Author: Adam Jackson <ajax@redhat.com>
> Date:   Fri Jul 16 14:46:29 2010 -0400
>
>      drm/i915: Initialize LVDS and eDP outputs before anything else
>
>      This makes them sort to the front in X, which makes them likely to be
>      the primary outputs if you haven't specified a preference in your DE,
>      which is likely to be what you want.
>
>      Signed-off-by: Adam Jackson <ajax@redhat.com>
>      Signed-off-by: Eric Anholt <eric@anholt.net>
>
> Sorting the connector list after the fact is much easier than trying
> to be clever with the init sequence.

Entirely reasonable.

Reviewed-by: Adam Jackson <ajax@redhat.com>

- ajax

Re: [PATCH 2/2] drm/i915: move panel connectors to the front

[Adam Jackson]

On 10/27/12 9:52 AM, Daniel Vetter wrote:
> This essentially reverts
>
> commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
> Author: Adam Jackson <ajax@redhat.com>
> Date:   Fri Jul 16 14:46:29 2010 -0400
>
>      drm/i915: Initialize LVDS and eDP outputs before anything else
>
> simply because it doesn't scale: It misses SDVO and DVO panels,
> and now with DDI encoders on haswell this is becoming unmanageable.
>
> Instead we simply sort the connector list after everything is
> set up.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Adam Jackson <ajax@redhat.com>

- ajax

Re: [PATCH 1/4] drm/doc: Helpers are not a Midlayer!

[Laurent Pinchart]

Hi Daniel,

Thanks for the patch.

On Wednesday 31 October 2012 10:11:47 Daniel Vetter wrote:
> I'm devoting all my wrath to that fight, so don't misname it ;-)

Right, my bad :-)

> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/DocBook/drm.tmpl | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index b030052..ca45155 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -1621,10 +1621,10 @@ void intel_crt_init(struct drm_device *dev)
>      </sect2>
>    </sect1>
> 
> -  <!-- Internals: mid-layer helper functions -->
> +  <!-- Internals: helper functions -->

Do you think we should come up with a more descriptive name ? We have 
different sets of helpers, what would you call this one ? Mode Set Helper 
Functions ?

>    <sect1>
> -    <title>Mid-layer Helper Functions</title>
> +    <title>Helper Functions</title>
>      <para>
>        The CRTC, encoder and connector functions provided by the drivers
>        implement the DRM API. They're called by the DRM core and ioctl
> handlers

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] drm/doc: integrate crtc helper api into docbook

[Laurent Pinchart]

Hi Daniel,

Thanks for the patch.

(text reflowed for review purpose to fit the e-mail line length limit)

On Wednesday 31 October 2012 10:11:48 Daniel Vetter wrote:
> - Add the missing doc for drm_helper_move_panel_connectors_to_head.
> - Fixup any outdated stuff in existing sections. I've only looked at
>   those kerneldoc headers that actually resulted in a complaint from
>   the kerneldoc parser tool.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

After fixing the small mistake below,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/DocBook/drm.tmpl    |  4 +++
>  drivers/gpu/drm/drm_crtc_helper.c | 54 ++++++++++++++++++++----------------
>  2 files changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index ca45155..43e706e 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2106,6 +2106,10 @@ void intel_crt_init(struct drm_device *dev)
>          </listitem>
>        </itemizedlist>
>      </sect2>
> +    <sect2>
> +      <title>Modeset Helper Functions Reference</title>
> +!Edrivers/gpu/drm/drm_crtc_helper.c
> +    </sect2>
>    </sect1>
> 
>    <!-- Internals: vertical blanking -->
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> b/drivers/gpu/drm/drm_crtc_helper.c index 1021f505..e2e19ef 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -92,22 +92,21 @@ static void drm_mode_validate_flag(struct drm_connector
> *connector,
> 
>  /**
>   * drm_helper_probe_single_connector_modes - get complete set of display
>   * modes
> - * @dev: DRM device
> + * @connector: connector to probe
>   * @maxX: max width for modes
>   * @maxY: max height for modes
>   *
>   * LOCKING:
>   * Caller must hold mode config lock.
>   *
> - * Based on @dev's mode_config layout, scan all the connectors and try to
> - * detect modes on them.  Modes will first be added to the connector's
> - * probed_modes list, then culled (based on validity and the @maxX, @maxY
> - * parameters) and put into the normal modes list.
> - *
> - * Intended to be used either at bootup time or when major configuration
> - * changes have occurred.
> + * Based on the helper callbacks implemented by @connector try to detect
> + * all valid modes.  Modes will first be added to the connector's
> + * probed_modes list, then culled (based on validity and the @maxX, @maxY
> + * parameters) and put into the normal modes list.
>   *
> - * FIXME: take into account monitor limits
> + * Intended to be use as a generic implementation of the ->probe()
> + * @connector callback for drivers that use the crtc helpers for output
> + * mode filtering and conector detection.
>   *
>   * RETURNS:
>   * Number of modes found on @connector.
> @@ -353,17 +352,23 @@ drm_crtc_prepare_encoders(struct drm_device *dev)
>  }
> 
>  /**
> - * drm_crtc_set_mode - set a mode
> + * drm_crtc_set_mode - internal helper to set a mode

The function is actually now called drm_crtc_helper_set_mode.

>   * @crtc: CRTC to program
>   * @mode: mode to use
>   * @x: width of mode
>   * @y: height of mode
> + * @old_fb: old framebuffer, for cleanup
>   *
>   * LOCKING:
>   * Caller must hold mode config lock.
>   *
>   * Try to set @mode on @crtc.  Give @crtc and its associated connectors a
> - * chance to fixup or reject the mode prior to trying to set it.
> + * chance to fixup or reject the mode prior to trying to set it. This is an
> + * internal helper that drivers could e.g. use to update properties that
> + * require the entire output pipe to be disabled and re-enabled in a new
> + * configuration. For example for changing whether audio is enabled on a
> + * hdmi link or for changing panel fitter or dither attributes. It is also
> + * called by the drm_crtc_helper_set_config() helper function.
>   *
>   * RETURNS:
>   * True if the mode was set successfully, or false otherwise.
> @@ -519,20 +524,19 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
> 
>  /**
>   * drm_crtc_helper_set_config - set a new config from userspace
> - * @crtc: CRTC to setup
> - * @crtc_info: user provided configuration
> - * @new_mode: new mode to set
> - * @connector_set: set of connectors for the new config
> - * @fb: new framebuffer
> + * @set: mode set configuration
>   *
>   * LOCKING:
>   * Caller must hold mode config lock.
>   *
> - * Setup a new configuration, provided by the user in @crtc_info, and
> - * enable it.
> + * Setup a new configuration, provided by the upper layers (either an ioctl
> + * call from userspace or internally e.g. from the fbdev suppport code) in
> + * @set, and enable it. This is the main helper functions for drivers that
> + * implement kernel mode setting with the crtc helper functions and the
> + * assorted ->prepare(), ->modeset() and ->commit() interfaces.
>   *
>   * RETURNS:
> - * Zero. (FIXME)
> + * Returns 0 on success, -ERRNO on failure.
>   */
>  int drm_crtc_helper_set_config(struct drm_mode_set *set)
>  {
> @@ -828,12 +832,14 @@ static int drm_helper_choose_crtc_dpms(struct drm_crtc
> *crtc) }
> 
>  /**
> - * drm_helper_connector_dpms
> - * @connector affected connector
> - * @mode DPMS mode
> + * drm_helper_connector_dpms() - connector dpms helper implementation
> + * @connector: affected connector
> + * @mode: DPMS mode
>   *
> - * Calls the low-level connector DPMS function, then
> - * calls appropriate encoder and crtc DPMS functions as well
> + * This is the main helper function provided by the crtc helper framework
> + * for implementing the DPMS connector attribute. It computes the new
> + * desired DPMS state for all encoders and crtcs in the output mesh and
> + * calls the ->dpsm() callback provided by the driver appropriately.
>   */
>  void drm_helper_connector_dpms(struct drm_connector *connector, int mode)
>  {
-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] drm/doc: integrate fb helper reference into docs

[Laurent Pinchart]

Hi Daniel,

Thanks for the patch.

On Wednesday 31 October 2012 10:11:49 Daniel Vetter wrote:
> Again only minimal changes to make kerneldoc no longer shout. Plus a
> little introduction in the form of a inline DOC: section to quickly
> explain what this is all about.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

With the error below fixed,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/DocBook/drm.tmpl  |  5 +++++
>  drivers/gpu/drm/drm_fb_helper.c | 19 ++++++++++++++-----
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 43e706e..15729eb 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2110,6 +2110,11 @@ void intel_crt_init(struct drm_device *dev)
>        <title>Modeset Helper Functions Reference</title>
>  !Edrivers/gpu/drm/drm_crtc_helper.c
>      </sect2>
> +    <sect2>
> +      <title>fbdev Helper Functions Reference</title>
> +!Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers
> +!Edrivers/gpu/drm/drm_fb_helper.c
> +    </sect2>
>    </sect1>
> 
>    <!-- Internals: vertical blanking -->
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c index 4d58d7e..b6e9893 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -41,7 +41,14 @@ MODULE_AUTHOR("David Airlie, Jesse Barnes");
>  MODULE_DESCRIPTION("DRM KMS helper");
>  MODULE_LICENSE("GPL and additional rights");
> 
> -static LIST_HEAD(kernel_fb_helper_list);
> +/**
> + * DOC: fbdev helpers
> + *
> + * The fb helper functions are useful to provide an fbdev on top of a drm
> + * kernel mode setting driver. They can be used mostly independantely from
> + * the crtc helper functions used by many drivers to implement the kernel
> + * mode setting interfaces.  static LIST_HEAD(kernel_fb_helper_list).
> + */

Unless you plan to modify the kerneldoc infrastructure to extract code from 
the comments and feed it to the compiler, I'm pretty sure this will break 
compilation ;-)

>  /* simple single crtc case helper function */
>  int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper
> *fb_helper) @@ -1291,12 +1298,14 @@ static void drm_setup_crtcs(struct
> drm_fb_helper *fb_helper)
> 
>  /**
>   * drm_helper_initial_config - setup a sane initial connector configuration
> - * @dev: DRM device
> + * @fb_helper: fb_helper device struct
> + * @bpp_sel: bpp value to use for the framebuffer configuration
>   *
>   * LOCKING:
> - * Called at init time, must take mode config lock.
> + * Called at init time by the driver to set up the @fb_helper initial
> + * configuration, must take mode config lock.
>   *
> - * Scan the CRTCs and connectors and try to put together an initial setup.
> + * Scans the CRTCs and connectors and tries to put together an initial
> + * setup. At the moment, this is a cloned configuration across all heads
> + * with a new framebuffer object as the backing store.
>   *
> @@ -1330,7 +1339,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
> 
>  /**
>   * drm_fb_helper_hotplug_event - respond to a hotplug notification by
> - *                               probing all the outputs attached to the
> fb.
> + *                               probing all the outputs attached to the fb
>   * @fb_helper: the drm_fb_helper
>   *
>   * LOCKING:

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/4] drm/doc: add new dp helpers into drm DocBook

[Laurent Pinchart]

Hi Daniel,

Thanks for the patch.

On Wednesday 31 October 2012 10:11:50 Daniel Vetter wrote:
> I didn't bother with documenting the really trivial new "extract
> something from dpcd" helpers, but the i2c over aux ch is now
> documented a bit.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/DocBook/drm.tmpl  |  6 ++++++
>  drivers/gpu/drm/drm_dp_helper.c | 20 ++++++++++++++++++++
>  include/drm/drm_dp_helper.h     |  7 +++++++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 15729eb..71edb57 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2115,6 +2115,12 @@ void intel_crt_init(struct drm_device *dev)
>  !Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers
>  !Edrivers/gpu/drm/drm_fb_helper.c
>      </sect2>
> +    <sect2>
> +      <title>Display Port Helper Functions Reference</title>
> +!Pdrivers/gpu/drm/drm_dp_helper.c dp helpers
> +!Iinclude/drm/drm_dp_helper.h
> +!Edrivers/gpu/drm/drm_dp_helper.c
> +    </sect2>
>    </sect1>
> 
>    <!-- Internals: vertical blanking -->
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c index 3c4cccd..2ff6482 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -30,6 +30,15 @@
>  #include <drm/drm_dp_helper.h>
>  #include <drm/drmP.h>
> 
> +/**
> + * DOC: dp helpers
> + *
> + * These functions contain some come logic and helpers at various

s/come/core/

> + * abstraction levels to deal with Display Port sink devices and related
> + * things like DP aux channel transfers, EDID reading over DP aux channels,
> + * decoding certain DPCD blocks, ...
> + */
> +
>  /* Run a single AUX_CH I2C transaction, writing/reading data as necessary
> */ static int
>  i2c_algo_dp_aux_transaction(struct i2c_adapter *adapter, int mode,
> @@ -193,6 +202,17 @@ i2c_dp_aux_prepare_bus(struct i2c_adapter *adapter)
>  	return 0;
>  }
> 
> +/**
> + * i2c_dp_aux_add_bus() - register a i2c adaptar using the aux ch helper

s/a i2c adaptar/an I2C adapter/

(feel free to drop the caps if preferred)

> + * @adapter: i2c adapter to register
> + *
> + * This registers an i2c adapater that uses dp aux channel as it's
> + * underlaying transport. The driver needs to fill out the
> + * &i2c_algo_dp_aux_data structure which will be used by the the i2c over

s/the the/the/

> + * dp aux algo.

s/algo/algorithm/ ?

> + *
> + * RETURNS:
> + * 0 on success, -ERRNO on failure.
> + */
>  int
>  i2c_dp_aux_add_bus(struct i2c_adapter *adapter)
>  {
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index c09d367..3f94ede 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -312,6 +312,13 @@
>  #define MODE_I2C_READ	4
>  #define MODE_I2C_STOP	8
> 
> +/**
> + * struct i2c_algo_dp_aux_data - driver interface structure for i2c over dp
> + * aux algo

s/algo/algorithm/ ?

> + * @running: set by the algo indicating whether an i2c is ongoing or
> + * whether the i2c bus is quiescent
> + * @address: i2c target address for the currently ongoing transfer
> + * @aux_ch: driver callback to transfer a single byte of the i2c payload
> + */
>  struct i2c_algo_dp_aux_data {
>  	bool running;
>  	u16 address;
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] drm/i915: move panel connectors to the front

[Daniel Vetter]

On Wed, Oct 31, 2012 at 04:05:33PM -0400, Adam Jackson wrote:
> On 10/27/12 9:52 AM, Daniel Vetter wrote:
> >This essentially reverts
> >
> >commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
> >Author: Adam Jackson <ajax@redhat.com>
> >Date:   Fri Jul 16 14:46:29 2010 -0400
> >
> >     drm/i915: Initialize LVDS and eDP outputs before anything else
> >
> >simply because it doesn't scale: It misses SDVO and DVO panels,
> >and now with DDI encoders on haswell this is becoming unmanageable.
> >
> >Instead we simply sort the connector list after everything is
> >set up.
> >
> >Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Adam Jackson <ajax@redhat.com>

Slurped both this and the drm helper patch into dinq, thanks for the
review. I'll annoy Dave about including the doc patches into his tree
directly, he should get the pull which has the drm_dp_helper.c rename
tomorrow.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/4] drm/doc: integrate fb helper reference into docs

[Dave Airlie]

On Thu, Nov 1, 2012 at 11:45 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Again only minimal changes to make kerneldoc no longer shout. Plus a
> little introduction in the form of a inline DOC: section to quickly
> explain what this is all about.

Did someone build these?

> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 4d58d7e..320dcbc 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -41,7 +41,14 @@ MODULE_AUTHOR("David Airlie, Jesse Barnes");
>  MODULE_DESCRIPTION("DRM KMS helper");
>  MODULE_LICENSE("GPL and additional rights");
>
> -static LIST_HEAD(kernel_fb_helper_list);

because that one liner seriously harshes my buzz.

Do better next time!

Dave.

[PATCH 3/4] drm/doc: integrate fb helper reference into docs

[Daniel Vetter]

Again only minimal changes to make kerneldoc no longer shout. Plus a
little introduction in the form of a inline DOC: section to quickly
explain what this is all about.

v2: Fixup spelling fail.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl  |  5 +++++
 drivers/gpu/drm/drm_fb_helper.c | 19 ++++++++++++++-----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index c2b31b9..fadc48b 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2110,6 +2110,11 @@ void intel_crt_init(struct drm_device *dev)
       <title>Modeset Helper Functions Reference</title>
 !Edrivers/gpu/drm/drm_crtc_helper.c
     </sect2>
+    <sect2>
+      <title>fbdev Helper Functions Reference</title>
+!Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers
+!Edrivers/gpu/drm/drm_fb_helper.c
+    </sect2>
   </sect1>
 
   <!-- Internals: vertical blanking -->
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4d58d7e..320dcbc 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -41,7 +41,14 @@ MODULE_AUTHOR("David Airlie, Jesse Barnes");
 MODULE_DESCRIPTION("DRM KMS helper");
 MODULE_LICENSE("GPL and additional rights");
 
-static LIST_HEAD(kernel_fb_helper_list);
+/**
+ * DOC: fbdev helpers
+ *
+ * The fb helper functions are useful to provide an fbdev on top of a drm kernel
+ * mode setting driver. They can be used mostly independantely from the crtc
+ * helper functions used by many drivers to implement the kernel mode setting
+ * interfaces.
+ */
 
 /* simple single crtc case helper function */
 int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper)
@@ -1291,12 +1298,14 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
 
 /**
  * drm_helper_initial_config - setup a sane initial connector configuration
- * @dev: DRM device
+ * @fb_helper: fb_helper device struct
+ * @bpp_sel: bpp value to use for the framebuffer configuration
  *
  * LOCKING:
- * Called at init time, must take mode config lock.
+ * Called at init time by the driver to set up the @fb_helper initial
+ * configuration, must take the mode config lock.
  *
- * Scan the CRTCs and connectors and try to put together an initial setup.
+ * Scans the CRTCs and connectors and tries to put together an initial setup.
  * At the moment, this is a cloned configuration across all heads with
  * a new framebuffer object as the backing store.
  *
@@ -1330,7 +1339,7 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
 
 /**
  * drm_fb_helper_hotplug_event - respond to a hotplug notification by
- *                               probing all the outputs attached to the fb.
+ *                               probing all the outputs attached to the fb
  * @fb_helper: the drm_fb_helper
  *
  * LOCKING:
-- 
1.7.11.7